github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-07-16 15:14:02 -05:00
Code Issues 3.0k Packages Projects Releases 58 Wiki Activity

[PR #1867] [MERGED] [Bugfix]: Account filter for budgeted and offbudget accounts #101692

New Issue
Closed
opened 2026-05-29 23:29:29 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-05-29 23:29:29 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/1867
Author: @sreetamdas
Created: 11/6/2023
Status: ✅ Merged
Merged: 11/7/2023
Merged by: @joel-jeremy

Base: master ← Head: bugfix-1577

📝 Commits (3)

  • c17d4d5 Update AQL filter for budgeted and offbudget accounts
  • d9d4a98 Add release note
  • 02e068f Merge remote-tracking branch 'origin/master' into bugfix-1577

📊 Changes

2 files changed (+14 additions, -4 deletions)

View changed files

📝 packages/loot-core/src/client/queries.ts (+8 -4)
➕ upcoming-release-notes/1867.md (+6 -0)

📄 Description

Fix getAccountFilter() for budgeted and offbudget accounts, which would cause schedules involving budgeted accounts and/or non-account payees to appear in both the "For budget" and "Off budget" account.

Details

This was due to incorrect SQLite queries as a result of getAccountFilter() in this block in Account.js:

let filterByAccount = queries.getAccountFilter(params.id, "_account");
let filterByPayee = queries.getAccountFilter(params.id, "_payee.transfer_acct");

// edited

return (q) => {
	q = q.filter({
		$and: [{ "_account.closed": false }],
		$or: [filterByAccount, filterByPayee],
	});
	return q.orderBy({ next_date: "desc" });
};

Since filterByAccount, filterByPayee is (correctly) placed in an $or operation, the inner operations in getAccountFilter() (below) are also compiled using compileOr:

// getAccountFilter(accountId, field = "account")
if (accountId === "budgeted") {
	return {
		[`${field}.offbudget`]: false,
		[`${field}.closed`]: false,
	};
} else if (accountId === "offbudget") {
	return {
		[`${field}.offbudget`]: true,
		[`${field}.closed`]: false,
	};
}

This results in incorrectly checking only if the payee/account are not-closed.

Further details

The SQLite queries that the AQL compiles down to for the two accounts are:

SQL queries, summary below 
-- "For budget" account
    SELECT
-- edited for brevity

	payees2.id AS _payee,
	accounts3.id AS _account,
FROM
	v_schedules
-- edited for brevity

	LEFT JOIN payees payees2 ON payees2.id = v_schedules._payee
		AND payees2.tombstone = 0
	LEFT JOIN accounts accounts3 ON accounts3.id = v_schedules._account
		AND accounts3.tombstone = 0
	LEFT JOIN accounts accounts4 ON accounts4.id = payees2.transfer_acct
		AND accounts4.tombstone = 0

WHERE ((accounts3.closed = 0)
	AND(accounts3.offbudget = 0
		OR accounts3.closed = 0
		OR accounts4.offbudget = 0
		OR accounts4.closed = 0))
-- edited for brevity

-- "Off budget" account
    SELECT
-- edited for brevity

	payees2.id AS _payee,
	accounts3.id AS _account,
FROM
	v_schedules
-- edited for brevity

	LEFT JOIN payees payees2 ON payees2.id = v_schedules._payee
		AND payees2.tombstone = 0
	LEFT JOIN accounts accounts3 ON accounts3.id = v_schedules._account
		AND accounts3.tombstone = 0
	LEFT JOIN accounts accounts4 ON accounts4.id = payees2.transfer_acct
		AND accounts4.tombstone = 0

WHERE ((accounts3.closed = 0)
	AND(accounts3.offbudget = 1
		OR accounts3.closed = 0
		OR accounts4.offbudget = 1
		OR accounts4.closed = 0))
-- edited for brevity

The important part is the WHERE clause:

WHERE ((accounts3.closed = 0)
	AND(accounts3.offbudget = 1 -- 0 in case of "For budget"
		OR accounts3.closed = 0
		OR accounts4.offbudget = 1 -- 0 in case of "For budget"
		OR accounts4.closed = 0))

This causes issues since we are simply checking for either the payee or account to be a not-closed payee/account, which (almost?) always results to true. This is also problematic for cases when one of the payees is not an account.

To fix, the correct SQL query would need to check that the payee/account is not closed and its offbudget value, like so:

WHERE ((accounts3.closed = 0)
	AND((accounts3.offbudget = 1 -- 0 in case of "For budget"
		AND accounts3.closed = 0)
	OR(accounts4.offbudget = 1 -- 0 in case of "For budget"
		AND accounts4.closed = 0)))

For this, the upstream AQL query needs to be updated, while preserving the original intended behaviour to show a scheduled transfers appear in both accounts (implemented in #955) (whenever applicable):

// getAccountFilter(accountId, field = "account")
if (accountId === "budgeted") {
  return {
    $and: [
      { [`${field}.offbudget`]: false },
      { [`${field}.closed`]: false },
    ],
  };
} else if (accountId === "offbudget") {
  return {
    $and: [
      { [`${field}.offbudget`]: true },
      { [`${field}.closed`]: false },
    ],
  };
}

Verifying

To test this PR, here's a budget built on top of the one mentioned in the original issue (#1577):
actual_budget_1867_2023-11-07.zip

Screenshots

Schedules:

image
Before:

For budget:

image

Off budget:

image
After:

For budget:

image

Off budget:

image

Fixes #1577

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/1867 **Author:** [@sreetamdas](https://github.com/sreetamdas) **Created:** 11/6/2023 **Status:** ✅ Merged **Merged:** 11/7/2023 **Merged by:** [@joel-jeremy](https://github.com/joel-jeremy) **Base:** `master` ← **Head:** `bugfix-1577` --- ### 📝 Commits (3) - [`c17d4d5`](https://github.com/actualbudget/actual/commit/c17d4d5a94de5c6b5edae41ec1df7728c10b3e30) Update AQL filter for budgeted and offbudget accounts - [`d9d4a98`](https://github.com/actualbudget/actual/commit/d9d4a983bf60389248c6810a24f0a66424791ba3) Add release note - [`02e068f`](https://github.com/actualbudget/actual/commit/02e068f6e5272fdd41f27b541cdb163212a2e27c) Merge remote-tracking branch 'origin/master' into bugfix-1577 ### 📊 Changes **2 files changed** (+14 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `packages/loot-core/src/client/queries.ts` (+8 -4) ➕ `upcoming-release-notes/1867.md` (+6 -0) </details> ### 📄 Description Fix `getAccountFilter()` for budgeted and offbudget accounts, which would cause schedules involving budgeted accounts and/or non-account payees to appear in both the "For budget" and "Off budget" account. --- ### Details This was due to incorrect SQLite queries as a result of `getAccountFilter()` in this block in [`Account.js`](https://github.com/actualbudget/actual/blob/4b712699a87a5bb6a5a6809981533a6b6538e813/packages/desktop-client/src/components/accounts/Account.js#L2015-L2034): ```js let filterByAccount = queries.getAccountFilter(params.id, "_account"); let filterByPayee = queries.getAccountFilter(params.id, "_payee.transfer_acct"); // edited return (q) => { q = q.filter({ $and: [{ "_account.closed": false }], $or: [filterByAccount, filterByPayee], }); return q.orderBy({ next_date: "desc" }); }; ``` Since `filterByAccount, filterByPayee` is (correctly) placed in an `$or` operation, the _inner_ operations in [`getAccountFilter()` (below)](https://github.com/actualbudget/actual/blob/a15f80e77133ac38443becbb3ddabd92d86c69f4/packages/loot-core/src/client/queries.ts#L15-L24) are also compiled using [`compileOr`](https://github.com/actualbudget/actual/blob/a15f80e77133ac38443becbb3ddabd92d86c69f4/packages/loot-core/src/server/aql/compiler.ts#L766): ```js // getAccountFilter(accountId, field = "account") if (accountId === "budgeted") { return { [`${field}.offbudget`]: false, [`${field}.closed`]: false, }; } else if (accountId === "offbudget") { return { [`${field}.offbudget`]: true, [`${field}.closed`]: false, }; } ``` This results in incorrectly checking only if the payee/account are not-closed. <details> <summary>Further details</summary> The SQLite queries that the AQL compiles down to for the two accounts are: <details> <summary>SQL queries, summary below</summary> ```sql -- "For budget" account SELECT -- edited for brevity payees2.id AS _payee, accounts3.id AS _account, FROM v_schedules -- edited for brevity LEFT JOIN payees payees2 ON payees2.id = v_schedules._payee AND payees2.tombstone = 0 LEFT JOIN accounts accounts3 ON accounts3.id = v_schedules._account AND accounts3.tombstone = 0 LEFT JOIN accounts accounts4 ON accounts4.id = payees2.transfer_acct AND accounts4.tombstone = 0 WHERE ((accounts3.closed = 0) AND(accounts3.offbudget = 0 OR accounts3.closed = 0 OR accounts4.offbudget = 0 OR accounts4.closed = 0)) -- edited for brevity ``` ```sql -- "Off budget" account SELECT -- edited for brevity payees2.id AS _payee, accounts3.id AS _account, FROM v_schedules -- edited for brevity LEFT JOIN payees payees2 ON payees2.id = v_schedules._payee AND payees2.tombstone = 0 LEFT JOIN accounts accounts3 ON accounts3.id = v_schedules._account AND accounts3.tombstone = 0 LEFT JOIN accounts accounts4 ON accounts4.id = payees2.transfer_acct AND accounts4.tombstone = 0 WHERE ((accounts3.closed = 0) AND(accounts3.offbudget = 1 OR accounts3.closed = 0 OR accounts4.offbudget = 1 OR accounts4.closed = 0)) -- edited for brevity ``` </details> The important part is the `WHERE` clause: ```sql WHERE ((accounts3.closed = 0) AND(accounts3.offbudget = 1 -- 0 in case of "For budget" OR accounts3.closed = 0 OR accounts4.offbudget = 1 -- 0 in case of "For budget" OR accounts4.closed = 0)) ``` This causes issues since we are simply checking for either the payee or account to be a not-closed payee/account, which (almost?) always results to `true`. This is also problematic for cases when one of the payees is not an account. To fix, the correct SQL query would need to check that the payee/account is not closed _and_ its `offbudget` value, like so: ```sql WHERE ((accounts3.closed = 0) AND((accounts3.offbudget = 1 -- 0 in case of "For budget" AND accounts3.closed = 0) OR(accounts4.offbudget = 1 -- 0 in case of "For budget" AND accounts4.closed = 0))) ``` For this, the upstream AQL query needs to be updated, while preserving the original intended behaviour to show a scheduled transfers appear in both accounts (implemented in #955) (whenever applicable): ```js // getAccountFilter(accountId, field = "account") if (accountId === "budgeted") { return { $and: [ { [`${field}.offbudget`]: false }, { [`${field}.closed`]: false }, ], }; } else if (accountId === "offbudget") { return { $and: [ { [`${field}.offbudget`]: true }, { [`${field}.closed`]: false }, ], }; } ``` </details> --- ### Verifying To test this PR, here's a budget built on top of [the one mentioned in the original issue (#1577)](https://github.com/actualbudget/actual/issues/1577#issuecomment-1694500710): [actual_budget_1867_2023-11-07.zip](https://github.com/actualbudget/actual/files/13276192/actual_budget_bugfix_1867_2023-11-07.zip) <details> <summary>Screenshots</summary> Schedules: <img width="1184" alt="image" src="https://github.com/actualbudget/actual/assets/11270438/f0c431ee-7ddf-4958-90d3-4a5d1ad0a821"> <details> <summary>Before:</summary> For budget: <img width="1466" alt="image" src="https://github.com/actualbudget/actual/assets/11270438/2c4593c2-7c83-4c5b-aa5f-835309795417"> Off budget: <img width="1465" alt="image" src="https://github.com/actualbudget/actual/assets/11270438/92b4632e-818a-4c7c-bcac-514f2582f07a"> </details> <details> <summary> After:</summary> For budget: <img width="1461" alt="image" src="https://github.com/actualbudget/actual/assets/11270438/4ee46117-c10e-4e32-8674-316ec9838fc1"> Off budget: <img width="1463" alt="image" src="https://github.com/actualbudget/actual/assets/11270438/e8e40fae-3f7a-4016-8a7d-75df14b208bf"> </details> </details> --- Fixes #1577 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-29 23:29:30 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
tech-support
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#101692
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?