From 8006e5e1f6373aae39d3c38068d694e142bc85a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elisi=C3=A1rio=20Couto?= Date: Wed, 10 Sep 2025 00:22:45 +0100 Subject: [PATCH] refactor: remove unused hide_missing_ids functionality - Remove hide_missing_ids parameter from all database functions - Remove hide_missing_ids from API routes and query parameters - Remove hide_missing_ids filtering logic from SQLite queries - Update all tests to remove hide_missing_ids assertions - Clean up codebase since internalTransactionId extraction is now fixed This functionality was added as a workaround for missing internalTransactionId values, but we've now fixed the root cause by properly extracting transaction IDs from raw data during sync, making this workaround unnecessary. --- frontend/package.json | 2 +- leggen/database/sqlite.py | 11 --- leggen/utils/database.py | 14 ++- leggend/api/routes/transactions.py | 9 -- leggend/services/database_service.py | 131 +++++++++++++++++++++++++-- tests/unit/test_api_transactions.py | 1 - tests/unit/test_database_service.py | 7 +- 7 files changed, 138 insertions(+), 37 deletions(-) diff --git a/frontend/package.json b/frontend/package.json index f2bf6dd..44f6006 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -4,7 +4,7 @@ "version": "0.0.0", "type": "module", "scripts": { - "dev": "vite", + "dev": "VITE_API_URL=http://localhost:8000/api/v1 vite", "build": "tsc -b && vite build", "lint": "eslint .", "preview": "vite preview" diff --git a/leggen/database/sqlite.py b/leggen/database/sqlite.py index 010d389..af31f9c 100644 --- a/leggen/database/sqlite.py +++ b/leggen/database/sqlite.py @@ -210,7 +210,6 @@ def get_transactions( min_amount=None, max_amount=None, search=None, - hide_missing_ids=True, ): """Get transactions from SQLite database with optional filtering""" from pathlib import Path @@ -250,11 +249,6 @@ def get_transactions( query += " AND description LIKE ?" params.append(f"%{search}%") - if hide_missing_ids: - query += ( - " AND internalTransactionId IS NOT NULL AND internalTransactionId != ''" - ) - # Add ordering and pagination query += " ORDER BY transactionDate DESC" @@ -403,11 +397,6 @@ def get_transaction_count(account_id=None, **filters): query += " AND description LIKE ?" params.append(f"%{filters['search']}%") - if filters.get("hide_missing_ids", True): - query += ( - " AND internalTransactionId IS NOT NULL AND internalTransactionId != ''" - ) - try: cursor.execute(query, params) count = cursor.fetchone()[0] diff --git a/leggen/utils/database.py b/leggen/utils/database.py index c1ce9e2..a0a68b7 100644 --- a/leggen/utils/database.py +++ b/leggen/utils/database.py @@ -69,8 +69,13 @@ def save_transactions(ctx: click.Context, account: str) -> list: ",".join(transaction.get("remittanceInformationUnstructuredArray", [])), ) + # Extract transaction ID, using transactionId as fallback when internalTransactionId is missing + transaction_id = transaction.get("internalTransactionId") or transaction.get( + "transactionId" + ) + t = { - "internalTransactionId": transaction.get("internalTransactionId"), + "internalTransactionId": transaction_id, "institutionId": account_info["institution_id"], "iban": account_info.get("iban", "N/A"), "transactionDate": min_date, @@ -105,8 +110,13 @@ def save_transactions(ctx: click.Context, account: str) -> list: ",".join(transaction.get("remittanceInformationUnstructuredArray", [])), ) + # Extract transaction ID, using transactionId as fallback when internalTransactionId is missing + transaction_id = transaction.get("internalTransactionId") or transaction.get( + "transactionId" + ) + t = { - "internalTransactionId": transaction.get("internalTransactionId"), + "internalTransactionId": transaction_id, "institutionId": account_info["institution_id"], "iban": account_info.get("iban", "N/A"), "transactionDate": min_date, diff --git a/leggend/api/routes/transactions.py b/leggend/api/routes/transactions.py index 799cd23..701bf3b 100644 --- a/leggend/api/routes/transactions.py +++ b/leggend/api/routes/transactions.py @@ -18,9 +18,6 @@ async def get_all_transactions( summary_only: bool = Query( default=True, description="Return transaction summaries only" ), - hide_missing_ids: bool = Query( - default=True, description="Hide transactions without internalTransactionId" - ), date_from: Optional[str] = Query( default=None, description="Filter from date (YYYY-MM-DD)" ), @@ -50,7 +47,6 @@ async def get_all_transactions( min_amount=min_amount, max_amount=max_amount, search=search, - hide_missing_ids=hide_missing_ids, ) # Get total count for pagination info (respecting the same filters) @@ -61,7 +57,6 @@ async def get_all_transactions( min_amount=min_amount, max_amount=max_amount, search=search, - hide_missing_ids=hide_missing_ids, ) # Get total count for pagination info @@ -126,9 +121,6 @@ async def get_all_transactions( async def get_transaction_stats( days: int = Query(default=30, description="Number of days to include in stats"), account_id: Optional[str] = Query(default=None, description="Filter by account ID"), - hide_missing_ids: bool = Query( - default=True, description="Hide transactions without internalTransactionId" - ), ) -> APIResponse: """Get transaction statistics for the last N days from database""" try: @@ -146,7 +138,6 @@ async def get_transaction_stats( date_from=date_from, date_to=date_to, limit=None, # Get all matching transactions for stats - hide_missing_ids=hide_missing_ids, ) # Calculate stats diff --git a/leggend/services/database_service.py b/leggend/services/database_service.py index 00af994..7b0bfad 100644 --- a/leggend/services/database_service.py +++ b/leggend/services/database_service.py @@ -93,8 +93,13 @@ class DatabaseService: ",".join(transaction.get("remittanceInformationUnstructuredArray", [])), ) + # Extract transaction ID, using transactionId as fallback when internalTransactionId is missing + transaction_id = transaction.get("internalTransactionId") or transaction.get( + "transactionId" + ) + return { - "internalTransactionId": transaction.get("internalTransactionId"), + "internalTransactionId": transaction_id, "institutionId": account_info["institution_id"], "iban": account_info.get("iban", "N/A"), "transactionDate": min_date, @@ -116,7 +121,6 @@ class DatabaseService: min_amount: Optional[float] = None, max_amount: Optional[float] = None, search: Optional[str] = None, - hide_missing_ids: bool = True, ) -> List[Dict[str, Any]]: """Get transactions from SQLite database""" if not self.sqlite_enabled: @@ -133,7 +137,6 @@ class DatabaseService: min_amount=min_amount, max_amount=max_amount, search=search, - hide_missing_ids=hide_missing_ids, ) logger.debug(f"Retrieved {len(transactions)} transactions from database") return transactions @@ -149,7 +152,6 @@ class DatabaseService: min_amount: Optional[float] = None, max_amount: Optional[float] = None, search: Optional[str] = None, - hide_missing_ids: bool = True, ) -> int: """Get total count of transactions from SQLite database""" if not self.sqlite_enabled: @@ -166,9 +168,7 @@ class DatabaseService: # Remove None values filters = {k: v for k, v in filters.items() if v is not None} - count = sqlite_db.get_transaction_count( - account_id=account_id, hide_missing_ids=hide_missing_ids, **filters - ) + count = sqlite_db.get_transaction_count(account_id=account_id, **filters) logger.debug(f"Total transaction count: {count}") return count except Exception as e: @@ -259,6 +259,7 @@ class DatabaseService: return await self._migrate_balance_timestamps_if_needed() + await self._migrate_null_transaction_ids_if_needed() async def _migrate_balance_timestamps_if_needed(self): """Check and migrate balance timestamps if needed""" @@ -379,6 +380,122 @@ class DatabaseService: logger.error(f"Balance timestamp migration failed: {e}") raise + async def _migrate_null_transaction_ids_if_needed(self): + """Check and migrate null transaction IDs if needed""" + try: + if await self._check_null_transaction_ids_migration_needed(): + logger.info("Null transaction IDs migration needed, starting...") + await self._migrate_null_transaction_ids() + logger.info("Null transaction IDs migration completed") + else: + logger.info("No null transaction IDs found to migrate") + except Exception as e: + logger.error(f"Null transaction IDs migration failed: {e}") + raise + + async def _check_null_transaction_ids_migration_needed(self) -> bool: + """Check if null transaction IDs need migration""" + from pathlib import Path + + db_path = Path.home() / ".config" / "leggen" / "leggen.db" + if not db_path.exists(): + return False + + try: + conn = sqlite3.connect(str(db_path)) + cursor = conn.cursor() + + # Check for transactions with null or empty internalTransactionId + cursor.execute(""" + SELECT COUNT(*) + FROM transactions + WHERE (internalTransactionId IS NULL OR internalTransactionId = '') + AND json_extract(rawTransaction, '$.transactionId') IS NOT NULL + """) + + count = cursor.fetchone()[0] + conn.close() + + return count > 0 + + except Exception as e: + logger.error(f"Failed to check null transaction IDs migration status: {e}") + return False + + async def _migrate_null_transaction_ids(self): + """Populate null internalTransactionId fields using transactionId from raw data""" + from pathlib import Path + + db_path = Path.home() / ".config" / "leggen" / "leggen.db" + if not db_path.exists(): + logger.warning("Database file not found, skipping migration") + return + + try: + conn = sqlite3.connect(str(db_path)) + cursor = conn.cursor() + + # Get all transactions with null/empty internalTransactionId but valid transactionId in raw data + cursor.execute(""" + SELECT rowid, json_extract(rawTransaction, '$.transactionId') as transactionId + FROM transactions + WHERE (internalTransactionId IS NULL OR internalTransactionId = '') + AND json_extract(rawTransaction, '$.transactionId') IS NOT NULL + ORDER BY rowid + """) + + null_records = cursor.fetchall() + total_records = len(null_records) + + if total_records == 0: + logger.info("No null transaction IDs found to migrate") + conn.close() + return + + logger.info( + f"Migrating {total_records} transaction records with null internalTransactionId" + ) + + # Update in batches + batch_size = 100 + migrated_count = 0 + + for i in range(0, total_records, batch_size): + batch = null_records[i : i + batch_size] + + for rowid, transaction_id in batch: + try: + # Update the record with the transactionId from raw data + cursor.execute( + """ + UPDATE transactions + SET internalTransactionId = ? + WHERE rowid = ? + """, + (str(transaction_id), rowid), + ) + + migrated_count += 1 + + if migrated_count % 100 == 0: + logger.info( + f"Migrated {migrated_count}/{total_records} transaction records" + ) + + except Exception as e: + logger.error(f"Failed to migrate record {rowid}: {e}") + continue + + # Commit batch + conn.commit() + + conn.close() + logger.info(f"Successfully migrated {migrated_count} transaction records") + + except Exception as e: + logger.error(f"Null transaction IDs migration failed: {e}") + raise + def _unix_to_datetime_string(self, unix_timestamp: float) -> str: """Convert Unix timestamp to datetime string""" dt = datetime.fromtimestamp(unix_timestamp) diff --git a/tests/unit/test_api_transactions.py b/tests/unit/test_api_transactions.py index 4e9b1db..bb99740 100644 --- a/tests/unit/test_api_transactions.py +++ b/tests/unit/test_api_transactions.py @@ -166,7 +166,6 @@ class TestTransactionsAPI: min_amount=-50.0, max_amount=0.0, search="Coffee", - hide_missing_ids=True, ) def test_get_transactions_empty_result( diff --git a/tests/unit/test_database_service.py b/tests/unit/test_database_service.py index 7bf3393..1494725 100644 --- a/tests/unit/test_database_service.py +++ b/tests/unit/test_database_service.py @@ -99,7 +99,6 @@ class TestDatabaseService: min_amount=None, max_amount=None, search=None, - hide_missing_ids=True, ) async def test_get_transactions_from_db_with_filters( @@ -130,7 +129,6 @@ class TestDatabaseService: min_amount=-50.0, max_amount=0.0, search="Coffee", - hide_missing_ids=True, ) async def test_get_transactions_from_db_sqlite_disabled(self, database_service): @@ -160,9 +158,7 @@ class TestDatabaseService: ) assert result == 42 - mock_get_count.assert_called_once_with( - account_id="test-account-123", hide_missing_ids=True - ) + mock_get_count.assert_called_once_with(account_id="test-account-123") async def test_get_transaction_count_from_db_with_filters(self, database_service): """Test getting transaction count with filters.""" @@ -182,7 +178,6 @@ class TestDatabaseService: date_from="2025-09-01", min_amount=-100.0, search="Coffee", - hide_missing_ids=True, ) async def test_get_transaction_count_from_db_sqlite_disabled(