fix: Prevent duplicate account links

Before, for each sync run, accounts were re-linked together. That is
incorrect, they should only be linked once.

Furthermore, the account status for Gocardless was incorrect: it always
defaulted to "linked". That is now fixed: it now defaults to "active".
This commit is contained in:
2025-11-27 21:04:41 +01:00
parent 21ef49ee38
commit 334f6ce277
5 changed files with 88 additions and 15 deletions

View File

@@ -258,18 +258,19 @@ impl TransactionSource for GoCardlessAdapter {
if let Some(req_accounts) = req.accounts { if let Some(req_accounts) = req.accounts {
for acc_id in req_accounts { for acc_id in req_accounts {
let iban = if let Some(iban) = cache.get_iban(&acc_id) { let (iban, status) = if let Some(iban) = cache.get_iban(&acc_id) {
iban (iban, "active".to_string()) // Cached accounts are active
} else { } else {
// Fetch if not cached // Fetch if not cached
match client.get_account(&acc_id).await { match client.get_account(&acc_id).await {
Ok(details) => { Ok(details) => {
let iban = details.iban.unwrap_or_default(); let iban = details.iban.unwrap_or_default();
let status = details.status.unwrap_or_else(|| "unknown".to_string());
cache.insert(acc_id.clone(), iban.clone()); cache.insert(acc_id.clone(), iban.clone());
cache.save(); cache.save();
iban (iban, status)
} }
Err(_) => "Unknown".to_string(), Err(_) => ("Unknown".to_string(), "error".to_string()),
} }
}; };
@@ -277,7 +278,7 @@ impl TransactionSource for GoCardlessAdapter {
id: acc_id, id: acc_id,
iban, iban,
currency: "EUR".to_string(), // Assuming EUR for now currency: "EUR".to_string(), // Assuming EUR for now
status: "linked".to_string(), status,
}); });
} }
} }

View File

@@ -59,7 +59,12 @@ impl LinkStore {
source_account: &Account, source_account: &Account,
dest_account: &Account, dest_account: &Account,
auto_linked: bool, auto_linked: bool,
) -> String { ) -> Option<String> {
// Check if link already exists
if self.links.iter().any(|l| l.source_account_id == source_account.id && l.dest_account_id == dest_account.id) {
return None; // Link already exists
}
let id = format!("link_{}", self.next_id); let id = format!("link_{}", self.next_id);
self.next_id += 1; self.next_id += 1;
let link = AccountLink { let link = AccountLink {
@@ -70,7 +75,7 @@ impl LinkStore {
auto_linked, auto_linked,
}; };
self.links.push(link); self.links.push(link);
id Some(id)
} }
pub fn set_alias(&mut self, link_id: &str, alias: String) -> Result<()> { pub fn set_alias(&mut self, link_id: &str, alias: String) -> Result<()> {
@@ -125,3 +130,63 @@ pub fn auto_link_accounts(
// Could add name similarity matching here // Could add name similarity matching here
links links
} }
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_add_link_prevents_duplicates() {
let mut store = LinkStore::default();
let src = Account {
id: "src1".to_string(),
iban: "NL01".to_string(),
currency: "EUR".to_string(),
};
let dest = Account {
id: "dest1".to_string(),
iban: "NL01".to_string(),
currency: "EUR".to_string(),
};
// First call should create a link
let first_result = store.add_link(&src, &dest, true);
assert!(first_result.is_some());
assert_eq!(store.links.len(), 1);
// Second call should not create a duplicate
let second_result = store.add_link(&src, &dest, true);
assert!(second_result.is_none());
assert_eq!(store.links.len(), 1); // Still only one link
}
#[test]
fn test_add_link_allows_different_accounts() {
let mut store = LinkStore::default();
let src1 = Account {
id: "src1".to_string(),
iban: "NL01".to_string(),
currency: "EUR".to_string(),
};
let dest1 = Account {
id: "dest1".to_string(),
iban: "NL01".to_string(),
currency: "EUR".to_string(),
};
let dest2 = Account {
id: "dest2".to_string(),
iban: "NL02".to_string(),
currency: "EUR".to_string(),
};
// Link src1 to dest1
let result1 = store.add_link(&src1, &dest1, true);
assert!(result1.is_some());
assert_eq!(store.links.len(), 1);
// Link src1 to dest2 (different destination)
let result2 = store.add_link(&src1, &dest2, true);
assert!(result2.is_some());
assert_eq!(store.links.len(), 2); // Two different links
}
}

View File

@@ -121,7 +121,7 @@ pub struct AccountSummary {
pub id: String, pub id: String,
pub iban: String, pub iban: String,
pub currency: String, pub currency: String,
pub status: String, // e.g., "active", "expired", "linked" pub status: String, // e.g., "active", "blocked", "suspended"
} }
#[derive(Clone, Debug, Serialize)] #[derive(Clone, Debug, Serialize)]

View File

@@ -58,7 +58,11 @@ pub async fn run_sync(
for (src_idx, dest_idx) in links { for (src_idx, dest_idx) in links {
let src = &all_source_accounts[src_idx]; let src = &all_source_accounts[src_idx];
let dest = &all_dest_accounts[dest_idx]; let dest = &all_dest_accounts[dest_idx];
link_store.add_link(src, dest, true); if let Some(_link_id) = link_store.add_link(src, dest, true) {
info!("Created new account link: {} -> {}", src.id, dest.id);
} else {
info!("Account link already exists: {} -> {}", src.id, dest.id);
}
} }
link_store.save().map_err(SyncError::SourceError)?; link_store.save().map_err(SyncError::SourceError)?;

View File

@@ -375,12 +375,15 @@ async fn handle_link(subcommand: LinkCommands) -> anyhow::Result<()> {
.cloned(); .cloned();
if let (Some(src), Some(dst)) = (source_acc, dest_acc) { if let (Some(src), Some(dst)) = (source_acc, dest_acc) {
let link_id = link_store.add_link(&src, &dst, false); if let Some(link_id) = link_store.add_link(&src, &dst, false) {
link_store.save()?; link_store.save()?;
println!( println!(
"Created link {} between {} and {}", "Created link {} between {} and {}",
link_id, src.iban, dst.iban link_id, src.iban, dst.iban
); );
} else {
println!("Link between {} and {} already exists", src.iban, dst.iban);
}
} else { } else {
println!("Account not found. Ensure accounts are discovered via sync first."); println!("Account not found. Ensure accounts are discovered via sync first.");
} }