From 6e687dc9b5fb6ec823be6670553d8a77af7bc596 Mon Sep 17 00:00:00 2001 From: Matthias Date: Tue, 23 Jun 2026 12:37:41 +0200 Subject: [PATCH] Fix FinTS transaction deduplication --- ...5 - Fix-FinTS-transaction-deduplication.md | 52 +++++ convex/bank/fintsMap.test.ts | 65 ++++++ convex/bank/fintsMap.ts | 50 ++++- convex/imports.test.ts | 198 ++++++++++++++++++ convex/imports.ts | 13 ++ convex/schema.ts | 1 + convex/transactions.test.ts | 72 +++++++ convex/transactions.ts | 2 +- 8 files changed, 451 insertions(+), 2 deletions(-) create mode 100644 backlog/tasks/task-15 - Fix-FinTS-transaction-deduplication.md create mode 100644 convex/bank/fintsMap.test.ts create mode 100644 convex/imports.test.ts diff --git a/backlog/tasks/task-15 - Fix-FinTS-transaction-deduplication.md b/backlog/tasks/task-15 - Fix-FinTS-transaction-deduplication.md new file mode 100644 index 0000000..0748b52 --- /dev/null +++ b/backlog/tasks/task-15 - Fix-FinTS-transaction-deduplication.md @@ -0,0 +1,52 @@ +--- +id: TASK-15 +title: Fix FinTS transaction deduplication +status: Done +assignee: [] +created_date: '2026-06-23 10:22' +updated_date: '2026-06-23 10:36' +labels: [] +dependencies: [] +priority: high +ordinal: 15000 +--- + +## Description + + +Repair FinTS import deduplication so placeholder MT940/CAMT references do not cause valid comdirect Girokonto transactions to be skipped. + + +## Acceptance Criteria + +- [x] #1 FinTS placeholder references such as NONREF and NOTPROVIDED are not stored as externalRef +- [x] #2 Multiple FinTS rows with placeholder references but distinct transaction content import successfully +- [x] #3 Real duplicate externalRef values are still skipped +- [x] #4 Repeated import of the same rows skips via dedupHash +- [x] #5 Focused tests cover FinTS mapping and import commit behavior + + +## Implementation Plan + + +1. Add failing regression tests for FinTS placeholder references and import skip behavior +2. Normalize FinTS external references so placeholders are discarded +3. Add import skip diagnostics for externalRef vs dedupHash +4. Run focused tests and project verification + + +## Implementation Notes + + +Implemented FinTS externalRef normalization for MT940/CAMT placeholder values and added import commit diagnostics split by externalRef and dedupHash skips. Verified with focused Vitest, broader Convex tests, focused ESLint, diff whitespace check, and production build. + +Follow-up from live Convex logs: all 42 rows were skipped by externalRef. Read-only Convex data inspection showed FinTS externalRef values like POS 13, which are MT940 positional markers and collide across statement periods. Extended normalization to discard POS-style references and added regression coverage for legacy POS externalRef records not blocking new rows. + +Follow-up from visible UI gaps: imported rows existed, but transactions.list used by_user_effmonth ordering for assignment-month mode, which sorts by effectiveMonth and creation time rather than bookingDate. Added by_user_effmonth_booking index and switched effective-basis listing to it so current month rows are ordered by booking date descending. Added regression coverage. + + +## Final Summary + + +Fixed FinTS transaction deduplication by discarding placeholder and POS-style external references, added import skip diagnostics, and corrected effective-month transaction listing to order by booking date. Verified with focused and related Vitest suites, focused ESLint, diff check, and production build. + diff --git a/convex/bank/fintsMap.test.ts b/convex/bank/fintsMap.test.ts new file mode 100644 index 0000000..2f9b1a2 --- /dev/null +++ b/convex/bank/fintsMap.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test } from "vitest"; +import { mapFinTsTransaction } from "./fintsMap"; + +const salaryShift = { + enabled: true, + categoryNames: ["Gehalt & Besoldung"], + dayThreshold: 25, +}; + +function baseTransaction(overrides: Partial[0]> = {}) { + return { + valueDate: new Date(2026, 5, 22), + entryDate: new Date(2026, 5, 22), + amount: -12.34, + transactionType: "NMSC", + bankReference: "", + customerReference: "", + bookingText: "Kartenzahlung", + purpose: "REWE SAGT DANKE", + remoteName: "REWE", + ...overrides, + }; +} + +describe("mapFinTsTransaction", () => { + test("does not expose MT940 placeholder references as external references", () => { + const placeholders = [ + "NONREF", + "NOTPROVIDED", + "NOT PROVIDED", + "N/A", + "POS 13", + "POS-42", + "POS_1155", + "-", + "", + ]; + + for (const placeholder of placeholders) { + const mapped = mapFinTsTransaction( + baseTransaction({ + bankReference: placeholder, + customerReference: placeholder, + }), + [], + salaryShift, + ); + + expect(mapped.externalRef).toBeUndefined(); + } + }); + + test("keeps real FinTS references for provider-level duplicate detection", () => { + const mapped = mapFinTsTransaction( + baseTransaction({ + bankReference: "5J2C21XL0470L56V/39761", + customerReference: "NONREF", + }), + [], + salaryShift, + ); + + expect(mapped.externalRef).toBe("5J2C21XL0470L56V/39761"); + }); +}); diff --git a/convex/bank/fintsMap.ts b/convex/bank/fintsMap.ts index 8c450ee..4111671 100644 --- a/convex/bank/fintsMap.ts +++ b/convex/bank/fintsMap.ts @@ -14,6 +14,54 @@ export type FintsStatementTransaction = { customerReference?: string; }; +const FINTS_EXTERNAL_REF_PLACEHOLDERS = new Set([ + "0", + "00", + "000", + "0000", + "00000", + "000000", + "0000000", + "00000000", + "000000000", + "0000000000", + "KEINE", + "N/A", + "NA", + "NONE", + "NONREF", + "NOREF", + "NOTPROVIDED", + "NULL", + "UNBEKANNT", +]); + +function normalizeFinTsExternalRefValue(value?: string): string | undefined { + const trimmed = value?.trim(); + if (!trimmed) return undefined; + + const compact = trimmed.replace(/[\s_-]+/g, "").toUpperCase(); + if ( + !compact || + FINTS_EXTERNAL_REF_PLACEHOLDERS.has(compact) || + /^POS\d+$/.test(compact) + ) { + return undefined; + } + + return trimmed; +} + +export function normalizeFinTsExternalRef( + bankReference?: string, + customerReference?: string, +): string | undefined { + return ( + normalizeFinTsExternalRefValue(bankReference) ?? + normalizeFinTsExternalRefValue(customerReference) + ); +} + export function formatFinTsDate(date: Date): string { const y = date.getFullYear(); const m = String(date.getMonth() + 1).padStart(2, "0"); @@ -55,7 +103,7 @@ export function mapFinTsTransaction( vorgang, isPending: false, rawText: rawText || undefined, - externalRef: tx.bankReference || tx.customerReference || undefined, + externalRef: normalizeFinTsExternalRef(tx.bankReference, tx.customerReference), categoryName, assignedMonth, effectiveMonth, diff --git a/convex/imports.test.ts b/convex/imports.test.ts new file mode 100644 index 0000000..4d958db --- /dev/null +++ b/convex/imports.test.ts @@ -0,0 +1,198 @@ +/// + +import { convexTest } from "convex-test"; +import { makeFunctionReference } from "convex/server"; +import { describe, expect, test } from "vitest"; +import type { Id } from "./_generated/dataModel"; +import schema from "./schema"; + +const modules = import.meta.glob("./**/*.ts"); +delete modules["./imports.test.ts"]; + +const commitRowsInternal = makeFunctionReference<"mutation">( + "imports:commitRowsInternal", +); + +async function seedImportFixture() { + const t = convexTest(schema, modules); + + const seeded = await t.run(async (ctx) => { + const userId = await ctx.db.insert("users", { + name: "Import User", + email: "import@example.com", + }); + const accountId = await ctx.db.insert("accounts", { + userId, + name: "Girokonto", + type: "giro", + iban: "DE40200411110592825400", + openingBalance: 0, + currency: "EUR", + isArchived: false, + externalId: "592825400", + }); + + return { userId, accountId }; + }); + + return { t, seeded }; +} + +describe("imports.commitRowsInternal", () => { + test("imports multiple FinTS rows without external references when transaction content differs", async () => { + const { t, seeded } = await seedImportFixture(); + + const result = await t.mutation(commitRowsInternal, { + userId: seeded.userId, + filename: "fints-sync-2026-06-15-2026-06-23", + source: "fints", + accountId: seeded.accountId as Id<"accounts">, + rows: [ + { + accountId: seeded.accountId as Id<"accounts">, + bookingDate: "2026-06-22", + valueDate: "2026-06-22", + description: "REWE", + amount: -12.34, + vorgang: "NMSC", + isPending: false, + rawText: "REWE SAGT DANKE", + }, + { + accountId: seeded.accountId as Id<"accounts">, + bookingDate: "2026-06-22", + valueDate: "2026-06-22", + description: "ALDI", + amount: -23.45, + vorgang: "NMSC", + isPending: false, + rawText: "ALDI SAGT DANKE", + }, + ], + }); + + const rows = await t.run(async (ctx) => + ctx.db + .query("transactions") + .withIndex("by_user_account", (q) => + q.eq("userId", seeded.userId).eq("accountId", seeded.accountId), + ) + .collect(), + ); + + expect(result.importedCount).toBe(2); + expect(result.skippedCount).toBe(0); + expect(rows.map((row) => row.description).sort()).toEqual(["ALDI", "REWE"]); + expect(rows.every((row) => row.externalRef === undefined)).toBe(true); + }); + + test("does not let a legacy positional external reference block new FinTS rows without external references", async () => { + const { t, seeded } = await seedImportFixture(); + + await t.run(async (ctx) => { + await ctx.db.insert("transactions", { + userId: seeded.userId, + accountId: seeded.accountId, + bookingDate: "2024-09-30", + valueDate: "2024-09-30", + description: "Old positional transaction", + amount: -9.99, + isPending: false, + effectiveMonth: "2024-09", + dedupHash: "legacy-dedup", + externalRef: "592825400:POS 13", + }); + }); + + const result = await t.mutation(commitRowsInternal, { + userId: seeded.userId, + filename: "fints-sync-2026-06-15-2026-06-23", + source: "fints", + accountId: seeded.accountId as Id<"accounts">, + rows: [ + { + accountId: seeded.accountId as Id<"accounts">, + bookingDate: "2026-06-22", + valueDate: "2026-06-22", + description: "HEM TANKSTELLE", + amount: -62.01, + vorgang: "NMSC", + isPending: false, + rawText: "HEM TANKSTELLE", + }, + ], + }); + + expect(result.importedCount).toBe(1); + expect(result.skippedCount).toBe(0); + }); + + test("still skips real duplicate external references", async () => { + const { t, seeded } = await seedImportFixture(); + const row = { + accountId: seeded.accountId as Id<"accounts">, + bookingDate: "2026-06-22", + valueDate: "2026-06-22", + description: "Kartenzahlung", + amount: -12.34, + vorgang: "NMSC", + isPending: false, + rawText: "Kartenzahlung", + externalRef: "592825400:5J2C21XL0470L56V/39761", + }; + + const first = await t.mutation(commitRowsInternal, { + userId: seeded.userId, + filename: "fints-sync-first", + source: "fints", + accountId: seeded.accountId as Id<"accounts">, + rows: [row], + }); + const second = await t.mutation(commitRowsInternal, { + userId: seeded.userId, + filename: "fints-sync-second", + source: "fints", + accountId: seeded.accountId as Id<"accounts">, + rows: [{ ...row, description: "Different text same bank ref", amount: -99 }], + }); + + expect(first.importedCount).toBe(1); + expect(first.skippedCount).toBe(0); + expect(second.importedCount).toBe(0); + expect(second.skippedCount).toBe(1); + }); + + test("skips repeated rows without external references via dedup hash", async () => { + const { t, seeded } = await seedImportFixture(); + const row = { + accountId: seeded.accountId as Id<"accounts">, + bookingDate: "2026-06-22", + valueDate: "2026-06-22", + description: "REWE", + amount: -12.34, + vorgang: "NMSC", + isPending: false, + rawText: "REWE SAGT DANKE", + }; + + const first = await t.mutation(commitRowsInternal, { + userId: seeded.userId, + filename: "fints-sync-first", + source: "fints", + accountId: seeded.accountId as Id<"accounts">, + rows: [row], + }); + const second = await t.mutation(commitRowsInternal, { + userId: seeded.userId, + filename: "fints-sync-second", + source: "fints", + accountId: seeded.accountId as Id<"accounts">, + rows: [row], + }); + + expect(first.importedCount).toBe(1); + expect(first.skippedCount).toBe(0); + expect(second.importedCount).toBe(0); + expect(second.skippedCount).toBe(1); + }); +}); diff --git a/convex/imports.ts b/convex/imports.ts index 6003e59..aaec7b5 100644 --- a/convex/imports.ts +++ b/convex/imports.ts @@ -132,6 +132,8 @@ async function commitRowsHandler( let importedCount = 0; let skippedCount = 0; + let skippedByExternalRef = 0; + let skippedByDedupHash = 0; for (const row of args.rows) { if (row.externalRef) { @@ -143,6 +145,7 @@ async function commitRowsHandler( .unique(); if (existingRef) { skippedCount++; + skippedByExternalRef++; continue; } } @@ -162,6 +165,7 @@ async function commitRowsHandler( .unique(); if (existingDedup) { skippedCount++; + skippedByDedupHash++; continue; } @@ -204,5 +208,14 @@ async function commitRowsHandler( status: "completed", }); + console.info("[imports] Commit-Ergebnis", { + source: args.source, + rowCount: args.rows.length, + importedCount, + skippedCount, + skippedByExternalRef, + skippedByDedupHash, + }); + return { importId, importedCount, skippedCount }; } diff --git a/convex/schema.ts b/convex/schema.ts index c9ab57e..ccc209a 100644 --- a/convex/schema.ts +++ b/convex/schema.ts @@ -77,6 +77,7 @@ export default defineSchema({ .index("by_user", ["userId"]) .index("by_user_booking", ["userId", "bookingDate"]) .index("by_user_effmonth", ["userId", "effectiveMonth"]) + .index("by_user_effmonth_booking", ["userId", "effectiveMonth", "bookingDate"]) .index("by_user_category", ["userId", "categoryId"]) .index("by_user_account", ["userId", "accountId"]) .index("by_user_account_booking", ["userId", "accountId", "bookingDate"]) diff --git a/convex/transactions.test.ts b/convex/transactions.test.ts index 0826038..50b02e7 100644 --- a/convex/transactions.test.ts +++ b/convex/transactions.test.ts @@ -186,4 +186,76 @@ describe("transactions.list", () => { expect(effectiveResult.page.map((tx) => tx._id)).toEqual([seeded.shiftedId]); expect(bookingResult.page).toEqual([]); }); + + test("orders effective month results by booking date instead of import creation time", async () => { + const t = convexTest(schema, modules); + + const seeded = await t.run(async (ctx) => { + const userId = await ctx.db.insert("users", { + name: "Ordering User", + email: "ordering@example.com", + }); + const accountId = await ctx.db.insert("accounts", { + userId, + name: "Girokonto", + type: "checking", + openingBalance: 0, + currency: "EUR", + isArchived: false, + }); + + const olderCreatedNewestBookingId = await ctx.db.insert("transactions", { + userId, + accountId, + bookingDate: "2026-06-23", + description: "Newest booking created first", + amount: -10, + isPending: false, + effectiveMonth: "2026-06", + }); + const newerCreatedOldestBookingId = await ctx.db.insert("transactions", { + userId, + accountId, + bookingDate: "2026-06-15", + description: "Oldest booking created later", + amount: -20, + isPending: false, + effectiveMonth: "2026-06", + }); + const middleBookingId = await ctx.db.insert("transactions", { + userId, + accountId, + bookingDate: "2026-06-22", + description: "Middle booking created last", + amount: -30, + isPending: false, + effectiveMonth: "2026-06", + }); + + return { + userId, + olderCreatedNewestBookingId, + newerCreatedOldestBookingId, + middleBookingId, + }; + }); + + const asUser = t.withIdentity({ + subject: `${seeded.userId}|test-session`, + tokenIdentifier: `test:${seeded.userId}`, + }); + + const result = await asUser.query(api.transactions.list, { + paginationOpts: { cursor: null, numItems: 20 }, + from: "2026-06-01", + to: "2026-06-30", + basis: "effective", + }); + + expect(result.page.map((tx) => tx._id)).toEqual([ + seeded.olderCreatedNewestBookingId, + seeded.middleBookingId, + seeded.newerCreatedOldestBookingId, + ]); + }); }); diff --git a/convex/transactions.ts b/convex/transactions.ts index 9cbaad9..7ab4b11 100644 --- a/convex/transactions.ts +++ b/convex/transactions.ts @@ -67,7 +67,7 @@ export const list = query({ if (basis === "effective") { q = ctx.db .query("transactions") - .withIndex("by_user_effmonth", (iq) => { + .withIndex("by_user_effmonth_booking", (iq) => { if (fromMonth && toMonth) { return iq.eq("userId", userId).gte("effectiveMonth", fromMonth).lte("effectiveMonth", toMonth); }