mirror of
https://github.com/actualbudget/actual.git
synced 2026-03-21 06:58:47 -05:00
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 <matiss@mja.lv> * 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 <matiss@mja.lv> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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 => {
|
||||
|
||||
@@ -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<typeof TableHeader>['headers'] = [
|
||||
@@ -1071,6 +1075,16 @@ export function ImportTransactionsModal({
|
||||
</CheckboxToggle>
|
||||
)}
|
||||
|
||||
{(isOfxFile(filetype) || isCamtFile(filetype)) && reconcile && (
|
||||
<CheckboxToggle
|
||||
id="form_reimport_deleted"
|
||||
checked={reimportDeleted}
|
||||
onChange={setReimportDeleted}
|
||||
>
|
||||
<Trans>Reimport deleted transactions</Trans>
|
||||
</CheckboxToggle>
|
||||
)}
|
||||
|
||||
{/*Import Options */}
|
||||
{(filetype === 'qif' || filetype === 'csv') && (
|
||||
<View style={{ marginTop: 10 }}>
|
||||
@@ -1187,6 +1201,15 @@ export function ImportTransactionsModal({
|
||||
>
|
||||
<Trans>Merge with existing transactions</Trans>
|
||||
</CheckboxToggle>
|
||||
{reconcile && (
|
||||
<CheckboxToggle
|
||||
id="form_reimport_deleted_csv"
|
||||
checked={reimportDeleted}
|
||||
onChange={setReimportDeleted}
|
||||
>
|
||||
<Trans>Reimport deleted transactions</Trans>
|
||||
</CheckboxToggle>
|
||||
)}
|
||||
</View>
|
||||
)}
|
||||
|
||||
|
||||
@@ -201,7 +201,7 @@ This method is mainly for custom importers that want to skip all the automatic s
|
||||
|
||||
#### `importTransactions`
|
||||
|
||||
<Method name="importTransactions" args={[{ name: 'accountId', type: 'id'}, { name: 'transactions', type: 'Transaction[]'}]} returns="Promise<{ errors, added, updated }>" />
|
||||
<Method name="importTransactions" args={[{ name: 'accountId', type: 'id'}, { name: 'transactions', type: 'Transaction[]'}, { name: 'opts = {}', type: 'object?'}]} returns="Promise<{ errors, added, updated }>" />
|
||||
|
||||
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
|
||||
|
||||
@@ -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.
|
||||
:::
|
||||
|
||||
@@ -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<ImportTransactionsResult> {
|
||||
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: [],
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -508,6 +508,7 @@ export async function reconcileTransactions(
|
||||
isPreview = false,
|
||||
defaultCleared = true,
|
||||
updateDates = false,
|
||||
reimportDeleted?: boolean,
|
||||
): Promise<ReconcileTransactionsResult> {
|
||||
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();
|
||||
|
||||
|
||||
@@ -26,6 +26,7 @@ import type {
|
||||
export type ImportTransactionsOpts = {
|
||||
defaultCleared?: boolean;
|
||||
dryRun?: boolean;
|
||||
reimportDeleted?: boolean;
|
||||
};
|
||||
|
||||
export type ApiHandlers = {
|
||||
|
||||
Reference in New Issue
Block a user