From b9ca74e7e67c3877728b749a42f15f0c0d906561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elisi=C3=A1rio=20Couto?= Date: Wed, 24 Sep 2025 19:57:03 +0100 Subject: [PATCH] feat(api): Add bank logo support and fix banks endpoint type errors. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend changes: - Add logo field to AccountDetails model - Update accounts API endpoints to include logo data - Add database migration for logo column in accounts table - Implement institution details fetching from GoCardless API - Enrich account data with institution logos during sync - Fix type errors in banks endpoint with proper response parsing Frontend changes: - Add failedImages state to track logo loading failures - Implement conditional rendering to show bank logos when available - Add proper error handling with fallback to Building2 icon - Fix image sizing to w-6 h-6 sm:w-8 sm:h-8 for proper display - Update Account interface to include optional logo field - Remove unused useState import from System component 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- .gitignore | 1 + .mcp.json | 4 +- frontend/src/components/AccountSettings.tsx | 17 ++++- frontend/src/components/Settings.tsx | 17 ++++- frontend/src/components/System.tsx | 1 - frontend/src/types/api.ts | 1 + leggen/api/models/accounts.py | 1 + leggen/api/routes/accounts.py | 2 + leggen/api/routes/banks.py | 7 +- leggen/services/database_service.py | 83 ++++++++++++++++++++- leggen/services/gocardless_service.py | 17 +++-- leggen/services/sync_service.py | 23 +++++- 12 files changed, 154 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index ce32d5e..0ea4d02 100644 --- a/.gitignore +++ b/.gitignore @@ -165,3 +165,4 @@ leggen.db *.db config.toml .claude/ +.playwright-mcp/ diff --git a/.mcp.json b/.mcp.json index 29a0d33..771f770 100644 --- a/.mcp.json +++ b/.mcp.json @@ -7,10 +7,10 @@ "mcp" ] }, - "browsermcp": { + "playwright": { "command": "npx", "args": [ - "@browsermcp/mcp@latest" + "@playwright/mcp@latest" ] } } diff --git a/frontend/src/components/AccountSettings.tsx b/frontend/src/components/AccountSettings.tsx index a367efb..e1f7665 100644 --- a/frontend/src/components/AccountSettings.tsx +++ b/frontend/src/components/AccountSettings.tsx @@ -78,6 +78,7 @@ export default function AccountSettings() { const [editingAccountId, setEditingAccountId] = useState(null); const [editingName, setEditingName] = useState(""); + const [failedImages, setFailedImages] = useState>(new Set()); const queryClient = useQueryClient(); @@ -194,8 +195,20 @@ export default function AccountSettings() { {/* Mobile layout - stack vertically */}
-
- +
+ {account.logo && !failedImages.has(account.id) ? ( + {`${account.institution_id} { + console.warn(`Failed to load bank logo for ${account.institution_id}: ${account.logo}`); + setFailedImages(prev => new Set([...prev, account.id])); + }} + /> + ) : ( + + )}
{editingAccountId === account.id ? ( diff --git a/frontend/src/components/Settings.tsx b/frontend/src/components/Settings.tsx index ebc8d8f..fafd7fb 100644 --- a/frontend/src/components/Settings.tsx +++ b/frontend/src/components/Settings.tsx @@ -79,6 +79,7 @@ const getStatusIndicator = (status: string) => { export default function Settings() { const [editingAccountId, setEditingAccountId] = useState(null); const [editingName, setEditingName] = useState(""); + const [failedImages, setFailedImages] = useState>(new Set()); const queryClient = useQueryClient(); @@ -280,8 +281,20 @@ export default function Settings() { {/* Mobile layout - stack vertically */}
-
- +
+ {account.logo && !failedImages.has(account.id) ? ( + {`${account.institution_id} { + console.warn(`Failed to load bank logo for ${account.institution_id}: ${account.logo}`); + setFailedImages(prev => new Set([...prev, account.id])); + }} + /> + ) : ( + + )}
{editingAccountId === account.id ? ( diff --git a/frontend/src/components/System.tsx b/frontend/src/components/System.tsx index 31ef097..a81e993 100644 --- a/frontend/src/components/System.tsx +++ b/frontend/src/components/System.tsx @@ -1,5 +1,4 @@ import { useQuery } from "@tanstack/react-query"; -import { useState } from "react"; import { RefreshCw, AlertCircle, diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts index 12365e4..d40597a 100644 --- a/frontend/src/types/api.ts +++ b/frontend/src/types/api.ts @@ -13,6 +13,7 @@ export interface Account { name?: string; display_name?: string; currency?: string; + logo?: string; created: string; last_accessed?: string; balances: AccountBalance[]; diff --git a/leggen/api/models/accounts.py b/leggen/api/models/accounts.py index 52c35e6..82e7262 100644 --- a/leggen/api/models/accounts.py +++ b/leggen/api/models/accounts.py @@ -26,6 +26,7 @@ class AccountDetails(BaseModel): name: Optional[str] = None display_name: Optional[str] = None currency: Optional[str] = None + logo: Optional[str] = None created: datetime last_accessed: Optional[datetime] = None balances: List[AccountBalance] = [] diff --git a/leggen/api/routes/accounts.py b/leggen/api/routes/accounts.py index 6db2c5b..2f79bcd 100644 --- a/leggen/api/routes/accounts.py +++ b/leggen/api/routes/accounts.py @@ -55,6 +55,7 @@ async def get_all_accounts() -> APIResponse: name=db_account.get("name"), display_name=db_account.get("display_name"), currency=db_account.get("currency"), + logo=db_account.get("logo"), created=db_account["created"], last_accessed=db_account.get("last_accessed"), balances=balances, @@ -115,6 +116,7 @@ async def get_account_details(account_id: str) -> APIResponse: name=db_account.get("name"), display_name=db_account.get("display_name"), currency=db_account.get("currency"), + logo=db_account.get("logo"), created=db_account["created"], last_accessed=db_account.get("last_accessed"), balances=balances, diff --git a/leggen/api/routes/banks.py b/leggen/api/routes/banks.py index d767723..03b5b52 100644 --- a/leggen/api/routes/banks.py +++ b/leggen/api/routes/banks.py @@ -21,18 +21,19 @@ async def get_bank_institutions( ) -> APIResponse: """Get available bank institutions for a country""" try: - institutions_data = await gocardless_service.get_institutions(country) + institutions_response = await gocardless_service.get_institutions(country) + institutions_data = institutions_response.get("results", []) institutions = [ BankInstitution( id=inst["id"], name=inst["name"], bic=inst.get("bic"), - transaction_total_days=inst["transaction_total_days"], + transaction_total_days=int(inst["transaction_total_days"]), countries=inst["countries"], logo=inst.get("logo"), ) - for inst in institutions_data.get("results", []) + for inst in institutions_data ] return APIResponse( diff --git a/leggen/services/database_service.py b/leggen/services/database_service.py index 73b533f..c8f977b 100644 --- a/leggen/services/database_service.py +++ b/leggen/services/database_service.py @@ -217,6 +217,7 @@ class DatabaseService: await self._migrate_to_composite_key_if_needed() await self._migrate_add_display_name_if_needed() await self._migrate_add_sync_operations_if_needed() + await self._migrate_add_logo_if_needed() async def _migrate_balance_timestamps_if_needed(self): """Check and migrate balance timestamps if needed""" @@ -1133,7 +1134,8 @@ class DatabaseService: created DATETIME, last_accessed DATETIME, last_updated DATETIME, - display_name TEXT + display_name TEXT, + logo TEXT )""" ) @@ -1170,8 +1172,9 @@ class DatabaseService: created, last_accessed, last_updated, - display_name - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", + display_name, + logo + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", ( account_data["id"], account_data["institution_id"], @@ -1183,6 +1186,7 @@ class DatabaseService: account_data.get("last_accessed"), account_data.get("last_updated", account_data["created"]), display_name, + account_data.get("logo"), ), ) conn.commit() @@ -1516,6 +1520,79 @@ class DatabaseService: logger.error(f"Sync operations table migration failed: {e}") raise + async def _migrate_add_logo_if_needed(self): + """Check and add logo column to accounts table if needed""" + try: + if await self._check_logo_migration_needed(): + logger.info("Logo column migration needed, starting...") + await self._migrate_add_logo() + logger.info("Logo column migration completed") + else: + logger.info("Logo column already exists") + except Exception as e: + logger.error(f"Logo column migration failed: {e}") + raise + + async def _check_logo_migration_needed(self) -> bool: + """Check if logo column needs to be added to accounts table""" + db_path = path_manager.get_database_path() + if not db_path.exists(): + return False + + try: + conn = sqlite3.connect(str(db_path)) + cursor = conn.cursor() + + # Check if accounts table exists + cursor.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='accounts'" + ) + if not cursor.fetchone(): + conn.close() + return False + + # Check if logo column exists + cursor.execute("PRAGMA table_info(accounts)") + columns = cursor.fetchall() + + # Check if logo column exists + has_logo = any(col[1] == "logo" for col in columns) + + conn.close() + return not has_logo + + except Exception as e: + logger.error(f"Failed to check logo migration status: {e}") + return False + + async def _migrate_add_logo(self): + """Add logo column to accounts table""" + db_path = path_manager.get_database_path() + if not db_path.exists(): + logger.warning("Database file not found, skipping migration") + return + + try: + conn = sqlite3.connect(str(db_path)) + cursor = conn.cursor() + + logger.info("Adding logo column to accounts table...") + + # Add the logo column + cursor.execute(""" + ALTER TABLE accounts + ADD COLUMN logo TEXT + """) + + conn.commit() + conn.close() + + logger.info("Logo column migration completed successfully") + + except Exception as e: + logger.error(f"Logo column migration failed: {e}") + raise + async def persist_sync_operation(self, sync_operation: Dict[str, Any]) -> int: """Persist sync operation to database and return the ID""" if not self.sqlite_enabled: diff --git a/leggen/services/gocardless_service.py b/leggen/services/gocardless_service.py index 14a3c09..43cd541 100644 --- a/leggen/services/gocardless_service.py +++ b/leggen/services/gocardless_service.py @@ -11,14 +11,13 @@ from leggen.utils.paths import path_manager def _log_rate_limits(response): """Log GoCardless API rate limit headers""" - limit = response.headers.get("X-RateLimit-Limit") - remaining = response.headers.get("X-RateLimit-Remaining") - reset = response.headers.get("X-RateLimit-Reset") - account_success_reset = response.headers.get("X-RateLimit-Account-Success-Reset") + limit = response.headers.get("http_x_ratelimit_limit") + remaining = response.headers.get("http_x_ratelimit_remaining") + reset = response.headers.get("http_x_ratelimit_reset") - if limit or remaining or reset or account_success_reset: + if limit or remaining or reset: logger.info( - f"GoCardless rate limits - Limit: {limit}, Remaining: {remaining}, Reset: {reset}s, Account Success Reset: {account_success_reset}" + f"GoCardless rate limits - Limit: {limit}, Remaining: {remaining}, Reset: {reset}s" ) @@ -162,3 +161,9 @@ class GoCardlessService: return await self._make_authenticated_request( "GET", f"{self.base_url}/accounts/{account_id}/transactions/" ) + + async def get_institution_details(self, institution_id: str) -> Dict[str, Any]: + """Get institution details by ID""" + return await self._make_authenticated_request( + "GET", f"{self.base_url}/institutions/{institution_id}/" + ) diff --git a/leggen/services/sync_service.py b/leggen/services/sync_service.py index 5e03a4f..0030374 100644 --- a/leggen/services/sync_service.py +++ b/leggen/services/sync_service.py @@ -15,6 +15,7 @@ class SyncService: self.database = DatabaseService() self.notifications = NotificationService() self._sync_status = SyncStatus(is_running=False) + self._institution_logos = {} # Cache for institution logos async def get_sync_status(self) -> SyncStatus: """Get current sync status""" @@ -77,7 +78,7 @@ class SyncService: # Get balances to extract currency information balances = await self.gocardless.get_account_balances(account_id) - # Enrich account details with currency and persist + # Enrich account details with currency and institution logo if account_details and balances: enriched_account_details = account_details.copy() @@ -90,6 +91,26 @@ class SyncService: if currency: enriched_account_details["currency"] = currency + # Get institution details to fetch logo + institution_id = enriched_account_details.get("institution_id") + if institution_id: + try: + institution_details = ( + await self.gocardless.get_institution_details( + institution_id + ) + ) + enriched_account_details["logo"] = ( + institution_details.get("logo", "") + ) + logger.info( + f"Fetched logo for institution {institution_id}: {enriched_account_details.get('logo', 'No logo')}" + ) + except Exception as e: + logger.warning( + f"Failed to fetch institution details for {institution_id}: {e}" + ) + # Persist enriched account details to database await self.database.persist_account_details( enriched_account_details