diff --git a/banks2ff/src/adapters/firefly/client.rs b/banks2ff/src/adapters/firefly/client.rs index 00fd159..026f46e 100644 --- a/banks2ff/src/adapters/firefly/client.rs +++ b/banks2ff/src/adapters/firefly/client.rs @@ -201,7 +201,8 @@ impl TransactionDestination for FireflyAdapter { if is_active { result.push(Account { id: acc.id, - iban: acc.attributes.iban.unwrap_or_default(), + name: Some(acc.attributes.name), + iban: acc.attributes.iban, currency: "EUR".to_string(), }); } diff --git a/banks2ff/src/adapters/gocardless/cache.rs b/banks2ff/src/adapters/gocardless/cache.rs index 5779186..f3fcbec 100644 --- a/banks2ff/src/adapters/gocardless/cache.rs +++ b/banks2ff/src/adapters/gocardless/cache.rs @@ -1,14 +1,150 @@ use crate::adapters::gocardless::encryption::Encryption; +use crate::core::models::AccountData; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::fs; use std::path::Path; use tracing::warn; +#[derive(Debug, Serialize, Deserialize)] +pub enum CachedAccount { + GoCardless(GoCardlessAccount), + Firefly(FireflyAccount), +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct GoCardlessAccount { + pub id: String, + pub iban: Option, + pub name: Option, // From AccountDetail.name + pub display_name: Option, // From AccountDetail.displayName + pub owner_name: Option, // From Account.owner_name + pub status: Option, // From Account.status + pub institution_id: Option, // From Account.institution_id + pub created: Option, // From Account.created + pub last_accessed: Option, // From Account.last_accessed + pub product: Option, // From AccountDetail.product + pub cash_account_type: Option, // From AccountDetail.cashAccountType +} + +#[derive(Debug, Serialize, Deserialize)] +pub struct FireflyAccount { + pub id: String, + pub name: String, // From Account.name + pub account_type: String, // From Account.type + pub iban: Option, // From Account.iban + pub active: Option, // From Account.active + pub order: Option, // From Account.order + pub created_at: Option, // From Account.created_at + pub account_role: Option, // From Account.account_role + pub object_group_id: Option, // From Account.object_group_id + pub object_group_title: Option, // From Account.object_group_title + pub object_group_order: Option, // From Account.object_group_order + pub currency_id: Option, // From Account.currency_id + pub currency_name: Option, // From Account.currency_name + pub currency_code: Option, // From Account.currency_code + pub currency_symbol: Option, // From Account.currency_symbol + pub currency_decimal_places: Option, // From Account.currency_decimal_places + pub primary_currency_id: Option, // From Account.primary_currency_id + pub primary_currency_name: Option, // From Account.primary_currency_name + pub primary_currency_code: Option, // From Account.primary_currency_code + pub primary_currency_symbol: Option, // From Account.primary_currency_symbol + pub primary_currency_decimal_places: Option, // From Account.primary_currency_decimal_places + pub opening_balance: Option, // From Account.opening_balance + pub pc_opening_balance: Option, // From Account.pc_opening_balance + pub debt_amount: Option, // From Account.debt_amount + pub pc_debt_amount: Option, // From Account.pc_debt_amount + pub notes: Option, // From Account.notes + pub monthly_payment_date: Option, // From Account.monthly_payment_date + pub credit_card_type: Option, // From Account.credit_card_type + pub account_number: Option, // From Account.account_number + pub bic: Option, // From Account.bic + pub opening_balance_date: Option, // From Account.opening_balance_date + pub liability_type: Option, // From Account.liability_type + pub liability_direction: Option, // From Account.liability_direction + pub interest: Option, // From Account.interest + pub interest_period: Option, // From Account.interest_period + pub include_net_worth: Option, // From Account.include_net_worth + pub longitude: Option, // From Account.longitude + pub latitude: Option, // From Account.latitude + pub zoom_level: Option, // From Account.zoom_level + pub last_activity: Option, // From Account.last_activity +} + +impl crate::core::models::AccountData for CachedAccount { + fn id(&self) -> &str { + match self { + CachedAccount::GoCardless(acc) => &acc.id, + CachedAccount::Firefly(acc) => &acc.id, + } + } + + fn iban(&self) -> Option<&str> { + match self { + CachedAccount::GoCardless(acc) => acc.iban.as_deref(), + CachedAccount::Firefly(acc) => acc.iban.as_deref(), + } + } + + fn display_name(&self) -> Option { + match self { + CachedAccount::GoCardless(acc) => acc.display_name.clone() + .or_else(|| acc.name.clone()) + .or_else(|| acc.owner_name.as_ref().map(|owner| format!("{} Account", owner))) + .or_else(|| acc.iban.as_ref().map(|iban| { + if iban.len() > 4 { + format!("{}****{}", &iban[..4], &iban[iban.len()-4..]) + } else { + iban.to_string() + } + })), + CachedAccount::Firefly(acc) => Some(acc.name.clone()), + } + } +} + +impl AccountData for GoCardlessAccount { + fn id(&self) -> &str { + &self.id + } + + fn iban(&self) -> Option<&str> { + self.iban.as_deref() + } + + fn display_name(&self) -> Option { + // Priority: display_name > name > owner_name > masked IBAN + self.display_name.clone() + .or_else(|| self.name.clone()) + .or_else(|| self.owner_name.as_ref().map(|owner| format!("{} Account", owner))) + .or_else(|| self.iban.as_ref().map(|iban| { + if iban.len() > 4 { + format!("{}****{}", &iban[..4], &iban[iban.len()-4..]) + } else { + iban.to_string() + } + })) + } +} + +impl AccountData for FireflyAccount { + fn id(&self) -> &str { + &self.id + } + + fn iban(&self) -> Option<&str> { + self.iban.as_deref() + } + + fn display_name(&self) -> Option { + Some(self.name.clone()) + } +} + #[derive(Debug, Serialize, Deserialize, Default)] pub struct AccountCache { - /// Map of Account ID -> IBAN - pub accounts: HashMap, + /// Map of Account ID -> Full Account Data + pub accounts: HashMap, } impl AccountCache { @@ -61,11 +197,25 @@ impl AccountCache { } } - pub fn get_iban(&self, account_id: &str) -> Option { - self.accounts.get(account_id).cloned() + pub fn get_account(&self, account_id: &str) -> Option<&CachedAccount> { + self.accounts.get(account_id) } - pub fn insert(&mut self, account_id: String, iban: String) { - self.accounts.insert(account_id, iban); + pub fn get_account_data(&self, account_id: &str) -> Option<&dyn AccountData> { + match self.accounts.get(account_id)? { + CachedAccount::GoCardless(acc) => Some(acc as &dyn AccountData), + CachedAccount::Firefly(acc) => Some(acc as &dyn AccountData), + } } + + pub fn get_display_name(&self, account_id: &str) -> Option { + self.get_account_data(account_id)?.display_name() + } + + pub fn insert(&mut self, account: CachedAccount) { + let account_id = account.id().to_string(); + self.accounts.insert(account_id, account); + } + + } diff --git a/banks2ff/src/adapters/gocardless/client.rs b/banks2ff/src/adapters/gocardless/client.rs index ee749f1..1c54516 100644 --- a/banks2ff/src/adapters/gocardless/client.rs +++ b/banks2ff/src/adapters/gocardless/client.rs @@ -1,4 +1,4 @@ -use crate::adapters::gocardless::cache::AccountCache; +use crate::adapters::gocardless::cache::{AccountCache, CachedAccount, GoCardlessAccount}; use crate::adapters::gocardless::mapper::map_transaction; use crate::adapters::gocardless::transaction_cache::AccountTransactionCache; use crate::core::models::{ @@ -85,29 +85,43 @@ impl TransactionSource for GoCardlessAdapter { if let Some(req_accounts) = req.accounts { for acc_id in req_accounts { - // 1. Check Cache - let mut iban_opt = cache.get_iban(&acc_id); + // Always fetch fresh account data during sync + match client.get_account(&acc_id).await { + Ok(basic_account) => { + // Also try to fetch account details + let details_result = client.get_account_details(&acc_id).await; - // 2. Fetch if missing - if iban_opt.is_none() { - match client.get_account(&acc_id).await { - Ok(details) => { - let new_iban = details.iban.unwrap_or_default(); - cache.insert(acc_id.clone(), new_iban.clone()); - cache.save(); - iban_opt = Some(new_iban); - } - Err(e) => { - // If rate limit hit here, we might want to skip this account and continue? - // But get_account is critical to identify the account. - // If we fail here, we can't match. - warn!("Failed to fetch details for account {}: {}", acc_id, e); - continue; - } + let gc_account = GoCardlessAccount { + id: basic_account.id.clone(), + iban: basic_account.iban, + owner_name: basic_account.owner_name, + status: basic_account.status, + institution_id: basic_account.institution_id, + created: basic_account.created, + last_accessed: basic_account.last_accessed, + // Include details if available + name: details_result.as_ref().ok().and_then(|d| d.account.name.clone()), + display_name: details_result.as_ref().ok().and_then(|d| d.account.display_name.clone()), + product: details_result.as_ref().ok().and_then(|d| d.account.product.clone()), + cash_account_type: details_result.as_ref().ok().and_then(|d| d.account.cash_account_type.clone()), + }; + + cache.insert(CachedAccount::GoCardless(gc_account)); + cache.save(); + } + Err(e) => { + // If rate limit hit here, we might want to skip this account and continue? + // But get_account is critical to identify the account. + // If we fail here, we can't match. + warn!("Failed to fetch details for account {}: {}", acc_id, e); + continue; } } - let iban = iban_opt.unwrap_or_default(); + let iban = cache.get_account_data(&acc_id) + .and_then(|acc| acc.iban()) + .unwrap_or("") + .to_string(); let mut keep = true; if let Some(ref wanted) = wanted_set { @@ -119,9 +133,17 @@ impl TransactionSource for GoCardlessAdapter { } if keep { + // Try to get account name from cache if available + let name = cache.get_account(&acc_id) + .and_then(|acc| match acc { + CachedAccount::GoCardless(gc_acc) => gc_acc.name.clone(), + _ => None, + }); + accounts.push(Account { id: acc_id, - iban, + name, + iban: Some(iban), currency: "EUR".to_string(), }); } @@ -237,50 +259,18 @@ impl TransactionSource for GoCardlessAdapter { #[instrument(skip(self))] async fn list_accounts(&self) -> Result> { - let mut client = self.client.lock().await; - let mut cache = self.cache.lock().await; - - client.obtain_access_token().await?; - - let requisitions = client.get_requisitions().await?; + let cache = self.cache.lock().await; let mut summaries = Vec::new(); - for req in requisitions.results { - if req.status != "LN" { - continue; - } - - if let Some(agreement_id) = &req.agreement { - if client.is_agreement_expired(agreement_id).await? { - continue; - } - } - - if let Some(req_accounts) = req.accounts { - for acc_id in req_accounts { - let (iban, status) = if let Some(iban) = cache.get_iban(&acc_id) { - (iban, "active".to_string()) // Cached accounts are active - } else { - // Fetch if not cached - match client.get_account(&acc_id).await { - Ok(details) => { - 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.save(); - (iban, status) - } - Err(_) => ("Unknown".to_string(), "error".to_string()), - } - }; - - summaries.push(AccountSummary { - id: acc_id, - iban, - currency: "EUR".to_string(), // Assuming EUR for now - status, - }); - } + // Use cached account data for display + for account_id in cache.accounts.keys() { + if let Some(account_data) = cache.get_account_data(account_id) { + let summary = AccountSummary { + id: account_id.clone(), + iban: account_data.iban().unwrap_or("").to_string(), + currency: "EUR".to_string(), // GoCardless primarily uses EUR + }; + summaries.push(summary); } } @@ -297,8 +287,10 @@ impl TransactionSource for GoCardlessAdapter { .cache .lock() .await - .get_iban(account_id) - .unwrap_or_else(|| "Unknown".to_string()); + .get_account_data(account_id) + .and_then(|acc| acc.iban()) + .unwrap_or("Unknown") + .to_string(); let transaction_count = cache.ranges.iter().map(|r| r.transactions.len()).sum(); let last_sync_date = cache.ranges.iter().map(|r| r.end_date).max(); diff --git a/banks2ff/src/cli/formatters.rs b/banks2ff/src/cli/formatters.rs index d906d65..bea7535 100644 --- a/banks2ff/src/cli/formatters.rs +++ b/banks2ff/src/cli/formatters.rs @@ -29,12 +29,11 @@ impl Formattable for AccountSummary { fn to_table(&self) -> Table { let mut table = Table::new(); table.load_preset(UTF8_FULL); - table.set_header(vec!["ID", "IBAN", "Currency", "Status"]); + table.set_header(vec!["ID", "IBAN", "Currency"]); table.add_row(vec![ self.id.clone(), mask_iban(&self.iban), self.currency.clone(), - self.status.clone(), ]); table } diff --git a/banks2ff/src/cli/setup.rs b/banks2ff/src/cli/setup.rs index 7d866b4..f46ed36 100644 --- a/banks2ff/src/cli/setup.rs +++ b/banks2ff/src/cli/setup.rs @@ -1,4 +1,5 @@ use crate::adapters::firefly::client::FireflyAdapter; +use crate::adapters::gocardless::cache::AccountCache; use crate::adapters::gocardless::client::GoCardlessAdapter; use crate::debug::DebugLogger; use anyhow::Result; @@ -10,6 +11,7 @@ use std::env; pub struct AppContext { pub source: GoCardlessAdapter, pub destination: FireflyAdapter, + pub account_cache: AccountCache, } impl AppContext { @@ -49,6 +51,7 @@ impl AppContext { Ok(Self { source, destination, + account_cache: AccountCache::default(), }) } } diff --git a/banks2ff/src/core/linking.rs b/banks2ff/src/core/linking.rs index 202ab41..d3a847d 100644 --- a/banks2ff/src/core/linking.rs +++ b/banks2ff/src/core/linking.rs @@ -1,7 +1,6 @@ use crate::core::models::Account; use anyhow::Result; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; use std::fs; use std::path::Path; use tracing::warn; @@ -18,9 +17,6 @@ pub struct AccountLink { #[derive(Debug, Serialize, Deserialize, Default)] pub struct LinkStore { pub links: Vec, - pub source_accounts: HashMap>, // outer key: source type, inner: account id - pub dest_accounts: HashMap>, // outer key: dest type, inner: account id - next_id: usize, } impl LinkStore { @@ -61,12 +57,14 @@ impl LinkStore { auto_linked: bool, ) -> Option { // 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) { + 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); - self.next_id += 1; + let next_id = self.links.len() + 1; + let id = format!("link_{}", next_id); let link = AccountLink { id: id.clone(), source_account_id: source_account.id.clone(), @@ -96,22 +94,7 @@ impl LinkStore { self.links.iter().find(|l| l.source_account_id == source_id) } - pub fn update_source_accounts(&mut self, source_type: &str, accounts: Vec) { - let type_map = self - .source_accounts - .entry(source_type.to_string()) - .or_default(); - for account in accounts { - type_map.insert(account.id.clone(), account); - } - } - pub fn update_dest_accounts(&mut self, dest_type: &str, accounts: Vec) { - let type_map = self.dest_accounts.entry(dest_type.to_string()).or_default(); - for account in accounts { - type_map.insert(account.id.clone(), account); - } - } } pub fn auto_link_accounts( @@ -121,7 +104,7 @@ pub fn auto_link_accounts( let mut links = Vec::new(); for (i, source) in source_accounts.iter().enumerate() { for (j, dest) in dest_accounts.iter().enumerate() { - if source.iban == dest.iban && !source.iban.is_empty() { + if source.iban == dest.iban && source.iban.as_ref().map(|s| !s.is_empty()).unwrap_or(false) { links.push((i, j)); break; // First match } @@ -140,12 +123,14 @@ mod tests { let mut store = LinkStore::default(); let src = Account { id: "src1".to_string(), - iban: "NL01".to_string(), + name: Some("Source Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }; let dest = Account { id: "dest1".to_string(), - iban: "NL01".to_string(), + name: Some("Destination Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }; @@ -165,17 +150,20 @@ mod tests { let mut store = LinkStore::default(); let src1 = Account { id: "src1".to_string(), - iban: "NL01".to_string(), + name: Some("Source Account 1".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }; let dest1 = Account { id: "dest1".to_string(), - iban: "NL01".to_string(), + name: Some("Destination Account 1".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }; let dest2 = Account { id: "dest2".to_string(), - iban: "NL02".to_string(), + name: Some("Destination Account 2".to_string()), + iban: Some("NL02".to_string()), currency: "EUR".to_string(), }; diff --git a/banks2ff/src/core/models.rs b/banks2ff/src/core/models.rs index 091da13..62d2354 100644 --- a/banks2ff/src/core/models.rs +++ b/banks2ff/src/core/models.rs @@ -54,7 +54,8 @@ impl fmt::Debug for BankTransaction { #[derive(Clone, PartialEq, serde::Serialize, serde::Deserialize)] pub struct Account { pub id: String, - pub iban: String, + pub name: Option, // Account display name + pub iban: Option, // IBAN may not be available for all accounts pub currency: String, } @@ -68,6 +69,13 @@ impl fmt::Debug for Account { } } +/// Common interface for account data from different sources +pub trait AccountData { + fn id(&self) -> &str; + fn iban(&self) -> Option<&str>; + fn display_name(&self) -> Option; +} + #[cfg(test)] mod tests { use super::*; @@ -104,7 +112,8 @@ mod tests { fn test_account_debug_masks_iban() { let account = Account { id: "123".to_string(), - iban: "DE1234567890".to_string(), + name: Some("Test Account".to_string()), + iban: Some("DE1234567890".to_string()), currency: "EUR".to_string(), }; @@ -121,7 +130,6 @@ pub struct AccountSummary { pub id: String, pub iban: String, pub currency: String, - pub status: String, // e.g., "active", "blocked", "suspended" } #[derive(Clone, Debug, Serialize)] diff --git a/banks2ff/src/core/sync.rs b/banks2ff/src/core/sync.rs index f08b71b..162a798 100644 --- a/banks2ff/src/core/sync.rs +++ b/banks2ff/src/core/sync.rs @@ -1,3 +1,4 @@ +use crate::adapters::gocardless::cache::AccountCache; use crate::core::linking::{auto_link_accounts, LinkStore}; use crate::core::models::{Account, SyncError}; use crate::core::ports::{IngestResult, TransactionDestination, TransactionSource}; @@ -13,10 +14,11 @@ pub struct SyncResult { pub accounts_skipped_errors: usize, } -#[instrument(skip(source, destination))] +#[instrument(skip(source, destination, _account_cache))] pub async fn run_sync( source: impl TransactionSource, destination: impl TransactionDestination, + _account_cache: &AccountCache, cli_start_date: Option, cli_end_date: Option, dry_run: bool, @@ -50,8 +52,6 @@ pub async fn run_sync( .map_err(SyncError::DestinationError)?; let mut link_store = LinkStore::load(); - link_store.update_source_accounts("gocardless", all_source_accounts.clone()); - link_store.update_dest_accounts("firefly", all_dest_accounts.clone()); // Auto-link accounts let links = auto_link_accounts(&all_source_accounts, &all_dest_accounts); @@ -288,7 +288,8 @@ mod tests { .returning(|_| { Ok(vec![Account { id: "src_1".to_string(), - iban: "NL01".to_string(), + name: Some("Test Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -296,7 +297,8 @@ mod tests { source.expect_discover_accounts().returning(|| { Ok(vec![Account { id: "src_1".to_string(), - iban: "NL01".to_string(), + name: Some("Test Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -325,7 +327,8 @@ mod tests { dest.expect_discover_accounts().returning(|| { Ok(vec![Account { id: "dest_1".to_string(), - iban: "NL01".to_string(), + name: Some("Savings Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -345,7 +348,7 @@ mod tests { .returning(|_, _| Ok(())); // Execution - let res = run_sync(&source, &dest, None, None, false).await; + let res = run_sync(&source, &dest, &AccountCache::default(), None, None, false).await; assert!(res.is_ok()); } @@ -360,7 +363,8 @@ mod tests { dest.expect_discover_accounts().returning(|| { Ok(vec![Account { id: "dest_1".to_string(), - iban: "NL01".to_string(), + name: Some("Savings Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -368,7 +372,8 @@ mod tests { source.expect_get_accounts().with(always()).returning(|_| { Ok(vec![Account { id: "src_1".to_string(), - iban: "NL01".to_string(), + name: Some("Test Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -376,7 +381,8 @@ mod tests { source.expect_discover_accounts().returning(|| { Ok(vec![Account { id: "src_1".to_string(), - iban: "NL01".to_string(), + name: Some("Test Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -412,7 +418,7 @@ mod tests { .times(1) .returning(|_, _| Ok(())); - let res = run_sync(&source, &dest, None, None, false).await; + let res = run_sync(&source, &dest, &AccountCache::default(), None, None, false).await; assert!(res.is_ok()); } @@ -427,7 +433,8 @@ mod tests { dest.expect_discover_accounts().returning(|| { Ok(vec![Account { id: "dest_1".to_string(), - iban: "NL01".to_string(), + name: Some("Savings Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -435,7 +442,8 @@ mod tests { source.expect_get_accounts().with(always()).returning(|_| { Ok(vec![Account { id: "src_1".to_string(), - iban: "NL01".to_string(), + name: Some("Test Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -443,7 +451,8 @@ mod tests { source.expect_discover_accounts().returning(|| { Ok(vec![Account { id: "src_1".to_string(), - iban: "NL01".to_string(), + name: Some("Test Account".to_string()), + iban: Some("NL01".to_string()), currency: "EUR".to_string(), }]) }); @@ -474,7 +483,7 @@ mod tests { dest.expect_create_transaction().never(); dest.expect_update_transaction_external_id().never(); - let res = run_sync(source, dest, None, None, true).await; + let res = run_sync(source, dest, &AccountCache::default(), None, None, true).await; assert!(res.is_ok()); } } diff --git a/banks2ff/src/main.rs b/banks2ff/src/main.rs index 3e26741..586f62a 100644 --- a/banks2ff/src/main.rs +++ b/banks2ff/src/main.rs @@ -9,6 +9,7 @@ use crate::core::adapters::{ get_available_destinations, get_available_sources, is_valid_destination, is_valid_source, }; use crate::core::linking::LinkStore; +use crate::core::models::AccountData; use crate::core::ports::TransactionSource; use crate::core::sync::run_sync; use chrono::NaiveDate; @@ -224,7 +225,7 @@ async fn handle_sync( let context = AppContext::new(debug).await?; // Run sync - match run_sync(context.source, context.destination, start, end, dry_run).await { + match run_sync(context.source, context.destination, &context.account_cache, start, end, dry_run).await { Ok(result) => { info!("Sync completed successfully."); info!( @@ -324,6 +325,7 @@ async fn handle_transactions(subcommand: TransactionCommands) -> anyhow::Result< async fn handle_link(subcommand: LinkCommands) -> anyhow::Result<()> { let mut link_store = LinkStore::load(); + let account_cache = crate::adapters::gocardless::cache::AccountCache::load(); match subcommand { LinkCommands::List => { @@ -332,20 +334,12 @@ async fn handle_link(subcommand: LinkCommands) -> anyhow::Result<()> { } else { println!("Account Links:"); for link in &link_store.links { - let source_acc = link_store - .source_accounts - .get("gocardless") - .and_then(|m| m.get(&link.source_account_id)); - let dest_acc = link_store - .dest_accounts - .get("firefly") - .and_then(|m| m.get(&link.dest_account_id)); - let source_name = source_acc - .map(|a| format!("{} ({})", a.iban, a.id)) - .unwrap_or_else(|| link.source_account_id.clone()); - let dest_name = dest_acc - .map(|a| format!("{} ({})", a.iban, a.id)) - .unwrap_or_else(|| link.dest_account_id.clone()); + let source_name = account_cache + .get_display_name(&link.source_account_id) + .unwrap_or_else(|| format!("Account {}", &link.source_account_id)); + let dest_name = account_cache + .get_display_name(&link.dest_account_id) + .unwrap_or_else(|| format!("Account {}", &link.dest_account_id)); let alias_info = link .alias .as_ref() @@ -363,26 +357,40 @@ async fn handle_link(subcommand: LinkCommands) -> anyhow::Result<()> { dest_account, } => { // Assume source_account is gocardless id, dest_account is firefly id - let source_acc = link_store - .source_accounts - .get("gocardless") - .and_then(|m| m.get(&source_account)) - .cloned(); - let dest_acc = link_store - .dest_accounts - .get("firefly") - .and_then(|m| m.get(&dest_account)) - .cloned(); + let source_acc = account_cache.get_account(&source_account); + let dest_acc = account_cache.get_account(&dest_account); if let (Some(src), Some(dst)) = (source_acc, dest_acc) { - if let Some(link_id) = link_store.add_link(&src, &dst, false) { + // Create minimal Account structs for linking + let src_minimal = crate::core::models::Account { + id: src.id().to_string(), + name: Some(src.id().to_string()), // Use ID as name for linking + iban: src.iban().map(|s| s.to_string()), + currency: "EUR".to_string(), + }; + let dst_minimal = crate::core::models::Account { + id: dst.id().to_string(), + name: Some(dst.id().to_string()), // Use ID as name for linking + iban: dst.iban().map(|s| s.to_string()), + currency: "EUR".to_string(), + }; + + if let Some(link_id) = link_store.add_link(&src_minimal, &dst_minimal, false) { link_store.save()?; + let src_display = account_cache.get_display_name(&source_account) + .unwrap_or_else(|| source_account.clone()); + let dst_display = account_cache.get_display_name(&dest_account) + .unwrap_or_else(|| dest_account.clone()); println!( "Created link {} between {} and {}", - link_id, src.iban, dst.iban + link_id, src_display, dst_display ); } else { - println!("Link between {} and {} already exists", src.iban, dst.iban); + let src_display = account_cache.get_display_name(&source_account) + .unwrap_or_else(|| source_account.clone()); + let dst_display = account_cache.get_display_name(&dest_account) + .unwrap_or_else(|| dest_account.clone()); + println!("Link between {} and {} already exists", src_display, dst_display); } } else { println!("Account not found. Ensure accounts are discovered via sync first.");