diff --git a/src/pages/ImportPage.render.test.tsx b/src/pages/ImportPage.render.test.tsx index 0ded6ae..13ddbaa 100644 --- a/src/pages/ImportPage.render.test.tsx +++ b/src/pages/ImportPage.render.test.tsx @@ -230,7 +230,8 @@ describe("ImportPage — the step card (#398)", () => { scannedSources: SOURCES, importReport: report(0), }); - // Duplicates only: the file row is `completed`, the checklist counts it. + // Duplicates only: this file brought no transaction, and the step stays + // checked by the earlier import that brought the ones it duplicates. harness.onboarding = path(IMPORTED, true); const html = render(); expect(sentence(html)).toBe("t(onboarding.card.import.failed[])"); @@ -238,6 +239,20 @@ describe("ImportPage — the step card (#398)", () => { expect(html).not.toContain("onboarding.card.next"); }); + it("after 'Done' on an import that added nothing, the wizard at rest: no 'Done.', step 4 again on the first source card", () => { + // The file row is written, but the import step only counts a file that + // brought a transaction: the re-read keeps the step to do (review of #398). + harness.wizard = wizard({ importFolder: "/home/max/Relevés", scannedSources: SOURCES }); + harness.onboarding = path({ ...ZERO, sources: 1 }, true); + const html = render(); + expect(html).toContain("t(onboarding.card.stepOf[n=4,total=6])"); + expect(html).not.toContain("t(onboarding.card.done)"); + expect(sentence(html)).toBe("t(onboarding.card.import.sourceList[])"); + const target = highlighted(html); + expect(target.inner).toContain("Desjardins"); + expect(target.inner).not.toContain("Tangerine"); + }); + it("after 'Done' or 'Cancel', the wizard at rest: the card still says 'Done.' and points at nothing", () => { harness.wizard = wizard({ importFolder: "/home/max/Relevés", scannedSources: SOURCES }); harness.onboarding = path(IMPORTED, true); diff --git a/src/services/importedFileService.ts b/src/services/importedFileService.ts index a559164..6e6bb95 100644 --- a/src/services/importedFileService.ts +++ b/src/services/importedFileService.ts @@ -39,8 +39,9 @@ export async function createImportedFile(file: { "SELECT id FROM imported_files WHERE source_id = $1 AND filename = $2", [file.source_id, file.filename] ); - // Both branches write the status that the "import a first file" step counts - // (`status = 'completed'`), so each one is announced (ADR 0022). + // Both branches write the status that the "import a first file" step reads + // (`status = 'completed'`, counted once the file brought a transaction), so + // each one is announced (ADR 0022). if (existing.length > 0) { await db.execute( `UPDATE imported_files SET file_hash = $1, row_count = $2, status = $3, notes = $4, import_date = CURRENT_TIMESTAMP WHERE id = $5`, diff --git a/src/services/onboardingService.test.ts b/src/services/onboardingService.test.ts index 883e953..104be6b 100644 --- a/src/services/onboardingService.test.ts +++ b/src/services/onboardingService.test.ts @@ -131,7 +131,6 @@ describe("getOnboardingSnapshot (#351, #396)", () => { expect(db.select).toHaveBeenCalledTimes(4); const sqls = db.select.mock.calls.map((c) => String(c[0])); const aggregate = sqls.find((s) => s.includes("import_sources"))!; - expect(aggregate).toContain("FROM imported_files WHERE status = 'completed'"); expect(aggregate).toContain("FROM transactions WHERE is_manually_categorized = 1"); expect(aggregate).toContain("FROM budget_entries"); expect(aggregate).toMatch(/\(SELECT COUNT\(\*\) FROM transactions\)\s+AS transactions/); @@ -144,6 +143,21 @@ describe("getOnboardingSnapshot (#351, #396)", () => { expect(sqls.filter((s) => s.includes("LIKE 'onboarding_skipped_%'"))).toHaveLength(1); }); + // The tests run without SQLite, so the rule is pinned on the query itself. + // `executeImport` writes the file row `completed` before its first insert: a + // file whose rows were all in error keeps that row and adds no transaction, + // and must not tick "Import a first file" (review of #398). + it("counts a completed import only once the file brought a transaction", async () => { + const { db } = useFakeDb(); + await getOnboardingSnapshot(); + const aggregate = db.select.mock.calls + .map((c) => String(c[0])) + .find((s) => s.includes("import_sources"))!; + expect(aggregate).toMatch( + /\(SELECT COUNT\(\*\) FROM imported_files f\s+WHERE f\.status = 'completed'\s+AND EXISTS \(SELECT 1 FROM transactions t WHERE t\.file_id = f\.id\)\)\s+AS imports/, + ); + }); + it("is a pure read: it never writes, even on a profile D9 would hide", async () => { const { db } = useFakeDb({ transactions: 60 }); const unsubscribe = subscribeDataChanged(() => { diff --git a/src/services/onboardingService.ts b/src/services/onboardingService.ts index d56d11d..bdd78cc 100644 --- a/src/services/onboardingService.ts +++ b/src/services/onboardingService.ts @@ -54,10 +54,16 @@ interface CountersRow { transactions: number; } +// `imports` counts a `completed` file only once it brought a transaction: +// `executeImport` writes the row `completed` before inserting, and a file whose +// rows were all in error (or that holds only its header) keeps that row with +// no transaction. The sub-query is covered by `idx_transactions_file`. const COUNTERS_SQL = ` SELECT (SELECT COUNT(*) FROM import_sources) AS sources, - (SELECT COUNT(*) FROM imported_files WHERE status = 'completed') AS imports, + (SELECT COUNT(*) FROM imported_files f + WHERE f.status = 'completed' + AND EXISTS (SELECT 1 FROM transactions t WHERE t.file_id = f.id)) AS imports, (SELECT COUNT(*) FROM transactions WHERE is_manually_categorized = 1) AS manual_categorized, (SELECT COUNT(*) FROM budget_entries) AS budget_entries, (SELECT COUNT(*) FROM transactions) AS transactions diff --git a/src/shared/types/index.ts b/src/shared/types/index.ts index d1873b1..2d13df2 100644 --- a/src/shared/types/index.ts +++ b/src/shared/types/index.ts @@ -960,6 +960,7 @@ export interface AccountReturn { /** Aggregated counters behind the getting-started checklist, read in one query. */ export interface OnboardingCounters { sources: number; + /** `completed` imported files that brought at least one transaction. */ imports: number; manualCategorized: number; budgetEntries: number; diff --git a/src/utils/onboardingPath.ts b/src/utils/onboardingPath.ts index eb6f0d4..bc60b98 100644 --- a/src/utils/onboardingPath.ts +++ b/src/utils/onboardingPath.ts @@ -280,8 +280,8 @@ function checked(checklist: Checklist | null, key: ChecklistItemKey): boolean { * are tried in order, and the first that applies wins: * 1. the folder is not read yet: no card (null); * 2. an import is running: "Importing…". It comes before anything the - * counters say, because `executeImport` writes the `imported_files` row - * `completed` before inserting a single transaction; + * counters say, because the import step ticks as soon as the first + * transaction of the file lands, long before the import ends; * 3. the report: "Done." when the import added transactions, the failure * sentence when it added none (insertion failed, duplicates only), or when * the count is missing: never "Done." without one;