diff --git a/src/__integration__/import-format-roundtrip.test.ts b/src/__integration__/import-format-roundtrip.test.ts new file mode 100644 index 0000000..26427d8 --- /dev/null +++ b/src/__integration__/import-format-roundtrip.test.ts @@ -0,0 +1,262 @@ +/** + * Import format — save/reload integration (#324). + * + * The unit tests in `src/utils/importFormat.test.ts` prove the codec carries + * every field. This file proves the SQL on either side of it does too: a format + * written by `importSourceService` and read back is the same format, field by + * field. That is the test that would have caught the root bug — before v17 the + * INSERT simply had no `amount_mode` / `sign_convention` column to write to, so + * a positive-expense source silently came back inverted on its second import. + * + * Like `balance-flow.test.ts`, real `tauri-plugin-sql` cannot be started outside + * the Tauri WebView, so the services run against an in-memory FakeDb that + * interprets the handful of statements they actually issue. It stores what it + * is given, so `has_header` lands as the 0/1 integer SQLite stores and comes + * back as one — the exact shape the codec has to absorb. + */ + +import { describe, it, expect, beforeEach, vi } from "vitest"; + +vi.mock("../services/db", () => { + const getDb = vi.fn(); + return { + getDb, + withTransaction: vi.fn(async (fn: (db: unknown) => unknown) => fn(await getDb())), + }; +}); + +import { getDb } from "../services/db"; +import { + createSource, + getSourceByName, + updateSource, +} from "../services/importSourceService"; +import { + createTemplate, + getAllTemplates, + updateTemplate, +} from "../services/importConfigTemplateService"; +import { formatFromRow, formatToRow, FORMAT_FIELD_PAIRS } from "../utils/importFormat"; +import type { ImportFormat, SourceConfig } from "../shared/types"; + +// --------------------------------------------------------------------------- +// FakeDb — a tiny interpreter for the statements these two services issue. +// --------------------------------------------------------------------------- + +type Row = Record; + +function makeFakeDb() { + const tables: Record = { + import_sources: [], + import_config_templates: [], + }; + let nextId = 1; + + const insert = (sql: string, params: unknown[]) => { + const [, table, cols] = + /INSERT INTO (\w+) \(([^)]+)\)/.exec(sql) ?? []; + const columns = cols.split(",").map((c) => c.trim()); + const row: Row = { id: nextId++ }; + columns.forEach((c, i) => (row[c] = params[i])); + + if (/ON CONFLICT\(name\) DO UPDATE/.test(sql)) { + const clash = tables[table].find((r) => r.name === row.name); + if (clash) { + // Mimic `excluded.*`: overwrite every listed column, keep the id. + columns.forEach((c, i) => (clash[c] = params[i])); + nextId--; + return { lastInsertId: 0, rowsAffected: 1 }; + } + } + tables[table].push(row); + return { lastInsertId: row.id as number, rowsAffected: 1 }; + }; + + const update = (sql: string, params: unknown[]) => { + const [, table, assignments] = + /UPDATE (\w+)\s+SET ([\s\S]+?)\s+WHERE id\s*=\s*\$(\d+)/.exec(sql) ?? []; + const id = params[params.length - 1]; + const row = tables[table].find((r) => r.id === id); + if (!row) return { rowsAffected: 0 }; + for (const [, column, index] of assignments.matchAll( + /(\w+)\s*=\s*\$(\d+)/g + )) { + row[column] = params[Number(index) - 1]; + } + return { rowsAffected: 1 }; + }; + + return { + tables, + execute: vi.fn(async (sql: string, params: unknown[] = []) => { + if (sql.trimStart().startsWith("INSERT")) return insert(sql, params); + if (sql.trimStart().startsWith("UPDATE")) return update(sql, params); + throw new Error(`FakeDb: unsupported statement ${sql.slice(0, 40)}`); + }), + select: vi.fn(async (sql: string, params: unknown[] = []) => { + const [, table] = /FROM (\w+)/.exec(sql) ?? []; + const rows = tables[table]; + if (/WHERE name = \$1/.test(sql)) return rows.filter((r) => r.name === params[0]); + if (/WHERE id = \$1/.test(sql)) return rows.filter((r) => r.id === params[0]); + return [...rows].sort((a, b) => String(a.name).localeCompare(String(b.name))); + }), + }; +} + +let db: ReturnType; + +beforeEach(() => { + db = makeFakeDb(); + vi.mocked(getDb).mockResolvedValue(db as never); +}); + +// --------------------------------------------------------------------------- +// Fixtures — a credit-card statement: expenses are POSITIVE. This is the +// configuration the old code could not keep. +// --------------------------------------------------------------------------- + +const CREDIT_CARD: SourceConfig = { + name: "Visa Desjardins", + delimiter: ",", + encoding: "windows-1252", + dateFormat: "YYYY-MM-DD", + skipLines: 2, + hasHeader: false, + columnMapping: { date: 1, description: 3, amount: 4 }, + amountMode: "single", + signConvention: "positive_expense", +}; + +/** What `useImportWizard.selectSource` does on a stored source. */ +async function reload(name: string): Promise { + const stored = await getSourceByName(name); + if (!stored) throw new Error(`source ${name} not found`); + return { name: stored.name, ...formatFromRow(stored) }; +} + +function expectSameFormat(actual: ImportFormat, expected: ImportFormat) { + for (const field of Object.keys(FORMAT_FIELD_PAIRS) as Array) { + expect(actual[field], `field ${field}`).toEqual(expected[field]); + } +} + +// --------------------------------------------------------------------------- + +describe("configure -> save -> reload (#324)", () => { + it("returns the saved format field by field", async () => { + await createSource({ name: CREDIT_CARD.name, ...formatToRow(CREDIT_CARD) }); + expectSameFormat(await reload(CREDIT_CARD.name), CREDIT_CARD); + }); + + it("keeps positive_expense on the second import and every one after", async () => { + await createSource({ name: CREDIT_CARD.name, ...formatToRow(CREDIT_CARD) }); + + // Three consecutive imports re-save what was reloaded, as the wizard does. + let current = await reload(CREDIT_CARD.name); + for (let i = 0; i < 3; i++) { + const id = (await getSourceByName(CREDIT_CARD.name))!.id; + await updateSource(id, { name: current.name, ...formatToRow(current) }); + current = await reload(CREDIT_CARD.name); + } + + expect(current.signConvention).toBe("positive_expense"); + expectSameFormat(current, CREDIT_CARD); + }); + + it("stores has_header as the integer SQLite holds, not a JS boolean", async () => { + await createSource({ name: CREDIT_CARD.name, ...formatToRow(CREDIT_CARD) }); + expect(db.tables.import_sources[0].has_header).toBe(0); + expect((await reload(CREDIT_CARD.name)).hasHeader).toBe(false); + }); + + it("keeps the chosen mode when the mapping does not betray it", async () => { + const debitCredit: SourceConfig = { + ...CREDIT_CARD, + amountMode: "debit_credit", + // No debitAmount: the old restore re-inferred "single" from this shape. + columnMapping: { date: 1, description: 3, creditAmount: 5 }, + }; + await createSource({ name: debitCredit.name, ...formatToRow(debitCredit) }); + expect((await reload(debitCredit.name)).amountMode).toBe("debit_credit"); + }); + + it("updates the format in place when the source is re-configured", async () => { + const id = await createSource({ + name: CREDIT_CARD.name, + ...formatToRow(CREDIT_CARD), + }); + + const flipped: SourceConfig = { + ...CREDIT_CARD, + signConvention: "negative_expense", + amountMode: "debit_credit", + columnMapping: { date: 1, description: 3, debitAmount: 4, creditAmount: 5 }, + }; + await updateSource(id, { name: flipped.name, ...formatToRow(flipped) }); + + expectSameFormat(await reload(CREDIT_CARD.name), flipped); + expect(db.tables.import_sources).toHaveLength(1); + }); +}); + +describe("template_id is provenance, never format (#324)", () => { + async function seedLinkedPair() { + const templateId = await createTemplate({ + name: "Desjardins carte", + ...formatToRow(CREDIT_CARD), + }); + await createSource({ + name: CREDIT_CARD.name, + ...formatToRow(CREDIT_CARD), + template_id: templateId, + }); + return templateId; + } + + it("records the template the source was configured from", async () => { + const templateId = await seedLinkedPair(); + expect((await getSourceByName(CREDIT_CARD.name))!.template_id).toBe(templateId); + }); + + // The acceptance criterion: the eight source columns are authoritative, so a + // template edit cannot reach through the link and change how a source reads. + it("editing the template alters no linked source", async () => { + const templateId = await seedLinkedPair(); + + await updateTemplate(templateId, { + name: "Desjardins carte", + ...formatToRow({ + ...CREDIT_CARD, + delimiter: "\t", + encoding: "utf-8", + dateFormat: "DD/MM/YYYY", + skipLines: 0, + hasHeader: true, + columnMapping: { date: 0, description: 1, debitAmount: 2, creditAmount: 3 }, + amountMode: "debit_credit", + signConvention: "negative_expense", + }), + }); + + expectSameFormat(await reload(CREDIT_CARD.name), CREDIT_CARD); + + // …and the template really did change, so the test is not vacuous. + const [stored] = await getAllTemplates(); + expect(formatFromRow(stored).signConvention).toBe("negative_expense"); + expect(formatFromRow(stored).amountMode).toBe("debit_credit"); + }); + + it("survives a re-import that re-saves the format", async () => { + const templateId = await seedLinkedPair(); + const id = (await getSourceByName(CREDIT_CARD.name))!.id; + const current = await reload(CREDIT_CARD.name); + + await updateSource(id, { + name: current.name, + ...formatToRow(current), + template_id: templateId, + }); + + expect((await getSourceByName(CREDIT_CARD.name))!.template_id).toBe(templateId); + }); +}); diff --git a/src/components/import/ColumnMappingEditor.tsx b/src/components/import/ColumnMappingEditor.tsx index 6bba264..970b507 100644 --- a/src/components/import/ColumnMappingEditor.tsx +++ b/src/components/import/ColumnMappingEditor.tsx @@ -1,12 +1,19 @@ import { useTranslation } from "react-i18next"; import type { ColumnMapping, AmountMode } from "../../shared/types"; +import { clearMappingForMode } from "../../utils/importFormat"; interface ColumnMappingEditorProps { headers: string[]; mapping: ColumnMapping; amountMode: AmountMode; onMappingChange: (mapping: ColumnMapping) => void; - onAmountModeChange: (mode: AmountMode) => void; + /** + * The mode carries the pruned mapping with it. Both have to land in a single + * state update: the parent's handlers each spread the same `config` prop, so + * two consecutive calls would see the same stale value and the second would + * overwrite the first. + */ + onAmountModeChange: (mode: AmountMode, mapping: ColumnMapping) => void; } export default function ColumnMappingEditor({ @@ -18,6 +25,12 @@ export default function ColumnMappingEditor({ }: ColumnMappingEditorProps) { const { t } = useTranslation(); + // The mode is the source of truth for the mapping, not the reverse: leaving a + // mode drops its columns so a stale key cannot keep deciding how amounts are + // read (#324). + const selectMode = (mode: AmountMode) => + onAmountModeChange(mode, clearMappingForMode(mapping, mode)); + const columnOptions = headers.map((h, i) => (