✨ Bank sync avoid reimporting deleted transactions (#4644)

* matchTransactions imported_id query checks for deleted transactions

* added release notes

* removed stray import

* Added configuration option to control reimporting deleted transactions

* Updated release notes

* Unused import

* Typo

* Linting errors

* Fixed Checkbox id to match what it's for

* Added tooltip for the checkbox

---------

Co-authored-by: Alec Bakholdin <abakho@icims.com>

packages/desktop-client/src/components/banksync/EditSyncAccount.tsx

@@ -2,8 +2,11 @@ import React, { useMemo, useState } from 'react';
 import { Trans, useTranslation } from 'react-i18next';
 
 import { Button } from '@actual-app/components/button';
+import { SvgQuestion } from '@actual-app/components/icons/v1';
 import { Stack } from '@actual-app/components/stack';
 import { Text } from '@actual-app/components/text';
+import { Tooltip } from '@actual-app/components/tooltip';
+import { View } from '@actual-app/components/view';
 
 import { useTransactions } from 'loot-core/client/data-hooks/transactions';
 import {
@@ -122,6 +125,9 @@ export function EditSyncAccount({ account }: EditSyncAccountProps) {
   const [savedImportPending = true, setSavedImportPending] = useSyncedPref(
     `sync-import-pending-${account.id}`,
   );
+  const [savedReimportDeleted = true, setSavedReimportDeleted] = useSyncedPref(
+    `sync-reimport-deleted-${account.id}`,
+  );
 
   const [transactionDirection, setTransactionDirection] =
     useState<TransactionDirection>('payment');
@@ -131,6 +137,9 @@ export function EditSyncAccount({ account }: EditSyncAccountProps) {
   const [importNotes, setImportNotes] = useState(
     String(savedImportNotes) === 'true',
   );
+  const [reimportDeleted, setReimportDeleted] = useState(
+    String(savedReimportDeleted) === 'true',
+  );
   const [mappings, setMappings] = useState<Mappings>(
     mappingsFromString(savedMappings),
   );
@@ -168,6 +177,7 @@ export function EditSyncAccount({ account }: EditSyncAccountProps) {
     setSavedMappings(mappingsStr);
     setSavedImportPending(String(importPending));
     setSavedImportNotes(String(importNotes));
+    setSavedReimportDeleted(String(reimportDeleted));
     close();
   };
 
@@ -226,6 +236,31 @@ export function EditSyncAccount({ account }: EditSyncAccountProps) {
             <Trans>Import transaction notes</Trans>
           </CheckboxOption>
 
+          <CheckboxOption
+            id="form_reimport_deleted"
+            checked={reimportDeleted}
+            onChange={() => setReimportDeleted(!reimportDeleted)}
+          >
+            <Tooltip
+              content={t(
+                'By default imported transactions that you delete will be re-imported with the next bank sync operation. To disable this behaviour - untick this box.',
+              )}
+            >
+              <View
+                style={{
+                  display: 'flex',
+                  flexWrap: 'nowrap',
+                  flexDirection: 'row',
+                  alignItems: 'center',
+                  gap: 4,
+                }}
+              >
+                <Trans>Reimport deleted transactions</Trans>
+                <SvgQuestion height={12} width={12} cursor="pointer" />
+              </View>
+            </Tooltip>
+          </CheckboxOption>
+
           <Stack
             direction="row"
             justify="flex-end"

packages/loot-core/src/server/accounts/__snapshots__/sync.test.ts.snap

@@ -1,5 +1,87 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Account sync reconcile does rematch deleted transactions by default 1`] = `
+[
+  {
+    "account": "one",
+    "amount": 0,
+    "category": null,
+    "cleared": 1,
+    "date": 20200101,
+    "error": null,
+    "id": "id3",
+    "imported_id": "finid",
+    "imported_payee": null,
+    "is_child": 0,
+    "is_parent": 0,
+    "notes": null,
+    "parent_id": null,
+    "payee": null,
+    "payee_name": null,
+    "raw_synced_data": null,
+    "reconciled": 0,
+    "schedule": null,
+    "sort_order": 123456789,
+    "starting_balance_flag": 0,
+    "tombstone": 1,
+    "transfer_id": null,
+  },
+  {
+    "account": "one",
+    "amount": 0,
+    "category": null,
+    "cleared": 1,
+    "date": 20200101,
+    "error": null,
+    "id": "id4",
+    "imported_id": "finid",
+    "imported_payee": null,
+    "is_child": 0,
+    "is_parent": 0,
+    "notes": null,
+    "parent_id": null,
+    "payee": null,
+    "payee_name": null,
+    "raw_synced_data": null,
+    "reconciled": 0,
+    "schedule": null,
+    "sort_order": 123456789,
+    "starting_balance_flag": 0,
+    "tombstone": 0,
+    "transfer_id": null,
+  },
+]
+`;
+
+exports[`Account sync reconcile doesnt rematch deleted transactions if reimport disabled 1`] = `
+[
+  {
+    "account": "one",
+    "amount": 0,
+    "category": null,
+    "cleared": 1,
+    "date": 20200101,
+    "error": null,
+    "id": "id3",
+    "imported_id": "finid",
+    "imported_payee": null,
+    "is_child": 0,
+    "is_parent": 0,
+    "notes": null,
+    "parent_id": null,
+    "payee": null,
+    "payee_name": null,
+    "raw_synced_data": null,
+    "reconciled": 0,
+    "schedule": null,
+    "sort_order": 123456789,
+    "starting_balance_flag": 0,
+    "tombstone": 1,
+    "transfer_id": null,
+  },
+]
+`;
+
 exports[`Account sync reconcile handles transactions with undefined fields 1`] = `
 [
   {

packages/loot-core/src/server/accounts/sync.test.ts

@@ -1,5 +1,6 @@
 // @ts-strict-ignore
 import * as monthUtils from '../../shared/months';
+import { SyncedPrefs } from '../../types/prefs';
 import * as db from '../db';
 import { loadMappings } from '../db/mappings';
 import { post } from '../post';
@@ -123,6 +124,49 @@ describe('Account sync', () => {
     );
   });
 
+  test('reconcile doesnt rematch deleted transactions if reimport disabled', async () => {
+    const { id: acctId } = await prepareDatabase();
+    const reimportKey =
+      `sync-reimport-deleted-${acctId}` satisfies keyof SyncedPrefs;
+    await db.update('preferences', { id: reimportKey, value: 'false' });
+
+    await reconcileTransactions(acctId, [
+      { date: '2020-01-01', imported_id: 'finid' },
+    ]);
+
+    const transactions1 = await getAllTransactions();
+    expect(transactions1.length).toBe(1);
+
+    await db.deleteTransaction(transactions1[0]);
+
+    await reconcileTransactions(acctId, [
+      { date: '2020-01-01', imported_id: 'finid' },
+    ]);
+    const transactions2 = await getAllTransactions();
+    expect(transactions2.length).toBe(1);
+    expect(transactions2).toMatchSnapshot();
+  });
+
+  test('reconcile does rematch deleted transactions by default', async () => {
+    const { id: acctId } = await prepareDatabase();
+
+    await reconcileTransactions(acctId, [
+      { date: '2020-01-01', imported_id: 'finid' },
+    ]);
+
+    const transactions1 = await getAllTransactions();
+    expect(transactions1.length).toBe(1);
+
+    await db.deleteTransaction(transactions1[0]);
+
+    await reconcileTransactions(acctId, [
+      { date: '2020-01-01', imported_id: 'finid' },
+    ]);
+    const transactions2 = await getAllTransactions();
+    expect(transactions2.length).toBe(2);
+    expect(transactions2).toMatchSnapshot();
+  });
+
   test('reconcile run rules with inferred payee', async () => {
     const { id: acctId } = await prepareDatabase();
     await db.insertCategoryGroup({

packages/loot-core/src/server/accounts/sync.ts

@@ -618,6 +618,12 @@ export async function matchTransactions(
 ) {
   console.log('Performing transaction reconciliation matching');
 
+  const reimportDeleted = await runQuery(
+    q('preferences')
+      .filter({ id: `sync-reimport-deleted-${acctId}` })
+      .select('value'),
+  ).then(data => String(data?.data?.[0]?.value ?? 'true') === 'true');
+
   const hasMatched = new Set();
 
   const transactionNormalization = isBankSyncAccount
@@ -649,8 +655,11 @@ export async function matchTransactions(
     // is the highest fidelity match and should always be attempted
     // first.
     if (trans.imported_id) {
-      match = await db.first<db.DbViewTransaction>(
-        'SELECT * FROM v_transactions WHERE imported_id = ? AND account = ?',
+      const table = reimportDeleted
+        ? 'v_transactions'
+        : 'v_transactions_internal';
+      match = await db.first<db.DbTransaction>(
+        `SELECT * FROM ${table} WHERE imported_id = ? AND account = ?`,
         [trans.imported_id, acctId],
       );
 

packages/loot-core/src/types/prefs.d.ts

@@ -32,6 +32,7 @@ export type SyncedPrefs = Partial<
     | `csv-has-header-${string}`
     | `custom-sync-mappings-${string}`
     | `sync-import-pending-${string}`
+    | `sync-reimport-deleted-${string}`
     | `sync-import-notes-${string}`
     | `ofx-fallback-missing-payee-${string}`
     | `flip-amount-${string}-${'csv' | 'qif'}`

upcoming-release-notes/4644.md

@@ -0,0 +1,6 @@
+---
+category: Enhancements
+authors: [alecbakholdin]
+---
+
+Added ability to configure whether deleted transactions are reimported on bank sync