From 1cbf4b7850fdc3bbbfa5f7be92b3772ac6d64413 Mon Sep 17 00:00:00 2001 From: Julian Dominguez-Schatz Date: Tue, 28 Jan 2025 23:46:46 -0500 Subject: [PATCH 1/2] fix: allow child transactions to have different transfer payees --- .../__snapshots__/transfer.test.ts.snap | 135 ++++++++++++++++++ .../src/server/accounts/rules.test.ts | 25 ++++ .../src/server/accounts/transfer.test.ts | 51 +++++++ .../loot-core/src/server/accounts/transfer.ts | 23 --- 4 files changed, 211 insertions(+), 23 deletions(-) diff --git a/packages/loot-core/src/server/accounts/__snapshots__/transfer.test.ts.snap b/packages/loot-core/src/server/accounts/__snapshots__/transfer.test.ts.snap index c9b6f89f830..6890d6ba26f 100644 --- a/packages/loot-core/src/server/accounts/__snapshots__/transfer.test.ts.snap +++ b/packages/loot-core/src/server/accounts/__snapshots__/transfer.test.ts.snap @@ -1,5 +1,140 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Transfer split transfers are retained on child transactions 1`] = ` +Array [ + Object { + "account": "one", + "amount": 5000, + "category": null, + "cleared": 1, + "date": 20170101, + "error": null, + "id": "id5", + "imported_id": null, + "imported_payee": null, + "is_child": 0, + "is_parent": 0, + "notes": null, + "parent_id": null, + "payee": "id3", + "payee_name": "", + "reconciled": 0, + "schedule": null, + "sort_order": 123456789, + "starting_balance_flag": 0, + "tombstone": 0, + "transfer_id": "id6", + }, + Object { + "account": "two", + "amount": -5000, + "category": null, + "cleared": 0, + "date": 20170101, + "error": null, + "id": "id6", + "imported_id": null, + "imported_payee": null, + "is_child": 0, + "is_parent": 0, + "notes": null, + "parent_id": null, + "payee": "id2", + "payee_name": "", + "reconciled": 0, + "schedule": null, + "sort_order": 123456789, + "starting_balance_flag": 0, + "tombstone": 0, + "transfer_id": "id5", + }, +] +`; + +exports[`Transfer split transfers are retained on child transactions 2`] = ` +"Snapshot Diff: +- First value ++ Second value + +@@ -8,11 +8,11 @@ + \\"error\\": null, + \\"id\\": \\"id5\\", + \\"imported_id\\": null, + \\"imported_payee\\": null, + \\"is_child\\": 0, +- \\"is_parent\\": 0, ++ \\"is_parent\\": 1, + \\"notes\\": null, + \\"parent_id\\": null, + \\"payee\\": \\"id3\\", + \\"payee_name\\": \\"\\", + \\"reconciled\\": 0," +`; + +exports[`Transfer split transfers are retained on child transactions 3`] = ` +"Snapshot Diff: +- First value ++ Second value + +@@ -21,10 +21,56 @@ + \\"starting_balance_flag\\": 0, + \\"tombstone\\": 0, + \\"transfer_id\\": \\"id6\\", + }, + Object { ++ \\"account\\": \\"one\\", ++ \\"amount\\": 2000, ++ \\"category\\": null, ++ \\"cleared\\": 1, ++ \\"date\\": 20170101, ++ \\"error\\": null, ++ \\"id\\": \\"id7\\", ++ \\"imported_id\\": null, ++ \\"imported_payee\\": null, ++ \\"is_child\\": 1, ++ \\"is_parent\\": 0, ++ \\"notes\\": null, ++ \\"parent_id\\": \\"id5\\", ++ \\"payee\\": \\"id2\\", ++ \\"payee_name\\": \\"\\", ++ \\"reconciled\\": 0, ++ \\"schedule\\": null, ++ \\"sort_order\\": 123456789, ++ \\"starting_balance_flag\\": 0, ++ \\"tombstone\\": 0, ++ \\"transfer_id\\": \\"id8\\", ++ }, ++ Object { ++ \\"account\\": \\"one\\", ++ \\"amount\\": -2000, ++ \\"category\\": null, ++ \\"cleared\\": 0, ++ \\"date\\": 20170101, ++ \\"error\\": null, ++ \\"id\\": \\"id8\\", ++ \\"imported_id\\": null, ++ \\"imported_payee\\": null, ++ \\"is_child\\": 0, ++ \\"is_parent\\": 0, ++ \\"notes\\": null, ++ \\"parent_id\\": null, ++ \\"payee\\": \\"id2\\", ++ \\"payee_name\\": \\"\\", ++ \\"reconciled\\": 0, ++ \\"schedule\\": null, ++ \\"sort_order\\": 123456789, ++ \\"starting_balance_flag\\": 0, ++ \\"tombstone\\": 0, ++ \\"transfer_id\\": \\"id7\\", ++ }, ++ Object { + \\"account\\": \\"two\\", + \\"amount\\": -5000, + \\"category\\": null, + \\"cleared\\": 0, + \\"date\\": 20170101," +`; + exports[`Transfer transfers are properly de-categorized 1`] = ` Array [ Object { diff --git a/packages/loot-core/src/server/accounts/rules.test.ts b/packages/loot-core/src/server/accounts/rules.test.ts index fa39f6424e8..f5864666a3c 100644 --- a/packages/loot-core/src/server/accounts/rules.test.ts +++ b/packages/loot-core/src/server/accounts/rules.test.ts @@ -524,6 +524,31 @@ describe('Rule', () => { }); describe('split actions', () => { + test('splits can change the payee', () => { + const rule = new Rule({ + conditionsOp: 'and', + conditions: [{ op: 'is', field: 'payee', value: '123' }], + actions: [ + { + op: 'set-split-amount', + field: 'amount', + value: 100, + options: { splitIndex: 1, method: 'fixed-amount' }, + }, + { + op: 'set', + field: 'payee', + value: '456', + options: { splitIndex: 1 }, + }, + ], + }); + + expect(rule.exec({ payee: '123' })).toMatchObject({ + subtransactions: [{ payee: '456' }], + }); + }); + const fixedAmountRule = new Rule({ conditionsOp: 'and', conditions: [{ op: 'is', field: 'imported_payee', value: 'James' }], diff --git a/packages/loot-core/src/server/accounts/transfer.test.ts b/packages/loot-core/src/server/accounts/transfer.test.ts index ce77ce7e94a..faf6d634684 100644 --- a/packages/loot-core/src/server/accounts/transfer.test.ts +++ b/packages/loot-core/src/server/accounts/transfer.test.ts @@ -44,6 +44,9 @@ type Transaction = { notes?: string; payee: string; transfer_id?: string; + is_parent?: boolean; + is_child?: boolean; + parent_id?: string; }; describe('Transfer', () => { @@ -167,4 +170,52 @@ describe('Transfer', () => { await transfer.onUpdate(transaction); differ.expectToMatchDiff(await getAllTransactions()); }); + + test('split transfers are retained on child transactions', async () => { + // test: first add a txn having a transfer acct payee + // then mark it as `is_parent` and add a child txn + // the child txn should have a different transfer acct payee + // and `is_child` set to true + await prepareDatabase(); + + const [transferOne, transferTwo] = await Promise.all([ + db.first("SELECT * FROM payees WHERE transfer_acct = 'one'"), + db.first("SELECT * FROM payees WHERE transfer_acct = 'two'"), + ]); + + let parent: Transaction = { + account: 'one', + amount: 5000, + payee: transferTwo.id, + date: '2017-01-01', + }; + parent.id = await db.insertTransaction(parent); + await transfer.onInsert(parent); + parent = await db.getTransaction(parent.id); + + const differ = expectSnapshotWithDiffer(await getAllTransactions()); + + // mark the txn as parent + await db.updateTransaction({ id: parent.id, is_parent: true }); + await transfer.onUpdate(parent); + differ.expectToMatchDiff(await getAllTransactions()); + + // add a child txn + let child: Transaction = { + account: 'one', + amount: 2000, + payee: transferOne.id, + date: '2017-01-01', + is_child: true, + parent_id: parent.id, + }; + child.id = await db.insertTransaction(child); + await transfer.onInsert(child); + differ.expectToMatchDiff(await getAllTransactions()); + + // ensure that the child txn has the correct transfer acct payee + child = await db.getTransaction(child.id); + expect(child.transfer_id).not.toBe(parent.transfer_id); + expect(child.payee).toBe(transferOne.id); + }); }); diff --git a/packages/loot-core/src/server/accounts/transfer.ts b/packages/loot-core/src/server/accounts/transfer.ts index fa8969667e0..d180244dd9c 100644 --- a/packages/loot-core/src/server/accounts/transfer.ts +++ b/packages/loot-core/src/server/accounts/transfer.ts @@ -55,29 +55,6 @@ export async function addTransfer(transaction, transferredAccount) { [transaction.account], ); - // We need to enforce certain constraints with child transaction transfers - if (transaction.parent_id) { - const row = await db.first( - ` - SELECT p.id, p.transfer_acct FROM v_transactions t - LEFT JOIN payees p ON p.id = t.payee - WHERE t.id = ? - `, - [transaction.parent_id], - ); - - if (row.transfer_acct) { - if (row.id !== transaction.payee) { - // This child transaction is trying to use a transfer payee, - // but the parent is already using a different transfer payee. - // This is not allowed, so not only do we do nothing, we clear - // the payee of the child transaction to make it clear - await db.updateTransaction({ id: transaction.id, payee: null }); - return { id: transaction.id, payee: null }; - } - } - } - const id = await db.insertTransaction({ account: transferredAccount, amount: -transaction.amount, From e4e9c9180f98d3c5e9b0ede62200375e45e27f94 Mon Sep 17 00:00:00 2001 From: Julian Dominguez-Schatz Date: Wed, 29 Jan 2025 00:28:53 -0500 Subject: [PATCH 2/2] Add release notes --- upcoming-release-notes/4255.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 upcoming-release-notes/4255.md diff --git a/upcoming-release-notes/4255.md b/upcoming-release-notes/4255.md new file mode 100644 index 00000000000..3c975242380 --- /dev/null +++ b/upcoming-release-notes/4255.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [jfdoming] +--- + +Allow child transactions to have different transfer payees