5 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
le king fu
|
484c4beb47 |
fix(import): fail a row whose amount cell is unreadable beside a 0,00 sibling
Review finding on #325. The debit/credit rule tested isNaN(debit) && isNaN(credit), which only caught the case where BOTH sides fell. The unused column carries 0,00 in exactly the files this rule exists to fix, so an unreadable debit beside a 0,00 credit computed 0 - 0 = 0 and imported silently — the very bug, one cell over. Replayed on the PR's own unused-column-zero fixture with a currency suffix: 6 transactions imported at 0,00 with no error row. A mapped cell that is not empty but does not parse now fails the row whatever its sibling holds. An EMPTY cell keeps meaning 'this column does not apply to this row' and contributes zero, which is the normal shape of the format. Refs #325 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
le king fu
|
f377d760af |
fix(export): preserve import sources and templates across data export/import
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m41s
Exporting then re-importing data destroyed every import configuration: dataExportService serialised only categories, suppliers, keywords and transactions, then ran DELETE FROM import_sources on restore and replaced them with a synthetic 'Data Import' source. After restoring a backup, every source had to be reconfigured by hand. - Serialise import_sources and import_config_templates into the envelope, with an explicit format_version; a file without one is the earlier format and its missing arrays are treated as empty. - Wrap wipe + restore in withTransaction, which the service had nowhere: a constraint violation mid-restore used to destroy financial history with no rollback. - Restore templates BEFORE sources (template_id is a foreign key), upserting by name and remapping template_id through the resolved ids, so restoring into a profile that already has templates no longer hits UNIQUE(name). - Whitelist amount_mode and sign_convention at the import boundary with a readable message rather than an SQLite constraint error. Resolves #331 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
le king fu
|
ce19efd476 |
feat(import): make the preview mandatory and show what the amounts mean
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m42s
A confidence score reports how many rows were READ, never what they say:
the `all-positive` fixture scores a perfect 100 % while every credit is
imported as an expense, because each of those rows is perfectly readable.
Nothing between that score and the database looked at the signs — the
preview was an optional modal of 20 rows with no totals, and the final
confirmation listed the delimiter and the date format but neither the
amount mode nor the sign convention.
The `file-preview` step had been declared in `ImportWizardStep` since the
beginning and no dispatch ever aimed at it. It is a real step now,
traversed at every import and gated by nothing — in particular not by the
detection score, which is sign-blind by construction.
- `useImportWizard`: `parseAndPreview` parses and stops at the preview,
replacing `parsePreview` and the `parseAndCheckDuplicates` that jumped
straight to the duplicates ("skips preview step"); `checkDuplicates`,
dead code until now, is the preview's next button, so the rows the user
validated are the rows that get checked.
- `summarizeParsedRows`: the recap, pure and tested — outflows and their
total, inflows and theirs, rows in error. Totals stay SIGNED, since
magnitudes would hide the one thing the recap exists to expose. Computed
over the whole file, never over the twenty rows displayed.
- `flipSignFormat` + "Inverser les signes": the correction lands on the
CONFIGURATION, so it is persisted with the source and the next file from
that bank reads right on its own. In debit/credit mode it swaps the two
column indices rather than toggling a convention `mapRow` ignores there,
where a toggle would have been inert.
- `ImportConfirmation` states the amount mode, the sign convention (in the
mode that applies it) and the column mapping, named by header.
- `FilePreviewModal` removed: it was the redundant surface, and editing
the table alone would have mutated a still-live copy of it.
The `all-positive` KNOWN DEFECT marker is dropped rather than deleted. Its
three original expectations still hold — an unsigned file carries no
direction and detection cannot invent one — and two cases were added: the
recap tell (six outflows, zero inflows) and the honest limit, that
flipping this particular file only produces its mirror image.
1088 vitest (1054 before), tsc and vite build clean. No DB migration.
Resolves #329
|
||
|
le king fu
|
7b6f063094 |
fix(import): read amounts with an anchored parser and a real debit/credit rule
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m44s
Three ways an imported amount could be silently wrong, all of them passing validation, all of them fixed here. The two-column rule was `isNaN(credit) ? -debit : credit`, so the credit always won. Many banks write `0,00` in the unused column rather than leaving it empty, and `isNaN(0)` is false -- every debit of such a file imported as 0,00 and the expense simply vanished, with no error anywhere. The rule is `credit - debit` on magnitudes now, which needs no special case for a `0,00` cell (zero is the identity of the subtraction) and implements the documented convention even when an export negates its debits. A row unreadable in BOTH columns is an error instead of a free 0,00 transaction. `parseFrenchAmount` ended on `parseFloat`, which returns the longest valid PREFIX instead of rejecting. Measured before the fix: `"50,00-"` -> 5000, `"1 234,56 CR"` -> 123456, `"100,00 CAD"` -> 10000. A factor-100 error, and it passes `isNaN`, so those rows counted as VALID everywhere downstream -- which would have defeated the signed preview (#329), the safety net of the whole chantier. Validation is anchored over the whole normalized string now and any residual character yields NaN. NaN, not a rescued magnitude, for a trailing `CR`/`DB` or currency code. Two reasons: `CR`/`DB` carry a DIRECTION, so returning a magnitude for both would trade a loud failure for a silent SIGN error (the D/C-indicator shape is refused upstream by design, #328); and `"100,00 CAD"` is structurally identical to `"2025 Montant"`, so whitelisting a trailing word to rescue the first re-blinds `detectHeader` on the second. Two accounting forms ARE legitimate and supported: parentheses `(50,00)` and a trailing sign `50,00-`. The `?? 0` fallbacks read column 0 -- usually the date -- when the mapping was incomplete. An unmapped amount column is an explicit row error now, reported ahead of any per-row problem since it is a format error affecting every row. `1.234` is 1234 in a French column and 1.234 in an English one, and no rule applied to that cell ALONE can tell. `detectDecimalSeparator` arbitrates from the decisive siblings of the column and `parseFrenchAmount` takes the verdict as an option. Detection deliberately stays out of it: it runs before a column is known to be an amount column at all, so the verdict is applied where the value actually becomes a transaction. The rule itself moves out of the hook as a pure `mapRow(raw, format)` in `importFormat.ts`. That is what lets the corpus tests run the REAL rule -- the hand-written mirror in `csvAutoDetect.test.ts` and the static guard pinning five `parseFilesInternal` expressions are both deleted -- and what stops the detection score (#328) and the signed preview (#329) each re-implementing it. Hardening is global: the parser is shared by 11 call sites, 8 in `csvAutoDetect.ts` and 3 in the holdings CSV import (#245), where a price cell `150,25 CAD` used to store 15025. It is refused now and `buildDetailedLines` raises on the empty price. An unreadable QUANTITY was worse -- coerced to 0, so a zero-value position saved in silence; the draft keeps the offending text instead and the existing `snapshot_priced_quantity_required` fires. Row errors become i18n keys (`import.rowErrors.*`) rather than the raw English literals rendered straight into the preview table, since this adds a user-visible string. The report table also carries raw exception messages, so both render sites resolve through `isRowErrorKey` and never feed `t()` anything that is not ours. Test churn, per link 1's handoff (update the expectation, drop the marker, never delete the test): the three `#325` KNOWN DEFECT blocks in `amountParser.test.ts` flip, plus `unused-column-zero` in `csvAutoDetect.test.ts`. One block tagged `#328` flips too -- `header-numeric-label`, whose own comment reads "#328 adds a lexical signal to detectHeader, and #325 anchors the parser [...] either fix closes this". The anchored parser landed first. The other `#328` blocks (`debit-credit-reversed`, `absolute-indicator`) and the `#329` block are verified unchanged. CHANGELOG and docs stay centralized in the last link of the stack, as the plan specifies. 989 vitest (963 before), tsc + vite build clean, cargo check clean. No DB migration. Resolves #325 |
||
|
le king fu
|
7a604e0e0d |
fix(import): persist the import format and restore it faithfully
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m48s
The root bug of this chantier. `import_sources` carried no `amount_mode` and no `sign_convention` until v17, so restoring a configured source re-inferred the mode from `mapping.debitAmount !== undefined` and wrote `signConvention: "negative_expense"` outright (useImportWizard.ts:321-323). A credit-card statement configured for positive expenses came back on the default convention at its second import and `parseFilesInternal` negated every amount: expenses landed as income, with no error shown anywhere. The format is a read value now, not a guessed one. Two types and a codec, not one composed type. The four carriers are structurally incompatible -- `ImportSource.has_header` is declared boolean, `ImportConfigTemplate.has_header` is a number, `SourceConfig` is camelCase on a parsed mapping -- so the guarantee cannot come from a shared shape. It comes from `src/utils/importFormat.ts` being the single conversion point between `ImportFormatRow` (persisted: snake_case, mapping as JSON, `has_header` normalized to 0/1) and `ImportFormat` (domain), and from its completeness test. That test is enforced on two levels, and both were mutation-checked: `FORMAT_FIELD_PAIRS` is typed `Record<keyof ImportFormat, keyof ImportFormatRow>`, so a field added to the format fails to BUILD until it is listed; the test then compares each codec's real output keys against that table, so a field listed but not wired fails the TEST. Dropping `sign_convention` from `formatToRow` -- the shape of the original bug -- fails 14 tests. `formatFromRow` validates rather than falls back. The v17 CHECK admits `absolute_indicator` so the third amount mode ships without another migration, but the app cannot map one: falling through to the `single` branch would read the wrong column for every row, and anything other than `positive_expense` would silently mean `negative_expense`. It raises an `ImportFormatError` carrying an i18n key, and the wizard opens on a fresh configuration so "reconfigure this source" stays an action the user can actually take. Also here: - The config write moves from `checkDuplicatesInternal` to `executeImport`, so an import abandoned at the duplicate step leaves no configuration behind. It is the only write point in the hook and a guard test holds that. - Switching amount mode prunes the abandoned mode's columns, so the mode owns the mapping rather than the reverse. The column the `<select>` merely displays is deliberately not materialized -- #325 turns an unmapped amount column into an explicit row error, and writing a 0 here would make it unreachable. The mode and the pruned mapping land in ONE state update: the panel's handlers each spread the same `config` prop, so two calls would see the same stale value. - `template_id` is recorded and restored as provenance only, never re-read as format. `selectedTemplateId` is no longer blanked on every source selection. An acceptance test rewrites a template end to end and asserts the linked source reads identically, plus a non-vacuity check that the template really changed. - Both template writers go through the codec too, so a new format field cannot reach one table and miss the other. `parseFilesInternal` is untouched: link 1's static guard on its five pinned expressions still passes. 39 new tests (963 vitest total, was 924), build clean, `cargo check` clean, no migration. Resolves #324 |