From bd1085c1483dc1ca12c3ca735daf4d5eb45da99c Mon Sep 17 00:00:00 2001 From: le king fu Date: Thu, 13 Aug 2026 12:56:28 -0400 Subject: [PATCH] schema: add migration v17 carrying the full import format on import_sources `import_sources` carried only the mechanical CSV settings. The two fields that decide how an amount is READ -- `amount_mode` and `sign_convention` -- lived only on `import_config_templates`. That asymmetry is the root bug of this chantier: restoring a saved source re-inferred the mode from the mapping and hardcoded `signConvention: "negative_expense"` (useImportWizard.ts:321-323), so a source configured with positive expenses silently flipped back on its second import. After v17 both tables carry the same eight format fields. v17 is strictly additive -- v1 to v16 are untouched, and the diff is pure insertion. The four columns are defaulted or nullable so the ALTERs are safe on a populated database: - amount_mode / sign_convention carry a CHECK, same pattern as v15 on balance_accounts.kind. amount_mode admits 'absolute_indicator' from the start so the third amount mode ships without another migration, while the database still refuses a corrupted value today. - header_signature stores the normalized header labels seen at the last successful import, for drift detection. - template_id is a provenance tag only, never re-read as format: the eight source columns are authoritative, so editing a template changes no linked source. ON DELETE SET NULL keeps the source and its format when a template goes. The backfill reproduces exactly the rule the wizard applied on the fly, so no source changes behaviour on migration. It tests `column_mapping LIKE '%debitAmount%'` rather than json_extract, so the migration depends on no JSON1 extension in the bundled SQLite. sign_convention is deliberately not backfilled: its DEFAULT restores precisely the value the code hardcoded, the only past convention that can be inferred. The four columns are mirrored into consolidated_schema.sql. They are inert there on the production path -- that script runs after every migration and only uses CREATE TABLE IF NOT EXISTS, so new profiles receive them from v17 -- but it stays the tested reference definition, and a parity test now compares it against the v1->v17 chain column by column, DEFAULT by DEFAULT, CHECK by CHECK and FK by FK. Both halves of that test were mutation-checked to confirm they fail on drift. 5 new tests (111 Rust total): v17 on a populated v16 database with a child row, the backfill against 5 mapping shapes, the CHECKs, the provenance-tag semantics, and the consolidated parity. Resolves #323 --- .../src/database/consolidated_schema.sql | 16 + src-tauri/src/lib.rs | 507 ++++++++++++++++++ 2 files changed, 523 insertions(+) diff --git a/src-tauri/src/database/consolidated_schema.sql b/src-tauri/src/database/consolidated_schema.sql index 17f5503..44725bf 100644 --- a/src-tauri/src/database/consolidated_schema.sql +++ b/src-tauri/src/database/consolidated_schema.sql @@ -15,6 +15,22 @@ CREATE TABLE IF NOT EXISTS import_sources ( column_mapping TEXT NOT NULL, skip_lines INTEGER NOT NULL DEFAULT 0, has_header INTEGER NOT NULL DEFAULT 1, + -- Full import format (migration v17). These four columns are INERT on the + -- production path: this script runs AFTER tauri-plugin-sql has applied every + -- migration and only uses CREATE TABLE IF NOT EXISTS, so a brand-new profile + -- actually receives them from v17. They are mirrored here to keep this file + -- the tested reference definition -- a parity test compares it against the + -- v1->v17 chain so the DEFAULT and CHECK of the two cannot drift apart. + -- `amount_mode` admits 'absolute_indicator' from the start so the third mode + -- can ship without another migration. `template_id` is a provenance tag + -- only, never re-read as format: the eight format columns above are + -- authoritative, so editing a template changes no linked source. + amount_mode TEXT NOT NULL DEFAULT 'single' + CHECK (amount_mode IN ('single','debit_credit','absolute_indicator')), + sign_convention TEXT NOT NULL DEFAULT 'negative_expense' + CHECK (sign_convention IN ('negative_expense','positive_expense')), + header_signature TEXT, + template_id INTEGER REFERENCES import_config_templates(id) ON DELETE SET NULL, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 4549108..c02d769 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -313,6 +313,58 @@ pub fn run() { DROP TABLE _v16_guard;", kind: MigrationKind::Up, }, + // Migration v17 — the full import format on the sources themselves (#323). + // + // Until now `import_sources` carried only the mechanical CSV settings + // (delimiter, encoding, date format, column mapping). The two fields + // that decide how an amount is READ — `amount_mode` and + // `sign_convention` — existed only on `import_config_templates`. That + // asymmetry is the root bug of this chantier: restoring a saved source + // re-inferred the mode from the mapping and hardcoded + // `signConvention: "negative_expense"` (`useImportWizard.ts:321-323`), + // so a source configured with positive expenses silently flipped back + // on its second import. After v17 both tables carry the same eight + // format fields and the asymmetry is gone structurally. + // + // All four columns are additive and either defaulted or nullable, so + // the ALTERs are safe on a populated v16 database: + // - `amount_mode` / `sign_convention` carry a CHECK, same pattern as + // the v15 `balance_accounts.kind`. `amount_mode` admits + // 'absolute_indicator' from the start: that third mode (absolute + // amount + a D/C indicator column) is out of scope today but must + // be implementable without another migration, and the constraint + // still refuses a corrupted value right now. + // - `header_signature` stores the normalized header labels seen at + // the last successful import, for drift detection. NULL until an + // import records one, and permanently NULL for headerless files. + // - `template_id` is a PROVENANCE TAG only, never re-read as format: + // the eight columns of the source are authoritative, so editing a + // template must not change any linked source. ON DELETE SET NULL + // keeps the source and its format intact when a template is deleted. + // + // The backfill reproduces EXACTLY the rule the wizard applies on the + // fly today (`useImportWizard.ts:321` — a mapping carrying + // `debitAmount` means debit/credit, anything else means single amount), + // so no existing source changes behaviour on migration. The test is + // `LIKE '%debitAmount%'` rather than `json_extract` so the migration + // depends on no JSON1 extension in the bundled SQLite. `sign_convention` + // is deliberately NOT backfilled: its DEFAULT restores precisely the + // value the code hardcoded, which is the only past convention that can + // be inferred — guessing anything else would silently rewrite meaning. + Migration { + version: 17, + description: "add amount_mode, sign_convention, header_signature and template_id to import_sources", + sql: "ALTER TABLE import_sources ADD COLUMN amount_mode TEXT NOT NULL DEFAULT 'single' \ + CHECK (amount_mode IN ('single','debit_credit','absolute_indicator')); \ + ALTER TABLE import_sources ADD COLUMN sign_convention TEXT NOT NULL DEFAULT 'negative_expense' \ + CHECK (sign_convention IN ('negative_expense','positive_expense')); \ + ALTER TABLE import_sources ADD COLUMN header_signature TEXT; \ + ALTER TABLE import_sources ADD COLUMN template_id INTEGER \ + REFERENCES import_config_templates(id) ON DELETE SET NULL; \ + UPDATE import_sources SET amount_mode = 'debit_credit' \ + WHERE column_mapping LIKE '%debitAmount%';", + kind: MigrationKind::Up, + }, ]; tauri::Builder::default() @@ -3415,5 +3467,460 @@ mod tests { .unwrap(); assert_eq!(xyz_secs, 0, "no security for a priced account without asset_type"); } + + // ========================================================================= + // Migration v17 — the full import format on import_sources (#323) + // ------------------------------------------------------------------------- + // What these tests guarantee: + // - v17 applies on a POPULATED v16 database with zero loss: every existing + // source keeps its identity and its mechanical CSV settings, and the + // child rows that reference it survive the ALTERs. + // - the backfill reproduces EXACTLY the rule the wizard applied on the fly + // (`useImportWizard.ts:321`), so no source changes behaviour on + // migration — a mapping carrying `debitAmount` becomes 'debit_credit', + // everything else stays 'single', and `sign_convention` lands on the + // value the code used to hardcode. + // - the two CHECKs really bite (an unknown enum value is refused, and + // 'absolute_indicator' is already admitted so the third amount mode + // needs no further migration). + // - `template_id` is a provenance tag: deleting the template it points at + // leaves the source alive with a NULL tag, and editing a template + // changes no linked source's format. + // - consolidated_schema.sql and the v1→v17 chain agree column for column, + // DEFAULT for DEFAULT, CHECK for CHECK, FK for FK. + // + // "v1..v16 unchanged" is NOT asserted here: no checksum harness exists in + // this crate (V10_SQL..V17_SQL are hand-kept copies, and the real checksums + // only live at runtime in `_sqlx_migrations`). It is verified by the diff — + // v17 adds strictly new lines and touches no earlier migration string. + // ========================================================================= + + /// Production v3 SQL — kept in sync with the Migration { version: 3 } entry. + const V3_SQL: &str = + "ALTER TABLE import_sources ADD COLUMN has_header INTEGER NOT NULL DEFAULT 1;"; + + /// Production v5 SQL — kept in sync with the Migration { version: 5 } entry. + const V5_SQL: &str = "CREATE TABLE IF NOT EXISTS import_config_templates ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL UNIQUE, + delimiter TEXT NOT NULL DEFAULT ';', + encoding TEXT NOT NULL DEFAULT 'utf-8', + date_format TEXT NOT NULL DEFAULT 'DD/MM/YYYY', + skip_lines INTEGER NOT NULL DEFAULT 0, + has_header INTEGER NOT NULL DEFAULT 1, + column_mapping TEXT NOT NULL, + amount_mode TEXT NOT NULL DEFAULT 'single', + sign_convention TEXT NOT NULL DEFAULT 'negative_expense', + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP + );"; + + /// Production v17 SQL — kept in sync with the Migration { version: 17 } entry. + const V17_SQL: &str = "ALTER TABLE import_sources ADD COLUMN amount_mode TEXT NOT NULL DEFAULT 'single' \ + CHECK (amount_mode IN ('single','debit_credit','absolute_indicator')); \ + ALTER TABLE import_sources ADD COLUMN sign_convention TEXT NOT NULL DEFAULT 'negative_expense' \ + CHECK (sign_convention IN ('negative_expense','positive_expense')); \ + ALTER TABLE import_sources ADD COLUMN header_signature TEXT; \ + ALTER TABLE import_sources ADD COLUMN template_id INTEGER \ + REFERENCES import_config_templates(id) ON DELETE SET NULL; \ + UPDATE import_sources SET amount_mode = 'debit_credit' \ + WHERE column_mapping LIKE '%debitAmount%';"; + + /// Build the pre-v17 state of the two tables v17 touches. `import_sources` + /// and `import_config_templates` are shaped by exactly three migrations — + /// v1 (creation), v3 (`has_header`) and v5 (the templates table). None of + /// v2, v4 and v6→v16 alters either table, so this IS their v16 shape; the + /// parity test below re-proves it by comparing the result against the + /// consolidated reference definition. + fn db_pre_v17() -> Connection { + let conn = Connection::open_in_memory().expect("open in-memory db"); + conn.execute("PRAGMA foreign_keys = ON;", []) + .expect("enable FKs"); + conn.execute_batch(crate::database::SCHEMA) + .expect("apply v1 SCHEMA"); + conn.execute_batch(V3_SQL).expect("apply v3"); + conn.execute_batch(V5_SQL).expect("apply v5"); + conn + } + + /// Insert a source and return its id. Only `name` and `column_mapping` have + /// no default at v16, so everything else is left to the schema. + fn seed_source(conn: &Connection, name: &str, mapping: &str) -> i64 { + conn.execute( + "INSERT INTO import_sources (name, column_mapping) VALUES (?1, ?2)", + rusqlite::params![name, mapping], + ) + .unwrap(); + conn.last_insert_rowid() + } + + /// (name, type, notnull, dflt_value) for every column of `table`, sorted by + /// name. Sorting drops physical column order on purpose: ALTER TABLE can + /// only append, while a CREATE TABLE places a column where it reads best — + /// the order is not part of the contract, the definitions are. + fn column_shape(conn: &Connection, table: &str) -> Vec<(String, String, i64, Option)> { + let mut cols: Vec<(String, String, i64, Option)> = conn + .prepare(&format!( + "SELECT name, type, \"notnull\", dflt_value FROM pragma_table_info('{table}')" + )) + .unwrap() + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?))) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + cols.sort(); + cols + } + + /// (referenced table, from column, to column, ON DELETE action) per FK. + fn fk_shape(conn: &Connection, table: &str) -> Vec<(String, String, String, String)> { + let mut fks: Vec<(String, String, String, String)> = conn + .prepare(&format!( + "SELECT \"table\", \"from\", \"to\", on_delete FROM pragma_foreign_key_list('{table}')" + )) + .unwrap() + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?))) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + fks.sort(); + fks + } + + #[test] + fn migration_v17_applies_on_a_populated_v16_db() { + let conn = db_pre_v17(); + + // A realistic populated v16 profile: a configured source, a template and + // an imported file hanging off the source by FK. + let src = seed_source( + &conn, + "Desjardins", + r#"{"date":"Date","description":"Description","amount":"Montant"}"#, + ); + conn.execute( + "UPDATE import_sources SET delimiter = ',', encoding = 'windows-1252', \ + date_format = '%Y-%m-%d', skip_lines = 3, has_header = 0 WHERE id = ?1", + [src], + ) + .unwrap(); + conn.execute( + "INSERT INTO import_config_templates (name, column_mapping) VALUES ('Modèle A', '{}')", + [], + ) + .unwrap(); + conn.execute( + "INSERT INTO imported_files (source_id, filename, file_hash) \ + VALUES (?1, 'janvier.csv', 'deadbeef')", + [src], + ) + .unwrap(); + + conn.execute_batch(V17_SQL).expect("apply v17 on a v16 db"); + + // The four columns landed. + let cols: Vec = column_shape(&conn, "import_sources") + .into_iter() + .map(|(n, _, _, _)| n) + .collect(); + for expected in &[ + "amount_mode", + "sign_convention", + "header_signature", + "template_id", + ] { + assert!( + cols.contains(&expected.to_string()), + "v17 must add {expected} to import_sources" + ); + } + + // Nothing the source already carried was touched. + let (name, delim, enc, fmt, skip, header, mapping): ( + String, + String, + String, + String, + i64, + i64, + String, + ) = conn + .query_row( + "SELECT name, delimiter, encoding, date_format, skip_lines, has_header, \ + column_mapping FROM import_sources WHERE id = ?1", + [src], + |r| { + Ok(( + r.get(0)?, + r.get(1)?, + r.get(2)?, + r.get(3)?, + r.get(4)?, + r.get(5)?, + r.get(6)?, + )) + }, + ) + .unwrap(); + assert_eq!(name, "Desjardins"); + assert_eq!(delim, ","); + assert_eq!(enc, "windows-1252"); + assert_eq!(fmt, "%Y-%m-%d"); + assert_eq!(skip, 3); + assert_eq!(header, 0); + assert_eq!( + mapping, + r#"{"date":"Date","description":"Description","amount":"Montant"}"# + ); + + // The new columns land on their defaults: a source that never carried a + // format now carries the exact one the code applied to it. + let (mode, sign, sig, tpl): (String, String, Option, Option) = conn + .query_row( + "SELECT amount_mode, sign_convention, header_signature, template_id \ + FROM import_sources WHERE id = ?1", + [src], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?, r.get(3)?)), + ) + .unwrap(); + assert_eq!(mode, "single"); + assert_eq!(sign, "negative_expense"); + assert!(sig.is_none(), "no header signature until an import records one"); + assert!(tpl.is_none(), "an existing source has no known provenance"); + + // The child row and its FK survived the ALTERs. + let files: i64 = conn + .query_row( + "SELECT COUNT(*) FROM imported_files WHERE source_id = ?1", + [src], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(files, 1, "imported_files must survive the v17 ALTERs"); + let templates: i64 = conn + .query_row("SELECT COUNT(*) FROM import_config_templates", [], |r| { + r.get(0) + }) + .unwrap(); + assert_eq!(templates, 1, "templates are untouched by v17"); + } + + #[test] + fn migration_v17_backfill_reproduces_the_wizard_rule() { + let conn = db_pre_v17(); + + // The rule being frozen (`useImportWizard.ts:321`): + // mapping.debitAmount !== undefined ? "debit_credit" : "single" + let cases: &[(&str, &str, &str)] = &[ + ( + "Deux colonnes", + r#"{"date":"Date","description":"Libellé","debitAmount":"Débit","creditAmount":"Crédit"}"#, + "debit_credit", + ), + ( + "Débit seul", + r#"{"date":"Date","description":"Libellé","debitAmount":"Retrait"}"#, + "debit_credit", + ), + ( + "Montant unique", + r#"{"date":"Date","description":"Libellé","amount":"Montant"}"#, + "single", + ), + ( + "Crédit seul", + r#"{"date":"Date","description":"Libellé","creditAmount":"Dépôt"}"#, + "single", + ), + ( + "Mapping minimal", + r#"{"date":"Date","description":"Libellé"}"#, + "single", + ), + ]; + for (name, mapping, _) in cases { + seed_source(&conn, name, mapping); + } + + conn.execute_batch(V17_SQL).expect("apply v17"); + + for (name, _, expected_mode) in cases { + let (mode, sign): (String, String) = conn + .query_row( + "SELECT amount_mode, sign_convention FROM import_sources WHERE name = ?1", + [name], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .unwrap(); + assert_eq!( + mode, *expected_mode, + "{name}: the backfill must reproduce the wizard rule" + ); + // The wizard hardcoded this on every restore, so every migrated + // source must land on it — that is what "no behaviour change" means. + assert_eq!( + sign, "negative_expense", + "{name}: sign_convention restores the previously hardcoded value" + ); + } + } + + #[test] + fn migration_v17_checks_reject_unknown_enum_values() { + let conn = db_pre_v17(); + conn.execute_batch(V17_SQL).expect("apply v17"); + + let set = |col: &str, val: &str| { + conn.execute( + &format!("INSERT INTO import_sources (name, column_mapping, {col}) VALUES (?1, '{{}}', ?2)"), + rusqlite::params![format!("{col}-{val}"), val], + ) + }; + + // The third amount mode is admitted from the start: implementing it later + // must not require another migration. + assert!( + set("amount_mode", "absolute_indicator").is_ok(), + "absolute_indicator must be accepted by the v17 CHECK" + ); + assert!(set("amount_mode", "debit_credit").is_ok()); + assert!(set("sign_convention", "positive_expense").is_ok()); + + // A corrupted value — e.g. restored from a hand-edited SREF backup — is + // refused by the database rather than silently mis-read at parse time. + assert!( + set("amount_mode", "montants_bizarres").is_err(), + "an unknown amount_mode must be refused" + ); + assert!( + set("sign_convention", "whatever").is_err(), + "an unknown sign_convention must be refused" + ); + } + + #[test] + fn migration_v17_template_id_is_a_nullable_provenance_tag() { + let conn = db_pre_v17(); + conn.execute_batch(V17_SQL).expect("apply v17"); + + conn.execute( + "INSERT INTO import_config_templates (name, column_mapping, amount_mode, sign_convention) \ + VALUES ('Desjardins', '{}', 'single', 'positive_expense')", + [], + ) + .unwrap(); + let tpl = conn.last_insert_rowid(); + let src = seed_source(&conn, "Compte chèque", "{}"); + conn.execute( + "UPDATE import_sources SET template_id = ?1, sign_convention = 'positive_expense' \ + WHERE id = ?2", + [tpl, src], + ) + .unwrap(); + + // Editing the template changes NO linked source: the eight format columns + // on the source are authoritative, the tag only records provenance. + conn.execute( + "UPDATE import_config_templates SET amount_mode = 'debit_credit', \ + sign_convention = 'negative_expense' WHERE id = ?1", + [tpl], + ) + .unwrap(); + let (mode, sign): (String, String) = conn + .query_row( + "SELECT amount_mode, sign_convention FROM import_sources WHERE id = ?1", + [src], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .unwrap(); + assert_eq!(mode, "single", "editing a template must not touch a source"); + assert_eq!(sign, "positive_expense"); + + // Deleting the template keeps the source and its format — only the tag + // goes (ON DELETE SET NULL). + conn.execute("DELETE FROM import_config_templates WHERE id = ?1", [tpl]) + .unwrap(); + let (tag, sign_after): (Option, String) = conn + .query_row( + "SELECT template_id, sign_convention FROM import_sources WHERE id = ?1", + [src], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .unwrap(); + assert!(tag.is_none(), "deleting a template nulls the provenance tag"); + assert_eq!( + sign_after, "positive_expense", + "deleting a template must not alter the source format" + ); + + // A dangling tag is refused outright. + assert!( + conn.execute( + "UPDATE import_sources SET template_id = 9999 WHERE id = ?1", + [src] + ) + .is_err(), + "template_id must point at a real template" + ); + } + + #[test] + fn consolidated_schema_matches_v17_chain_on_import_sources_at_parity() { + // The consolidated schema is the tested reference definition for new + // profiles. Without this test the DEFAULT or the CHECK of the four v17 + // columns could drift between the two definitions unnoticed — the + // columns are inert on the production path (this script runs after every + // migration and only uses CREATE TABLE IF NOT EXISTS), so nothing else + // would surface the divergence. + let chain = db_pre_v17(); + chain.execute_batch(V17_SQL).expect("apply v17"); + let consolidated = consolidated_db(); + + // Same columns, same types, same NOT NULL flags, same DEFAULTs. + assert_eq!( + column_shape(&consolidated, "import_sources"), + column_shape(&chain, "import_sources"), + "consolidated import_sources must match the v1→v17 chain" + ); + // Same FK, same ON DELETE action. + assert_eq!( + fk_shape(&consolidated, "import_sources"), + fk_shape(&chain, "import_sources"), + "consolidated import_sources must carry the same template_id FK" + ); + assert_eq!( + fk_shape(&consolidated, "import_sources"), + vec![( + "import_config_templates".to_string(), + "template_id".to_string(), + "id".to_string(), + "SET NULL".to_string(), + )], + ); + + // The CHECKs are not exposed by pragma, so prove them behaviourally on + // BOTH definitions — the same values must be accepted and refused. + for conn in [&consolidated, &chain] { + for (col, val, ok) in [ + ("amount_mode", "absolute_indicator", true), + ("amount_mode", "debit_credit", true), + ("amount_mode", "nope", false), + ("sign_convention", "positive_expense", true), + ("sign_convention", "nope", false), + ] { + let res = conn.execute( + &format!( + "INSERT INTO import_sources (name, column_mapping, {col}) \ + VALUES (?1, '{{}}', ?2)" + ), + rusqlite::params![format!("{col}-{val}"), val], + ); + assert_eq!( + res.is_ok(), + ok, + "{col} = {val} must behave identically in both definitions" + ); + } + } + } }