From 11eb911657d052236580f9c223174758ab7267a8 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sun, 18 Oct 2026 20:30:35 -0400 Subject: [PATCH] fix(import): translate the sample folder refusals, banner after a good rescan (#384) create_sample_import_folders refuses with stable codes instead of English sentences: sample_folder_exists:, import_folder_missing (no longer carrying the folder's path) and invalid_date. The wizard maps them to import.empty.sampleErrors.* through the pure sampleFolderError, the folder name interpolated from the new errorValues of the state; any other message stays raw. The raw refusal also goes to the Journal as a warning: a code, or a write error naming a sample folder and the OS error, never a path. scanFolderInternal now tells whether the scan went through, and the success banner only follows a rescan that did: a failed rescan leaves its error alone on screen. ImportEmptyState keeps folderName alone (null without a folder); the page passes "" for a folder stored as "", as hasFolder did. Rust tests: the three codes asserted exactly, the case of the second sample folder already there (nothing written), and {{M1}} pinned as the most recent month line by line against the templates. Co-Authored-By: Claude Opus 5.5 --- src-tauri/src/commands/fs_commands.rs | 89 +++++++-- .../import/ImportEmptyState.test.tsx | 35 ++-- src/components/import/ImportEmptyState.tsx | 13 +- src/hooks/useImportWizard.samples.test.ts | 176 ++++++++++++++++++ src/hooks/useImportWizard.test.ts | 93 +++++++++ src/hooks/useImportWizard.ts | 76 ++++++-- src/i18n/locales/en.json | 7 +- src/i18n/locales/fr.json | 7 +- src/pages/ImportPage.render.test.tsx | 19 ++ src/pages/ImportPage.tsx | 10 +- 10 files changed, 471 insertions(+), 54 deletions(-) create mode 100644 src/hooks/useImportWizard.samples.test.ts diff --git a/src-tauri/src/commands/fs_commands.rs b/src-tauri/src/commands/fs_commands.rs index 1f5f53c..e68fdb4 100644 --- a/src-tauri/src/commands/fs_commands.rs +++ b/src-tauri/src/commands/fs_commands.rs @@ -220,6 +220,15 @@ const SAMPLE_DESJARDINS_TEMPLATE: &str = const SAMPLE_TANGERINE_TEMPLATE: &str = include_str!("../../../src/__fixtures__/samples/exemple-tangerine.csv"); +/// Stable refusal codes of `create_sample_import_folders` (#384), translated by +/// the front (`sampleFolderError` in `src/hooks/useImportWizard.ts`, whose +/// tests read these three constants). `SAMPLE_ERROR_FOLDER_EXISTS` is followed +/// by `:` and the name of the sample folder found. Any other error keeps its +/// English text, shown raw. +const SAMPLE_ERROR_FOLDER_EXISTS: &str = "sample_folder_exists"; +const SAMPLE_ERROR_IMPORT_FOLDER_MISSING: &str = "import_folder_missing"; +const SAMPLE_ERROR_INVALID_DATE: &str = "invalid_date"; + /// Write the two onboarding sample statements (a Desjardins chequing account and /// a Tangerine credit card, both synthetic) into `import_folder`, one sub-folder /// each, so that the very next scan of the import folder shows two sources a @@ -231,18 +240,21 @@ const SAMPLE_TANGERINE_TEMPLATE: &str = /// `MM/YYYY` of the two complete months before `today` (`{{M1}}` the most /// recent), and each file is named `releve-YYYY-MM.csv` after `{{M1}}`. /// -/// Refuses to run when `import_folder` is not a directory, and when either -/// sample folder already exists — checked BEFORE anything is written, so an -/// existing folder is never touched. The command only ever receives the import -/// folder and a date: the names of what it writes are fixed here. Returns the -/// two folder NAMES (not paths). +/// Refuses to run when `today` is not a date (`invalid_date`), when +/// `import_folder` is not a directory (`import_folder_missing`, without the +/// path), and when either sample folder already exists +/// (`sample_folder_exists:`) — checked BEFORE anything is written, so an +/// existing folder is never touched. A write error keeps its text: the sample +/// folder's name, the file name and the OS error, never a path. The command +/// only ever receives the import folder and a date: the names of what it +/// writes are fixed here. Returns the two folder NAMES (not paths). #[tauri::command] pub fn create_sample_import_folders( import_folder: String, today: String, ) -> Result, String> { let today = NaiveDate::parse_from_str(&today, "%Y-%m-%d") - .map_err(|e| format!("Invalid date '{}': {}", today, e))?; + .map_err(|_| SAMPLE_ERROR_INVALID_DATE.to_string())?; let first_of_month = today .with_day(1) .ok_or_else(|| "Cannot compute the first day of the month".to_string())?; @@ -255,7 +267,7 @@ pub fn create_sample_import_folders( let root = Path::new(&import_folder); if !root.is_dir() { - return Err(format!("Folder does not exist: {}", import_folder)); + return Err(SAMPLE_ERROR_IMPORT_FOLDER_MISSING.to_string()); } let samples: [(&str, &str); 2] = [ @@ -267,7 +279,7 @@ pub fn create_sample_import_folders( // one of the folders keeps it untouched and gets neither. for (folder_name, _) in &samples { if root.join(folder_name).exists() { - return Err(format!("Folder already exists: {}", folder_name)); + return Err(format!("{}:{}", SAMPLE_ERROR_FOLDER_EXISTS, folder_name)); } } @@ -371,14 +383,39 @@ mod sample_folder_tests { } } + /// `{{M1}}` is the most recent month (#384): line by line, a template line + /// that carries `{{M1}}` comes out in `m1`, one that carries `{{M2}}` in + /// `m2`. `assert_dates_in` accepts either month on any line, so the two + /// tokens swapped would pass it. + fn assert_months_follow_tokens(content: &str, template: &str, m1: &str, m2: &str) { + let rendered: Vec<&str> = content.lines().collect(); + let source: Vec<&str> = template.lines().collect(); + assert_eq!(rendered.len(), source.len()); + let (mut m1_lines, mut m2_lines) = (0, 0); + for (out, tpl) in rendered.iter().zip(&source) { + let expected = if tpl.contains("{{M1}}") { + m1_lines += 1; + m1 + } else if tpl.contains("{{M2}}") { + m2_lines += 1; + m2 + } else { + continue; + }; + let date = out.split([';', ',']).next().unwrap(); + assert_eq!(&date[3..], expected, "template line {:?} came out as {:?}", tpl, out); + } + assert!(m1_lines > 0 && m2_lines > 0, "each template carries both tokens"); + } + #[test] fn refuses_a_missing_import_folder() { let dir = sandbox("missing"); let missing = dir.join("nope"); let result = create_sample_import_folders(as_arg(&missing), "2026-09-25".to_string()); cleanup(&dir); - let err = result.unwrap_err(); - assert!(err.contains("does not exist"), "{}", err); + // The code alone, translated by the front: never the folder's path. + assert_eq!(result.unwrap_err(), "import_folder_missing"); } #[test] @@ -392,12 +429,32 @@ mod sample_folder_tests { .next() .is_none(); cleanup(&dir); - let err = result.unwrap_err(); - assert!(err.contains(SAMPLE_DESJARDINS_FOLDER), "{}", err); + assert_eq!(result.unwrap_err(), "sample_folder_exists:Exemple Desjardins"); assert!(!tangerine_created, "Exemple Tangerine must not be created"); assert!(desjardins_empty, "the existing folder must stay untouched"); } + /// The second folder of the loop (#384): without the checks that run + /// before the first write, Exemple Desjardins would be written before the + /// loop reached Exemple Tangerine. + #[test] + fn refuses_when_tangerine_folder_exists_and_writes_nothing() { + let dir = sandbox("existing-tangerine"); + fs::create_dir(dir.join(SAMPLE_TANGERINE_FOLDER)).unwrap(); + let result = create_sample_import_folders(as_arg(&dir), "2026-09-25".to_string()); + let desjardins_created = dir.join(SAMPLE_DESJARDINS_FOLDER).exists(); + let tangerine_empty = fs::read_dir(dir.join(SAMPLE_TANGERINE_FOLDER)) + .unwrap() + .next() + .is_none(); + let entries = fs::read_dir(&dir).unwrap().count(); + cleanup(&dir); + assert_eq!(result.unwrap_err(), "sample_folder_exists:Exemple Tangerine"); + assert!(!desjardins_created, "Exemple Desjardins must not be created"); + assert!(tangerine_empty, "the existing folder must stay untouched"); + assert_eq!(entries, 1, "nothing but the existing folder"); + } + #[test] fn writes_both_folders_for_the_two_previous_months() { let dir = sandbox("success"); @@ -428,6 +485,9 @@ mod sample_folder_tests { assert_dates_in(&tangerine, &["08/2026", "07/2026"]); assert!(desjardins.contains("08/2026") && desjardins.contains("07/2026")); assert!(tangerine.contains("08/2026") && tangerine.contains("07/2026")); + // August, the month before the click, is `{{M1}}`; July is `{{M2}}`. + assert_months_follow_tokens(&desjardins, SAMPLE_DESJARDINS_TEMPLATE, "08/2026", "07/2026"); + assert_months_follow_tokens(&tangerine, SAMPLE_TANGERINE_TEMPLATE, "08/2026", "07/2026"); } #[test] @@ -436,8 +496,7 @@ mod sample_folder_tests { let result = create_sample_import_folders(as_arg(&dir), "pas-une-date".to_string()); let anything_written = fs::read_dir(&dir).unwrap().next().is_some(); cleanup(&dir); - let err = result.unwrap_err(); - assert!(err.contains("Invalid date"), "{}", err); + assert_eq!(result.unwrap_err(), "invalid_date"); assert!(!anything_written); } @@ -458,6 +517,8 @@ mod sample_folder_tests { assert_dates_in(&tangerine, &["12/2026", "11/2026"]); assert!(desjardins.contains("12/2026") && desjardins.contains("11/2026")); assert!(tangerine.contains("12/2026") && tangerine.contains("11/2026")); + assert_months_follow_tokens(&desjardins, SAMPLE_DESJARDINS_TEMPLATE, "12/2026", "11/2026"); + assert_months_follow_tokens(&tangerine, SAMPLE_TANGERINE_TEMPLATE, "12/2026", "11/2026"); } } diff --git a/src/components/import/ImportEmptyState.test.tsx b/src/components/import/ImportEmptyState.test.tsx index 6c7aa91..bd7e652 100644 --- a/src/components/import/ImportEmptyState.test.tsx +++ b/src/components/import/ImportEmptyState.test.tsx @@ -61,7 +61,6 @@ type LinkEl = ReactElement<{ to: string; children?: ReactNode }>; function render(overrides: Partial = {}) { const props: ImportEmptyStateProps = { - hasFolder: false, folderName: null, onBrowse: vi.fn(), onCreateSample: vi.fn(), @@ -77,7 +76,7 @@ function render(overrides: Partial = {}) { describe("ImportEmptyState — no folder (#356)", () => { it("offers one primary 'choose a folder' button wired to onBrowse, and the guide link", () => { - const { props, tree, buttons, links } = render({ hasFolder: false }); + const { props, tree, buttons, links } = render({ folderName: null }); expect(texts(tree)).toContain("t(import.empty.noFolder.title)"); expect(texts(tree)).toContain("t(import.empty.noFolder.description)"); @@ -96,7 +95,7 @@ describe("ImportEmptyState — no folder (#356)", () => { }); it("never shows the sample or refresh actions without a folder", () => { - const { tree } = render({ hasFolder: false }); + const { tree } = render({ folderName: null }); expect(texts(tree)).not.toContain("t(import.empty.createSample)"); expect(texts(tree)).not.toContain("t(import.empty.refresh)"); }); @@ -104,7 +103,7 @@ describe("ImportEmptyState — no folder (#356)", () => { describe("ImportEmptyState — folder without sources (#356)", () => { it("offers 'create a sample folder' as primary, 'refresh' as secondary, and the guide link", () => { - const { props, tree, buttons, links } = render({ hasFolder: true, folderName: "Relevés" }); + const { props, tree, buttons, links } = render({ folderName: "Relevés" }); expect(texts(tree)).toContain("t(import.empty.noSources.title)"); @@ -125,7 +124,7 @@ describe("ImportEmptyState — folder without sources (#356)", () => { }); it("disables the create button while the sample folders are being written", () => { - const { buttons } = render({ hasFolder: true, folderName: "Relevés", isCreatingSample: true }); + const { buttons } = render({ folderName: "Relevés", isCreatingSample: true }); expect(buttons[0].props.disabled).toBe(true); // Refresh stays available: it is not the action in flight. expect(buttons[1].props.disabled).toBe(false); @@ -133,12 +132,22 @@ describe("ImportEmptyState — folder without sources (#356)", () => { it("names the folder in the description, never the full path", () => { const folderName = folderNameFromPath("C:\\Users\\max\\Documents\\Relevés"); - const { tree } = render({ hasFolder: true, folderName }); + const { tree } = render({ folderName }); const description = texts(tree).find((s) => s.startsWith("t(import.empty.noSources.description")); expect(description).toBe("t(import.empty.noSources.description[folderName=Relevés])"); expect(description).not.toContain("Users"); }); + + // `folderName` alone tells the variants apart (#384): null is "no folder", + // any string a folder, an empty one included (what the page hands over for + // an import folder stored as "", as `hasFolder` did before it). + it("an empty folder name is still a folder: the 'no source' variant", () => { + const { tree, buttons } = render({ folderName: "" }); + expect(texts(tree)).toContain("t(import.empty.noSources.title)"); + expect(texts(tree)).toContain("t(import.empty.noSources.description[folderName=])"); + expect(texts(buttons[0].props.children)).toContain("t(import.empty.createSample)"); + }); }); describe("ImportEmptyState — expected structure of the folder (#422)", () => { @@ -153,8 +162,8 @@ describe("ImportEmptyState — expected structure of the folder (#422)", () => { ].map((name) => `t(import.empty.structure.${name})`); it.each([ - ["no folder", { hasFolder: false }], - ["a folder without sources", { hasFolder: true, folderName: "Relevés" }], + ["no folder", { folderName: null }], + ["a folder without sources", { folderName: "Relevés" }], ] as const)("shows the expected structure with %s, above the guide link", (_, overrides) => { const { tree } = render(overrides); const all = texts(tree); @@ -170,7 +179,7 @@ describe("ImportEmptyState — getting-started target (#398)", () => { find(tree, (e) => e.props["data-onboarding-highlight"] === "true"); it("highlights 'Choose a folder' without a folder, and only it", () => { - const { tree } = render({ hasFolder: false, onboardingTarget: true }); + const { tree } = render({ folderName: null, onboardingTarget: true }); const targets = highlighted(tree); expect(targets).toHaveLength(1); expect(targets[0].type).toBe("button"); @@ -178,16 +187,16 @@ describe("ImportEmptyState — getting-started target (#398)", () => { }); it("highlights 'Create a sample folder' in a folder without sources, not 'Refresh'", () => { - const { tree } = render({ hasFolder: true, folderName: "Relevés", onboardingTarget: true }); + const { tree } = render({ folderName: "Relevés", onboardingTarget: true }); const targets = highlighted(tree); expect(targets).toHaveLength(1); expect(texts(targets[0].props.children)).toContain("t(import.empty.createSample)"); }); it("highlights nothing without the property", () => { - expect(highlighted(render({ hasFolder: false }).tree)).toHaveLength(0); - expect(highlighted(render({ hasFolder: true, folderName: "Relevés" }).tree)).toHaveLength(0); - expect(highlighted(render({ hasFolder: true, onboardingTarget: false }).tree)).toHaveLength(0); + expect(highlighted(render({ folderName: null }).tree)).toHaveLength(0); + expect(highlighted(render({ folderName: "Relevés" }).tree)).toHaveLength(0); + expect(highlighted(render({ folderName: "Relevés", onboardingTarget: false }).tree)).toHaveLength(0); }); }); diff --git a/src/components/import/ImportEmptyState.tsx b/src/components/import/ImportEmptyState.tsx index 24d5618..c2f950f 100644 --- a/src/components/import/ImportEmptyState.tsx +++ b/src/components/import/ImportEmptyState.tsx @@ -8,9 +8,11 @@ import ImportFolderStructure from "./ImportFolderStructure"; export const CSV_EXPORT_GUIDE_PATH = "/settings/users#import-export"; export interface ImportEmptyStateProps { - /** An import folder is configured; the empty state then speaks of its content. */ - hasFolder: boolean; - /** Last segment of the import folder path, never the full path. */ + /** + * Last segment of the import folder path, never the full path; null when no + * import folder is configured. With a folder, the empty state speaks of its + * content (#384: one prop, no separate `hasFolder` to disagree with it). + */ folderName: string | null; onBrowse: () => void; onCreateSample: () => void; @@ -43,7 +45,6 @@ export function folderNameFromPath(path: string): string { * so it is tested by direct call. */ export default function ImportEmptyState({ - hasFolder, folderName, onBrowse, onCreateSample, @@ -67,7 +68,7 @@ export default function ImportEmptyState({ ); - if (!hasFolder) { + if (folderName === null) { return ( } @@ -84,7 +85,7 @@ export default function ImportEmptyState({ } title={t("import.empty.noSources.title")} - description={t("import.empty.noSources.description", { folderName: folderName ?? "" })} + description={t("import.empty.noSources.description", { folderName })} primaryAction={{ label: t("import.empty.createSample"), onClick: onCreateSample, diff --git a/src/hooks/useImportWizard.samples.test.ts b/src/hooks/useImportWizard.samples.test.ts new file mode 100644 index 0000000..5b10706 --- /dev/null +++ b/src/hooks/useImportWizard.samples.test.ts @@ -0,0 +1,176 @@ +// useImportWizard — "Create a sample folder", its banners and its Journal line (#384). +// +// React's hooks come from the shared harness (src/test/hookHarness.ts, #409): +// the hook runs as a plain function, its reducer is applied, `commit()` runs +// the mount read of the import folder (and its first scan), and the test then +// calls `createSampleFolders` from the next render, as the page's button does. +// `invoke` answers per command; the Journal is the real log service. +// +// What is pinned: the success banner only after a rescan that went through +// (a failed rescan leaves its error alone on screen), a refusal of the Rust +// command shown by its translation key and values, any other message raw, and +// the Journal line of a refusal, which never carries the import folder's path +// or name (the Journal travels with a feedback report). +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +vi.mock("react", async (orig) => (await import("../test/hookHarness")).fakeReact(await orig())); +const ipc = vi.hoisted(() => ({ + /** The stored import folder: its name must never reach the Journal. */ + importFolder: "/home/jean/Relevés bancaires", + /** What each command answers, in call order: a value, or an Error or a string to reject with. */ + answers: {} as Record, +})); +vi.mock("@tauri-apps/api/core", () => ({ + invoke: vi.fn(async (command: string) => { + const queue = ipc.answers[command] ?? []; + if (queue.length === 0) throw new Error(`unexpected command ${command}`); + const answer = queue.shift(); + if (answer instanceof Error || typeof answer === "string") throw answer; + return answer; + }), +})); +vi.mock("../services/db", () => ({ getDb: vi.fn(), withTransaction: vi.fn() })); +vi.mock("../services/userPreferenceService", () => ({ + getImportFolder: vi.fn(async () => ipc.importFolder), + setImportFolder: vi.fn(), +})); +vi.mock("../services/importSourceService", () => ({ + getAllSources: vi.fn(async () => []), + getSourceByName: vi.fn(async () => null), + createSource: vi.fn(), + updateSource: vi.fn(), +})); +vi.mock("../services/importedFileService", () => ({ + existsByHash: vi.fn(), + createImportedFile: vi.fn(), + updateFileStatus: vi.fn(), + getFilesBySourceId: vi.fn(async () => []), +})); +vi.mock("../services/importConfigTemplateService", () => ({ + getAllTemplates: vi.fn(async () => []), + createTemplate: vi.fn(), + updateTemplate: vi.fn(), + deleteTemplate: vi.fn(), +})); + +import { invoke } from "@tauri-apps/api/core"; +import { hookHarness } from "../test/hookHarness"; +import { useImportWizard } from "./useImportWizard"; +import { clearLogs, getLogs, initLogCapture } from "../services/logService"; +import type { ImportFolderScan } from "../shared/types"; + +const harness = hookHarness(); + +const IMPORT_FOLDER = ipc.importFolder; +const NAMES = ["Exemple Desjardins", "Exemple Tangerine"]; +const EMPTY_SCAN: ImportFolderScan = { sources: [], root_files: [] }; +const SAMPLE_SCAN: ImportFolderScan = { + sources: NAMES.map((name) => ({ folder_name: name, folder_path: `${IMPORT_FOLDER}/${name}`, files: [] })), + root_files: [], +}; + +/** Lets the hook's promises settle. */ +const flush = async () => { + for (let i = 0; i < 20; i++) await Promise.resolve(); +}; + +/** Mounts the wizard on IMPORT_FOLDER (read, then scanned empty) and returns its next render. */ +async function mount() { + harness.render(useImportWizard); + harness.commit(); + await flush(); + return harness.render(useImportWizard); +} + +/** Clicks "Create a sample folder" and returns the state after it. */ +async function createSamples() { + const wizard = await mount(); + expect(wizard.state.importFolder).toBe(IMPORT_FOLDER); + await wizard.createSampleFolders(); + return harness.render(useImportWizard).state; +} + +const journal = () => getLogs().map((l) => `${l.level}: ${l.message}`); +const created = () => + harness.dispatch.mock.calls.filter(([a]) => (a as { type: string }).type === "SET_SAMPLE_CREATED"); + +beforeEach(() => { + harness.clear(); + vi.mocked(invoke).mockClear(); + initLogCapture(); + clearLogs(); + ipc.answers = { scan_import_folder: [EMPTY_SCAN] }; +}); + +afterEach(() => { + harness.unmount(); +}); + +describe("createSampleFolders — the banners (#384)", () => { + it("rescan went through: the success banner names the folders, no error", async () => { + ipc.answers.create_sample_import_folders = [NAMES]; + ipc.answers.scan_import_folder.push(SAMPLE_SCAN); + const state = await createSamples(); + expect(state.sampleCreated).toEqual(NAMES); + expect(state.scannedSources).toBe(SAMPLE_SCAN.sources); + expect(state.error).toBeNull(); + expect(state.isLoading).toBe(false); + }); + + it("rescan failed: the error banner alone, never the success one", async () => { + ipc.answers.create_sample_import_folders = [NAMES]; + ipc.answers.scan_import_folder.push(new Error(`Folder does not exist: ${IMPORT_FOLDER}`)); + const state = await createSamples(); + expect(state.sampleCreated).toBeNull(); + expect(created()).toEqual([]); + expect(state.error).toBe(`Folder does not exist: ${IMPORT_FOLDER}`); + // The folders were written all the same: the Journal says so, by name. + expect(journal()).toContain("info: [import] Sample folders created: Exemple Desjardins, Exemple Tangerine"); + // The rescan's error carries the path: it stays on screen, out of the Journal. + expect(journal().join("\n")).not.toContain("Relevés"); + }); +}); + +describe("createSampleFolders — a refusal (#384, D2)", () => { + it("a sample folder already there: its key and name, no scan, no success banner", async () => { + ipc.answers.create_sample_import_folders = ["sample_folder_exists:Exemple Desjardins"]; + const state = await createSamples(); + expect(state.error).toBe("import.empty.sampleErrors.folderExists"); + expect(state.errorValues).toEqual({ name: "Exemple Desjardins" }); + expect(state.sampleCreated).toBeNull(); + expect(state.isLoading).toBe(false); + // The mount's scan only: a refused creation is not rescanned. + expect(vi.mocked(invoke).mock.calls.filter(([c]) => c === "scan_import_folder")).toHaveLength(1); + }); + + it.each([ + ["import_folder_missing", "import.empty.sampleErrors.importFolderMissing"], + ["invalid_date", "import.empty.sampleErrors.invalidDate"], + ])("%s: its key, without values", async (code, key) => { + ipc.answers.create_sample_import_folders = [code]; + const state = await createSamples(); + expect(state.error).toBe(key); + expect(state.errorValues).toBeNull(); + }); + + it("any other message is shown raw", async () => { + const raw = "Cannot write Exemple Desjardins/releve-2026-09.csv: Permission denied (os error 13)"; + ipc.answers.create_sample_import_folders = [raw]; + const state = await createSamples(); + expect(state.error).toBe(raw); + expect(state.errorValues).toBeNull(); + }); + + it.each([ + "sample_folder_exists:Exemple Tangerine", + "import_folder_missing", + "invalid_date", + "Cannot create folder Exemple Desjardins: Permission denied (os error 13)", + ])("the Journal gets the raw refusal as a warning, never the import folder (%s)", async (raw) => { + ipc.answers.create_sample_import_folders = [raw]; + await createSamples(); + expect(journal()).toEqual([`warn: [import] Sample folders not created: ${raw}`]); + expect(journal().join("\n")).not.toContain("Relevés"); + expect(journal().join("\n")).not.toContain("/home/jean"); + }); +}); diff --git a/src/hooks/useImportWizard.test.ts b/src/hooks/useImportWizard.test.ts index 169c35a..0640038 100644 --- a/src/hooks/useImportWizard.test.ts +++ b/src/hooks/useImportWizard.test.ts @@ -22,10 +22,13 @@ vi.mock("../services/db", () => { }; }); +import { readFileSync } from "fs"; +import { resolve } from "path"; import { wizardReducer, initialWizardState, formatLocalDate, + sampleFolderError, type WizardAction, type WizardState, } from "./useImportWizard"; @@ -220,3 +223,93 @@ describe("wizardReducer — rootFiles (#427)", () => { expect(after.rootFiles).toBe(before.rootFiles); }); }); + +describe("sampleFolderError — the refusals of create_sample_import_folders (#384, D2)", () => { + it("a sample folder already there: its key, with the folder's name", () => { + expect(sampleFolderError("sample_folder_exists:Exemple Desjardins")).toEqual({ + key: "import.empty.sampleErrors.folderExists", + values: { name: "Exemple Desjardins" }, + }); + expect(sampleFolderError("sample_folder_exists:Exemple Tangerine")).toEqual({ + key: "import.empty.sampleErrors.folderExists", + values: { name: "Exemple Tangerine" }, + }); + }); + + it("the import folder missing, the date unreadable: their keys, without values", () => { + expect(sampleFolderError("import_folder_missing")).toEqual({ + key: "import.empty.sampleErrors.importFolderMissing", + }); + expect(sampleFolderError("invalid_date")).toEqual({ key: "import.empty.sampleErrors.invalidDate" }); + }); + + it("any other message is unknown, so the banner shows it raw", () => { + for (const raw of [ + "Cannot write Exemple Desjardins/releve-2026-09.csv: Permission denied (os error 13)", + "Cannot create folder Exemple Tangerine: File exists (os error 17)", + "Cannot compute the previous month", + // The messages of the versions before #384, and codes bent out of shape. + "Folder already exists: Exemple Desjardins", + "Folder does not exist: /home/max/Relevés", + "sample_folder_exists:", + "sample_folder_exists", + "import_folder_missing: /home/max/Relevés", + "Invalid date 'x': input contains invalid characters", + "", + ]) { + expect(sampleFolderError(raw), raw).toBeNull(); + } + }); + + // The codes are Rust constants (`SAMPLE_ERROR_*` of fs_commands.rs): a code + // renamed or added there, with no mapping here, would reach the user raw. + // Read from the source like onboarding-samples.test.ts reads the seed. A PR + // that touches only `src-tauri/**` skips `check-frontend`: the drift then + // shows at the next frontend run. + it("maps every refusal code the Rust command declares", () => { + const rust = readFileSync( + resolve(import.meta.dirname, "../../src-tauri/src/commands/fs_commands.rs"), + "utf-8", + ); + const codes = [...rust.matchAll(/^const SAMPLE_ERROR_[A-Z_]+: &str = "([a-z_]+)";$/gm)].map((m) => m[1]); + expect(codes.sort()).toEqual(["import_folder_missing", "invalid_date", "sample_folder_exists"]); + for (const code of codes) { + const raw = code === "sample_folder_exists" ? `${code}:Exemple Desjardins` : code; + expect(sampleFolderError(raw), code).not.toBeNull(); + } + }); +}); + +describe("wizardReducer — errorValues (#384)", () => { + it("starts with none", () => { + expect(initialWizardState.errorValues).toBeNull(); + }); + + it("SET_ERROR sets the values with the key, and any later SET_ERROR drops them", () => { + const refused = wizardReducer(initialWizardState, { + type: "SET_ERROR", + payload: "import.empty.sampleErrors.folderExists", + values: { name: "Exemple Desjardins" }, + }); + expect(refused.error).toBe("import.empty.sampleErrors.folderExists"); + expect(refused.errorValues).toEqual({ name: "Exemple Desjardins" }); + + const raw = wizardReducer(refused, { type: "SET_ERROR", payload: "folder not found" }); + expect(raw.error).toBe("folder not found"); + expect(raw.errorValues).toBeNull(); + const cleared = wizardReducer(refused, { type: "SET_ERROR", payload: null }); + expect(cleared.error).toBeNull(); + expect(cleared.errorValues).toBeNull(); + }); + + it("RESET drops them with the error", () => { + const refused = wizardReducer(initialWizardState, { + type: "SET_ERROR", + payload: "import.empty.sampleErrors.folderExists", + values: { name: "Exemple Desjardins" }, + }); + const after = wizardReducer(refused, { type: "RESET" }); + expect(after.error).toBeNull(); + expect(after.errorValues).toBeNull(); + }); +}); diff --git a/src/hooks/useImportWizard.ts b/src/hooks/useImportWizard.ts index 0ebfb10..75d22c0 100644 --- a/src/hooks/useImportWizard.ts +++ b/src/hooks/useImportWizard.ts @@ -61,7 +61,7 @@ import { mapRow, ROW_ERROR_KEYS, } from "../utils/importFormat"; -import { logInfo } from "../services/logService"; +import { logInfo, logWarn } from "../services/logService"; /** Error text for the banner: an i18n key when we have one, the raw message otherwise. */ function errorMessage(e: unknown): string { @@ -69,6 +69,32 @@ function errorMessage(e: unknown): string { return e instanceof Error ? e.message : String(e); } +/** A refusal of `create_sample_import_folders` as the banner says it: an i18n key and its values. */ +export interface SampleFolderError { + key: string; + values?: Record; +} + +const SAMPLE_FOLDER_EXISTS = "sample_folder_exists:"; + +/** + * The banner text of a refusal of `create_sample_import_folders` (#384). The + * command refuses with a stable code (its constants `SAMPLE_ERROR_*` in + * `fs_commands.rs`): `sample_folder_exists:`, `import_folder_missing`, + * `invalid_date`, each turned here into its `import.empty.sampleErrors.*` key, + * with the name of the sample folder for the first. Any other message (a + * write error of the system) gives null: the banner shows it raw. + */ +export function sampleFolderError(raw: string): SampleFolderError | null { + if (raw.startsWith(SAMPLE_FOLDER_EXISTS)) { + const name = raw.slice(SAMPLE_FOLDER_EXISTS.length); + return name ? { key: "import.empty.sampleErrors.folderExists", values: { name } } : null; + } + if (raw === "import_folder_missing") return { key: "import.empty.sampleErrors.importFolderMissing" }; + if (raw === "invalid_date") return { key: "import.empty.sampleErrors.invalidDate" }; + return null; +} + export interface WizardState { step: ImportWizardStep; importFolder: string | null; @@ -101,6 +127,12 @@ export interface WizardState { importProgress: { current: number; total: number; file: string }; isLoading: boolean; error: string | null; + /** + * Values `error` interpolates when it is an i18n key that needs some (the + * name of a sample folder that already exists, #384); null otherwise. Set + * and cleared with `error` by `SET_ERROR`, dropped with it by `RESET`. + */ + errorValues: Record | null; configuredSourceNames: Set; importedFilesBySource: Map>; configTemplates: ImportConfigTemplate[]; @@ -139,7 +171,7 @@ export interface WizardState { export type WizardAction = | { type: "SET_LOADING"; payload: boolean } - | { type: "SET_ERROR"; payload: string | null } + | { type: "SET_ERROR"; payload: string | null; values?: Record } | { type: "SET_STEP"; payload: ImportWizardStep } | { type: "SET_IMPORT_FOLDER"; payload: string | null } | { type: "SET_IMPORT_FOLDER_LOADED" } @@ -196,6 +228,7 @@ const initialState: WizardState = { importProgress: { current: 0, total: 0, file: "" }, isLoading: false, error: null, + errorValues: null, configuredSourceNames: new Set(), importedFilesBySource: new Map(), configTemplates: [], @@ -215,7 +248,7 @@ export function wizardReducer(state: WizardState, action: WizardAction): WizardS case "SET_LOADING": return { ...state, isLoading: action.payload }; case "SET_ERROR": - return { ...state, error: action.payload, isLoading: false }; + return { ...state, error: action.payload, errorValues: action.values ?? null, isLoading: false }; case "SET_STEP": return { ...state, step: action.payload }; case "SET_IMPORT_FOLDER": @@ -390,8 +423,13 @@ export function useImportWizard() { dispatch({ type: "SET_CONFIG_TEMPLATES", payload: templates }); }, []); + /** + * Scan `folder` and read the configured sources. Never throws: a failure + * lands in the error banner, and the result says whether the scan went + * through, for a caller that has more to show after it (#384). + */ const scanFolderInternal = useCallback( - async (folder: string) => { + async (folder: string): Promise => { dispatch({ type: "SET_LOADING", payload: true }); try { // The root file names stay out of the logs (#427): on screen only. @@ -400,11 +438,13 @@ export function useImportWizard() { }); dispatch({ type: "SET_SCANNED_SOURCES", payload: scan }); await loadConfiguredSources(); + return true; } catch (e) { dispatch({ type: "SET_ERROR", payload: e instanceof Error ? e.message : String(e), }); + return false; } }, [loadConfiguredSources] @@ -435,10 +475,17 @@ export function useImportWizard() { /** * Write the two sample bank folders into the import folder (#356), rescan * it so their cards show up without another click, then raise the success - * banner. The Rust command refuses when either folder already exists and - * writes nothing; its message lands in the regular error banner. Only the - * folder NAMES reach the logs: the logs travel with a feedback report, the - * absolute path of the import folder must not. + * banner — only once the rescan went through: a failed rescan puts up its + * own error, and the two banners would contradict each other (#384). + * + * The Rust command refuses with a stable code (a folder already there, the + * import folder missing, an unreadable date) and writes nothing; + * `sampleFolderError` turns the code into the error banner's key, any other + * message stays raw. Only NAMES reach the logs, never a path: the logs + * travel with a feedback report, the absolute path of the import folder + * must not. Hence the raw refusal is logged as is: a code, or a write error + * that names a sample folder, its file and the OS error. The rescan's own + * error, which can carry the path, is not logged. */ const createSampleFolders = useCallback(async () => { const folder = state.importFolder; @@ -451,13 +498,14 @@ export function useImportWizard() { today: formatLocalDate(new Date()), }); logInfo(`[import] Sample folders created: ${names.join(", ")}`); - await scanFolderInternal(folder); - dispatch({ type: "SET_SAMPLE_CREATED", payload: names }); + if (await scanFolderInternal(folder)) { + dispatch({ type: "SET_SAMPLE_CREATED", payload: names }); + } } catch (e) { - dispatch({ - type: "SET_ERROR", - payload: e instanceof Error ? e.message : String(e), - }); + const raw = e instanceof Error ? e.message : String(e); + logWarn(`[import] Sample folders not created: ${raw}`); + const known = sampleFolderError(raw); + dispatch({ type: "SET_ERROR", payload: known ? known.key : raw, values: known?.values }); } }, [state.importFolder, scanFolderInternal]); diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 1f31f84..fec4c1c 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -334,7 +334,12 @@ "account2": "Chequing account: another source", "rootNote": "Files placed directly in the parent folder are ignored." }, - "sampleCreated": "Two sample folders were created: {{names}}. Click one of them to start the import. Delete them once your real statements are in place." + "sampleCreated": "Two sample folders were created: {{names}}. Click one of them to start the import. Delete them once your real statements are in place.", + "sampleErrors": { + "folderExists": "\"{{name}}\" already exists in the import folder: no sample folder was created. Refresh the list to see it, or rename it before trying again.", + "importFolderMissing": "The import folder cannot be found: no sample folder was created. Choose the folder again, then retry.", + "invalidDate": "Today's date could not be read: no sample folder was created." + } }, "rootFiles": { "message_one": "{{count}} file sits directly in “{{folder}}” and is ignored: {{names}}. Move it into a subfolder per account.", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 9442c90..4b50318 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -334,7 +334,12 @@ "account2": "Compte chèques : une autre source", "rootNote": "Les fichiers posés directement dans le dossier parent sont ignorés." }, - "sampleCreated": "Deux dossiers d'exemple ont été créés : {{names}}. Cliquez sur l'un d'eux pour lancer l'import. Supprimez-les quand vos vrais relevés sont en place." + "sampleCreated": "Deux dossiers d'exemple ont été créés : {{names}}. Cliquez sur l'un d'eux pour lancer l'import. Supprimez-les quand vos vrais relevés sont en place.", + "sampleErrors": { + "folderExists": "« {{name}} » existe déjà dans le dossier d'import : aucun dossier d'exemple n'a été créé. Actualisez la liste pour le voir, ou renommez-le avant de réessayer.", + "importFolderMissing": "Le dossier d'import est introuvable : aucun dossier d'exemple n'a été créé. Choisissez-le de nouveau, puis réessayez.", + "invalidDate": "La date du jour n'a pas pu être lue : aucun dossier d'exemple n'a été créé." + } }, "rootFiles": { "message_one": "{{count}} fichier est posé directement dans « {{folder}} » et est ignoré : {{names}}. Déplacez-le dans un sous-dossier par compte.", diff --git a/src/pages/ImportPage.render.test.tsx b/src/pages/ImportPage.render.test.tsx index 9095276..3270e49 100644 --- a/src/pages/ImportPage.render.test.tsx +++ b/src/pages/ImportPage.render.test.tsx @@ -367,3 +367,22 @@ describe("ImportPage — files placed directly in the import folder (#427)", () expect(render()).not.toContain("import.rootFiles"); }); }); + +describe("ImportPage — the error banner of the sample folders (#384)", () => { + it("a refusal of the command: its key, with the folder's name", () => { + harness.wizard = wizard({ + importFolder: "/home/max/Relevés", + error: "import.empty.sampleErrors.folderExists", + errorValues: { name: "Exemple Desjardins" }, + }); + expect(render()).toContain( + "t(import.empty.sampleErrors.folderExists[name=Exemple Desjardins,defaultValue=import.empty.sampleErrors.folderExists])", + ); + }); + + it("any other message: raw, through `defaultValue`", () => { + const raw = "Cannot write Exemple Desjardins/releve-2026-09.csv: Permission denied (os error 13)"; + harness.wizard = wizard({ importFolder: "/home/max/Relevés", error: raw }); + expect(render()).toContain(`t(${raw}[defaultValue=${raw}])`); + }); +}); diff --git a/src/pages/ImportPage.tsx b/src/pages/ImportPage.tsx index d964e32..6fe6039 100644 --- a/src/pages/ImportPage.tsx +++ b/src/pages/ImportPage.tsx @@ -102,11 +102,12 @@ export default function ImportPage() { {/* The wizard reports a translation key when it has one (a stored - format it cannot decode) and a raw message otherwise; `defaultValue` - renders the latter unchanged. + format it cannot decode, a refusal of the sample folders with its + values, #384) and a raw message otherwise; `defaultValue` renders + the latter unchanged. */}

- {t(state.error, { defaultValue: state.error })} + {t(state.error, { ...(state.errorValues ?? {}), defaultValue: state.error })}

)} @@ -151,8 +152,7 @@ export default function ImportPage() { (showEmptyState ? ( <>