diff --git a/src/utils/importFormat.test.ts b/src/utils/importFormat.test.ts index 0026c7c..8e450ae 100644 --- a/src/utils/importFormat.test.ts +++ b/src/utils/importFormat.test.ts @@ -416,6 +416,35 @@ describe("mapRow — debit/credit is a subtraction, not a nullity test (#325)", ).toBe(60); }); + it("errors on an unreadable cell even when its sibling parses", () => { + // The review finding on #336: testing `isNaN(debit) && isNaN(credit)` only + // caught the case where BOTH sides fell. But 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. + expect( + outcome(mapRow(["05/01/2025", "EPICERIE", "n/a", "0,00"], DC_FORMAT)) + ).toBe(ROW_ERROR_KEYS.invalidAmount); + expect( + outcome(mapRow(["05/01/2025", "EPICERIE", "84,32 CAD", "0,00"], DC_FORMAT)) + ).toBe(ROW_ERROR_KEYS.invalidAmount); + // Symmetric: an unreadable credit beside a readable debit. + expect( + outcome(mapRow(["15/01/2025", "PAIE", "0,00", "1 250 $ CAD"], DC_FORMAT)) + ).toBe(ROW_ERROR_KEYS.invalidAmount); + }); + + it("still treats an EMPTY cell as absent, not as unreadable", () => { + // The distinction the fix rests on: empty means "this column does not apply + // to this row", which is the normal shape of a debit/credit file. + expect( + outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", ""], DC_FORMAT)) + ).toBe(-84.32); + expect( + outcome(mapRow(["05/01/2025", "EPICERIE", "84,32", " "], DC_FORMAT)) + ).toBe(-84.32); + }); + it("errors when NEITHER column is readable, instead of importing 0", () => { expect(outcome(mapRow(["05/01/2025", "X", "", ""], DC_FORMAT))).toBe( ROW_ERROR_KEYS.invalidAmount diff --git a/src/utils/importFormat.ts b/src/utils/importFormat.ts index 15cdc9c..7bbd836 100644 --- a/src/utils/importFormat.ts +++ b/src/utils/importFormat.ts @@ -277,8 +277,12 @@ export interface MapRowOptions { * convention (both columns hold positive numbers) even when an export negates * its debits. * - * A row whose amount is unreadable in BOTH columns is an error, never a 0: an - * amount nobody could read must not enter the ledger as a free transaction. + * A mapped cell that is NOT EMPTY but does not parse is an error, whatever its + * sibling holds. Testing `isNaN(debit) && isNaN(credit)` was not enough: the + * unused column carries `0,00` in exactly the files this rule exists to fix, so + * an unreadable debit beside a `0,00` credit parsed as 0 − 0 = 0 and imported + * silently — the very bug, one cell over. An EMPTY cell is different: it means + * the column does not apply to this row, and contributes zero. */ export function mapRow( raw: string[], @@ -304,6 +308,22 @@ export function mapRow( decimalSeparator: options.decimalSeparators?.get(col), }); + /** + * Read one side of a debit/credit pair. `present` separates "the column does + * not apply to this row" (empty cell, contributes zero) from "the column says + * something we cannot read" (an error) — a distinction `isNaN` alone cannot + * make once the other side parses. + */ + const readSide = ( + col: number | undefined + ): { present: boolean; readable: boolean; value: number } => { + if (col === undefined) return { present: false, readable: true, value: 0 }; + const text = raw[col]?.trim() ?? ""; + if (!text) return { present: false, readable: true, value: 0 }; + const value = readAmount(col); + return { present: true, readable: !isNaN(value), value }; + }; + // 1. Configuration. An unmapped amount column used to fall back to `?? 0`, // reading column 0 — usually the date — for every row of the file. That is // a format error affecting the whole import, so it is reported before any @@ -325,14 +345,20 @@ export function mapRow( // 3. Amount. let amount: number; if (format.amountMode === "debit_credit") { - const debit = debitMapped ? readAmount(mapping.debitAmount!) : NaN; - const credit = creditMapped ? readAmount(mapping.creditAmount!) : NaN; - if (isNaN(debit) && isNaN(credit)) { + const debit = readSide(debitMapped ? mapping.debitAmount : undefined); + const credit = readSide(creditMapped ? mapping.creditAmount : undefined); + // A cell that holds something we cannot read fails the row even when its + // sibling parses — otherwise the `0,00` filler silently answers for it. + if (!debit.readable || !credit.readable) { + return fail(ROW_ERROR_KEYS.invalidAmount); + } + // Both empty: the row states no amount at all. + if (!debit.present && !credit.present) { return fail(ROW_ERROR_KEYS.invalidAmount); } amount = - (isNaN(credit) ? 0 : Math.abs(credit)) - - (isNaN(debit) ? 0 : Math.abs(debit)); + (credit.present ? Math.abs(credit.value) : 0) - + (debit.present ? Math.abs(debit.value) : 0); } else { amount = readAmount(mapping.amount!); if (isNaN(amount)) return fail(ROW_ERROR_KEYS.invalidAmount);