github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-03-09 03:32:54 -05:00
Code Issues 250 Packages Projects Releases 53 Wiki Activity

[PR #247] [MERGED] Route aggregate queries in transaction grouped mode through the correct layer to remove deleted transactions #3080

New Issue
Closed
opened 2026-02-28 20:36:00 -06:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-02-28 20:36:00 -06:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/247
Author: @jlongster
Created: 9/7/2022
Status: ✅ Merged
Merged: 3/12/2023
Merged by: @MatissJanis

Base: master ← Head: fix-aggregate-grouped-query

📝 Commits (2)

  • 5f6f968 Route aggregate queries in transaction grouped mode through the correct layer to remove deleted transactions
  • b0163ab Fix test for aggregate queries with grouped transactions

📊 Changes

3 files changed (+24 additions, -14 deletions)

View changed files

📝 packages/loot-core/src/server/aql/compiler.js (+2 -2)
📝 packages/loot-core/src/server/aql/schema/executors.js (+21 -11)
📝 packages/loot-core/src/server/aql/schema/executors.test.js (+1 -1)

📄 Description

Don't merge yet: these changes are ready, but I want to add tests before merging

I recently migrated my personal usage of Actual over to the open-source version and imported a bunch of transactions. I have a lot of history in Actual, including a lot of weird edge cases like deleted split transactions. While reconciling I noticed that my account balance shown at the top was incorrect, even though the running balance was current.

Digging into this, I discovered that we aren't correctly handling aggregate queries when querying transactions in the "grouped" mode. Aggregate queries don't make sense in the "grouped" mode. Grouped means that you want a list of transactions that include both the parent and child transactions (when they are split). If you are summing up all the amount, you only want to consider non-parent transactions. So we switch it back to "inline" mode, but the way we did this previously was to manually stitch the query together.

Even though was add SQL to ignore deleted transactions, we still possibly include them. A child transaction may not be marked as deleted, even though the parent transaction is deleted. When a parent transaction is deleted, all child transactions should be considered deleted as well, regardless of their tombstone status. This is what the v_transactions_internal_alive view does. Previously we weren't going through this view though, so we could still potentially include split transactions even though they've been deleted.

This is little hacky, but it fixes the immediate problem. We fall back to the inline mode by modifying the where clause, and we also adjust the view that it queries to use the correct one.

🔄 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/247 **Author:** [@jlongster](https://github.com/jlongster) **Created:** 9/7/2022 **Status:** ✅ Merged **Merged:** 3/12/2023 **Merged by:** [@MatissJanis](https://github.com/MatissJanis) **Base:** `master` ← **Head:** `fix-aggregate-grouped-query` --- ### 📝 Commits (2) - [`5f6f968`](https://github.com/actualbudget/actual/commit/5f6f968d0b4ed41e34a60e2404c7a869e08c9b28) Route aggregate queries in transaction grouped mode through the correct layer to remove deleted transactions - [`b0163ab`](https://github.com/actualbudget/actual/commit/b0163ab8d4919849ab38e82cdd1e207b506255ca) Fix test for aggregate queries with grouped transactions ### 📊 Changes **3 files changed** (+24 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `packages/loot-core/src/server/aql/compiler.js` (+2 -2) 📝 `packages/loot-core/src/server/aql/schema/executors.js` (+21 -11) 📝 `packages/loot-core/src/server/aql/schema/executors.test.js` (+1 -1) </details> ### 📄 Description **Don't merge yet**: these changes are ready, but I want to add tests before merging I recently migrated my personal usage of Actual over to the open-source version and imported a bunch of transactions. I have a _lot_ of history in Actual, including a lot of weird edge cases like deleted split transactions. While reconciling I noticed that my account balance shown at the top was incorrect, even though the running balance was current. Digging into this, I discovered that we aren't correctly handling aggregate queries when querying transactions in the "grouped" mode. Aggregate queries don't make sense in the "grouped" mode. Grouped means that you want a list of transactions that include both the parent and child transactions (when they are split). If you are summing up all the amount, you only want to consider non-parent transactions. So we switch it back to "inline" mode, but the way we did this previously was to manually stitch the query together. Even though was add SQL to ignore deleted transactions, we still possibly include them. A child transaction may not be marked as deleted, even though the parent transaction is deleted. When a parent transaction is deleted, all child transactions should be considered deleted as well, regardless of their tombstone status. This is what the `v_transactions_internal_alive` view does. Previously we weren't going through this view though, so we could still potentially include split transactions even though they've been deleted. This is little hacky, but it fixes the immediate problem. We fall back to the inline mode by modifying the where clause, and we also adjust the view that it queries to use the correct one. --- <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-02-28 20:36:00 -06: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
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#3080
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?