From 93e784a0fec06f0586b2c18794ec3f6c00a7755c Mon Sep 17 00:00:00 2001
From: Matthew Strasiotto <39424834+strazto@users.noreply.github.com>
Date: Sat, 8 Jun 2024 03:47:16 +1000
Subject: [PATCH] fix(#2562): Prevent transaction deduplication for imported
 transactions (#2770)

* fix(#2562): Prevent transaction deduplication for imported transactions

* chore(): eslint fixes

* chore(): Add release note file

* fix(#2562): Allow transaction deduplication if transaction being imported is null

* chore: Rename release note, add strazto as author

* test(loot-core): Add test case for new logic

* docs(release-notes.loot-core): Add pmoon00 as author

* test(loot-core): Update test case to not be affected by unrelated bug

* test(loot-core): fix linter

---------

Co-authored-by: Mohamed El Mahdali <mohamed.elmahdali.developer@gmail.com>
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
---
 .../src/server/accounts/sync.test.ts          | 104 ++++++++++++++++++
 .../loot-core/src/server/accounts/sync.ts     |   9 +-
 upcoming-release-notes/2770.md                |   6 +
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 upcoming-release-notes/2770.md

diff --git a/packages/loot-core/src/server/accounts/sync.test.ts b/packages/loot-core/src/server/accounts/sync.test.ts
index 1ea00a4d0..13a9160a8 100644
--- a/packages/loot-core/src/server/accounts/sync.test.ts
+++ b/packages/loot-core/src/server/accounts/sync.test.ts
@@ -342,4 +342,108 @@ describe('Account sync', () => {
       'bakkerij-renamed',
     ]);
   });
+
+  test('reconcile does not merge transactions with different ‘imported_id’ values', async () => {
+    const { id } = await prepareDatabase();
+
+    let payees = await getAllPayees();
+    expect(payees.length).toBe(0);
+
+    // Add first transaction
+    await reconcileTransactions(id, [
+      {
+        date: '2024-04-05',
+        amount: -1239,
+        imported_payee: 'Acme Inc.',
+        payee_name: 'Acme Inc.',
+        imported_id: 'b85cdd57-5a1c-4ca5-bd54-12e5b56fa02c',
+        notes: 'TEST TRANSACTION',
+        cleared: true,
+      },
+    ]);
+
+    payees = await getAllPayees();
+    expect(payees.length).toBe(1);
+
+    let transactions = await getAllTransactions();
+    expect(transactions.length).toBe(1);
+
+    // Add second transaction
+    await reconcileTransactions(id, [
+      {
+        date: '2024-04-06',
+        amount: -1239,
+        imported_payee: 'Acme Inc.',
+        payee_name: 'Acme Inc.',
+        imported_id: 'ca1589b2-7bc3-4587-a157-476170b383a7',
+        notes: 'TEST TRANSACTION',
+        cleared: true,
+      },
+    ]);
+
+    payees = await getAllPayees();
+    expect(payees.length).toBe(1);
+
+    transactions = await getAllTransactions();
+    expect(transactions.length).toBe(2);
+
+    expect(
+      transactions.find(
+        t => t.imported_id === 'b85cdd57-5a1c-4ca5-bd54-12e5b56fa02c',
+      ).amount,
+    ).toBe(-1239);
+    expect(
+      transactions.find(
+        t => t.imported_id === 'ca1589b2-7bc3-4587-a157-476170b383a7',
+      ).amount,
+    ).toBe(-1239);
+  });
+
+  test(
+    'given an imported tx with no imported_id, ' +
+      'when an existing transaction that has an imported_id and matches amount and is within 7 days of imported tx,' +
+      'then imported tx should reconcile with existing transaction from fuzzy match',
+    async () => {
+      const { id } = await prepareDatabase();
+
+      let payees = await getAllPayees();
+      expect(payees.length).toBe(0);
+
+      const existingTx = {
+        date: '2024-04-05',
+        amount: -1239,
+        imported_payee: 'Acme Inc.',
+        payee_name: 'Acme Inc.',
+        imported_id: 'b85cdd57-5a1c-4ca5-bd54-12e5b56fa02c',
+        notes: 'TEST TRANSACTION',
+        cleared: true,
+      };
+
+      // Add transaction to represent existing transaction with imoprted_id
+      await reconcileTransactions(id, [existingTx]);
+
+      payees = await getAllPayees();
+      expect(payees.length).toBe(1);
+
+      let transactions = await getAllTransactions();
+      expect(transactions.length).toBe(1);
+
+      // Import transaction similar to existing but with different date and no imported_id
+      await reconcileTransactions(id, [
+        {
+          ...existingTx,
+          date: '2024-04-06',
+          imported_id: null,
+        },
+      ]);
+
+      payees = await getAllPayees();
+      expect(payees.length).toBe(1);
+
+      transactions = await getAllTransactions();
+      expect(transactions.length).toBe(1);
+
+      expect(transactions[0].amount).toBe(-1239);
+    },
+  );
 });
diff --git a/packages/loot-core/src/server/accounts/sync.ts b/packages/loot-core/src/server/accounts/sync.ts
index 39f48bcdb..2b9609204 100644
--- a/packages/loot-core/src/server/accounts/sync.ts
+++ b/packages/loot-core/src/server/accounts/sync.ts
@@ -399,8 +399,15 @@ export async function reconcileTransactions(
       // fields.
       fuzzyDataset = await db.all(
         `SELECT id, is_parent, date, imported_id, payee, category, notes, reconciled FROM v_transactions
-           WHERE date >= ? AND date <= ? AND amount = ? AND account = ?`,
+           WHERE
+             -- If both ids are set, and we didn't match earlier then skip dedup
+             ( imported_id IS NULL OR ? IS NULL )
+             -- Look 7 days ahead, 7 days behind
+             AND date >= ? AND date <= ? AND amount = ?
+             AND account = ?
+        `,
         [
+          trans.imported_id || null,
           db.toDateRepr(monthUtils.subDays(trans.date, 7)),
           db.toDateRepr(monthUtils.addDays(trans.date, 7)),
           trans.amount || 0,
diff --git a/upcoming-release-notes/2770.md b/upcoming-release-notes/2770.md
new file mode 100644
index 000000000..3681a1e52
--- /dev/null
+++ b/upcoming-release-notes/2770.md
@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [ttlgeek, strazto, pmoon00]
+---
+
+Prevent transaction deduplication for imported transactions
-- 
GitLab