♻️ (synced-prefs) move budget type to synced prefs #3427
Conversation
actual-github-bot
bot
changed the title
| @@ -174,17 +174,3 @@ export async function prewarmAllMonths( | |||
| months.map(month => prewarmMonth(budgetType, spreadsheet, month)), | |||
| ); | |||
| } | |||
|
|
|||
| export async function switchBudgetType( | |||
There was a problem hiding this comment.
Not necessary anymore IMO. We already do the prewarming operation in the budget page - inside a useEffect hook. So whenever budgetType changes - the prewarming will be performed.
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 No assets were bigger 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 |
MatissJanis
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@coderabbitai full review |
Actions performedFull review triggered. |
WalkthroughThe changes involve a comprehensive update to the management of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Server
participant Database
User->>UI: Request Budget Type
UI->>Server: Fetch Budget Type
Server->>Database: Query Budget Type
Database-->>Server: Return Budget Type
Server-->>UI: Send Budget Type
UI-->>User: Display Budget Type
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
Outside diff range and nitpick comments (1)
upcoming-release-notes/3352.md (1)
6-6: Fix the typo in the change description.The change description seems to have a typo or grammatical error. It currently reads:
Support translations in Translation support for desktop-client/src/components/budget/BalanceWithCarryover.tsx
Consider rephrasing it to something like:
Added translation support for desktop-client/src/components/budget/BalanceWithCarryover.tsx
This will make the description clearer and easier to understand.
-Support translations in Translation support for desktop-client/src/components/budget/BalanceWithCarryover.tsx +Added translation support for desktop-client/src/components/budget/BalanceWithCarryover.tsx
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx (2 hunks)
- packages/desktop-client/src/components/budget/index.tsx (2 hunks)
- packages/desktop-client/src/components/budget/util.ts (3 hunks)
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (3 hunks)
- packages/desktop-client/src/components/mobile/budget/index.tsx (1 hunks)
- packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (1 hunks)
- packages/desktop-client/src/components/settings/Experimental.tsx (1 hunks)
- packages/loot-core/migrations/1723665565000_prefs.js (1 hunks)
- packages/loot-core/src/server/budget/actions.ts (1 hunks)
- packages/loot-core/src/server/main.ts (1 hunks)
- packages/loot-core/src/server/prefs.ts (1 hunks)
- packages/loot-core/src/server/sheet.ts (1 hunks)
- packages/loot-core/src/server/sync/index.ts (1 hunks)
- packages/loot-core/src/types/prefs.d.ts (1 hunks)
- packages/loot-core/src/types/server-handlers.d.ts (0 hunks)
- upcoming-release-notes/3352.md (1 hunks)
- upcoming-release-notes/3427.md (1 hunks)
Files not reviewed due to no reviewable changes (1)
- packages/loot-core/src/types/server-handlers.d.ts
Additional comments not posted (25)
upcoming-release-notes/3427.md (1)
1-6: Release notes look good!The release notes follow the standard format and accurately describe the change being made in this PR. The category, author, and description are all relevant and clear.
packages/loot-core/migrations/1723665565000_prefs.js (1)
15-15: LGTM! The change aligns with the transition ofbudgetTypeto synced preferences.Uncommenting the entry for
'budgetType'in theSYNCED_PREF_KEYSarray is a logical step in the process of transitioning thebudgetTypepreference from metadata to synced preferences. This change ensures thatbudgetTypeis now actively managed as part of the synchronization process, allowing it to be stored and retrieved alongside other preference keys.The overall functionality is enhanced by treating
budgetTypeconsistently with the other keys in the synchronization logic, which may affect how user preferences are persisted and accessed across sessions. This change promotes a more unified and reliable approach to managing user preferences.packages/desktop-client/src/components/settings/BudgetTypeSettings.tsx (6)
1-1: LGTM!The
Reactimport is necessary and correctly added.
3-3: LGTM!The
useSyncedPrefhook is correctly imported and aligns with the changes mentioned in the summary.
4-4: LGTM!The
Buttoncomponent is correctly imported and used in the component.
11-11: LGTM!The
useSyncedPrefhook is correctly used to manage thebudgetTypestate, and the default value is set appropriately.
14-15: LGTM!The logic for updating the
budgetTypestate is correct, and thesetBudgetTypefunction is correctly used.
21-24: LGTM!The
Buttoncomponent is correctly used with theonSwitchTypefunction, and the dynamic button text accurately reflects the action of switching between budgeting types.packages/loot-core/src/server/prefs.ts (1)
Line range hint
47-55: Clarify the handling ofbudgetTypeafter the removal from sync logic.The removal of
budgetTypefrom the sync logic in thesavePrefsfunction suggests a change in how this preference is being managed. It would be helpful to understand the reasoning behind this modification and howbudgetTypeis being handled in other parts of the application after this change.Please ensure that the removal of
budgetTypefrom this section does not introduce any unintended consequences or inconsistencies in the application's behavior related to budget types.packages/loot-core/src/types/prefs.d.ts (1)
15-15: Verify the impact of movingbudgetTypetoSyncedPrefs.The change is consistent with the AI-generated summary and implies that
budgetTypewill now be synchronized across devices.Run the following script to verify the usage of
budgetTypeand ensure that all references have been updated to useSyncedPrefs:Additionally, consider the implications of synchronizing
budgetTypeacross devices on the user experience and data consistency. Ensure that the synchronization behavior aligns with the intended design.Verification successful
Verification successful:
budgetTypemigration toSyncedPrefsis complete and consistent.The codebase analysis confirms that:
- All references to
MetadataPrefs.budgetTypehave been removed.budgetTypeis now accessed using theuseSyncedPrefhook, indicating proper integration withSyncedPrefs.- The change has been consistently applied across various components and utilities.
No further action is required regarding this change.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `budgetType` have been updated to use `SyncedPrefs`. # Test: Search for references to `budgetType`. Expect: No references to `MetadataPrefs.budgetType`. rg --type typescript $'budgetType'Length of output: 68
Script:
#!/bin/bash # Description: Verify all references to `budgetType` have been updated to use `SyncedPrefs`. # Search for all occurrences of `budgetType` echo "Searching for all occurrences of budgetType:" rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' --type ts --type js 'budgetType' echo -e "\nSearching for any remaining references to MetadataPrefs.budgetType:" rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' --type ts --type js 'MetadataPrefs\.budgetType' echo -e "\nSearching for new references to SyncedPrefs.budgetType:" rg --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' --type ts --type js 'SyncedPrefs\.budgetType'Length of output: 7854
packages/desktop-client/src/components/settings/Experimental.tsx (1)
72-72: LGTM!The change from
useMetadataPreftouseSyncedPreffor retrieving thebudgetTypepreference is a good refactor. UsinguseSyncedPrefpotentially improves the synchronization and consistency of thebudgetTypepreference across different components or sessions in the application.The default value for
budgetTyperemains 'rollover', ensuring that existing functionality is preserved. The logic for determining the enabled state of the budget report feature and the condition for blocking the toggle also remains unchanged, indicating that the core functionality of the feature toggle is intact despite the underlying change in preference management.packages/desktop-client/src/components/budget/util.ts (3)
7-7: LGTM!The import statement for
SyncedPrefstype is correct and aligns with the transition fromMetadataPrefstoSyncedPrefsmentioned in the summary.
144-144: LGTM!The
budgetTypeparameter type change in theprewarmMonthfunction is correct and aligns with the transition fromMetadataPrefstoSyncedPrefs.
159-159: LGTM!The
budgetTypeparameter type change in theprewarmAllMonthsfunction is correct and aligns with the transition fromMetadataPrefstoSyncedPrefs.packages/loot-core/src/server/sheet.ts (1)
198-201: Approve the changes to retrievebudgetTypefrom the database.The new implementation ensures that the
budgetTypeis always fetched from the database, which is a more robust and centralized approach compared to the previous implementation that relied on theprefsmodule. This change allows for more dynamic updates to user preferences without requiring changes to the codebase.The default value of
'rollover'is used if nobudgetTypeis found in the database, which maintains backward compatibility and prevents unexpected behavior.packages/loot-core/src/server/budget/actions.ts (2)
30-30: LGTM!The change in the function logic to call
isReflectBudget()instead of usingprefs.getPrefs()is consistent with the alterations mentioned in the summary. The implementation looks good.
34-37: Looks good!The change in the function implementation to retrieve
budgetTypefrom the database instead of usingprefs.getPrefs()is a good improvement. It enhances the reliability of the budget type retrieval by ensuring it is sourced directly from the database.The SQL query and the logic to determine if the budget type is 'report' are implemented correctly.
packages/desktop-client/src/components/budget/index.tsx (2)
28-28: LGTM!The import change from
useMetadataPreftouseSyncedPrefaligns with the PR objective of moving the budget type to synced preferences. This is a good refactor to centralize preference management.
81-81: VerifyuseSyncedPrefusage forbudgetTypeacross the codebase.The change from
useMetadataPreftouseSyncedPreffor managing thebudgetTypepreference aligns with the PR objective and is a good refactor.However, it's important to ensure that all relevant components have been updated to use
useSyncedPrefforbudgetTypeto maintain consistency across the application.Run the following script to verify the usage of
useSyncedPrefforbudgetType:Verification successful
Migration to
useSyncedPreffor 'budgetType' successfully implementedThe verification process confirms that the migration from
useMetadataPreftouseSyncedPreffor the 'budgetType' preference has been successfully and consistently implemented across the codebase. Key observations:
- No occurrences of
useMetadataPref('budgetType')were found.useSyncedPref('budgetType')is now used in multiple components, including:
- Mobile and desktop budget components
- Category autocomplete
- Settings (Experimental and BudgetTypeSettings)
The change has been applied comprehensively, maintaining consistency across different parts of the application. The default value of 'rollover' is also consistently used, ensuring backward compatibility.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify `useSyncedPref` is used for `budgetType` across the codebase. # Test: Search for the usage of `useMetadataPref` with `budgetType`. Expect: No occurrences. rg --type typescript -A 5 $'useMetadataPref\(\'budgetType\'\)' # Test: Search for the usage of `useSyncedPref` with `budgetType`. Expect: Only occurrences of the new usage. rg --type typescript -A 5 $'useSyncedPref\(\'budgetType\'\)'Length of output: 203
Script:
#!/bin/bash # Description: Verify `useSyncedPref` is used for `budgetType` across the codebase. # Test: Search for the usage of `useMetadataPref` with `budgetType`. Expect: No occurrences. echo "Searching for useMetadataPref('budgetType'):" fd -e ts -e tsx | xargs rg -n -A 5 "useMetadataPref\('budgetType'\)" echo "" echo "Searching for useSyncedPref('budgetType'):" # Test: Search for the usage of `useSyncedPref` with `budgetType`. Expect: Only occurrences of the new usage. fd -e ts -e tsx | xargs rg -n -A 5 "useSyncedPref\('budgetType'\)"Length of output: 3681
packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx (1)
382-382: LGTM!The change from
useMetadataPreftouseSyncedPreffor retrieving thebudgetTypepreference aligns with the PR objective and should improve the consistency of preference synchronization. The default value and the subsequent logic remain unchanged, ensuring no functional impact.packages/desktop-client/src/components/mobile/budget/index.tsx (1)
45-45: LGTM!The change from
useMetadataPreftouseSyncedPreffor retrieving thebudgetTypepreference is a good move. It ensures that the preference is consistently synchronized across components or sessions, potentially improving the user experience.The surrounding logic remains intact, and the change is isolated to the preference retrieval without affecting the functionality of the
Budgetcomponent.packages/loot-core/src/server/sync/index.ts (1)
362-366: LGTM!The new conditional logic for handling the
budgetTypepreference synchronization is a great addition. It ensures that the application state is updated in real-time when thebudgetTypepreference is detected in the incoming messages.This change improves the responsiveness of the application and provides immediate feedback to the user when the
budgetTypepreference is updated. The code segment is logically correct and aligns with the objective of enhancing the synchronization of thebudgetTypepreference.Good job on streamlining the control flow by centralizing the handling of the
budgetTypepreference in one location.packages/loot-core/src/server/main.ts (1)
1980-1984: LGTM!The change to retrieve the budget type from the database instead of user preferences is a good refactor for centralization and consistency. The default value of
'rollover'maintains backward compatibility.packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx (2)
228-229: LGTM! Verify theuseSyncedPrefhook implementation.The change from
useMetadataPreftouseSyncedPreffor managing thebudgetTypepreference looks good. It suggests a shift towards a more centralized approach to preference synchronization.Please ensure that the
useSyncedPrefhook is implemented correctly and provides the expected functionality for consistent preference management across the application.
361-362: LGTM! Verify theuseSyncedPrefhook implementation.The change from
useMetadataPreftouseSyncedPreffor managing thebudgetTypepreference in theExpenseCategoryfunction looks good. It maintains consistency with the similar change made in theBudgetCellfunction.Please ensure that the
useSyncedPrefhook is implemented correctly and provides the expected functionality for consistent preference management across the application.
* marked files for translation * added release note * fixed linting warnings * 🐛 fix account filters being overridden (#3441) *Reduce package size (#3443) * reduce package size of all packages * release notes * Update beforePackHook.ts * [Maintenance] Cleanup react aria packages and dedupe (#3450) * Cleanup react aria packages and dedupe * Release notes * ♻️ (synced-prefs) moving the prefs from metadata.json to the db (#3423) *
Depends on #3423
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores