Fix transaction hooks and improve transactions loading experience in mobile #5415
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
joel-jeremy
changed the title
| isLoading, | ||
| ...(error && { error }), | ||
| reload, | ||
| loadMore, | ||
| isLoadingMore, | ||
| }; | ||
| } | ||
|
|
||
| function getCalculateRunningBalancesFn( |
There was a problem hiding this comment.
This doesn't work on the full transaction list, thats why I changed to a query. When the account is long, not all transactions are pulled, so this doesn't have all the data it needs to calculate properly.
There was a problem hiding this comment.
I see. Let me check. Do you have a test file I can test with?
There was a problem hiding this comment.
I used my personal budget with 1000+ transactions. If you compare to the desktop view running balance, you can see if the amounts are wrong as the desktop view is query based.
There was a problem hiding this comment.
Also, it looks like it put that comment in a confusing place. I meant to hook it to the calculate bottom up function.
There was a problem hiding this comment.
Its possible that a top down summation would work if you had the right starting amount
There was a problem hiding this comment.
This should be fixed now. We just need to recalculate running balance whenever we inverse some amounts/payees/accounts on the useAccountPreviewTransactions.
There was a problem hiding this comment.
The running balance of regular transactions is still incorrect.
WalkthroughRefactors transaction hooks and related mobile components to make running-balance calculation optional, configurable, and per-account-aware. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Would showing transactions before previews are loaded allow people to trigger a bank sync? The reason that the page was set into a loading state while previews were being loaded was because a bank sync couldn't be triggered until after the previews had loaded. See #5080 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts (1)
185-196: Remove redundant nullish coalescing operatorThe
startingBalanceparameter is required (not optional) in the function signature, so the nullish coalescing operator is unnecessary.- startingBalance ?? 0, + startingBalance,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts(7 hunks)packages/desktop-client/src/hooks/usePreviewTransactions.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/hooks/usePreviewTransactions.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Learnt from: csenel
PR: actualbudget/actual#3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Learnt from: MatissJanis
PR: actualbudget/actual#3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
Learning: In the `Transaction` component in `Transaction.tsx`, both `rawTransaction` and `transaction` should be included in the dependency arrays of `useMemo` hooks, even though `transaction` derives from `rawTransaction`.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: matt-fidd
PR: actualbudget/actual#4253
File: packages/sync-server/src/app-gocardless/banks/nationwide_naiagb21.js:41-44
Timestamp: 2025-02-11T17:25:39.615Z
Learning: In bank sync providers, core fields like transactionId and amount should be mutated directly on the transaction object as they aren't covered by the fallback normalization logic and shouldn't be exposed for user mapping. Display-related fields should use editedTrans.
Learnt from: matt-fidd
PR: actualbudget/actual#4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:155-158
Timestamp: 2025-01-16T14:30:36.518Z
Learning: In packages/loot-core/src/client/data-hooks/transactions.ts, the `upcomingLength` preference is always stored as a number in string format, so no additional type checking is needed when using `parseInt`.
Learnt from: lelemm
PR: actualbudget/actual#3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Learnt from: UnderKoen
PR: actualbudget/actual#3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts (19)
Learnt from: jfdoming
PR: #3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the AllTransactions component of Account.tsx, the useLayoutEffect hook is appropriately used to dispatch an action that closes splits for parent transactions when prependTransactions changes, ensuring this occurs synchronously before the component is painted.
Learnt from: csenel
PR: #3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Learnt from: joel-jeremy
PR: #3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React useTransactions hook defined in packages/loot-core/src/client/data-hooks/transactions.ts, changes to options (e.g., options.pageCount) should be applied on the next query without adding them to the useEffect dependency array, to avoid unnecessary rerenders when options change.
Learnt from: joel-jeremy
PR: #3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: MatissJanis
PR: #3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
Learning: In the Transaction component in Transaction.tsx, both rawTransaction and transaction should be included in the dependency arrays of useMemo hooks, even though transaction derives from rawTransaction.
Learnt from: lelemm
PR: #3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In packages/desktop-client/src/components/reports/reports/Summary.tsx, API calls like get-earliest-transaction are used without explicit error handling to maintain consistency with other components.
Learnt from: tlesicka
PR: #3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The onReorder function in Accounts.tsx was moved from Sidebar.tsx, and the targetId parameter remains typed as unknown intentionally.
Learnt from: matt-fidd
PR: #4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:155-158
Timestamp: 2025-01-16T14:30:36.518Z
Learning: In packages/loot-core/src/client/data-hooks/transactions.ts, the upcomingLength preference is always stored as a number in string format, so no additional type checking is needed when using parseInt.
Learnt from: matt-fidd
PR: #4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:200-200
Timestamp: 2025-01-16T14:29:13.188Z
Learning: In the scheduled transactions implementation within packages/loot-core/src/client/data-hooks/transactions.ts, the upcoming flag is set based on schedules.length > 0 to act as an override, where the first occurrence gets false and subsequent occurrences get true. This is intentional and should not be changed to date-based comparison.
Learnt from: matt-fidd
PR: #3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In packages/loot-core/src/client/actions/account.ts, within the syncAccounts function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.
Learnt from: matt-fidd
PR: #5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Learnt from: jfdoming
PR: #3641
File: packages/loot-core/src/server/accounts/rules.ts:687-693
Timestamp: 2024-10-12T19:11:05.790Z
Learning: In packages/loot-core/src/server/accounts/rules.ts, within the execSplitActions function, the zeroth index of newTransactions is reserved for actions that apply to all splits, so split transactions start from index 1.
Learnt from: matt-fidd
PR: #4253
File: packages/desktop-client/src/components/banksync/EditSyncAccount.tsx:154-160
Timestamp: 2025-01-29T19:44:02.950Z
Learning: In EditSyncAccount.tsx, mappings state is initialized with defaultMappings (containing both 'payment' and 'deposit' directions) if no saved mappings exist, so direction maps are guaranteed to exist.
Learnt from: UnderKoen
PR: #3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using Promise.all as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Learnt from: matt-fidd
PR: #4180
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:312-318
Timestamp: 2025-01-17T18:05:12.337Z
Learning: In the Actual Budget codebase, the schedule action handlers (onPost, onSkip, onComplete) in AccountTransactions.tsx follow a consistent pattern without explicit try-catch blocks, letting errors propagate up the call stack.
Learnt from: lelemm
PR: #4628
File: packages/sync-server/src/app-pluggyai/app-pluggyai.js:113-119
Timestamp: 2025-03-15T02:34:59.859Z
Learning: In Pluggy.ai transactions, the trans.amount field is always present and never null, while trans.amountInAccountCurrency may be null or undefined, requiring a null check before operations.
Learnt from: qedi-r
PR: #3527
File: packages/desktop-client/src/components/modals/CreateLocalAccountModal.tsx:47-47
Timestamp: 2024-09-28T17:03:43.286Z
Learning: Validating balance is outside the scope in CreateLocalAccountModal.tsx.
Learnt from: matt-fidd
PR: #4253
File: packages/sync-server/src/app-gocardless/banks/nationwide_naiagb21.js:41-44
Timestamp: 2025-02-11T17:25:39.615Z
Learning: In bank sync providers, core fields like transactionId and amount should be mutated directly on the transaction object as they aren't covered by the fallback normalization logic and shouldn't be exposed for user mapping. Display-related fields should use editedTrans.
Learnt from: matt-fidd
PR: #4253
File: packages/sync-server/src/app-gocardless/banks/seb_kort_bank_ab.js:19-27
Timestamp: 2025-02-11T17:26:44.539Z
Learning: In SEB bank integration (seb_kort_bank_ab.js), the transaction amount should be modified directly on the original transaction object as it serves as an identifier, and the modified amount is considered the true value. This is an intentional exception to the immutable pattern.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Wait for Netlify build to finish
🔇 Additional comments (2)
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts (2)
15-26: Good use of ReturnType utilityThe change from explicit type definition to
ReturnType<typeof usePreviewTransactions>follows DRY principles and ensures the type stays in sync with the actual return value.
88-133: Well-structured hook logic with proper memoizationThe implementation correctly handles the undefined accountId case, properly filters running balances to match the preview transactions, and includes all necessary dependencies in the useMemo hook.
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
Outdated
Show resolved
Hide resolved
Thanks for the catch! I have disabled PullToRefresh when transaction list is in loading state. |
|
/update-vrt |
|
This does help a little. It still doesn't fix the account ledger hesitating before scrolling on large accounts. This only happens after the preview transactions have loaded. The ledger scrolls fine while they are loading. One thing that confuses me is why the desktop view loads and acts much nicer even on a phone. The desktop view loads in ~1 sec where the mobile view takes ~6 seconds to load everything (on this account im testing) |
|
Long_test.zip I've not seen any performance hit by using a query to do the calculation. Maybe instead of storing all running balances we could calculate the current balance, then work back from there. That would work better with the paged loads than what I was doing of holding onto all balance values for all transactions even if you only viewed the most recent ones. |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx(3 hunks)packages/desktop-client/src/hooks/useTransactions.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/hooks/useTransactions.tspackages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable.
Files:
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: tlesicka
PR: actualbudget/actual#3689
File: packages/desktop-client/src/components/modals/LoadBackupModal.tsx:162-190
Timestamp: 2024-10-25T06:22:33.416Z
Learning: Adding progress indicators for backup operations in the budget application requires updates to the server backend, and may be beyond the scope of a single PR.
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Learnt from: csenel
PR: actualbudget/actual#3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: MatissJanis
PR: actualbudget/actual#3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
Learning: In the `Transaction` component in `Transaction.tsx`, both `rawTransaction` and `transaction` should be included in the dependency arrays of `useMemo` hooks, even though `transaction` derives from `rawTransaction`.
Learnt from: matt-fidd
PR: actualbudget/actual#4253
File: packages/sync-server/src/app-gocardless/banks/nationwide_naiagb21.js:41-44
Timestamp: 2025-02-11T17:25:39.615Z
Learning: In bank sync providers, core fields like transactionId and amount should be mutated directly on the transaction object as they aren't covered by the fallback normalization logic and shouldn't be exposed for user mapping. Display-related fields should use editedTrans.
Learnt from: matt-fidd
PR: actualbudget/actual#4170
File: packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx:570-581
Timestamp: 2025-01-17T12:00:27.629Z
Learning: The transaction amount conversion for income categories in TransactionEdit.jsx is intended as a quality-of-life feature to help users who forget to set the correct direction, not as a preventative measure. Users should still be able to manually enter negative amounts even for income categories.
Learnt from: lelemm
PR: actualbudget/actual#3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Learnt from: matt-fidd
PR: actualbudget/actual#4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:155-158
Timestamp: 2025-01-16T14:30:36.518Z
Learning: In packages/loot-core/src/client/data-hooks/transactions.ts, the `upcomingLength` preference is always stored as a number in string format, so no additional type checking is needed when using `parseInt`.
packages/desktop-client/src/hooks/useTransactions.ts (20)
Learnt from: joel-jeremy
PR: #3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React useTransactions hook defined in packages/loot-core/src/client/data-hooks/transactions.ts, changes to options (e.g., options.pageCount) should be applied on the next query without adding them to the useEffect dependency array, to avoid unnecessary rerenders when options change.
Learnt from: csenel
PR: #3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Learnt from: jfdoming
PR: #3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the AllTransactions component of Account.tsx, the useLayoutEffect hook is appropriately used to dispatch an action that closes splits for parent transactions when prependTransactions changes, ensuring this occurs synchronously before the component is painted.
Learnt from: matt-fidd
PR: #5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Learnt from: MatissJanis
PR: #3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
Learning: In the Transaction component in Transaction.tsx, both rawTransaction and transaction should be included in the dependency arrays of useMemo hooks, even though transaction derives from rawTransaction.
Learnt from: matt-fidd
PR: #4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:155-158
Timestamp: 2025-01-16T14:30:36.518Z
Learning: In packages/loot-core/src/client/data-hooks/transactions.ts, the upcomingLength preference is always stored as a number in string format, so no additional type checking is needed when using parseInt.
Learnt from: matt-fidd
PR: #4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:200-200
Timestamp: 2025-01-16T14:29:13.188Z
Learning: In the scheduled transactions implementation within packages/loot-core/src/client/data-hooks/transactions.ts, the upcoming flag is set based on schedules.length > 0 to act as an override, where the first occurrence gets false and subsequent occurrences get true. This is intentional and should not be changed to date-based comparison.
Learnt from: jfdoming
PR: #3641
File: packages/loot-core/src/server/accounts/rules.ts:687-693
Timestamp: 2024-10-12T19:11:05.790Z
Learning: In packages/loot-core/src/server/accounts/rules.ts, within the execSplitActions function, the zeroth index of newTransactions is reserved for actions that apply to all splits, so split transactions start from index 1.
Learnt from: lelemm
PR: #4628
File: packages/sync-server/src/app-pluggyai/app-pluggyai.js:113-119
Timestamp: 2025-03-15T02:34:59.859Z
Learning: In Pluggy.ai transactions, the trans.amount field is always present and never null, while trans.amountInAccountCurrency may be null or undefined, requiring a null check before operations.
Learnt from: UnderKoen
PR: #3365
File: packages/loot-core/src/types/models/rule.d.ts:4-4
Timestamp: 2024-10-02T08:45:11.136Z
Learning: In packages/loot-core/src/server/accounts/transaction-rules.ts, the stage property can have legacy values 'cleanup' and 'modify', which are converted to 'pre'. The type remains string to accommodate these values and ensure correct usage.
Learnt from: joel-jeremy
PR: #3903
File: packages/desktop-client/src/components/mobile/accounts/Accounts.tsx:181-182
Timestamp: 2024-11-26T19:31:26.664Z
Learning: The balance type names onbudget-accounts-balance and offbudget-accounts-balance are both in lowercase in the codebase.
Learnt from: matt-fidd
PR: #4170
File: packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx:570-581
Timestamp: 2025-01-17T12:00:27.629Z
Learning: The transaction amount conversion for income categories in TransactionEdit.jsx is intended as a quality-of-life feature to help users who forget to set the correct direction, not as a preventative measure. Users should still be able to manually enter negative amounts even for income categories.
Learnt from: matt-fidd
PR: #4253
File: packages/sync-server/src/app-gocardless/banks/nationwide_naiagb21.js:41-44
Timestamp: 2025-02-11T17:25:39.615Z
Learning: In bank sync providers, core fields like transactionId and amount should be mutated directly on the transaction object as they aren't covered by the fallback normalization logic and shouldn't be exposed for user mapping. Display-related fields should use editedTrans.
Learnt from: lelemm
PR: #3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In packages/desktop-client/src/components/reports/reports/Summary.tsx, API calls like get-earliest-transaction are used without explicit error handling to maintain consistency with other components.
Learnt from: jfdoming
PR: #4147
File: packages/loot-core/src/platform/client/fetch/index.d.ts:44-45
Timestamp: 2025-01-18T03:51:56.741Z
Learning: Query results in packages/loot-core/src/platform/client/fetch/index.d.ts are intentionally typed as any due to their complex and varied nature (could be numbers, strings, or other types). While this could be improved with stricter types in the future, it requires significant work to properly type all possible Query results.
Learnt from: elijaholmos
PR: #5076
File: packages/desktop-client/src/components/CommandBar.tsx:70-70
Timestamp: 2025-06-03T23:19:44.814Z
Learning: The useReports hook in packages/desktop-client/src/hooks/useReports.ts always returns an array for the data property due to the ternary operator at line 62: queryData ? [...queryData] : [], ensuring data is never undefined even if the underlying query returns undefined.
Learnt from: lelemm
PR: #3828
File: packages/desktop-client/src/components/reports/reports/Calendar.tsx:575-631
Timestamp: 2024-11-12T18:18:07.282Z
Learning: In Calendar.tsx, transaction-related callbacks such as onBatchDelete, onBatchDuplicate, onCreateRule, and onScheduleAction are intentionally left as empty functions because these operations should not be usable on that page.
Learnt from: matt-fidd
PR: #4180
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:312-318
Timestamp: 2025-01-17T18:05:12.337Z
Learning: In the Actual Budget codebase, the schedule action handlers (onPost, onSkip, onComplete) in AccountTransactions.tsx follow a consistent pattern without explicit try-catch blocks, letting errors propagate up the call stack.
Learnt from: matt-fidd
PR: #4253
File: packages/sync-server/src/app-gocardless/banks/seb_kort_bank_ab.js:19-27
Timestamp: 2025-02-11T17:26:44.539Z
Learning: In SEB bank integration (seb_kort_bank_ab.js), the transaction amount should be modified directly on the original transaction object as it serves as an identifier, and the modified amount is considered the true value. This is an intentional exception to the immutable pattern.
Learnt from: matt-fidd
PR: #4253
File: packages/desktop-client/src/components/banksync/EditSyncAccount.tsx:154-160
Timestamp: 2025-01-29T19:44:02.950Z
Learning: In EditSyncAccount.tsx, mappings state is initialized with defaultMappings (containing both 'payment' and 'deposit' directions) if no saved mappings exist, so direction maps are guaranteed to exist.
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (15)
Learnt from: joel-jeremy
PR: #3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Learnt from: jfdoming
PR: #3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the AllTransactions component of Account.tsx, the useLayoutEffect hook is appropriately used to dispatch an action that closes splits for parent transactions when prependTransactions changes, ensuring this occurs synchronously before the component is painted.
Learnt from: joel-jeremy
PR: #3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React useTransactions hook defined in packages/loot-core/src/client/data-hooks/transactions.ts, changes to options (e.g., options.pageCount) should be applied on the next query without adding them to the useEffect dependency array, to avoid unnecessary rerenders when options change.
Learnt from: csenel
PR: #3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Learnt from: MatissJanis
PR: #3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
Learning: In the Transaction component in Transaction.tsx, both rawTransaction and transaction should be included in the dependency arrays of useMemo hooks, even though transaction derives from rawTransaction.
Learnt from: matt-fidd
PR: #5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Learnt from: lelemm
PR: #3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In packages/desktop-client/src/components/reports/reports/Summary.tsx, API calls like get-earliest-transaction are used without explicit error handling to maintain consistency with other components.
Learnt from: tlesicka
PR: #3554
File: packages/desktop-client/src/components/sidebar/Accounts.tsx:60-64
Timestamp: 2024-10-04T05:13:58.322Z
Learning: The onReorder function in Accounts.tsx was moved from Sidebar.tsx, and the targetId parameter remains typed as unknown intentionally.
Learnt from: matt-fidd
PR: #3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In packages/loot-core/src/client/actions/account.ts, within the syncAccounts function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.
Learnt from: jfdoming
PR: #3641
File: packages/loot-core/src/server/accounts/rules.ts:687-693
Timestamp: 2024-10-12T19:11:05.790Z
Learning: In packages/loot-core/src/server/accounts/rules.ts, within the execSplitActions function, the zeroth index of newTransactions is reserved for actions that apply to all splits, so split transactions start from index 1.
Learnt from: lelemm
PR: #3828
File: packages/desktop-client/src/components/reports/reports/Calendar.tsx:575-631
Timestamp: 2024-11-12T18:18:07.282Z
Learning: In Calendar.tsx, transaction-related callbacks such as onBatchDelete, onBatchDuplicate, onCreateRule, and onScheduleAction are intentionally left as empty functions because these operations should not be usable on that page.
Learnt from: matt-fidd
PR: #4180
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:312-318
Timestamp: 2025-01-17T18:05:12.337Z
Learning: In the Actual Budget codebase, the schedule action handlers (onPost, onSkip, onComplete) in AccountTransactions.tsx follow a consistent pattern without explicit try-catch blocks, letting errors propagate up the call stack.
Learnt from: joel-jeremy
PR: #3685
File: packages/desktop-client/src/components/schedules/index.tsx:26-31
Timestamp: 2024-10-22T05:32:55.520Z
Learning: In this codebase, ESLint requires dispatch from useDispatch to be included in the dependency array of useCallback hooks, even though dispatch is stable.
Learnt from: matt-fidd
PR: #4253
File: packages/desktop-client/src/components/banksync/EditSyncAccount.tsx:154-160
Timestamp: 2025-01-29T19:44:02.950Z
Learning: In EditSyncAccount.tsx, mappings state is initialized with defaultMappings (containing both 'payment' and 'deposit' directions) if no saved mappings exist, so direction maps are guaranteed to exist.
Learnt from: lelemm
PR: #4628
File: packages/sync-server/src/app-pluggyai/app-pluggyai.js:113-119
Timestamp: 2025-03-15T02:34:59.859Z
Learning: In Pluggy.ai transactions, the trans.amount field is always present and never null, while trans.amountInAccountCurrency may be null or undefined, requiring a null check before operations.
🪛 GitHub Check: typecheck
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
[failure] 282-282:
Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
[failure] 299-299:
Type 'number | null' is not assignable to type 'number | undefined'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze
- GitHub Check: test
🔇 Additional comments (7)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (2)
36-41: LGTM! Import changes align with the hook refactoring.The imports correctly reflect the enhanced
useTransactionshook that now supports running balance calculations, and the addition ofcalculateRunningBalancesTopDownfunction aligns with the PR's refactoring goals.
295-301: Verify running balance accuracy with paged loading.The integration of top-down running balance calculation with
accountBalanceValueas the starting balance should address previous concerns about accuracy on large transaction lists. This approach aligns with past feedback suggesting that "a top down summation would work if you had the right starting amount."However, given the past issues with running balance calculations on partially loaded transaction lists, please verify that this implementation produces accurate results compared to the desktop view, especially for accounts with 1000+ transactions where not all transactions are initially loaded.
You can test this by:
- Comparing mobile running balances to desktop running balances on the same large account
- Ensuring accuracy is maintained as more transactions are loaded via pagination
packages/desktop-client/src/hooks/useTransactions.ts (5)
12-22: LGTM! Well-designed type definitions.The type definitions provide good flexibility with
CalculateRunningBalancesOptionsupporting both boolean (for default behavior) and function (for custom implementations). The function signature properly includes all necessary parameters for running balance calculations.
32-62: Excellent documentation and option design.The JSDoc documentation clearly explains the purpose and behavior of the new options. The explanation about default bottom-up calculation working with descending date order, and the note about custom functions for different orderings, demonstrates good design foresight.
100-160: Clean integration of running balance calculation.The implementation properly integrates running balance calculation without disrupting existing functionality. Key strengths:
- Uses options ref to prevent unnecessary re-renders (consistent with codebase patterns)
- Calculates running balances when data loads
- Maintains proper error handling
- Clean state management
206-214: LGTM! Clean helper function implementation.The helper function properly handles all cases of the
CalculateRunningBalancesOptiontype with clear logic flow.
216-252: Bottom-up calculation logic looks correct.The implementation properly:
- Filters transactions based on splits option (only parent transactions for 'all')
- Uses
reduceRightfor reverse order calculation- Builds running balances backward from the starting balance
The mathematical logic appears sound for bottom-up calculation where each transaction's running balance includes all subsequent transactions.
Given the complexity of running balance calculations and past issues mentioned in PR comments, please verify this implementation produces correct results by comparing with known good balances (e.g., desktop view) on test accounts.
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
Outdated
Show resolved
Hide resolved
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
Outdated
Show resolved
Hide resolved
|
/update-vrt |
|
The balance looks correct now. Large accounts still have hesitation on scroll. I only see it on my phone and not on a desktop machine. One extra thing I noticed is that the preview transactions don't load until after the regular transactions get marked with schedule/transfer icons. The previews load very soon after that. I wonder what has caused there to be so much delay between transactions rendering and the schedule/transfer icons rendering. |
|
Im thinking that for now I will revert the feature. Then after the release I can add back the feature and we can keep working on it. Does that sound OK? |
Sorry just got back from vacation. Yup, that sounds good! |
|
When I get some time, or if you get to it first, I will make a PR that reverts the removal. Then you can merge that into this and we can keep trying to get it to work. |
joel-jeremy
force-pushed
the
fix-useTransactions-and-improve-loading-experience
branch
from
ad9c3a7 to
44972de
Compare
Sounds good! I think we can merge this first and then let's work on the PR that reverts the removal. This PR fixes the running balances on the hooks and removes the duplicate |
… + recalculate running balances if there are any inversed transaction amounts
joel-jeremy
force-pushed
the
fix-useTransactions-and-improve-loading-experience
branch
from
3a6527a to
bb548a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts (1)
73-73: Handle undefined accountId in preference keyWhen
accountIdis undefined, the preference key becomesshow-balances-undefined. Consider handling this case explicitly.- const [showBalances] = useSyncedPref(`show-balances-${accountId}`); + const [showBalances] = useSyncedPref( + accountId ? `show-balances-${accountId}` : 'show-balances-default' + );Alternatively, since the hook returns early when accountId is undefined anyway (lines 89-96), you could move this line inside the conditional logic after the accountId check.
packages/desktop-client/src/hooks/useTransactions.ts (1)
254-294: Critical issue: Top-down calculation logic is incorrectThe calculation logic has several problems that will produce incorrect running balances:
- Line 276: First transaction's balance doesn't include its own amount
- Line 283: Last transaction's balance is reset to only its amount, ignoring all previous calculations
- Line 290: The calculation subtracts the previous amount instead of the current amount
Apply this fix to correct the calculation logic:
export function calculateRunningBalancesTopDown( transactions: TransactionEntity[], splits: TransactionSplitsOption, startingBalance: IntegerAmount = 0, ) { return transactions .filter(t => { switch (splits) { case 'all': // Only calculate parent/non-split amounts return !t.parent_id; default: // inline // grouped // none return true; } }) .reduce((acc, transaction, index, arr) => { if (index === 0) { - // This is the first transaction in the list, - // so we set the running balance to the starting balance - acc.set(transaction.id, startingBalance); - return acc; - } - - if (index === arr.length - 1) { - // This is the last transaction in the list, - // so we set the running balance to the amount of the transaction - acc.set(transaction.id, transaction.amount); - return acc; - } - - const previousTransaction = arr[index - 1]; - const previousRunningBalance = acc.get(previousTransaction.id) ?? 0; - const previousAmount = previousTransaction.amount ?? 0; - const currentRunningBalance = previousRunningBalance - previousAmount; + // First transaction: current balance after this transaction + acc.set(transaction.id, startingBalance - transaction.amount); + } else { + // Subsequent transactions: subtract current amount from previous balance + const previousTransaction = arr[index - 1]; + const previousRunningBalance = acc.get(previousTransaction.id) ?? 0; + acc.set(transaction.id, previousRunningBalance - transaction.amount); + } - acc.set(transaction.id, currentRunningBalance); return acc; }, new Map<TransactionEntity['id'], IntegerAmount>()); }
🧹 Nitpick comments (4)
packages/desktop-client/src/hooks/useCategoryPreviewTransactions.ts (1)
80-82: Avoid array materialization when filtering Map (micro-optimization + less GC)
new Map([...allRunningBalances].filter(...))allocates an intermediate array of all entries before filtering. Build the filtered Map directly to reduce allocations and iterate only once.Apply this diff:
- const runningBalances = new Map( - [...allRunningBalances].filter(([id]) => transactionIds.has(id)), - ); + const runningBalances = new Map<string, number>(); + for (const [id, balance] of allRunningBalances) { + if (transactionIds.has(id)) { + runningBalances.set(id, balance); + } + }packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts (2)
159-183: Add type annotation forinversedproperty in mapped transactionsThe mapped transactions now include an
inversedproperty that isn't part of theTransactionEntitytype. Consider creating a dedicated type for these enhanced transactions to improve type safety.Add a type definition above the function:
type InversedTransactionEntity = TransactionEntity & { inversed: boolean };Then update the function signature:
-}: InverseBasedOnAccountProps): { - transactions: TransactionEntity[]; +}: InverseBasedOnAccountProps): { + transactions: InversedTransactionEntity[]; runningBalances: Map<TransactionEntity['id'], IntegerAmount>; } {
189-196: Potential performance optimization for running balance recalculationThe running balances are recalculated from scratch whenever any transaction is inversed. For large transaction lists, consider optimizing this by only adjusting the affected balances.
For accounts with many transactions, recalculating all running balances could be expensive. Consider implementing an incremental update approach where only the inversed transactions' balances are adjusted. However, given that preview transactions are typically limited in number and this matches the PR's intent to fix running balances, the current implementation is acceptable.
packages/desktop-client/src/hooks/useTransactions.ts (1)
148-159: Consider memoizing the calculation functionThe
getCalculateRunningBalancesFnfunction is called on every data update. While the function itself is lightweight, consider memoizing it to avoid repeated evaluations of the same condition.+import { useMemo } from 'react'; // In the component: - const calculateFn = getCalculateRunningBalancesFn( - optionsRef.current?.calculateRunningBalances, - ); + const calculateFn = useMemo( + () => getCalculateRunningBalancesFn( + optionsRef.current?.calculateRunningBalances, + ), + [optionsRef.current?.calculateRunningBalances] + );Note: Since
optionsRefis used to avoid re-renders, this optimization may have limited impact but would make the intent clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5415.mdis excluded by!**/*.md
📒 Files selected for processing (9)
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx(1 hunks)packages/desktop-client/src/components/mobile/budget/CategoryTransactions.tsx(2 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionList.tsx(3 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.tsx(1 hunks)packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts(9 hunks)packages/desktop-client/src/hooks/useCategoryPreviewTransactions.ts(2 hunks)packages/desktop-client/src/hooks/usePreviewTransactions.ts(4 hunks)packages/desktop-client/src/hooks/useTransactions.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionList.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx
- packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
- packages/desktop-client/src/components/mobile/budget/CategoryTransactions.tsx
- packages/desktop-client/src/hooks/usePreviewTransactions.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Use functional and declarative programming patterns; avoid classes.
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError).
Favor named exports for components and utilities.
Use TypeScript for all code; prefer interfaces over types.
Avoid enums; use objects or maps instead.
Avoid usinganyorunknownunless absolutely necessary. Look for type definitions in the codebase instead.
Avoid type assertions withasor!; prefer usingsatisfies.
Use the "function" keyword for pure functions.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
packages/desktop-client/src/hooks/useCategoryPreviewTransactions.tspackages/desktop-client/src/hooks/useTransactions.tspackages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
📚 Learning: 2024-10-22T02:08:48.162Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Applied to files:
packages/desktop-client/src/hooks/useCategoryPreviewTransactions.tspackages/desktop-client/src/hooks/useTransactions.ts
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
PR: actualbudget/actual#3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Applied to files:
packages/desktop-client/src/hooks/useTransactions.tspackages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
PR: actualbudget/actual#5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/hooks/useTransactions.tspackages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
PR: actualbudget/actual#3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Applied to files:
packages/desktop-client/src/hooks/useTransactions.ts
📚 Learning: 2024-11-12T19:52:52.889Z
Learnt from: lelemm
PR: actualbudget/actual#3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Applied to files:
packages/desktop-client/src/hooks/useTransactions.ts
📚 Learning: 2024-10-12T19:11:05.790Z
Learnt from: jfdoming
PR: actualbudget/actual#3641
File: packages/loot-core/src/server/accounts/rules.ts:687-693
Timestamp: 2024-10-12T19:11:05.790Z
Learning: In `packages/loot-core/src/server/accounts/rules.ts`, within the `execSplitActions` function, the zeroth index of `newTransactions` is reserved for actions that apply to all splits, so split transactions start from index 1.
Applied to files:
packages/desktop-client/src/hooks/useTransactions.ts
📚 Learning: 2024-10-09T20:30:39.127Z
Learnt from: UnderKoen
PR: actualbudget/actual#3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
Applied to files:
packages/desktop-client/src/hooks/useTransactions.ts
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
📚 Learning: 2024-10-07T11:45:10.127Z
Learnt from: tlesicka
PR: actualbudget/actual#3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:64-64
Timestamp: 2024-10-07T11:45:10.127Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, when `useMetadataPref('budgetName')` returns `undefined`, the code handles it appropriately without the need to assign a default value to `budgetName`.
Applied to files:
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
PR: actualbudget/actual#4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget app, the callback functions `onEditCategory` and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts
🧬 Code Graph Analysis (2)
packages/desktop-client/src/hooks/useTransactions.ts (3)
packages/loot-core/src/types/models/transaction.ts (1)
TransactionEntity(8-38)packages/loot-core/src/shared/util.ts (1)
IntegerAmount(392-392)packages/loot-core/src/shared/query.ts (1)
Query(22-151)
packages/desktop-client/src/hooks/useAccountPreviewTransactions.ts (6)
packages/desktop-client/src/hooks/useSyncedPref.ts (1)
useSyncedPref(12-25)packages/loot-core/src/types/models/account.ts (1)
AccountEntity(1-9)packages/loot-core/src/types/models/transaction.ts (1)
TransactionEntity(8-38)packages/loot-core/src/shared/util.ts (1)
IntegerAmount(392-392)packages/desktop-client/src/queries/index.ts (1)
transactions(62-78)packages/desktop-client/src/hooks/useTransactions.ts (1)
calculateRunningBalancesBottomUp(216-252)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Functional Desktop App
- GitHub Check: Analyze
- GitHub Check: test
- GitHub Check: Wait for Netlify build to finish
🔇 Additional comments (3)
packages/desktop-client/src/hooks/useCategoryPreviewTransactions.ts (2)
69-71: Propagating loading/error during early-return is correctReturning
isLoadinganderrorfromusePreviewTransactionshere prevents masking in-flight states and errors when there are no schedules (or category), which is the right behavior for downstream UX.
57-62: Options handling in usePreviewTransactions is correct
Confirmed thatusePreviewTransactionsstores the incomingoptionsin a ref (lines 69–73) and never includesoptionsin itsuseEffectdependency array (the effect at lines 152–160 only depends onfilter,isSchedulesLoading,schedules,statuses, andupcomingLength). This matches our established pattern—no changes required.packages/desktop-client/src/hooks/useTransactions.ts (1)
216-252: LGTM! Well-implemented bottom-up calculationThe bottom-up calculation correctly handles running balances by working backwards from the most recent transaction. The implementation properly:
- Filters split transactions when needed
- Starts with the correct initial balance for the last transaction
- Accumulates balances moving backwards through the list
joel-jeremy
deleted the
fix-useTransactions-and-improve-loading-experience
branch
Fix reverted changes on some hooks (could be due to incorrect rebase/merge), this PR aims to fix that + a change on the loading experience on mobile to display transactions as they load.
Meaning, normal transactions would be displayed even when the preview transactions are not yet loaded, and display the preview transactions as they load (a loading indicator would be displayed to indicate the preview transactions are still being loaded). This should improve the user experience because the preview transactions are slower to load than the normal transactions.