From 90c115e0c0ad3ad0472386c3761d1524e7c4d499 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 6 Jun 2026 13:00:57 -0400 Subject: [PATCH] feat(balance): convert existing priced accounts to detailed (migration v16) (#211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v16 is a purely additive, guarded, atomic data migration (Bilan détail par titre, Étape 2). It converts each existing single-security priced account into a detailed account holding exactly one position, with zero data loss. 1. Mints one shared balance_securities row per priced account symbol (normalized UPPER(TRIM), deduped via ON CONFLICT(symbol) DO NOTHING on the COLLATE NOCASE UNIQUE), ONLY for accounts whose category carries a real asset_type — balance_securities.asset_type is NOT NULL, and a priced account whose category has asset_type IS NULL has no valid routing. 2. Mirrors each existing priced line into one holding (qty/unit_price/value/ price_source/price_fetched_at copied; book_cost stays NULL — no retroactive acquisition cost). UNIQUE(line, security) + ON CONFLICT DO NOTHING makes a re-run a strict no-op. 3. Collapses the now-redundant per-line qty/unit_price to NULL ONLY where a holding now exists (the security fix — a line that got no holding, i.e. priced-without-asset_type, is never NULLed, so no silent data loss). NULLing both columns together preserves the lines' (both NULL | both NOT NULL) CHECK. A trailing TEMP-table CHECK(ok = 1) asserts the invariant 'qty NULLed ⇒ has a holding' and ABORTS on breach, rolling back the whole v16 transaction (sqlx wraps each migration in a transaction). Priced accounts without asset_type or without a symbol are left fully intact. Integration tests (in-memory SQLite, apply v10→v16 via execute_batch, mirroring the #210 migration-test style): convertible account gains a security + holding with values/history preserved and its line qty NULLed; non-convertible priced account untouched (qty intact, no holding); re-run idempotent; injected-failure mid-v16 aborts on the guard and a transaction rollback restores the pre-v16 state (zero securities/holdings, quantity intact). Co-Authored-By: Claude Opus 4.8 (1M context) --- src-tauri/src/lib.rs | 393 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 393 insertions(+) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 0b11c7d..8cea36d 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -250,6 +250,68 @@ pub fn run() { SELECT id FROM balance_categories WHERE kind = 'priced');", kind: MigrationKind::Up, }, + // Migration v16 — Bilan détail par titre (Étape 2), data conversion part. + // Converts each existing single-security "priced" account into a + // `detailed` account holding exactly one position, with zero data loss. + // + // Step 1 mints a shared, deduped `balance_securities` row per priced + // account symbol (normalized UPPER(TRIM), deduped via ON CONFLICT(symbol) + // DO NOTHING — the symbol UNIQUE is COLLATE NOCASE so case-variants merge). + // Critically it converts ONLY accounts whose category carries a real + // `asset_type` (stock|crypto): balance_securities.asset_type is NOT NULL, + // and a priced account whose category has asset_type IS NULL has no valid + // routing — it must be left intact. + // + // Step 2 mirrors each existing priced line into one holding + // (quantity/unit_price/value/price_source/price_fetched_at copied verbatim; + // book_cost stays NULL — no retroactive acquisition cost for historical + // lines). UNIQUE(snapshot_line_id, security_id) + ON CONFLICT DO NOTHING + // makes a re-run a strict no-op. + // + // Step 3 collapses the now-redundant per-line qty/unit_price to NULL so a + // detailed account's line keeps only its total `value` (the per-title + // breakdown lives in holdings). The 🔴 security fix: this NULLing is scoped + // to `id IN (SELECT snapshot_line_id FROM balance_snapshot_holdings)` — a + // line that never received a holding (priced-without-asset_type) is never + // NULLed, so no silent data loss. NULLing BOTH qty+unit_price together + // preserves balance_snapshot_lines' (both NULL | both NOT NULL) CHECK. + // + // The trailing TEMP-table guard asserts the invariant "qty was NULLed ⇒ a + // holding exists" and ABORTS (CHECK(ok = 1) on an inserted 0) so any breach + // rolls back the whole v16 transaction. Idempotent: ON CONFLICT no-ops on + // re-run, and the guard re-passes since every NULLed line still has its + // holding. + Migration { + version: 16, + description: "convert existing priced accounts to detailed (one security + mirror holding)", + sql: "INSERT INTO balance_securities (symbol, currency, asset_type) \ + SELECT DISTINCT UPPER(TRIM(a.symbol)), a.currency, c.asset_type \ + FROM balance_accounts a \ + JOIN balance_categories c ON c.id = a.balance_category_id \ + WHERE a.symbol IS NOT NULL AND c.asset_type IS NOT NULL \ + ON CONFLICT(symbol) DO NOTHING; \ + INSERT INTO balance_snapshot_holdings \ + (snapshot_line_id, security_id, quantity, unit_price, value, price_source, price_fetched_at) \ + SELECT sl.id, s.id, sl.quantity, sl.unit_price, sl.value, sl.price_source, sl.price_fetched_at \ + FROM balance_snapshot_lines sl \ + JOIN balance_accounts a ON a.id = sl.account_id \ + JOIN balance_securities s ON s.symbol = UPPER(TRIM(a.symbol)) \ + WHERE a.symbol IS NOT NULL AND sl.quantity IS NOT NULL \ + ON CONFLICT(snapshot_line_id, security_id) DO NOTHING; \ + UPDATE balance_snapshot_lines \ + SET quantity = NULL, unit_price = NULL \ + WHERE quantity IS NOT NULL \ + AND id IN (SELECT snapshot_line_id FROM balance_snapshot_holdings); \ + CREATE TEMP TABLE _v16_guard (ok INTEGER CHECK (ok = 1)); \ + INSERT INTO _v16_guard(ok) SELECT CASE WHEN EXISTS ( \ + SELECT 1 FROM balance_snapshot_lines sl \ + JOIN balance_accounts a ON a.id = sl.account_id \ + WHERE a.symbol IS NOT NULL AND sl.quantity IS NULL \ + AND NOT EXISTS (SELECT 1 FROM balance_snapshot_holdings h WHERE h.snapshot_line_id = sl.id) \ + ) THEN 0 ELSE 1 END; \ + DROP TABLE _v16_guard;", + kind: MigrationKind::Up, + }, ]; tauri::Builder::default() @@ -2204,6 +2266,337 @@ mod tests { assert_eq!(kind, "simple", "kind must default to 'simple'"); } + // ========================================================================= + // Migration v16 — Bilan détail par titre (Étape 2) — issue #211 + // ------------------------------------------------------------------------- + // Data conversion: each existing single-security priced account becomes a + // detailed account with one security + one mirror holding, zero data loss. + // - Step 1: one balance_securities per priced account symbol (normalized, + // deduped), ONLY for accounts whose category has a real asset_type. + // - Step 2: one holding per priced line (qty/price/value/source mirrored). + // - Step 3: NULL the per-line qty/unit_price ONLY where a holding now + // exists (the 🔴 fix — never NULL a line that got no holding). + // - Trailing TEMP-table CHECK(ok = 1) aborts if any NULLed line lacks a + // holding, rolling back the whole v16 transaction. + // + // V16_SQL is statement-equivalent to the Migration { version: 16 } entry, + // kept in sync by hand (same pattern as V10..V15 above). + // ========================================================================= + + /// Production v16 SQL — kept in sync with the Migration { version: 16 } entry. + const V16_SQL: &str = "INSERT INTO balance_securities (symbol, currency, asset_type) \ + SELECT DISTINCT UPPER(TRIM(a.symbol)), a.currency, c.asset_type \ + FROM balance_accounts a \ + JOIN balance_categories c ON c.id = a.balance_category_id \ + WHERE a.symbol IS NOT NULL AND c.asset_type IS NOT NULL \ + ON CONFLICT(symbol) DO NOTHING; \ + INSERT INTO balance_snapshot_holdings \ + (snapshot_line_id, security_id, quantity, unit_price, value, price_source, price_fetched_at) \ + SELECT sl.id, s.id, sl.quantity, sl.unit_price, sl.value, sl.price_source, sl.price_fetched_at \ + FROM balance_snapshot_lines sl \ + JOIN balance_accounts a ON a.id = sl.account_id \ + JOIN balance_securities s ON s.symbol = UPPER(TRIM(a.symbol)) \ + WHERE a.symbol IS NOT NULL AND sl.quantity IS NOT NULL \ + ON CONFLICT(snapshot_line_id, security_id) DO NOTHING; \ + UPDATE balance_snapshot_lines \ + SET quantity = NULL, unit_price = NULL \ + WHERE quantity IS NOT NULL \ + AND id IN (SELECT snapshot_line_id FROM balance_snapshot_holdings); \ + CREATE TEMP TABLE _v16_guard (ok INTEGER CHECK (ok = 1)); \ + INSERT INTO _v16_guard(ok) SELECT CASE WHEN EXISTS ( \ + SELECT 1 FROM balance_snapshot_lines sl \ + JOIN balance_accounts a ON a.id = sl.account_id \ + WHERE a.symbol IS NOT NULL AND sl.quantity IS NULL \ + AND NOT EXISTS (SELECT 1 FROM balance_snapshot_holdings h WHERE h.snapshot_line_id = sl.id) \ + ) THEN 0 ELSE 1 END; \ + DROP TABLE _v16_guard;"; + + /// Build a realistic pre-v16 DB: full v10→v15 chain applied, then two priced + /// accounts seeded — one CONVERTIBLE (seed `stock` category, asset_type set) + /// and one NON-CONVERTIBLE (custom priced category with asset_type still + /// NULL) — each with one snapshot line. Returns (conn, convertible_line_id, + /// non_convertible_line_id). + fn db_pre_v16() -> (Connection, i64, i64) { + let conn = db_through_v13(); + conn.execute_batch(V14_SQL).expect("apply v14"); + conn.execute_batch(V15_SQL).expect("apply v15"); + + // (a) Convertible: seed `stock` category carries asset_type = 'stock' (v10). + let stock_cat: i64 = conn + .query_row( + "SELECT id FROM balance_categories WHERE key = 'stock'", + [], + |r| r.get(0), + ) + .unwrap(); + conn.execute( + "INSERT INTO balance_accounts (balance_category_id, name, symbol, kind) \ + VALUES (?1, 'Courtage AAPL', ' aapl ', 'detailed')", + [stock_cat], + ) + .unwrap(); + let acc_a: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + + // (b) Non-convertible: a custom priced category with NO asset_type. + conn.execute( + "INSERT INTO balance_categories (key, i18n_key, kind, sort_order, is_seed) \ + VALUES ('custom_priced', 'custom', 'priced', 80, 0)", + [], + ) + .unwrap(); + let custom_cat: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + conn.execute( + "INSERT INTO balance_accounts (balance_category_id, name, symbol, kind) \ + VALUES (?1, 'Courtage XYZ', 'XYZ', 'detailed')", + [custom_cat], + ) + .unwrap(); + let acc_b: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + + // One snapshot, one priced line per account. + conn.execute( + "INSERT INTO balance_snapshots (snapshot_date) VALUES ('2026-06-01')", + [], + ) + .unwrap(); + let snap_id: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + conn.execute( + "INSERT INTO balance_snapshot_lines \ + (snapshot_id, account_id, quantity, unit_price, value, price_source, price_fetched_at) \ + VALUES (?1, ?2, 10.0, 50.0, 500.0, 'maximus-api', '2026-06-01T12:00:00')", + rusqlite::params![snap_id, acc_a], + ) + .unwrap(); + let line_a: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + conn.execute( + "INSERT INTO balance_snapshot_lines \ + (snapshot_id, account_id, quantity, unit_price, value, price_source) \ + VALUES (?1, ?2, 3.0, 7.0, 21.0, 'manual')", + rusqlite::params![snap_id, acc_b], + ) + .unwrap(); + let line_b: i64 = conn + .query_row("SELECT last_insert_rowid()", [], |r| r.get(0)) + .unwrap(); + + (conn, line_a, line_b) + } + + #[test] + fn migration_v16_converts_priced_with_asset_type_and_preserves_values() { + let (conn, line_a, _line_b) = db_pre_v16(); + conn.execute_batch(V16_SQL).expect("apply v16"); + + // A security was minted from the normalized symbol (' aapl ' → 'AAPL'). + let sec_id: i64 = conn + .query_row( + "SELECT id FROM balance_securities WHERE symbol = 'AAPL'", + [], + |r| r.get(0), + ) + .expect("security AAPL must exist (normalized UPPER(TRIM))"); + let (sym, cur, at): (String, String, String) = conn + .query_row( + "SELECT symbol, currency, asset_type FROM balance_securities WHERE id = ?1", + [sec_id], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap(); + assert_eq!(sym, "AAPL"); + assert_eq!(cur, "CAD"); + assert_eq!(at, "stock"); + + // One mirror holding on line_a with the original qty/price/value/source. + let (h_qty, h_price, h_val, h_src, h_fetched, h_book): ( + f64, + f64, + f64, + String, + String, + Option, + ) = conn + .query_row( + "SELECT quantity, unit_price, value, price_source, price_fetched_at, book_cost \ + FROM balance_snapshot_holdings WHERE snapshot_line_id = ?1", + [line_a], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?, r.get(4)?, r.get(5)?)), + ) + .expect("a holding must mirror the converted line"); + assert_eq!(h_qty, 10.0); + assert_eq!(h_price, 50.0); + assert_eq!(h_val, 500.0); + assert_eq!(h_src, "maximus-api"); + assert_eq!(h_fetched, "2026-06-01T12:00:00"); + assert!(h_book.is_none(), "book_cost stays NULL for historical lines"); + assert_eq!( + h_qty * h_price, + h_val, + "holding value must equal quantity * unit_price" + ); + + // The aggregated line keeps its total value but qty/unit_price are NULLed. + let (l_qty, l_price, l_val): (Option, Option, f64) = conn + .query_row( + "SELECT quantity, unit_price, value FROM balance_snapshot_lines WHERE id = ?1", + [line_a], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap(); + assert!(l_qty.is_none(), "converted line quantity must be NULL"); + assert!(l_price.is_none(), "converted line unit_price must be NULL"); + assert_eq!(l_val, 500.0, "converted line value must be preserved"); + } + + #[test] + fn migration_v16_leaves_priced_without_asset_type_intact() { + let (conn, _line_a, line_b) = db_pre_v16(); + conn.execute_batch(V16_SQL).expect("apply v16"); + + // No security minted for the symbol 'XYZ' (its category had no asset_type). + let xyz_secs: i64 = conn + .query_row( + "SELECT COUNT(*) FROM balance_securities WHERE symbol = 'XYZ'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(xyz_secs, 0, "no security for a priced account without asset_type"); + + // No holding attached to line_b. + let holdings_b: i64 = conn + .query_row( + "SELECT COUNT(*) FROM balance_snapshot_holdings WHERE snapshot_line_id = ?1", + [line_b], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(holdings_b, 0, "non-convertible line must get no holding"); + + // line_b is left fully intact — qty/unit_price NOT NULLed (no silent loss). + let (q, p, v): (Option, Option, f64) = conn + .query_row( + "SELECT quantity, unit_price, value FROM balance_snapshot_lines WHERE id = ?1", + [line_b], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap(); + assert_eq!(q, Some(3.0), "intact line keeps its quantity"); + assert_eq!(p, Some(7.0), "intact line keeps its unit_price"); + assert_eq!(v, 21.0, "intact line keeps its value"); + } + + #[test] + fn migration_v16_is_idempotent_on_rerun() { + let (conn, line_a, _line_b) = db_pre_v16(); + conn.execute_batch(V16_SQL).expect("first v16"); + // Re-running must be a strict no-op (ON CONFLICT no-ops, guard re-passes). + conn.execute_batch(V16_SQL).expect("second v16 (idempotent)"); + + let n_secs: i64 = conn + .query_row( + "SELECT COUNT(*) FROM balance_securities WHERE symbol = 'AAPL'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(n_secs, 1, "re-run must not duplicate the security"); + let n_holdings: i64 = conn + .query_row( + "SELECT COUNT(*) FROM balance_snapshot_holdings WHERE snapshot_line_id = ?1", + [line_a], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(n_holdings, 1, "re-run must not duplicate the holding"); + } + + #[test] + fn migration_v16_guard_aborts_and_rolls_back_on_injected_failure() { + // Simulate a buggy v16 where the holdings-insert step is skipped: step 3 + // would NULL a line that got no holding, and the trailing guard must + // catch the broken invariant and ABORT. We run the corrupted SQL inside + // an explicit rusqlite transaction that is dropped (→ rolled back) on the + // returned error — exactly how sqlx rolls back a failed migration. SQLite + // ABORT resolution only undoes the offending statement, so the whole- + // migration safety relies on the surrounding transaction being rolled + // back (which this test asserts). + let (mut conn, line_a, _line_b) = db_pre_v16(); + + // The corrupted batch: step 1 + step 3 + guard, with step 2 (holdings) + // intentionally removed. NULLing now happens on lines with no holding. + const V16_CORRUPT: &str = "\ + INSERT INTO balance_securities (symbol, currency, asset_type) \ + SELECT DISTINCT UPPER(TRIM(a.symbol)), a.currency, c.asset_type \ + FROM balance_accounts a \ + JOIN balance_categories c ON c.id = a.balance_category_id \ + WHERE a.symbol IS NOT NULL AND c.asset_type IS NOT NULL \ + ON CONFLICT(symbol) DO NOTHING; \ + UPDATE balance_snapshot_lines SET quantity = NULL, unit_price = NULL \ + WHERE quantity IS NOT NULL; \ + CREATE TEMP TABLE _v16_guard (ok INTEGER CHECK (ok = 1)); \ + INSERT INTO _v16_guard(ok) SELECT CASE WHEN EXISTS ( \ + SELECT 1 FROM balance_snapshot_lines sl \ + JOIN balance_accounts a ON a.id = sl.account_id \ + WHERE a.symbol IS NOT NULL AND sl.quantity IS NULL \ + AND NOT EXISTS (SELECT 1 FROM balance_snapshot_holdings h WHERE h.snapshot_line_id = sl.id) \ + ) THEN 0 ELSE 1 END; \ + DROP TABLE _v16_guard;"; + + { + let tx = conn.transaction().expect("begin tx"); + let res = tx.execute_batch(V16_CORRUPT); + assert!( + res.is_err(), + "the CHECK(ok = 1) guard must abort when a NULLed line lacks a holding" + ); + // tx dropped here without commit → ROLLBACK (mirrors sqlx on error). + } + + // Rollback left the DB at its pre-v16 state: zero holdings, qty intact. + let total_holdings: i64 = conn + .query_row("SELECT COUNT(*) FROM balance_snapshot_holdings", [], |r| r.get(0)) + .unwrap(); + assert_eq!(total_holdings, 0, "rollback must leave zero holdings"); + let (q, p): (Option, Option) = conn + .query_row( + "SELECT quantity, unit_price FROM balance_snapshot_lines WHERE id = ?1", + [line_a], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .unwrap(); + assert_eq!(q, Some(10.0), "rollback must leave quantity intact"); + assert_eq!(p, Some(50.0), "rollback must leave unit_price intact"); + // And no securities were committed either. + let total_secs: i64 = conn + .query_row("SELECT COUNT(*) FROM balance_securities", [], |r| r.get(0)) + .unwrap(); + assert_eq!(total_secs, 0, "rollback must leave zero securities"); + } + + #[test] + fn migration_v16_full_chain_applies_cleanly() { + // The whole v10→v16 chain on a fresh DB with no priced accounts is a + // no-op for v16 and must apply without error (guard passes vacuously). + let conn = db_through_v13(); + conn.execute_batch(V14_SQL).expect("apply v14"); + conn.execute_batch(V15_SQL).expect("apply v15"); + conn.execute_batch(V16_SQL).expect("apply v16 on empty data"); + let secs: i64 = conn + .query_row("SELECT COUNT(*) FROM balance_securities", [], |r| r.get(0)) + .unwrap(); + assert_eq!(secs, 0, "no securities when there are no priced accounts"); + } + // ------------------------------------------------------------------------- // Consolidated schema (new profiles) — issue #202 // ------------------------------------------------------------------------- -- 2.45.2