Consistent accounts terminology (For budget / Budgeted --> On-budget) #3903
Conversation
actual-github-bot
bot
changed the title
joel-jeremy
changed the title
✅ 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
View detailed bundle breakdownAdded No assets were added Removed No assets were removed 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 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
|
WalkthroughThe pull request implements a series of changes primarily focused on updating terminology and naming conventions related to account management across various components in the desktop client. Key modifications include renaming "Budgeted Accounts" to "On-Budget Accounts" in the test suite and associated components, ensuring consistency in the naming of account categories. Additionally, several components and tests have updated string literals to reflect this new terminology, changing phrases like "Available Funds" to "Available funds" and "Projected Savings" to "Projected savings." The changes also extend to method signatures, type definitions, and rendering logic, particularly in how account IDs are handled and displayed. Overall, the modifications aim to align the codebase with a new semantic structure for account categorization, while maintaining existing functionality and logic. Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/desktop-client/src/components/mobile/accounts/Accounts.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eslint-plugin". (The package "eslint-plugin-eslint-plugin" was not found when loaded as a Node module from the directory "/packages/eslint-plugin-actual".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eslint-plugin" was referenced from the config file in "packages/eslint-plugin-actual/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (142)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 4
🧹 Outside diff range and nitpick comments (9)
packages/desktop-client/src/components/budget/tracking/budgetsummary/Saved.tsx (1)
36-38: Consider consolidating i18n text keys for better maintainability.While the current implementation is functional, consider consolidating the "Projected savings:" string into a single i18n key to ensure future terminology updates can be made in one place.
- <Trans>Projected savings:</Trans> + <Trans i18nKey="labels.projectedSavings">Projected savings:</Trans> - left={t('Projected savings:')} + left={t('labels.projectedSavings')}Also applies to: 49-59
packages/desktop-client/src/components/sidebar/Account.tsx (1)
Terminology update is consistent, but one instance of old terminology remains
There's one remaining instance of old terminology "budgeted accounts" in
packages/desktop-client/src/components/reports/reports/CashFlow.tsx. All other occurrences use the new standardized "on-budget"/"off-budget" terminology consistently across the codebase.
packages/desktop-client/src/components/reports/reports/CashFlow.tsx: Update "budgeted accounts" to "on-budget accounts" in the cash flow description.🔗 Analysis chain
Line range hint
156-161: LGTM! Terminology update aligns with PR objectives.The change from 'For budget' to 'On-budget' correctly implements the standardized terminology.
Let's verify the terminology consistency across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of old terminology rg -i "for budget|budgeted accounts" --type tsx --type ts # Search for new terminology to ensure consistent usage rg -i "on-budget|off-budget" --type tsx --type tsLength of output: 167
Script:
#!/bin/bash # Search for any remaining instances of old terminology, using ripgrep without file type filters rg -i "for budget|budgeted accounts" # Search for new terminology to ensure consistent usage rg -i "on-budget|off-budget"Length of output: 3728
packages/loot-core/src/client/queries.ts (1)
162-168: Consider consistent casing in binding namesWhile the function names follow proper camelCase (
onBudgetAccountBalance), the binding names use kebab-case (onbudget-accounts-balance). Consider using consistent casing in the binding names:export function onBudgetAccountBalance() { return { - name: `onbudget-accounts-balance`, + name: `on-budget-accounts-balance`, query: q('transactions') .filter({ 'account.offbudget': false, 'account.closed': false }) .calculate({ $sum: '$amount' }), - } satisfies Binding<'account', 'onbudget-accounts-balance'>; + } satisfies Binding<'account', 'on-budget-accounts-balance'>; } export function offBudgetAccountBalance() { return { - name: `offbudget-accounts-balance`, + name: `off-budget-accounts-balance`, query: q('transactions') .filter({ 'account.offbudget': true, 'account.closed': false }) .calculate({ $sum: '$amount' }), - } satisfies Binding<'account', 'offbudget-accounts-balance'>; + } satisfies Binding<'account', 'off-budget-accounts-balance'>; }Also applies to: 171-177
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx (1)
Line range hint
207-207: Update variable name to match new terminologyThe variable
budgetedAccountsstill uses the old terminology. Consider renaming it toonBudgetAccountsto maintain consistency with the new terminology.- const budgetedAccounts = accounts.filter(account => account.offbudget === 0); + const onBudgetAccounts = accounts.filter(account => account.offbudget === 0);Remember to update all references to this variable in the component.
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (1)
223-223: Consider extracting the account ID type to reduce duplication.The accountId type is duplicated between
AccountTransactionsandTransactionListWithPreviews. Consider extracting it into a shared type definition.+type AccountIdType = AccountEntity['id'] | 'onbudget' | 'offbudget' | 'uncategorized'; export function AccountTransactions({ account, accountId, accountName, }: { readonly account?: AccountEntity; - readonly accountId?: AccountEntity['id'] | 'onbudget' | 'offbudget' | 'uncategorized'; + readonly accountId?: AccountIdType; readonly accountName: string; }) { function TransactionListWithPreviews({ account, accountId, accountName, }: { readonly account?: AccountEntity; - readonly accountId?: AccountEntity['id'] | 'onbudget' | 'offbudget' | 'uncategorized'; + readonly accountId?: AccountIdType; readonly accountName: AccountEntity['name'] | string; }) {packages/desktop-client/src/components/accounts/Account.tsx (1)
Line range hint
1-1: Consider coordination with component rewrite PR.Based on the learning note, this component is being rewritten in PR #3685. While the current changes are valid, please ensure coordination with the rewrite PR to avoid potential merge conflicts or duplicated effort.
packages/desktop-client/src/components/transactions/TransactionsTable.jsx (3)
1431-1431: Terminology update looks good but needs consistent hyphenationThe change from "Off Budget" to "Off-budget" aligns with the PR objective of standardizing terminology. However, to maintain full consistency, we should ensure all budget-related terms follow the same hyphenation pattern.
Consider applying consistent hyphenation to other budget-related terms throughout the codebase. For example:
- "on-budget" (with hyphen) when used as an adjective
- "off-budget" (with hyphen) when used as an adjective
Line range hint
1-24: Consider splitting this large component file into smaller modulesThe TransactionsTable component file is quite large (~2000 lines) and handles multiple responsibilities. This can make maintenance and testing more difficult.
Consider:
- Extracting the following into separate components:
- TransactionHeader
- TransactionError
- PayeeCell
- StatusCell
- Moving utility functions like
notesTagFormatter,getDisplayValueto a separate utils file- Separating the state management logic into custom hooks
1431-1434: Enhance accessibility for budget status textThe budget status text ("Off-budget", "Transfer") could benefit from improved accessibility.
Consider adding ARIA labels to provide more context:
- ? 'Off-budget' + ? <span aria-label="Off-budget account status">Off-budget</span> - : isBudgetTransfer - ? 'Transfer' + : isBudgetTransfer + ? <span aria-label="Transfer transaction">Transfer</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (23)
packages/desktop-client/e2e/accounts.test.js(1 hunks)packages/desktop-client/e2e/budget.test.js(1 hunks)packages/desktop-client/e2e/page-models/mobile-budget-page.js(2 hunks)packages/desktop-client/src/components/accounts/Account.tsx(4 hunks)packages/desktop-client/src/components/autocomplete/AccountAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/budget/envelope/budgetsummary/TotalsList.tsx(1 hunks)packages/desktop-client/src/components/budget/tracking/budgetsummary/Saved.tsx(2 hunks)packages/desktop-client/src/components/mobile/accounts/Account.tsx(1 hunks)packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx(3 hunks)packages/desktop-client/src/components/mobile/accounts/Accounts.tsx(4 hunks)packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx(1 hunks)packages/desktop-client/src/components/modals/CloseAccountModal.tsx(1 hunks)packages/desktop-client/src/components/reports/ReportOptions.ts(2 hunks)packages/desktop-client/src/components/reports/ReportSidebar.tsx(1 hunks)packages/desktop-client/src/components/sidebar/Account.tsx(1 hunks)packages/desktop-client/src/components/sidebar/Accounts.tsx(5 hunks)packages/desktop-client/src/components/spreadsheet/index.ts(1 hunks)packages/desktop-client/src/components/transactions/TransactionsTable.jsx(1 hunks)packages/desktop-client/src/hooks/useOnBudgetAccounts.ts(1 hunks)packages/loot-core/src/client/data-hooks/schedules.tsx(1 hunks)packages/loot-core/src/client/queries.ts(3 hunks)
✅ Files skipped from review due to trivial changes (8)
- packages/desktop-client/src/components/budget/envelope/budgetsummary/TotalsList.tsx
- packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx
- packages/desktop-client/src/components/modals/CloseAccountModal.tsx
- packages/desktop-client/src/components/reports/ReportOptions.ts
- packages/desktop-client/src/components/reports/ReportSidebar.tsx
- packages/desktop-client/src/hooks/useOnBudgetAccounts.ts
🧰 Additional context used
📓 Learnings (4)
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.
packages/desktop-client/src/components/mobile/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.
packages/desktop-client/src/components/mobile/accounts/Accounts.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.
packages/desktop-client/src/components/sidebar/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 (32)
packages/desktop-client/src/components/mobile/accounts/Account.tsx (2)
32-33: LGTM! Terminology changes align with PR objectives.
The renaming from 'budgeted' to 'onbudget' and the corresponding display text change to 'On-Budget Accounts' is consistent with the PR's goal of standardizing terminology.
32-35: Coordinate changes with the component rewrite PR.
Based on the learning note, this component is being rewritten in a separate PR. Please ensure these terminology changes are coordinated with that rewrite to avoid potential conflicts or rework.
Let's check for any potential conflicts:
packages/desktop-client/e2e/budget.test.js (1)
Line range hint 1-65: Consider adding test coverage for "on-budget" terminology.
Given that this PR's main objective is to standardize terminology from "For budget/Budget" to "On-budget", this test file might need additional test cases to verify the new terminology is correctly displayed in the UI.
Let's check if there are any existing tests for this functionality:
Consider adding test cases to verify:
- The account categorization displays "On-budget" consistently
- The navigation and filtering by account type works with the new terminology
- The account creation/editing flow uses the correct terminology
Would you like me to help draft these additional test cases?
packages/desktop-client/src/components/spreadsheet/index.ts (2)
13-13: LGTM! Terminology change aligns with project objectives.
The change from 'budgeted-accounts-balance' to 'onbudget-accounts-balance' maintains consistency with the existing 'offbudget-accounts-balance' property and aligns with the PR's goal of standardizing terminology.
13-13: Verify all usages of this property have been updated.
Since this is a type definition change, we should ensure all code referencing this property has been updated to use the new name.
✅ Verification successful
Based on the search results, I can generate the final response as we have sufficient information. The old property name "budgeted-accounts-balance" is not found anywhere in the codebase, while the new property name "onbudget-accounts-balance" is consistently used across all relevant files. The property is properly defined in the type system and referenced correctly in the queries and components.
All usages of the property have been updated correctly
The property 'onbudget-accounts-balance' is consistently used across the codebase:
- Type definition in
packages/desktop-client/src/components/spreadsheet/index.ts - Query binding in
packages/loot-core/src/client/queries.ts - Usage in mobile accounts component
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old property name
rg "budgeted-accounts-balance"
# Search for usages of the new property name to ensure the migration is complete
rg "onbudget-accounts-balance"
# Look for potential type usage in TypeScript files
ast-grep --pattern 'Spreadsheets["account"]'
Length of output: 538
packages/desktop-client/src/components/budget/tracking/budgetsummary/Saved.tsx (1)
36-36: LGTM! Capitalization changes align with UI text standardization.
The changes to "Projected savings:" maintain consistency with the new text formatting standards across the application.
Also applies to: 49-49
packages/loot-core/src/client/data-hooks/schedules.tsx (2)
164-164: LGTM! Terminology change aligns with standardization goals.
The update from 'budgeted' to 'onbudget' in the type definition is consistent with the PR's objective to standardize account terminology across the application.
164-164: Verify all callers have been updated to use 'onbudget'.
Since this is a breaking change in the type definition, we should ensure all callers of accountSchedulesQuery have been updated to use 'onbudget' instead of 'budgeted'.
✅ Verification successful
All callers have been verified and are using correct parameters
Based on the search results, there are no instances of accountSchedulesQuery being called with 'budgeted' parameter. All usages found in the codebase pass either an account ID or no parameter at all. The only occurrences of 'budgeted' in the client code are in queries.ts and are unrelated to this function. The type change appears to be safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining usage of 'budgeted' in function calls
# Expected: No occurrences of accountSchedulesQuery with 'budgeted' parameter
# Search for accountSchedulesQuery usage
rg "accountSchedulesQuery.*['\"]budgeted['\"]" --type ts --type tsx
# Also check for any remaining 'budgeted' terminology in nearby files
rg "\bbudgeted\b" --type ts --type tsx -g "packages/loot-core/src/client/**"
Length of output: 208
Script:
#!/bin/bash
# Search for accountSchedulesQuery usage with 'budgeted' parameter, adjusting file type flags
rg "accountSchedulesQuery.*['\"]budgeted['\"]" -t typescript -t javascriptreact -t typescriptreact
# Check for any remaining 'budgeted' terminology in client files
cd packages/loot-core/src/client
rg "\bbudgeted\b" -t typescript -t javascriptreact -t typescriptreact
# Also search for all usages of accountSchedulesQuery to verify parameters
rg "accountSchedulesQuery" -t typescript -t javascriptreact -t typescriptreact -A 2 -B 2
Length of output: 405
Script:
#!/bin/bash
# Search for accountSchedulesQuery usage with 'budgeted' parameter
rg "accountSchedulesQuery.*budgeted"
# Check for any remaining 'budgeted' terminology in client files
rg "\bbudgeted\b" --glob "packages/loot-core/src/client/**/*"
# Also search for all usages of accountSchedulesQuery to verify parameters
rg "accountSchedulesQuery" -A 2 -B 2
Length of output: 2609
packages/desktop-client/e2e/accounts.test.js (3)
57-57: LGTM: Test suite name updated correctly
The test suite name has been updated to use the new "On-Budget" terminology, maintaining proper title case.
67-67: LGTM: Account name expectation updated correctly
The account name expectation has been updated to use the new "On-Budget Accounts" terminology, maintaining consistent capitalization with the test suite name.
57-67: Verify complete terminology migration
Let's ensure all occurrences of the old terminology have been updated consistently across the codebase.
✅ Verification successful
Terminology is consistently used across the codebase
The codebase shows consistent usage of terminology:
- "On-budget" (with variations in capitalization) is used for account grouping
- "Budgeted" is used appropriately in budget-related functionality
- No instances of old "For budget" terminology were found
The variations in capitalization ("On-budget", "On-Budget", etc.) are acceptable as they appear in different UI contexts (labels, titles, etc.) where such variations are normal.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of old terminology that might need updating
echo "Checking for 'For budget' occurrences..."
rg -i "for budget" --type js --type jsx --type ts --type tsx
echo "Checking for 'Budgeted' occurrences..."
rg -i "budgeted" --type js --type jsx --type ts --type tsx
echo "Checking for current 'On-budget' occurrences to verify capitalization consistency..."
rg -i "on-budget" --type js --type jsx --type ts --type tsx
Length of output: 640
Script:
#!/bin/bash
# Search for any remaining instances of old terminology that might need updating
# Using simpler ripgrep commands without file type filters
echo "Checking for 'For budget' occurrences..."
rg -i "for budget"
echo "Checking for 'Budgeted' occurrences..."
rg -i "budgeted"
echo "Checking for current 'On-budget' occurrences to verify capitalization consistency..."
rg -i "on-budget"
Length of output: 40223
packages/desktop-client/src/components/sidebar/Accounts.tsx (5)
15-15: LGTM: Import statement aligns with terminology changes
The new import for useOnBudgetAccounts is properly structured and consistent with the PR's objective of standardizing terminology.
33-33: LGTM: Hook usage follows new terminology
The useOnBudgetAccounts hook is properly implemented, maintaining consistency with the new terminology standard.
116-116: LGTM: Account list rendering maintains functionality
The change to use onBudgetAccounts in the mapping is consistent with the new terminology while preserving the existing functionality.
135-137: Verify terminology consistency and consider query naming
The hyphenation in "Off-budget" correctly matches the "On-budget" style. The query name follows camelCase convention but differs slightly in structure:
onBudgetAccountBalance()offBudgetAccountBalance()
✅ Verification successful
Terminology and query naming are consistent across the codebase
The verification shows that:
- The "Off-budget" term is consistently hyphenated in both mobile and desktop components
- Query naming follows a consistent pattern:
onBudgetAccountBalance()offBudgetAccountBalance()
These are used consistently across components in desktop-client and loot-core packages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent hyphenation in labels
rg -g '*.{ts,tsx,js,jsx}' -A 1 -B 1 '"Off-budget"|"On-budget"'
# Check query naming consistency
rg -g '*.{ts,tsx,js,jsx}' "BudgetAccountBalance"
Length of output: 1336
103-107: Verify route and translation consistency
The changes to the on-budget account section look good, but let's verify the consistency of:
- The new route
/accounts/onbudget - The translation key for "On-budget"
✅ Verification successful
Route and translation usage are consistent
The verification shows that:
- The route
/accounts/onbudgetis consistently used in the sidebar component and follows the same pattern as other account routes (/accounts/offbudget,/accounts/uncategorized). - The translation key "On-budget" is used consistently across multiple components:
sidebar/Accounts.tsxsidebar/Account.tsxautocomplete/AccountAutocomplete.tsxe2e/accounts.test.js
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check route definition consistency
rg -g '*.{ts,tsx,js,jsx}' "'/accounts/onbudget'"
# Check translation key usage
rg -g '*.{json,ts,tsx,js,jsx}' '"On-budget"'
Length of output: 103
Script:
#!/bin/bash
# Let's try different approaches to find route and translation patterns
# Check route patterns in router configurations and components
rg -g '*.{ts,tsx,js,jsx}' "accounts/onbudget"
# Check translation patterns with single and double quotes
rg -g '*.{ts,tsx,js,jsx}' "'On-budget'"
rg -g '*.{ts,tsx,js,jsx}' "\"On-budget\""
# Look for route definitions in router setup files
fd "routes?" --type f --extension ts --extension tsx --extension js --extension jsx --exec rg -l "accounts"
# Search for translation files
fd "translations?" --type f --extension json --extension ts --extension tsx --exec rg -l "budget"
Length of output: 880
Script:
#!/bin/bash
# Let's check the router configuration to ensure the route is properly defined
rg -g '*.{ts,tsx,js,jsx}' -B 5 -A 5 "Route.*accounts"
# Check if there are any other route patterns similar to onbudget/offbudget
rg -g '*.{ts,tsx,js,jsx}' "/accounts/[a-zA-Z]+"
# Look for translation definitions in locale files
fd "en.json|en-US.json" --type f --exec rg -l "budget"
Length of output: 5653
packages/desktop-client/src/components/sidebar/Account.tsx (1)
Line range hint 1-224: Consider coordination with upcoming component rewrite.
Based on the retrieved learning, this component is being rewritten in a separate PR. While this terminology change is valid, ensure these changes are coordinated with the rewrite to prevent conflicts or duplicate work.
Let's check the status of the rewrite PR:
packages/desktop-client/src/components/autocomplete/AccountAutocomplete.tsx (1)
71-72: LGTM! The terminology changes align with the PR objectives.
The changes correctly implement the standardized terminology by:
- Using "Off-budget" instead of "Off Budget"
- Using "On-budget" instead of "For Budget"
This maintains consistency with the broader terminology changes across the application.
Let's verify the terminology consistency across the codebase:
✅ Verification successful
Terminology changes are consistent across the codebase
The search results confirm that:
- The new standardized terms "On-budget" and "Off-budget" are consistently used throughout the codebase
- Only two instances of old terminology remain, which are acceptable:
- In
transactions.ts: A code comment using "off budget" in a natural language context - In
CashFlow.tsx: Using "budgeted accounts" in a help text, which is a different context
- In
The changes in AccountAutocomplete.tsx align perfectly with the standardized terminology used across the application's UI components, tests, and business logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining inconsistent account terminology
# Search for variations of budget-related terms
echo "Checking for inconsistent terminology..."
rg -i "for budget|budgeted account|off budget" --type ts --type tsx
# Search for the new standardized terms to ensure consistency
echo "Verifying new terminology usage..."
rg -i "on-budget|off-budget" --type ts --type tsx
Length of output: 347
Script:
#!/bin/bash
# Description: Check for any remaining inconsistent terminology with correct file type flags
# Search for variations of budget-related terms
echo "Checking for inconsistent terminology..."
rg -i "for budget|budgeted account|off budget" -t typescript -t tsx
# Search for the new standardized terms to ensure consistency
echo "Verifying new terminology usage..."
rg -i "on-budget|off-budget" -t typescript -t tsx
Length of output: 361
Script:
#!/bin/bash
# Description: Check for any remaining inconsistent terminology using file extensions
# Search for variations of budget-related terms
echo "Checking for inconsistent terminology..."
rg -i "for budget|budgeted account|off budget" --type-add 'ts:*.{ts,tsx}'
# Search for the new standardized terms to ensure consistency
echo "Verifying new terminology usage..."
rg -i "on-budget|off-budget" --type-add 'ts:*.{ts,tsx}'
Length of output: 4087
packages/desktop-client/e2e/page-models/mobile-budget-page.js (2)
297-297: LGTM: Consistent error message update
The error message has been correctly updated to match the button name change, maintaining consistency in the codebase.
37-37: Verify if capitalization changes are within PR scope
While this change standardizes the capitalization from "Projected Savings" to "Projected savings", it appears unrelated to the main PR objective of standardizing "On-budget" terminology. Please clarify if standardizing text capitalization is an intended additional scope of this PR.
✅ Verification successful
Based on the search results, I can see that "Projected savings" appears consistently in lowercase across the codebase:
packages/desktop-client/src/components/budget/tracking/budgetsummary/Saved.tsx: <Trans>Projected savings:</Trans>
packages/desktop-client/src/components/budget/tracking/budgetsummary/Saved.tsx: left={t('Projected savings:')}
packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx: title="Projected savings"
The change from "Projected Savings" to "Projected savings" in the test file aligns with the actual implementation in the codebase, where it's consistently using sentence case. While this change wasn't explicitly mentioned in the PR objectives about "On-budget" terminology standardization, it appears to be a valid consistency fix to match the actual UI implementation.
Capitalization change is consistent with implementation
The change from "Projected Savings" to "Projected savings" in the test file correctly aligns with the actual UI implementation, where this text consistently appears in sentence case across the codebase. This change improves test accuracy by matching the real UI behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other capitalization changes in the codebase
# to determine if this is part of a broader standardization effort
# Look for other instances of "Projected savings" or "Projected Savings"
rg -i "projected.?savings"
# Look for similar capitalization patterns
rg "([A-Z][a-z]+\s[A-Z][a-z]+)"
Length of output: 96932
packages/loot-core/src/client/queries.ts (3)
32-32: LGTM: Type definition updated for consistent terminology
The type definition change from 'budgeted' to 'onbudget' aligns with the PR's objective of standardizing terminology.
71-71: LGTM: Transaction query parameter updated consistently
The type definition in the transactions function has been correctly updated to match the new terminology.
Line range hint 36-68: Verify consistent usage of new terminology across the codebase
The account filter logic has been correctly updated to use 'onbudget'. Let's verify that all related components and tests have been updated consistently.
✅ Verification successful
Terminology update is consistent across the codebase
The codebase shows consistent usage of the new terminology:
- Account filtering logic correctly uses 'onbudget' instead of 'budgeted'
- Database schema and queries consistently use 'offbudget' field with boolean values
- All account type checks and filters across components use the updated terminology
- No instances of outdated 'budgeted' terminology found in account type logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining instances of 'budgeted' in account-related code
# to ensure complete terminology migration
echo "Checking for remaining 'budgeted' terminology in account-related files..."
rg -i "budgeted.?accounts?" --type ts --type tsx
echo "Checking for new 'onbudget' terminology usage..."
rg -i "onbudget.?accounts?" --type ts --type tsx
Length of output: 433
Script:
#!/bin/bash
# Let's try with just the file extension without type specification
echo "Checking for remaining 'budgeted' terminology..."
rg -i "budgeted"
echo -e "\nChecking for 'onbudget' terminology..."
rg -i "onbudget"
Length of output: 59220
Script:
#!/bin/bash
# Let's check for any remaining instances of 'budgeted' in account filter logic specifically
echo "Checking account filter logic for terminology consistency..."
rg -C 3 "accountId.*=.*('budgeted'|'onbudget')"
echo -e "\nChecking account type checks..."
rg -C 3 "account\.(budgeted|onBudget|offbudget)"
Length of output: 16352
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx (2)
303-304: LGTM! Method names are consistent
The method names follow consistent camelCase formatting and align well with the PR's objective of standardizing terminology.
Line range hint 1-309: Verify complete terminology migration
Let's ensure all instances of the old terminology have been updated across the codebase.
✅ Verification successful
Terminology is consistently used across the codebase
The codebase consistently uses "off-budget" (with hyphen) in user-facing strings and "offbudget" (without hyphen) in code identifiers. The file under review follows this convention correctly:
- User-facing strings use "Off-budget" with hyphen (e.g.,
name="Off-budget") - Code identifiers use "offbudget" without hyphen (e.g.,
account.offbudget === 1) - The terms "budgeted" and "on budget" are used appropriately in their respective contexts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for remaining instances of old terminology
echo "Searching for 'budgeted' terminology..."
rg -i "budgeted.*accounts?" --type ts --type tsx
echo "Searching for 'for budget' terminology..."
rg -i "for.*budget" --type ts --type tsx
echo "Searching for inconsistent 'off-budget' formatting..."
rg -i "off.?budget" --type ts --type tsx
Length of output: 537
Script:
#!/bin/bash
# Description: Search for remaining instances of old terminology with correct file type flags
echo "Searching for 'budgeted' terminology..."
rg -i "budgeted.*accounts?"
echo "Searching for 'for budget' terminology..."
rg -i "for.*budget"
echo "Searching for inconsistent 'off-budget' formatting..."
rg -i "off.?budget"
Length of output: 36936
packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx (2)
345-351: LGTM! Verify the existence of new query functions.
The query function names have been updated to match the new terminology while maintaining the existing logic.
✅ Verification successful
Functions are properly defined and used across the codebase
The query functions onBudgetAccountBalance and offBudgetAccountBalance are correctly defined in packages/loot-core/src/client/queries.ts and are being used consistently across multiple components including:
packages/desktop-client/src/components/sidebar/Accounts.tsxpackages/desktop-client/src/components/mobile/accounts/Accounts.tsxpackages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of new query functions in the queries module
# Search for the new query function definitions
echo "Checking for new query function definitions..."
rg -A 2 "export (const|function) (on|off)BudgetAccountBalance" --type=ts --type=tsx
Length of output: 216
Script:
#!/bin/bash
# Let's try a different approach to search for these functions without file type restrictions
echo "Searching for onBudgetAccountBalance and offBudgetAccountBalance functions..."
rg "onBudgetAccountBalance|offBudgetAccountBalance" -A 2
# Also search in the queries module specifically
echo -e "\nSearching in files containing 'queries'..."
fd "queries" --type f --exec rg "onBudgetAccountBalance|offBudgetAccountBalance" {} -A 2
Length of output: 2740
56-60: LGTM! Verify consistent terminology usage across the codebase.
The type definition update aligns with the PR objective of standardizing to "on-budget" terminology.
✅ Verification successful
Terminology change from 'budgeted' to 'onbudget' is consistent with codebase
The search results confirm that the type definition change from 'budgeted' to 'onbudget' in AccountTransactions.tsx aligns with the broader codebase terminology:
- The term 'onbudget' is consistently used in account-related contexts (e.g.,
useOnBudgetAccounts,onBudgetAccountBalance, route/accounts/onbudget) - The term 'budgeted' is appropriately retained when referring to budget amounts/actions (e.g.,
totalBudgeted,budgetedValue)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent terminology usage across TypeScript/JavaScript files
# Look for any remaining instances of old terminology
# Search for old terminology
echo "Checking for remaining instances of old terminology..."
rg -i "budgeted.*accounts?" --type=ts --type=tsx --type=js --type=jsx
# Search for mix of new terminology to ensure consistency
echo "Checking new terminology usage..."
rg -i "on.?budget" --type=ts --type=tsx --type=js --type=jsx
Length of output: 386
Script:
#!/bin/bash
# Let's try with correct ripgrep file type specifications
# Search for old terminology
echo "Checking for remaining instances of old terminology..."
rg -i "budgeted.*accounts?" -t typescript -t javascriptreact -t javascript
# Search for mix of new terminology
echo "Checking new terminology usage..."
rg -i "on.?budget" -t typescript -t javascriptreact -t javascript
# Also search for type definitions with these terms
echo "Checking type definitions..."
ast-grep --pattern 'type $_ = $$$| "budgeted" |$$$'
ast-grep --pattern 'type $_ = $$$| "onbudget" |$$$'
Length of output: 585
Script:
#!/bin/bash
# Try simpler ripgrep commands without file type specifications
echo "Checking for remaining instances of old terminology..."
rg -i "budgeted"
echo "Checking new terminology usage..."
rg -i "on.?budget"
# Also check specific file extensions
echo "Checking files with .ts and .tsx extensions..."
fd -e ts -e tsx -x rg -i "budgeted|on.?budget" {}
Length of output: 83723
packages/desktop-client/src/components/accounts/Account.tsx (4)
844-847: LGTM! Consistent terminology change.
The changes in getAccountTitle method correctly implement the terminology standardization from "Budgeted Accounts" to "On-Budget Accounts".
1665-1667: LGTM! Consistent terminology in editable check.
The isNameEditable condition correctly uses the new 'onbudget' terminology.
1769-1771: LGTM! Consistent terminology in visibility check.
The showAccount condition correctly uses the new 'onbudget' terminology.
253-253: Verify type change impact across the codebase.
The change from 'budgeted' to 'onbudget' in AccountInternalProps type is consistent with the terminology standardization objective.
✅ Verification successful
Let me gather more specific information about the usage of this type and the actual string literals being used.
Type change from 'budgeted' to 'onbudget' is consistent across the codebase
The verification shows that:
- All instances of
accountIdtype definitions and usages consistently use 'onbudget' - No remaining instances of 'budgeted' string literal in relation to account types
- The type is used in multiple files (Account.tsx, queries.ts, schedules.tsx) and all are aligned with the new terminology
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining usages of 'budgeted' string literal in type definitions or prop assignments
# Search for 'budgeted' string literal in TypeScript files
rg -t typescript "['\"](budgeted)['\"]" --type-add "typescript:*.{ts,tsx}"
# Search for AccountInternalProps usage
ast-grep --pattern 'type $_ = AccountInternalProps'
Length of output: 3466
Script:
#!/bin/bash
# Search for usage of accountId with 'budgeted' or 'onbudget' string literals
rg -t typescript "accountId.*['\"](?:budgeted|onbudget)['\"]" --type-add "typescript:*.{ts,tsx}"
# Search for imports of AccountInternalProps
rg -t typescript "import.*AccountInternalProps" --type-add "typescript:*.{ts,tsx}"
# Search for actual usage of the Account component
ast-grep --pattern 'Account $$$'
Length of output: 1018
packages/desktop-client/src/components/transactions/TransactionsTable.jsx (1)
Line range hint 1-1434: Verify all budget-related terminology has been updated
Let's ensure all instances of budget-related terms have been updated consistently across the codebase.
✅ Verification successful
Budget-related terminology has been consistently updated
The codebase shows consistent usage of hyphenated terms:
- "off-budget" is used correctly throughout the UI components
- Budget transfer logic correctly handles "off-budget" vs "on-budget" accounts
- Related terms like "overbudgeted" and "to-be-budgeted" are also consistently hyphenated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of old terminology
echo "Checking for 'Off Budget' (without hyphen)..."
rg -i "off budget" --type js --type jsx --type ts --type tsx
echo "Checking for 'For Budget'..."
rg -i "for budget" --type js --type jsx --type ts --type tsx
echo "Checking for 'Budgeted'..."
rg -i "budgeted" --type js --type jsx --type ts --type tsx
Length of output: 517
Script:
#!/bin/bash
# Search for any remaining instances of old terminology using ripgrep without file type restrictions
echo "Checking for 'Off Budget' (without hyphen)..."
rg -i "off budget"
echo "Checking for 'For Budget'..."
rg -i "for budget"
echo "Checking for 'Budgeted'..."
rg -i "budgeted"
# Also check for hyphenated versions
echo "Checking for 'Off-Budget'..."
rg -i "off-budget"
echo "Checking for variations of budget terms..."
rg -i "budget" -A 2 -B 2 packages/desktop-client/src/components/transactions/TransactionsTable.jsx
Length of output: 42877
packages/desktop-client/src/components/mobile/accounts/Accounts.tsx
Outdated
Show resolved
Hide resolved
|
/update-vrt |
|
I can't nail it down to a grammatical rule, but I'm not sure about the hyphenated version. It looks cleaner to me as: "On budget" What do you think? Happy to be overridden here but would like to get some opinions before making the change. |
|
Sure we can do the unhyphenated version. As long as it's consistent across the app and docs :) |
Accounts that are on budget are either referred to as
For budget/Budgetedin the UI and the codebase. This PR aims to just refer to these accounts ason-budgetfor consistency.Changes are also reflected in docs actualbudget/docs#504