fix(onboarding): count an import only once the file brought a transaction

The "Import a first file" step counted `imported_files` rows in status
`completed`. `executeImport` writes that row before its first insert, so
an import that added nothing (a file holding only its header, or one
whose rows were all in error in the preview) ticked the step: the Import
card then said "Done." at rest and sent the user to /transactions, which
had nothing to categorize and led back to the import.

The counter now keeps a completed file only when a transaction points at
it (`EXISTS` on `transactions.file_id`, covered by
`idx_transactions_file`). Replayed with sqlite3 on
`consolidated_schema.sql`: 0 after an import without a valid row, 0
before a successful import's first insert and 1 after, 0 once the file
turns to `error`. Duplicates only change nothing: their transactions
came with an earlier completed file, and a restore writes its own.

The tests run without SQLite, so the rule is pinned on the query, and
the Import page is pinned at rest after such an import: step 4 again,
on the first source card, without "Done.".

Review of #398 (PR #404), option A chosen by Max.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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);

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`,

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(() => {

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

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;

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;