feat: add "last reconciled" timestamp to accounts #4459
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site 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 (largest 100 files by percent change)
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
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
tostasmistas
force-pushed
the
feat/accounts-last-reconciled
branch
from
2ad3da4 to
5eaaf45
Compare
add a `last_reconciled` column to the `accounts` table, which stores a UNIX timestamp indicating when each account was last reconciled upon initial release, all accounts will display "Not yet reconciled"; however, after completing reconciliation and pressing the "Done reconciling" button, the timestamp will be updated accordingly
tostasmistas
force-pushed
the
feat/accounts-last-reconciled
branch
from
5eaaf45 to
51a53f6
Compare
tostasmistas
changed the title
WalkthroughThe changes integrate tracking and error handling for account reconciliation across multiple areas of the codebase. In the desktop client, the reconciliation process now includes additional error checks to ensure the existence of an account and updates its Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/loot-core/src/mocks/index.ts (1)
25-25: Consider initializinglast_reconciledto null for new accounts.Currently, all mock accounts are initialized with the current timestamp as their
last_reconciledvalue, implying they've already been reconciled. This doesn't match the expected behavior where new accounts would start with a "Not yet reconciled" status.- last_reconciled: new Date().getTime().toString(), + last_reconciled: null,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4459.mdis excluded by!**/*.md
📒 Files selected for processing (8)
packages/desktop-client/src/components/accounts/Account.tsx(2 hunks)packages/desktop-client/src/components/accounts/Balance.tsx(5 hunks)packages/desktop-client/src/components/spreadsheet/index.ts(1 hunks)packages/loot-core/migrations/1740506588539_add_last_reconciled_at.sql(1 hunks)packages/loot-core/src/mocks/index.ts(1 hunks)packages/loot-core/src/server/accounts/app.ts(1 hunks)packages/loot-core/src/server/aql/schema/index.ts(1 hunks)packages/loot-core/src/types/models/account.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/loot-core/migrations/1740506588539_add_last_reconciled_at.sql
🧰 Additional context used
🧠 Learnings (1)
packages/desktop-client/src/components/accounts/Account.tsx (1)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
🔇 Additional comments (10)
packages/desktop-client/src/components/spreadsheet/index.ts (1)
17-17: LGTM: Properly typed new property for account reconciliation.The
lastReconciledproperty has been correctly added to the account type with the appropriatestring | nulltype, allowing it to represent both reconciled accounts (with timestamp strings) and unreconciled accounts (with null values).packages/loot-core/src/types/models/account.d.ts (1)
7-7: LGTM: Properly typed core data model property.The
last_reconciledproperty has been correctly added to theAccountEntitytype with the appropriatestring | nulltype, consistent with other timestamp fields in the model and allowing for accounts that haven't been reconciled yet.packages/loot-core/src/server/aql/schema/index.ts (1)
77-77: LGTM: Schema field added with correct type.The
last_reconciledfield has been correctly added to the schema with the string type, which aligns with the type definition in theAccountEntityinterface.packages/loot-core/src/server/accounts/app.ts (1)
64-74: Good implementation for conditional field updates.The function signature has been properly updated to accept the new
last_reconciledfield using TypeScript's utility types, and the implementation uses a conditional spread operator to only include the field when it's provided. This maintains backward compatibility with existing code while supporting the new functionality.packages/desktop-client/src/components/accounts/Account.tsx (2)
1013-1018: Good defensive programming with account existence check.Adding this account existence check before attempting to update the last_reconciled timestamp helps prevent potential runtime errors and provides a clear error message for debugging.
1044-1049: Last reconciled timestamp implementation looks good.The code properly captures the current timestamp and updates the account record with the reconciliation time when the user completes reconciliation. This ensures the timestamp is updated in the database for future reference.
packages/desktop-client/src/components/accounts/Balance.tsx (4)
7-8: Appropriate imports for date handling.The date-fns library is a good choice for handling relative time formatting and provides good localization support.
150-159: Well-implemented relative time formatter.The
tsToRelativeTimefunction handles null values gracefully, correctly parses the timestamp, and properly applies localization based on the user's preferred language.One small suggestion: Add a fallback for invalid date parsing that could occur if the timestamp format changes in the future.
const tsToRelativeTime = (ts: string | null, language: string): string => { if (!ts) return 'Unknown'; const parsed = new Date(parseInt(ts, 10)); + if (isNaN(parsed.getTime())) return 'Unknown'; const locale = locales[language.replace('-', '') as keyof typeof locales] ?? locales['enUS']; return formatDistanceToNow(parsed, { addSuffix: true, locale }); };
188-208: Good UI implementation for reconciliation status.The UI element is consistent with the existing design language and provides useful information to the user about the reconciliation status of the account.
294-299: Proper prop passing for the reconciliation status.The code correctly passes the
last_reconciledproperty from the account object to theMoreBalancescomponent, maintaining the data flow.
generate mock accounts with the `last_reconciled` value set to `null`, to match the expected behaviour that new accounts will start with a "Not yet reconciled" status
| locales[language.replace('-', '') as keyof typeof locales] ?? | ||
| locales['enUS']; | ||
|
|
||
| return formatDistanceToNow(parsed, { addSuffix: true, locale }); |
There was a problem hiding this comment.
We can also use formatDistanceToNowStrict which will not include the prefix "about".
This PR resolves #2967.
It adds a
last_reconciledcolumn to theaccountstable, which stores a UNIX timestamp indicating when each account was last reconciled. Upon initial release, all accounts will display "Not yet reconciled"; however, after completing reconciliation and pressing the "Done reconciling" button, the timestamp will be updated accordingly.