From 6d9b1a1d72ff736041001a9e4c6fa0a2759e7860 Mon Sep 17 00:00:00 2001 From: J B Date: Tue, 17 Mar 2026 13:25:22 -0400 Subject: [PATCH] Duplicate reimport fix in ui and API (#6926) * add options to override reimportDeleted * doc and default in ui to false * pr note * period * wording * [autofix.ci] apply automated fixes * docs clarity * actually test default behavior * Update upcoming-release-notes/6926.md Co-authored-by: Matiss Janis Aboltins * use new ImportTransactionOpts type for consistency * Release note wording Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Matiss Janis Aboltins Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- packages/api/methods.test.ts | 67 +++++++++++++++ .../desktop-client/src/accounts/mutations.ts | 6 ++ .../ImportTransactionsModal.tsx | 23 ++++++ packages/docs/docs/api/reference.md | 17 +++- packages/docs/docs/transactions/importing.md | 6 ++ packages/loot-core/src/server/accounts/app.ts | 7 +- .../src/server/accounts/sync.test.ts | 82 +++++++++++++++++++ .../loot-core/src/server/accounts/sync.ts | 16 ++-- packages/loot-core/src/types/api-handlers.ts | 1 + upcoming-release-notes/6926.md | 6 ++ 10 files changed, 222 insertions(+), 9 deletions(-) create mode 100644 upcoming-release-notes/6926.md diff --git a/packages/api/methods.test.ts b/packages/api/methods.test.ts index 3b23865aa8..8dc66bcb39 100644 --- a/packages/api/methods.test.ts +++ b/packages/api/methods.test.ts @@ -896,6 +896,73 @@ describe('API CRUD operations', () => { ); expect(transactions[0].notes).toBeNull(); }); + + test('Transactions: reimportDeleted=false prevents reimporting deleted transactions', async () => { + const accountId = await api.createAccount({ name: 'test-account' }, 0); + + // Import a transaction + const result1 = await api.importTransactions(accountId, [ + { + date: '2023-11-03', + imported_id: 'reimport-test-1', + amount: 100, + account: accountId, + }, + ]); + expect(result1.added).toHaveLength(1); + + // Delete the transaction + await api.deleteTransaction(result1.added[0]); + + // Reimport the same transaction with reimportDeleted=false + const result2 = await api.importTransactions( + accountId, + [ + { + date: '2023-11-03', + imported_id: 'reimport-test-1', + amount: 100, + account: accountId, + }, + ], + { reimportDeleted: false }, + ); + + // Should match the deleted transaction and not create a new one + expect(result2.added).toHaveLength(0); + expect(result2.updated).toHaveLength(0); + }); + + test('Transactions: reimportDeleted=true reimports deleted transactions', async () => { + const accountId = await api.createAccount({ name: 'test-account' }, 0); + + // Import a transaction + const result1 = await api.importTransactions(accountId, [ + { + date: '2023-11-03', + imported_id: 'reimport-test-2', + amount: 200, + account: accountId, + }, + ]); + expect(result1.added).toHaveLength(1); + + // Delete the transaction + await api.deleteTransaction(result1.added[0]); + + // Reimport the same transaction relying on reimportDeleted=true default + const result2 = await api.importTransactions(accountId, [ + { + date: '2023-11-03', + imported_id: 'reimport-test-2', + amount: 200, + account: accountId, + }, + ]); + + // Should create a new transaction since deleted ones are ignored + expect(result2.added).toHaveLength(1); + }); }); //apis: createSchedule, getSchedules, updateSchedule, deleteSchedule diff --git a/packages/desktop-client/src/accounts/mutations.ts b/packages/desktop-client/src/accounts/mutations.ts index d15aa24376..7f3634e7b4 100644 --- a/packages/desktop-client/src/accounts/mutations.ts +++ b/packages/desktop-client/src/accounts/mutations.ts @@ -207,6 +207,7 @@ export function useMoveAccountMutation() { type ImportPreviewTransactionsPayload = { accountId: string; transactions: TransactionEntity[]; + reimportDeleted?: boolean; }; export function useImportPreviewTransactionsMutation() { @@ -218,6 +219,7 @@ export function useImportPreviewTransactionsMutation() { mutationFn: async ({ accountId, transactions, + reimportDeleted, }: ImportPreviewTransactionsPayload) => { const { errors = [], updatedPreview } = await send( 'transactions-import', @@ -225,6 +227,7 @@ export function useImportPreviewTransactionsMutation() { accountId, transactions, isPreview: true, + opts: reimportDeleted !== undefined ? { reimportDeleted } : undefined, }, ); @@ -259,6 +262,7 @@ type ImportTransactionsPayload = { accountId: string; transactions: TransactionEntity[]; reconcile: boolean; + reimportDeleted?: boolean; }; export function useImportTransactionsMutation() { @@ -271,6 +275,7 @@ export function useImportTransactionsMutation() { accountId, transactions, reconcile, + reimportDeleted, }: ImportTransactionsPayload) => { if (!reconcile) { await send('api/transactions-add', { @@ -289,6 +294,7 @@ export function useImportTransactionsMutation() { accountId, transactions, isPreview: false, + opts: reimportDeleted !== undefined ? { reimportDeleted } : undefined, }); errors.forEach(error => { diff --git a/packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.tsx b/packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.tsx index ecb67993d8..da0dc86f85 100644 --- a/packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.tsx +++ b/packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.tsx @@ -211,6 +211,7 @@ export function ImportTransactionsModal({ const [flipAmount, setFlipAmount] = useState(false); const [multiplierEnabled, setMultiplierEnabled] = useState(false); const [reconcile, setReconcile] = useState(true); + const [reimportDeleted, setReimportDeleted] = useState(false); const [importNotes, setImportNotes] = useState(true); // This cannot be set after parsing the file, because changing it @@ -718,6 +719,7 @@ export function ImportTransactionsModal({ accountId, transactions: finalTransactions, reconcile, + reimportDeleted, }, { onSuccess: async didChange => { @@ -766,6 +768,7 @@ export function ImportTransactionsModal({ { accountId, transactions: previewTransactionsToImport, + reimportDeleted, }, { onSuccess: previewTrx => { @@ -832,6 +835,7 @@ export function ImportTransactionsModal({ startDate, fieldMappings, parseDateFormat, + reimportDeleted, ]); const headers: ComponentProps['headers'] = [ @@ -1071,6 +1075,16 @@ export function ImportTransactionsModal({ )} + {(isOfxFile(filetype) || isCamtFile(filetype)) && reconcile && ( + + Reimport deleted transactions + + )} + {/*Import Options */} {(filetype === 'qif' || filetype === 'csv') && ( @@ -1187,6 +1201,15 @@ export function ImportTransactionsModal({ > Merge with existing transactions + {reconcile && ( + + Reimport deleted transactions + + )} )} diff --git a/packages/docs/docs/api/reference.md b/packages/docs/docs/api/reference.md index 072c74afce..6c9cc17afb 100644 --- a/packages/docs/docs/api/reference.md +++ b/packages/docs/docs/api/reference.md @@ -201,7 +201,7 @@ This method is mainly for custom importers that want to skip all the automatic s #### `importTransactions` - + Adds multiple transactions at once, while going through the same process as importing a file or downloading transactions from a bank. In particular, all rules are run on the specified transactions before adding them. @@ -211,6 +211,21 @@ The import will "reconcile" transactions to avoid adding duplicates. Transaction It will also create transfers if a transfer payee is specified. See [transfers](#transfers). +This method has the following optional flags (passed as the `opts` object): + +- `defaultCleared`: whether imported transactions should be marked as cleared (defaults to `true`) +- `dryRun`: if `true`, returns what would be added/updated without actually modifying the database (defaults to `false`) +- `reimportDeleted`: if `true`, transactions that were previously imported and then deleted will be reimported; if `false`, they will be skipped (defaults to `true` for backward compatibility — note that the [file import UI](/docs/transactions/importing#avoiding-duplicate-transactions) defaults to `false`) + +Example using opts: + +```js +await api.importTransactions(accountId, transactions, { + reimportDeleted: false, + defaultCleared: false, +}); +``` + This method returns an object with the following fields: - `added`: an array of ids of transactions that were added diff --git a/packages/docs/docs/transactions/importing.md b/packages/docs/docs/transactions/importing.md index a7ba294acc..02026ff0e5 100644 --- a/packages/docs/docs/transactions/importing.md +++ b/packages/docs/docs/transactions/importing.md @@ -50,3 +50,9 @@ Actual will automatically try to avoid duplicate transactions. This works best w After checking the **id**, Actual will look for transactions around the same date, with the same amount, and with a similar payee. If it thinks the transaction already exists, it will avoid creating a duplicate. This means you can manually enter a transaction, and later it will be matched when you import it from a file. It will always favor the imported transaction. If it matches a manually-entered transaction, it will update the date to match the imported transaction. **Keeping dates in sync with your bank is important** as it allows you to compare the balance at any point in time with your bank. + +When "Merge with existing transactions" is enabled, a **Reimport deleted transactions** checkbox is also available. When unchecked (the default for file imports), any transactions that were previously imported and then deleted will not be reimported. Enable this option if you want deleted transactions to reappear during import. + +:::note +The [API](/docs/api/reference#importtransactions) defaults `reimportDeleted` to `true` for backward compatibility. If you are importing via the API and want to skip deleted transactions, pass `reimportDeleted: false` explicitly. +::: diff --git a/packages/loot-core/src/server/accounts/app.ts b/packages/loot-core/src/server/accounts/app.ts index 38ce4f05a5..8bb6eaba4f 100644 --- a/packages/loot-core/src/server/accounts/app.ts +++ b/packages/loot-core/src/server/accounts/app.ts @@ -1,6 +1,7 @@ import { t } from 'i18next'; import { v4 as uuidv4 } from 'uuid'; +import type { ImportTransactionsOpts } from '#types/api-handlers'; import { captureException } from '../../platform/exceptions'; import * as asyncStorage from '../../platform/server/asyncStorage'; import * as connection from '../../platform/server/connection'; @@ -1148,9 +1149,7 @@ async function importTransactions({ accountId: AccountEntity['id']; transactions: ImportTransactionEntity[]; isPreview: boolean; - opts?: { - defaultCleared?: boolean; - }; + opts?: ImportTransactionsOpts; }): Promise { if (typeof accountId !== 'string') { throw APIError('transactions-import: accountId must be an id'); @@ -1164,6 +1163,8 @@ async function importTransactions({ true, isPreview, opts?.defaultCleared, + false, + opts?.reimportDeleted, ); return { errors: [], diff --git a/packages/loot-core/src/server/accounts/sync.test.ts b/packages/loot-core/src/server/accounts/sync.test.ts index 6ca0bdcbe8..4787f50e6f 100644 --- a/packages/loot-core/src/server/accounts/sync.test.ts +++ b/packages/loot-core/src/server/accounts/sync.test.ts @@ -173,6 +173,88 @@ describe('Account sync', () => { expect(transactions2).toMatchSnapshot(); }); + test('reconcile doesnt rematch deleted transactions with reimportDeleted override false', async () => { + const { id: acctId } = await prepareDatabase(); + + await reconcileTransactions(acctId, [ + { date: '2020-01-01', imported_id: 'finid-override' }, + ]); + + const transactions1 = await getAllTransactions(); + expect(transactions1.length).toBe(1); + + await db.deleteTransaction(transactions1[0]); + + await reconcileTransactions( + acctId, + [{ date: '2020-01-01', imported_id: 'finid-override' }], + false, + true, + false, + true, + false, + false, + ); + const transactions2 = await getAllTransactions(); + expect(transactions2.length).toBe(1); + }); + + test('reconcile does rematch deleted transactions with reimportDeleted override true', async () => { + const { id: acctId } = await prepareDatabase(); + + await reconcileTransactions(acctId, [ + { date: '2020-01-01', imported_id: 'finid-override2' }, + ]); + + const transactions1 = await getAllTransactions(); + expect(transactions1.length).toBe(1); + + await db.deleteTransaction(transactions1[0]); + + await reconcileTransactions( + acctId, + [{ date: '2020-01-01', imported_id: 'finid-override2' }], + false, + true, + false, + true, + false, + true, + ); + const transactions2 = await getAllTransactions(); + expect(transactions2.length).toBe(2); + }); + + test('reimportDeleted override takes precedence over stored preference', async () => { + const { id: acctId } = await prepareDatabase(); + const reimportKey = + `sync-reimport-deleted-${acctId}` satisfies keyof SyncedPrefs; + // Preference says reimport (true), but override says don't (false) + await db.update('preferences', { id: reimportKey, value: 'true' }); + + await reconcileTransactions(acctId, [ + { date: '2020-01-01', imported_id: 'finid-precedence' }, + ]); + + const transactions1 = await getAllTransactions(); + expect(transactions1.length).toBe(1); + + await db.deleteTransaction(transactions1[0]); + + await reconcileTransactions( + acctId, + [{ date: '2020-01-01', imported_id: 'finid-precedence' }], + false, + true, + false, + true, + false, + false, + ); + const transactions2 = await getAllTransactions(); + expect(transactions2.length).toBe(1); + }); + test('reconcile run rules with inferred payee', async () => { const { id: acctId } = await prepareDatabase(); await db.insertCategoryGroup({ diff --git a/packages/loot-core/src/server/accounts/sync.ts b/packages/loot-core/src/server/accounts/sync.ts index 1638a22ec7..35659642dd 100644 --- a/packages/loot-core/src/server/accounts/sync.ts +++ b/packages/loot-core/src/server/accounts/sync.ts @@ -508,6 +508,7 @@ export async function reconcileTransactions( isPreview = false, defaultCleared = true, updateDates = false, + reimportDeleted?: boolean, ): Promise { logger.log('Performing transaction reconciliation'); @@ -526,6 +527,7 @@ export async function reconcileTransactions( transactions, isBankSyncAccount, strictIdChecking, + reimportDeleted, ); // Finally, generate & commit the changes @@ -663,14 +665,18 @@ export async function matchTransactions( transactions, isBankSyncAccount = false, strictIdChecking = true, + reimportDeletedOverride?: boolean, ) { logger.log('Performing transaction reconciliation matching'); - const reimportDeleted = await aqlQuery( - q('preferences') - .filter({ id: `sync-reimport-deleted-${acctId}` }) - .select('value'), - ).then(data => String(data?.data?.[0]?.value ?? 'true') === 'true'); + const reimportDeleted = + reimportDeletedOverride !== undefined + ? reimportDeletedOverride + : await aqlQuery( + q('preferences') + .filter({ id: `sync-reimport-deleted-${acctId}` }) + .select('value'), + ).then(data => String(data?.data?.[0]?.value ?? 'true') === 'true'); const hasMatched = new Set(); diff --git a/packages/loot-core/src/types/api-handlers.ts b/packages/loot-core/src/types/api-handlers.ts index 1545b18f10..ccd1439e27 100644 --- a/packages/loot-core/src/types/api-handlers.ts +++ b/packages/loot-core/src/types/api-handlers.ts @@ -26,6 +26,7 @@ import type { export type ImportTransactionsOpts = { defaultCleared?: boolean; dryRun?: boolean; + reimportDeleted?: boolean; }; export type ApiHandlers = { diff --git a/upcoming-release-notes/6926.md b/upcoming-release-notes/6926.md new file mode 100644 index 0000000000..281d13111a --- /dev/null +++ b/upcoming-release-notes/6926.md @@ -0,0 +1,6 @@ +--- +category: Bugfixes +authors: [totallynotjon] +--- + +Import: added override option allowing users to control whether previously deleted (or merged) transactions are reimported.