Fix updateTransaction corrupting split parents with partial updates#7242
Conversation
When `api.updateTransaction(id, { notes: '...' })` is called on a split
parent, the `updateTransaction` helper replaces the parent with the
sparse update object (`{ id, notes }`) instead of merging it with
the existing transaction data. This causes `recalculateSplit` to see
`amount` as `undefined` (→ 0), which doesn't match the children's
total and sets a `SplitTransactionError` on the parent. `makeChild`
also inherits undefined `account`, `date`, and `cleared` values,
potentially creating broken child rows.
Fix: merge the incoming partial fields (`{ ...trans, ...transaction }`)
so all existing properties are preserved.
Add a test that performs a notes-only update on a split parent and
asserts no error is set and the amount stays intact.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hello contributor! We would love to review your PR! Before we can do that, please make sure:
We do this to reduce the TOIL the core contributor team has to go through for each PR and to allow for speedy reviews and merges. For more information, please see our Contributing Guide. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdateTransaction merge behavior changed: when updating a split parent, the incoming transaction is merged with the existing parent to preserve non-provided fields. A test was added to verify partial parent updates (e.g., only Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
matt-fidd
left a comment
There was a problem hiding this comment.
Thanks for this, same comments as the other one here
…note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for actualbudget-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…7242) * [AI] Fix updateTransaction corrupting split parents with partial updates When `api.updateTransaction(id, { notes: '...' })` is called on a split parent, the `updateTransaction` helper replaces the parent with the sparse update object (`{ id, notes }`) instead of merging it with the existing transaction data. This causes `recalculateSplit` to see `amount` as `undefined` (→ 0), which doesn't match the children's total and sets a `SplitTransactionError` on the parent. `makeChild` also inherits undefined `account`, `date`, and `cleared` values, potentially creating broken child rows. Fix: merge the incoming partial fields (`{ ...trans, ...transaction }`) so all existing properties are preserved. Add a test that performs a notes-only update on a split parent and asserts no error is set and the amount stays intact. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [AI] Add release notes for PR #7242 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: remove verbose comment and simplify release note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: L. Warren Thompson <lwarrenthompson@Warren-MBP.local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…7242) * [AI] Fix updateTransaction corrupting split parents with partial updates When `api.updateTransaction(id, { notes: '...' })` is called on a split parent, the `updateTransaction` helper replaces the parent with the sparse update object (`{ id, notes }`) instead of merging it with the existing transaction data. This causes `recalculateSplit` to see `amount` as `undefined` (→ 0), which doesn't match the children's total and sets a `SplitTransactionError` on the parent. `makeChild` also inherits undefined `account`, `date`, and `cleared` values, potentially creating broken child rows. Fix: merge the incoming partial fields (`{ ...trans, ...transaction }`) so all existing properties are preserved. Add a test that performs a notes-only update on a split parent and asserts no error is set and the amount stays intact. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [AI] Add release notes for PR #7242 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: remove verbose comment and simplify release note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: L. Warren Thompson <lwarrenthompson@Warren-MBP.local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…7242) * [AI] Fix updateTransaction corrupting split parents with partial updates When `api.updateTransaction(id, { notes: '...' })` is called on a split parent, the `updateTransaction` helper replaces the parent with the sparse update object (`{ id, notes }`) instead of merging it with the existing transaction data. This causes `recalculateSplit` to see `amount` as `undefined` (→ 0), which doesn't match the children's total and sets a `SplitTransactionError` on the parent. `makeChild` also inherits undefined `account`, `date`, and `cleared` values, potentially creating broken child rows. Fix: merge the incoming partial fields (`{ ...trans, ...transaction }`) so all existing properties are preserved. Add a test that performs a notes-only update on a split parent and asserts no error is set and the amount stays intact. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [AI] Add release notes for PR #7242 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: remove verbose comment and simplify release note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: L. Warren Thompson <lwarrenthompson@Warren-MBP.local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
) * trim down some unused/unnecessary dependencies (#7350) * fix github actions inconsistencies * fix pinning of transitive deps in eslint-plugin * drop use of node-fetch in api * drop md5 dependency in favour of node:crypto * drop slash * drop unused top level packages * add note about node-polyfills warning * remove unused deps from desktop-client * drop pegjs types * note * drop node-jq * [Doc] More tour image (mostly) updates & a hotkey fix (#7328) * Fix keyboard shortcut Mac key for undo operations Updated keyboard shortcut instructions for Mac & make consistent. * Add files via upload * Fix undo shortcut from 'K' to 'Z' Updated keyboard shortcut for undo operation in payees guide. COFFEE! * Revise budget section for clarity and consistency Updated category descriptions and improved Markdown support details. * Add files via upload * Fix grammatical error in budget.md * Fix typo and clarify Markdown description in budget.md Corrected a typo in the documentation regarding the chevrons and clarified the description of rendered Markdown. * Fix spelling error in budget documentation Corrected the spelling of 'cheverons' to 'chevrons'. * Add files via upload * Remove redundant text in budget.md * Fix formatting issues in payees.md * count points script should fetch the release note from the PR directly (#7309) * get pr release note from PR, not top of master * note * [AI] Mobile: Post transaction today on global account lists (#7311) (#7322) * [AI] Mobile: pass today for Post transaction today on global account lists (#7311) All Accounts, On budget, and Off budget transaction lists now forward the today flag to schedule/post-transaction, matching single-account mobile and desktop behavior. Made-with: Cursor * [AI] Add release note for PR 7322 (#7311) Made-with: Cursor * [AI] Tighten release note wording for PR 7322 (imperative) Made-with: Cursor --------- Co-authored-by: Pranay Mac M1 <pranayseela@yahoo.com> --------- Co-authored-by: Matt Fiddaman <github@m.fiddaman.uk> Co-authored-by: Pranay S <pranayritvik@gmail.com> Co-authored-by: Pranay Mac M1 <pranayseela@yahoo.com> Co-authored-by: youngcw <calebyoung94@gmail.com> * Implement Sankey report for spent and budgeted money (#7220) * Implement Sankey graph report * Add release notes * Update VRT screenshots Auto-generated by VRT workflow PR: #6068 * Remove local debug settings * [autofix.ci] apply automated fixes * Update VRT screenshots Auto-generated by VRT workflow PR: #6068 * Improve graphs from comments * Fix lints * coderabit fixes * Fix filtering and UI enhancements * remove pngs * Fix typecheck * Another type issue * Update VRT screenshots Auto-generated by VRT workflow PR: #6068 * Update VRT screenshots Auto-generated by VRT workflow PR: #6068 * Fix strict typing issues * Update report page Now better conforms with components from other reports, e.g. by reusing Header Makes it possible to display a period longer than one month. * Change view description order * Formatting and cleanup * Removed difference section, as it will be difficult to get a reliable view across months * Introduce the Timeframe param, similar to Spending report, to allow saving a Live sliding window. * Allow filtering just the last month * Fix linting errors * Remove all information about income * Remove debugging statement * Sort categories and subcategories by amount * Move compact mode to spreadsheet to fix Card view more easily * Update tests file * Add release notes * Rename release notes to match PR# * Fix autofix.ci issues * Update packages/desktop-client/e2e/sankey.test.ts Enable experimental feature fall all tests, pr. coderabbit recommendation Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add sankey-card to isWidgetType * Gate Sankey routes to prevent direct URL bypass * Fix typo * Change node transformation to work by key instead of name, to remove risk of duplicate issues Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Prevent false-positive pass in month-change test. * Translate mode to a proper label * Fix message for empty data * Enabled LoadingIndicator until data is ready * Change card default mode * More robust filtering * Fixed issue with budgeted spreadsheet not using 'end' date * Allow copying SankeyCard to dashboard * Fix typing and linting issues * Remove e2e tests I cannot currently get them to pass, because I dont fully understand playwright and how they are supposed to work. I can see that they don't exist for other reports. We can add them later if required. * Remove unecessary sankey reference * Refactor spreadsheet * Remove dead code from SankeyGraph * Collect to Other if too many subcategories * Edit wrong comment * Linting and typechecking * Show remaining amount to budget * Hide description on narrow device * Add visual clue if 'To budget' is larger than 'Budgeted' and would extend below the edge of the graph * Add colors to the links * Fix report card showing subcategories instead of main categories * Add tooltip info to Other on SankeyCard * Create globalOther flag and implement greedy category reduction algorithm * Allow user to select between Global or Per category Other * Allow user to choose number of subcategories to show * Allow user to select how subcategories are sorted * Fix budget filtering * [autofix.ci] apply automated fixes * Condense sorting and Other-grouping to one option * Implement Sort as budget option * Dynamically adjust topN based on SankeyCard height * Remove old feature flags from previous PR --------- Co-authored-by: andrewhumble <43395285+andrewhumble@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: youngcw <calebyoung94@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fix yarn generate:icons command (#7281) * fix icon templates with `module.exports` to `export default` * Add `@svgr/babel-plugin-add-jsx-attribute` to dependencies * Run `yarn generate:icons`, and set prettier singleQuote to reduce changes * Add release note * Add temporary fix for `SvgChartArea` * Add `ChartArea` svg from the existing tsx * CI rerun * Standardise ledger scrolling when using keyboard shortcuts (#7283) * Standardise table keyboard navigation by preventing browser scroll with arrow keys * Add release note * Apply the preventDefault() in specific cases so that it is not applied to default --------- Co-authored-by: youngcw <calebyoung94@gmail.com> * Fix updateTransaction corrupting split parents with partial updates (#7242) * [AI] Fix updateTransaction corrupting split parents with partial updates When `api.updateTransaction(id, { notes: '...' })` is called on a split parent, the `updateTransaction` helper replaces the parent with the sparse update object (`{ id, notes }`) instead of merging it with the existing transaction data. This causes `recalculateSplit` to see `amount` as `undefined` (→ 0), which doesn't match the children's total and sets a `SplitTransactionError` on the parent. `makeChild` also inherits undefined `account`, `date`, and `cleared` values, potentially creating broken child rows. Fix: merge the incoming partial fields (`{ ...trans, ...transaction }`) so all existing properties are preserved. Add a test that performs a notes-only update on a split parent and asserts no error is set and the amount stays intact. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [AI] Add release notes for PR #7242 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: remove verbose comment and simplify release note Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: L. Warren Thompson <lwarrenthompson@Warren-MBP.local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * [AI] Add electron conditions to loot-core platform/server exports and fix imports - Add "electron" condition to platform/server exports (asyncStorage, connection, fetch, fs, sqlite) so they resolve to .electron.ts files when using the electron export condition - Remove broken ./client/platform export referencing non-existent files - Convert deep relative imports in electron files to subpath imports (#types/prefs, #server/errors, #server/mutators) https://claude.ai/code/session_01FPpKnozt42Mf79YHAT6ytM * [AI] Convert remaining relative imports to subpath imports in electron files - Convert ../fs, ../log, ../../exceptions to subpath imports (#platform/server/fs, #platform/server/log, #platform/exceptions) - Add electron-conditional entries to the imports field in package.json for all 5 platform/server modules with electron variants - Add resolve.conditions: ['electron'] to vite.desktop.config.ts so the electron condition is recognized during desktop builds https://claude.ai/code/session_01FPpKnozt42Mf79YHAT6ytM * Add release notes for PR #7383 * [AI] Fix API build and test failures from conditional exports - Add "api" condition to all 5 platform/server exports and imports entries so the API build resolves to .api.ts variants correctly - Add resolve.conditions and ssr.resolve.conditions: ['api'] to packages/api/vite.config.ts - Add explicit #platform/server/log and #platform/exceptions entries to the imports field (array fallback in #* wildcard doesn't work for directory modules) - Revert #platform/server/fs back to relative ../fs import in asyncStorage/index.electron.ts — subpath imports for platform modules with electron variants don't work in vitest because the test runner doesn't propagate resolve.conditions to Node's import resolution https://claude.ai/code/session_01FPpKnozt42Mf79YHAT6ytM * fix: apply CodeRabbit auto-fixes Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * [autofix.ci] apply automated fixes * Enhance package.json and Vite configurations for Electron support; refactor imports to use new path aliases. Added new paths for server and shared modules, updated SSR settings, and improved test configurations for better module resolution. * [AI] Merge electron condition with default Vite conditions in vitest config Co-authored-by: Matiss Janis Aboltins <MatissJanis@users.noreply.github.com> * [AI] Move @ts-strict-ignore comment to first line in reset.ts Co-authored-by: Matiss Janis Aboltins <MatissJanis@users.noreply.github.com> --------- Co-authored-by: Matt Fiddaman <github@m.fiddaman.uk> Co-authored-by: Juulz <julesmcn@gmail.com> Co-authored-by: Pranay S <pranayritvik@gmail.com> Co-authored-by: Pranay Mac M1 <pranayseela@yahoo.com> Co-authored-by: youngcw <calebyoung94@gmail.com> Co-authored-by: Emil Tveden Bjerglund <emilbp@gmail.com> Co-authored-by: andrewhumble <43395285+andrewhumble@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: James Skinner <56730344+JSkinnerUK@users.noreply.github.com> Co-authored-by: L. Warren Thompson <warren.thompson@zuirail.com> Co-authored-by: L. Warren Thompson <lwarrenthompson@Warren-MBP.local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Matiss Janis Aboltins <MatissJanis@users.noreply.github.com>
Summary
When
api.updateTransaction(id, { notes: '...' })is called on a split parent transaction, theupdateTransaction()helper inshared/transactions.tsreplaces the parent with the sparse update object instead of merging it with the existing transaction data. This causes:recalculateSplit()seesamountasundefined(→ 0), which doesn't match the children's total, and sets aSplitTransactionErroron the parentmakeChild()inheritsundefinedvalues foraccount,date, andclearedfrom the sparse parent objectRoot cause: Line 265 of
transactions.tsusestransaction(the sparse partial update) directly instead of merging withtrans(the existing full transaction):Fix: Merge the incoming partial fields with the existing transaction data so all properties (
amount,account,date, etc.) are preserved.Test added: A new test case performs a notes-only partial update on a split parent and asserts no
SplitTransactionErroris set and the amount stays intact.Bundle Stats
View detailed bundle stats
desktop-client
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/shared/transactions.tsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
loot-core
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/shared/transactions.tsView detailed bundle breakdown
Added
Removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged
api
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/shared/transactions.tsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
cli
Total
View detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged