mirror of
https://github.com/elisiariocouto/leggen.git
synced 2025-12-13 11:22:21 +00:00
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.
This commit is contained in:
@@ -4,7 +4,7 @@
|
|||||||
"version": "0.0.0",
|
"version": "0.0.0",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"dev": "vite",
|
"dev": "VITE_API_URL=http://localhost:8000/api/v1 vite",
|
||||||
"build": "tsc -b && vite build",
|
"build": "tsc -b && vite build",
|
||||||
"lint": "eslint .",
|
"lint": "eslint .",
|
||||||
"preview": "vite preview"
|
"preview": "vite preview"
|
||||||
|
|||||||
@@ -210,7 +210,6 @@ def get_transactions(
|
|||||||
min_amount=None,
|
min_amount=None,
|
||||||
max_amount=None,
|
max_amount=None,
|
||||||
search=None,
|
search=None,
|
||||||
hide_missing_ids=True,
|
|
||||||
):
|
):
|
||||||
"""Get transactions from SQLite database with optional filtering"""
|
"""Get transactions from SQLite database with optional filtering"""
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -250,11 +249,6 @@ def get_transactions(
|
|||||||
query += " AND description LIKE ?"
|
query += " AND description LIKE ?"
|
||||||
params.append(f"%{search}%")
|
params.append(f"%{search}%")
|
||||||
|
|
||||||
if hide_missing_ids:
|
|
||||||
query += (
|
|
||||||
" AND internalTransactionId IS NOT NULL AND internalTransactionId != ''"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Add ordering and pagination
|
# Add ordering and pagination
|
||||||
query += " ORDER BY transactionDate DESC"
|
query += " ORDER BY transactionDate DESC"
|
||||||
|
|
||||||
@@ -403,11 +397,6 @@ def get_transaction_count(account_id=None, **filters):
|
|||||||
query += " AND description LIKE ?"
|
query += " AND description LIKE ?"
|
||||||
params.append(f"%{filters['search']}%")
|
params.append(f"%{filters['search']}%")
|
||||||
|
|
||||||
if filters.get("hide_missing_ids", True):
|
|
||||||
query += (
|
|
||||||
" AND internalTransactionId IS NOT NULL AND internalTransactionId != ''"
|
|
||||||
)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
cursor.execute(query, params)
|
cursor.execute(query, params)
|
||||||
count = cursor.fetchone()[0]
|
count = cursor.fetchone()[0]
|
||||||
|
|||||||
@@ -69,8 +69,13 @@ def save_transactions(ctx: click.Context, account: str) -> list:
|
|||||||
",".join(transaction.get("remittanceInformationUnstructuredArray", [])),
|
",".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 = {
|
t = {
|
||||||
"internalTransactionId": transaction.get("internalTransactionId"),
|
"internalTransactionId": transaction_id,
|
||||||
"institutionId": account_info["institution_id"],
|
"institutionId": account_info["institution_id"],
|
||||||
"iban": account_info.get("iban", "N/A"),
|
"iban": account_info.get("iban", "N/A"),
|
||||||
"transactionDate": min_date,
|
"transactionDate": min_date,
|
||||||
@@ -105,8 +110,13 @@ def save_transactions(ctx: click.Context, account: str) -> list:
|
|||||||
",".join(transaction.get("remittanceInformationUnstructuredArray", [])),
|
",".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 = {
|
t = {
|
||||||
"internalTransactionId": transaction.get("internalTransactionId"),
|
"internalTransactionId": transaction_id,
|
||||||
"institutionId": account_info["institution_id"],
|
"institutionId": account_info["institution_id"],
|
||||||
"iban": account_info.get("iban", "N/A"),
|
"iban": account_info.get("iban", "N/A"),
|
||||||
"transactionDate": min_date,
|
"transactionDate": min_date,
|
||||||
|
|||||||
@@ -18,9 +18,6 @@ async def get_all_transactions(
|
|||||||
summary_only: bool = Query(
|
summary_only: bool = Query(
|
||||||
default=True, description="Return transaction summaries only"
|
default=True, description="Return transaction summaries only"
|
||||||
),
|
),
|
||||||
hide_missing_ids: bool = Query(
|
|
||||||
default=True, description="Hide transactions without internalTransactionId"
|
|
||||||
),
|
|
||||||
date_from: Optional[str] = Query(
|
date_from: Optional[str] = Query(
|
||||||
default=None, description="Filter from date (YYYY-MM-DD)"
|
default=None, description="Filter from date (YYYY-MM-DD)"
|
||||||
),
|
),
|
||||||
@@ -50,7 +47,6 @@ async def get_all_transactions(
|
|||||||
min_amount=min_amount,
|
min_amount=min_amount,
|
||||||
max_amount=max_amount,
|
max_amount=max_amount,
|
||||||
search=search,
|
search=search,
|
||||||
hide_missing_ids=hide_missing_ids,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
# Get total count for pagination info (respecting the same filters)
|
# Get total count for pagination info (respecting the same filters)
|
||||||
@@ -61,7 +57,6 @@ async def get_all_transactions(
|
|||||||
min_amount=min_amount,
|
min_amount=min_amount,
|
||||||
max_amount=max_amount,
|
max_amount=max_amount,
|
||||||
search=search,
|
search=search,
|
||||||
hide_missing_ids=hide_missing_ids,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
# Get total count for pagination info
|
# Get total count for pagination info
|
||||||
@@ -126,9 +121,6 @@ async def get_all_transactions(
|
|||||||
async def get_transaction_stats(
|
async def get_transaction_stats(
|
||||||
days: int = Query(default=30, description="Number of days to include in 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"),
|
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:
|
) -> APIResponse:
|
||||||
"""Get transaction statistics for the last N days from database"""
|
"""Get transaction statistics for the last N days from database"""
|
||||||
try:
|
try:
|
||||||
@@ -146,7 +138,6 @@ async def get_transaction_stats(
|
|||||||
date_from=date_from,
|
date_from=date_from,
|
||||||
date_to=date_to,
|
date_to=date_to,
|
||||||
limit=None, # Get all matching transactions for stats
|
limit=None, # Get all matching transactions for stats
|
||||||
hide_missing_ids=hide_missing_ids,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
# Calculate stats
|
# Calculate stats
|
||||||
|
|||||||
@@ -93,8 +93,13 @@ class DatabaseService:
|
|||||||
",".join(transaction.get("remittanceInformationUnstructuredArray", [])),
|
",".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 {
|
return {
|
||||||
"internalTransactionId": transaction.get("internalTransactionId"),
|
"internalTransactionId": transaction_id,
|
||||||
"institutionId": account_info["institution_id"],
|
"institutionId": account_info["institution_id"],
|
||||||
"iban": account_info.get("iban", "N/A"),
|
"iban": account_info.get("iban", "N/A"),
|
||||||
"transactionDate": min_date,
|
"transactionDate": min_date,
|
||||||
@@ -116,7 +121,6 @@ class DatabaseService:
|
|||||||
min_amount: Optional[float] = None,
|
min_amount: Optional[float] = None,
|
||||||
max_amount: Optional[float] = None,
|
max_amount: Optional[float] = None,
|
||||||
search: Optional[str] = None,
|
search: Optional[str] = None,
|
||||||
hide_missing_ids: bool = True,
|
|
||||||
) -> List[Dict[str, Any]]:
|
) -> List[Dict[str, Any]]:
|
||||||
"""Get transactions from SQLite database"""
|
"""Get transactions from SQLite database"""
|
||||||
if not self.sqlite_enabled:
|
if not self.sqlite_enabled:
|
||||||
@@ -133,7 +137,6 @@ class DatabaseService:
|
|||||||
min_amount=min_amount,
|
min_amount=min_amount,
|
||||||
max_amount=max_amount,
|
max_amount=max_amount,
|
||||||
search=search,
|
search=search,
|
||||||
hide_missing_ids=hide_missing_ids,
|
|
||||||
)
|
)
|
||||||
logger.debug(f"Retrieved {len(transactions)} transactions from database")
|
logger.debug(f"Retrieved {len(transactions)} transactions from database")
|
||||||
return transactions
|
return transactions
|
||||||
@@ -149,7 +152,6 @@ class DatabaseService:
|
|||||||
min_amount: Optional[float] = None,
|
min_amount: Optional[float] = None,
|
||||||
max_amount: Optional[float] = None,
|
max_amount: Optional[float] = None,
|
||||||
search: Optional[str] = None,
|
search: Optional[str] = None,
|
||||||
hide_missing_ids: bool = True,
|
|
||||||
) -> int:
|
) -> int:
|
||||||
"""Get total count of transactions from SQLite database"""
|
"""Get total count of transactions from SQLite database"""
|
||||||
if not self.sqlite_enabled:
|
if not self.sqlite_enabled:
|
||||||
@@ -166,9 +168,7 @@ class DatabaseService:
|
|||||||
# Remove None values
|
# Remove None values
|
||||||
filters = {k: v for k, v in filters.items() if v is not None}
|
filters = {k: v for k, v in filters.items() if v is not None}
|
||||||
|
|
||||||
count = sqlite_db.get_transaction_count(
|
count = sqlite_db.get_transaction_count(account_id=account_id, **filters)
|
||||||
account_id=account_id, hide_missing_ids=hide_missing_ids, **filters
|
|
||||||
)
|
|
||||||
logger.debug(f"Total transaction count: {count}")
|
logger.debug(f"Total transaction count: {count}")
|
||||||
return count
|
return count
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -259,6 +259,7 @@ class DatabaseService:
|
|||||||
return
|
return
|
||||||
|
|
||||||
await self._migrate_balance_timestamps_if_needed()
|
await self._migrate_balance_timestamps_if_needed()
|
||||||
|
await self._migrate_null_transaction_ids_if_needed()
|
||||||
|
|
||||||
async def _migrate_balance_timestamps_if_needed(self):
|
async def _migrate_balance_timestamps_if_needed(self):
|
||||||
"""Check and migrate balance timestamps if needed"""
|
"""Check and migrate balance timestamps if needed"""
|
||||||
@@ -379,6 +380,122 @@ class DatabaseService:
|
|||||||
logger.error(f"Balance timestamp migration failed: {e}")
|
logger.error(f"Balance timestamp migration failed: {e}")
|
||||||
raise
|
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:
|
def _unix_to_datetime_string(self, unix_timestamp: float) -> str:
|
||||||
"""Convert Unix timestamp to datetime string"""
|
"""Convert Unix timestamp to datetime string"""
|
||||||
dt = datetime.fromtimestamp(unix_timestamp)
|
dt = datetime.fromtimestamp(unix_timestamp)
|
||||||
|
|||||||
@@ -166,7 +166,6 @@ class TestTransactionsAPI:
|
|||||||
min_amount=-50.0,
|
min_amount=-50.0,
|
||||||
max_amount=0.0,
|
max_amount=0.0,
|
||||||
search="Coffee",
|
search="Coffee",
|
||||||
hide_missing_ids=True,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_get_transactions_empty_result(
|
def test_get_transactions_empty_result(
|
||||||
|
|||||||
@@ -99,7 +99,6 @@ class TestDatabaseService:
|
|||||||
min_amount=None,
|
min_amount=None,
|
||||||
max_amount=None,
|
max_amount=None,
|
||||||
search=None,
|
search=None,
|
||||||
hide_missing_ids=True,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
async def test_get_transactions_from_db_with_filters(
|
async def test_get_transactions_from_db_with_filters(
|
||||||
@@ -130,7 +129,6 @@ class TestDatabaseService:
|
|||||||
min_amount=-50.0,
|
min_amount=-50.0,
|
||||||
max_amount=0.0,
|
max_amount=0.0,
|
||||||
search="Coffee",
|
search="Coffee",
|
||||||
hide_missing_ids=True,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
async def test_get_transactions_from_db_sqlite_disabled(self, database_service):
|
async def test_get_transactions_from_db_sqlite_disabled(self, database_service):
|
||||||
@@ -160,9 +158,7 @@ class TestDatabaseService:
|
|||||||
)
|
)
|
||||||
|
|
||||||
assert result == 42
|
assert result == 42
|
||||||
mock_get_count.assert_called_once_with(
|
mock_get_count.assert_called_once_with(account_id="test-account-123")
|
||||||
account_id="test-account-123", hide_missing_ids=True
|
|
||||||
)
|
|
||||||
|
|
||||||
async def test_get_transaction_count_from_db_with_filters(self, database_service):
|
async def test_get_transaction_count_from_db_with_filters(self, database_service):
|
||||||
"""Test getting transaction count with filters."""
|
"""Test getting transaction count with filters."""
|
||||||
@@ -182,7 +178,6 @@ class TestDatabaseService:
|
|||||||
date_from="2025-09-01",
|
date_from="2025-09-01",
|
||||||
min_amount=-100.0,
|
min_amount=-100.0,
|
||||||
search="Coffee",
|
search="Coffee",
|
||||||
hide_missing_ids=True,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
async def test_get_transaction_count_from_db_sqlite_disabled(
|
async def test_get_transaction_count_from_db_sqlite_disabled(
|
||||||
|
|||||||
Reference in New Issue
Block a user