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>
This commit is contained in:
parent
e2b8eb8b22
commit
484c4beb47
2 changed files with 62 additions and 7 deletions
|
|
@ -416,6 +416,35 @@ describe("mapRow — debit/credit is a subtraction, not a nullity test (#325)",
|
||||||
).toBe(60);
|
).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", () => {
|
it("errors when NEITHER column is readable, instead of importing 0", () => {
|
||||||
expect(outcome(mapRow(["05/01/2025", "X", "", ""], DC_FORMAT))).toBe(
|
expect(outcome(mapRow(["05/01/2025", "X", "", ""], DC_FORMAT))).toBe(
|
||||||
ROW_ERROR_KEYS.invalidAmount
|
ROW_ERROR_KEYS.invalidAmount
|
||||||
|
|
|
||||||
|
|
@ -277,8 +277,12 @@ export interface MapRowOptions {
|
||||||
* convention (both columns hold positive numbers) even when an export negates
|
* convention (both columns hold positive numbers) even when an export negates
|
||||||
* its debits.
|
* its debits.
|
||||||
*
|
*
|
||||||
* A row whose amount is unreadable in BOTH columns is an error, never a 0: an
|
* A mapped cell that is NOT EMPTY but does not parse is an error, whatever its
|
||||||
* amount nobody could read must not enter the ledger as a free transaction.
|
* 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(
|
export function mapRow(
|
||||||
raw: string[],
|
raw: string[],
|
||||||
|
|
@ -304,6 +308,22 @@ export function mapRow(
|
||||||
decimalSeparator: options.decimalSeparators?.get(col),
|
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`,
|
// 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
|
// 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
|
// a format error affecting the whole import, so it is reported before any
|
||||||
|
|
@ -325,14 +345,20 @@ export function mapRow(
|
||||||
// 3. Amount.
|
// 3. Amount.
|
||||||
let amount: number;
|
let amount: number;
|
||||||
if (format.amountMode === "debit_credit") {
|
if (format.amountMode === "debit_credit") {
|
||||||
const debit = debitMapped ? readAmount(mapping.debitAmount!) : NaN;
|
const debit = readSide(debitMapped ? mapping.debitAmount : undefined);
|
||||||
const credit = creditMapped ? readAmount(mapping.creditAmount!) : NaN;
|
const credit = readSide(creditMapped ? mapping.creditAmount : undefined);
|
||||||
if (isNaN(debit) && isNaN(credit)) {
|
// 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);
|
return fail(ROW_ERROR_KEYS.invalidAmount);
|
||||||
}
|
}
|
||||||
amount =
|
amount =
|
||||||
(isNaN(credit) ? 0 : Math.abs(credit)) -
|
(credit.present ? Math.abs(credit.value) : 0) -
|
||||||
(isNaN(debit) ? 0 : Math.abs(debit));
|
(debit.present ? Math.abs(debit.value) : 0);
|
||||||
} else {
|
} else {
|
||||||
amount = readAmount(mapping.amount!);
|
amount = readAmount(mapping.amount!);
|
||||||
if (isNaN(amount)) return fail(ROW_ERROR_KEYS.invalidAmount);
|
if (isNaN(amount)) return fail(ROW_ERROR_KEYS.invalidAmount);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue