Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix bug - Manually entered split transactions are not cleared on import #200 #1465

Merged
merged 17 commits into from
Aug 23, 2023
Merged

Fix bug - Manually entered split transactions are not cleared on import #200 #1465

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
4cb6349
Added clear transactions on import option
kstockk Aug 3, 2023
390f485
Merge branch 'master' of github.com:kstockk/actual
kstockk Aug 3, 2023
a1bbbd2
Added release note
kstockk Aug 3, 2023
4bb1e64
Merge branch 'master' into master
kstockk Aug 4, 2023
2952bc3
Added cleared column to csv export
kstockk Aug 4, 2023
d7727f1
fixed Manually entered split transactions are not cleared on import
kstockk Aug 5, 2023
cb55c94
Merge branch 'import-split-bug-200'
kstockk Aug 5, 2023
9b607fd
Merge branch 'actualbudget:master' into master
kstockk Aug 5, 2023
b0c11c1
Revert "Added cleared column to csv export"
kstockk Aug 5, 2023
eb5a9e0
Merge branch 'master' of github.com:kstockk/actual
kstockk Aug 5, 2023
51bdd34
added release note
kstockk Aug 5, 2023
2e695b6
Copied same code to Gocardless
kstockk Aug 5, 2023
a755b4a
Updated var name
kstockk Aug 6, 2023
5228b9f
Merge branch 'master' into master
kstockk Aug 6, 2023
5a95a72
Updated to only query changed transactions instead of all
kstockk Aug 23, 2023
4b10ec1
Merge branch 'master' of github.com:kstockk/actual
kstockk Aug 23, 2023
4ff0984
Merge branch 'master' into master
kstockk Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/loot-core/src/server/accounts/sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,20 @@ export async function reconcileGoCardlessTransactions(acctId, transactions) {
await createNewPayees(payeesToCreate, [...added, ...updated]);
await batchUpdateTransactions({ added, updated });

// Make sure the "cleared" flag is synced up with the parent
// transactions
let clearedRows = await db.all(`
SELECT t.id, p.cleared FROM v_transactions t
LEFT JOIN v_transactions p ON t.parent_id = p.id
WHERE t.is_child = 1 AND t.cleared != p.cleared
`);

let updatedClearedRows = clearedRows.map(row => ({
id: row.id,
cleared: row.cleared === 1,
}));
await batchUpdateTransactions({ updated: updatedClearedRows });

Copy link
Contributor

@kyrias kyrias Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to query every transaction like this seems very inefficient. I think the following would be a better solution:

diff --git a/packages/loot-core/src/server/accounts/sync.ts b/packages/loot-core/src/server/accounts/sync.ts
index 4733dc6f..4cbea6d5 100644
--- a/packages/loot-core/src/server/accounts/sync.ts
+++ b/packages/loot-core/src/server/accounts/sync.ts
@@ -431,7 +431,7 @@ export async function reconcileGoCardlessTransactions(acctId, transactions) {
       // matched transaction. See the final pass below for the needed
       // fields.
       fuzzyDataset = await db.all(
-        `SELECT id, date, imported_id, payee, category, notes FROM v_transactions
+        `SELECT id, is_parent, date, imported_id, payee, category, notes FROM v_transactions
            WHERE date >= ? AND date <= ? AND amount = ? AND account = ? AND is_child = 0`,
         [
           db.toDateRepr(monthUtils.subDays(trans.date, 4)),
@@ -509,6 +509,17 @@ export async function reconcileGoCardlessTransactions(acctId, transactions) {
       if (hasFieldsChanged(existing, updates, Object.keys(updates))) {
         updated.push({ id: existing.id, ...updates });
       }
+
+      if (existing.is_parent && existing.cleared !== updates.cleared) {
+        const children = await db.all(
+          'SELECT id FROM v_transactions WHERE parent_id = ?',
+          [existing.id],
+        );
+        for (const child of children) {
+          updated.push({ id: child.id, cleared: updates.cleared });
+        }
+      }
     } else {
       // Insert a new transaction
       let finalTransaction = {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Thanks for for the suggestion

I've updated it in both the reconcileGoCardlessTransactions and reconcileTransactions functions

Please review

return {
added: added.map(trans => trans.id),
updated: updated.map(trans => trans.id),
Expand Down Expand Up @@ -682,6 +696,20 @@ export async function reconcileTransactions(acctId, transactions) {
await createNewPayees(payeesToCreate, [...added, ...updated]);
await batchUpdateTransactions({ added, updated });

// Make sure the "cleared" flag is synced up with the parent
// transactions
let clearedRows = await db.all(`
SELECT t.id, p.cleared FROM v_transactions t
LEFT JOIN v_transactions p ON t.parent_id = p.id
WHERE t.is_child = 1 AND t.cleared != p.cleared
`);

let updatedClearedRows = clearedRows.map(row => ({
id: row.id,
cleared: row.cleared === 1,
}));
await batchUpdateTransactions({ updated: updatedClearedRows });

return {
added: added.map(trans => trans.id),
updated: updated.map(trans => trans.id),
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/1465.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Bugfix
authors: [kstockk]
---

Fixed clearing split transactions when importing matched transactions
Loading