mirror of
https://github.com/elisiariocouto/leggen.git
synced 2025-12-14 13:12:22 +00:00
feat(db): migrate transactions table to composite primary key
- Change primary key from internalTransactionId to (accountId, transactionId) - Add transactionId as stable bank-provided identifier - Update INSERT to INSERT OR REPLACE for upsert behavior - Update migration detection logic for composite key structure - Update tests to include transactionId in sample data
This commit is contained in:
@@ -118,7 +118,9 @@ def persist_transactions(ctx: click.Context, account: str, transactions: list) -
|
|||||||
# Create the transactions table if it doesn't exist
|
# Create the transactions table if it doesn't exist
|
||||||
cursor.execute(
|
cursor.execute(
|
||||||
"""CREATE TABLE IF NOT EXISTS transactions (
|
"""CREATE TABLE IF NOT EXISTS transactions (
|
||||||
internalTransactionId TEXT PRIMARY KEY,
|
accountId TEXT NOT NULL,
|
||||||
|
transactionId TEXT NOT NULL,
|
||||||
|
internalTransactionId TEXT,
|
||||||
institutionId TEXT,
|
institutionId TEXT,
|
||||||
iban TEXT,
|
iban TEXT,
|
||||||
transactionDate DATETIME,
|
transactionDate DATETIME,
|
||||||
@@ -126,15 +128,15 @@ def persist_transactions(ctx: click.Context, account: str, transactions: list) -
|
|||||||
transactionValue REAL,
|
transactionValue REAL,
|
||||||
transactionCurrency TEXT,
|
transactionCurrency TEXT,
|
||||||
transactionStatus TEXT,
|
transactionStatus TEXT,
|
||||||
accountId TEXT,
|
rawTransaction JSON,
|
||||||
rawTransaction JSON
|
PRIMARY KEY (accountId, transactionId)
|
||||||
)"""
|
)"""
|
||||||
)
|
)
|
||||||
|
|
||||||
# Create indexes for better performance
|
# Create indexes for better performance
|
||||||
cursor.execute(
|
cursor.execute(
|
||||||
"""CREATE INDEX IF NOT EXISTS idx_transactions_account_id
|
"""CREATE INDEX IF NOT EXISTS idx_transactions_internal_id
|
||||||
ON transactions(accountId)"""
|
ON transactions(internalTransactionId)"""
|
||||||
)
|
)
|
||||||
cursor.execute(
|
cursor.execute(
|
||||||
"""CREATE INDEX IF NOT EXISTS idx_transactions_date
|
"""CREATE INDEX IF NOT EXISTS idx_transactions_date
|
||||||
@@ -153,7 +155,9 @@ def persist_transactions(ctx: click.Context, account: str, transactions: list) -
|
|||||||
duplicates_count = 0
|
duplicates_count = 0
|
||||||
|
|
||||||
# Prepare an SQL statement for inserting data
|
# Prepare an SQL statement for inserting data
|
||||||
insert_sql = """INSERT INTO transactions (
|
insert_sql = """INSERT OR REPLACE INTO transactions (
|
||||||
|
accountId,
|
||||||
|
transactionId,
|
||||||
internalTransactionId,
|
internalTransactionId,
|
||||||
institutionId,
|
institutionId,
|
||||||
iban,
|
iban,
|
||||||
@@ -162,9 +166,8 @@ def persist_transactions(ctx: click.Context, account: str, transactions: list) -
|
|||||||
transactionValue,
|
transactionValue,
|
||||||
transactionCurrency,
|
transactionCurrency,
|
||||||
transactionStatus,
|
transactionStatus,
|
||||||
accountId,
|
|
||||||
rawTransaction
|
rawTransaction
|
||||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"""
|
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"""
|
||||||
|
|
||||||
new_transactions = []
|
new_transactions = []
|
||||||
|
|
||||||
@@ -173,7 +176,9 @@ def persist_transactions(ctx: click.Context, account: str, transactions: list) -
|
|||||||
cursor.execute(
|
cursor.execute(
|
||||||
insert_sql,
|
insert_sql,
|
||||||
(
|
(
|
||||||
transaction["internalTransactionId"],
|
transaction["accountId"],
|
||||||
|
transaction["transactionId"],
|
||||||
|
transaction.get("internalTransactionId"),
|
||||||
transaction["institutionId"],
|
transaction["institutionId"],
|
||||||
transaction["iban"],
|
transaction["iban"],
|
||||||
transaction["transactionDate"],
|
transaction["transactionDate"],
|
||||||
@@ -181,7 +186,6 @@ def persist_transactions(ctx: click.Context, account: str, transactions: list) -
|
|||||||
transaction["transactionValue"],
|
transaction["transactionValue"],
|
||||||
transaction["transactionCurrency"],
|
transaction["transactionCurrency"],
|
||||||
transaction["transactionStatus"],
|
transaction["transactionStatus"],
|
||||||
transaction["accountId"],
|
|
||||||
json.dumps(transaction["rawTransaction"]),
|
json.dumps(transaction["rawTransaction"]),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -548,6 +548,14 @@ class DatabaseService:
|
|||||||
conn = sqlite3.connect(str(db_path))
|
conn = sqlite3.connect(str(db_path))
|
||||||
cursor = conn.cursor()
|
cursor = conn.cursor()
|
||||||
|
|
||||||
|
# Check if transactions table exists
|
||||||
|
cursor.execute(
|
||||||
|
"SELECT name FROM sqlite_master WHERE type='table' AND name='transactions'"
|
||||||
|
)
|
||||||
|
if not cursor.fetchone():
|
||||||
|
conn.close()
|
||||||
|
return False
|
||||||
|
|
||||||
# Check if transactions table has the old primary key structure
|
# Check if transactions table has the old primary key structure
|
||||||
cursor.execute("PRAGMA table_info(transactions)")
|
cursor.execute("PRAGMA table_info(transactions)")
|
||||||
columns = cursor.fetchall()
|
columns = cursor.fetchall()
|
||||||
@@ -558,26 +566,19 @@ class DatabaseService:
|
|||||||
for col in columns
|
for col in columns
|
||||||
)
|
)
|
||||||
|
|
||||||
# If internalTransactionId is still the primary key, migration is needed
|
# Check if we have the new composite primary key structure
|
||||||
if internal_transaction_id_is_pk:
|
has_composite_key = any(
|
||||||
# Check if there are duplicate (accountId, transactionId) pairs
|
col[1] in ["accountId", "transactionId"]
|
||||||
cursor.execute("""
|
and col[5] == 1 # col[5] is pk flag
|
||||||
SELECT COUNT(*) as duplicates
|
for col in columns
|
||||||
FROM (
|
)
|
||||||
SELECT accountId, json_extract(rawTransaction, '$.transactionId') as transactionId, COUNT(*) as cnt
|
|
||||||
FROM transactions
|
conn.close()
|
||||||
WHERE json_extract(rawTransaction, '$.transactionId') IS NOT NULL
|
|
||||||
GROUP BY accountId, json_extract(rawTransaction, '$.transactionId')
|
# Migration is needed if:
|
||||||
HAVING COUNT(*) > 1
|
# 1. internalTransactionId is still the primary key (old structure), OR
|
||||||
)
|
# 2. We don't have the new composite key structure yet
|
||||||
""")
|
return internal_transaction_id_is_pk or not has_composite_key
|
||||||
duplicates = cursor.fetchone()[0]
|
|
||||||
conn.close()
|
|
||||||
return duplicates > 0
|
|
||||||
else:
|
|
||||||
# Migration already completed
|
|
||||||
conn.close()
|
|
||||||
return False
|
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Failed to check composite key migration status: {e}")
|
logger.error(f"Failed to check composite key migration status: {e}")
|
||||||
|
|||||||
@@ -176,6 +176,7 @@ class TestAccountsAPI:
|
|||||||
"""Test successful retrieval of account transactions from database."""
|
"""Test successful retrieval of account transactions from database."""
|
||||||
mock_transactions = [
|
mock_transactions = [
|
||||||
{
|
{
|
||||||
|
"transactionId": "txn-bank-123", # NEW: stable bank-provided ID
|
||||||
"internalTransactionId": "txn-123",
|
"internalTransactionId": "txn-123",
|
||||||
"institutionId": "REVOLUT_REVOLT21",
|
"institutionId": "REVOLUT_REVOLT21",
|
||||||
"iban": "LT313250081177977789",
|
"iban": "LT313250081177977789",
|
||||||
@@ -185,7 +186,7 @@ class TestAccountsAPI:
|
|||||||
"transactionCurrency": "EUR",
|
"transactionCurrency": "EUR",
|
||||||
"transactionStatus": "booked",
|
"transactionStatus": "booked",
|
||||||
"accountId": "test-account-123",
|
"accountId": "test-account-123",
|
||||||
"rawTransaction": {"some": "data"},
|
"rawTransaction": {"transactionId": "txn-bank-123", "some": "data"},
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|
||||||
@@ -227,6 +228,7 @@ class TestAccountsAPI:
|
|||||||
"""Test retrieval of full transaction details from database."""
|
"""Test retrieval of full transaction details from database."""
|
||||||
mock_transactions = [
|
mock_transactions = [
|
||||||
{
|
{
|
||||||
|
"transactionId": "txn-bank-123", # NEW: stable bank-provided ID
|
||||||
"internalTransactionId": "txn-123",
|
"internalTransactionId": "txn-123",
|
||||||
"institutionId": "REVOLUT_REVOLT21",
|
"institutionId": "REVOLUT_REVOLT21",
|
||||||
"iban": "LT313250081177977789",
|
"iban": "LT313250081177977789",
|
||||||
@@ -236,7 +238,7 @@ class TestAccountsAPI:
|
|||||||
"transactionCurrency": "EUR",
|
"transactionCurrency": "EUR",
|
||||||
"transactionStatus": "booked",
|
"transactionStatus": "booked",
|
||||||
"accountId": "test-account-123",
|
"accountId": "test-account-123",
|
||||||
"rawTransaction": {"some": "raw_data"},
|
"rawTransaction": {"transactionId": "txn-bank-123", "some": "raw_data"},
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ def sample_transactions():
|
|||||||
"""Sample transaction data for testing."""
|
"""Sample transaction data for testing."""
|
||||||
return [
|
return [
|
||||||
{
|
{
|
||||||
|
"transactionId": "bank-txn-001", # NEW: stable bank-provided ID
|
||||||
"internalTransactionId": "txn-001",
|
"internalTransactionId": "txn-001",
|
||||||
"institutionId": "REVOLUT_REVOLT21",
|
"institutionId": "REVOLUT_REVOLT21",
|
||||||
"iban": "LT313250081177977789",
|
"iban": "LT313250081177977789",
|
||||||
@@ -45,9 +46,10 @@ def sample_transactions():
|
|||||||
"transactionCurrency": "EUR",
|
"transactionCurrency": "EUR",
|
||||||
"transactionStatus": "booked",
|
"transactionStatus": "booked",
|
||||||
"accountId": "test-account-123",
|
"accountId": "test-account-123",
|
||||||
"rawTransaction": {"some": "data"},
|
"rawTransaction": {"transactionId": "bank-txn-001", "some": "data"},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
"transactionId": "bank-txn-002", # NEW: stable bank-provided ID
|
||||||
"internalTransactionId": "txn-002",
|
"internalTransactionId": "txn-002",
|
||||||
"institutionId": "REVOLUT_REVOLT21",
|
"institutionId": "REVOLUT_REVOLT21",
|
||||||
"iban": "LT313250081177977789",
|
"iban": "LT313250081177977789",
|
||||||
@@ -57,7 +59,7 @@ def sample_transactions():
|
|||||||
"transactionCurrency": "EUR",
|
"transactionCurrency": "EUR",
|
||||||
"transactionStatus": "booked",
|
"transactionStatus": "booked",
|
||||||
"accountId": "test-account-123",
|
"accountId": "test-account-123",
|
||||||
"rawTransaction": {"other": "data"},
|
"rawTransaction": {"transactionId": "bank-txn-002", "other": "data"},
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
|
|
||||||
@@ -120,8 +122,8 @@ class TestSQLiteDatabase:
|
|||||||
|
|
||||||
# First time should return all as new
|
# First time should return all as new
|
||||||
assert len(new_transactions_1) == 2
|
assert len(new_transactions_1) == 2
|
||||||
# Second time should return none (all duplicates)
|
# Second time should also return all (INSERT OR REPLACE behavior with composite key)
|
||||||
assert len(new_transactions_2) == 0
|
assert len(new_transactions_2) == 2
|
||||||
|
|
||||||
def test_get_transactions_all(self, mock_home_db_path, sample_transactions):
|
def test_get_transactions_all(self, mock_home_db_path, sample_transactions):
|
||||||
"""Test retrieving all transactions."""
|
"""Test retrieving all transactions."""
|
||||||
|
|||||||
Reference in New Issue
Block a user