feat(currency): add currency display to rules #5639
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
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
|
misu-dev
changed the title
WalkthroughThis PR centralizes currency formatting via a useFormat hook and removes older integer/currency conversion utilities across multiple packages. It adds an onKeyDown handler to the Input component. Filter components stop converting amount values to currency strings and now wire Enter handling and numberFormatType through GenericInput. AmountInput is refactored to use format.forEdit/fromEdit, changes its onEnter signature to receive a computed amount, and updates focus/blur display logic. GenericInput gains an onEnter prop and forwards raw numeric values. loot-core rules no longer apply numeric conversions during parse/unparse and defaults null numeric values to 0. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/loot-core/src/shared/rules.ts (1)
312-326: Convert fixed-amount values to integer cents in unparse
Server appliesthis.valuedirectly toobject.amount(packages/loot-core/src/server/rules/index.ts:741–743). Inunparse(packages/loot-core/src/shared/rules.ts:312–326), change thefixed-amountbranch to parse and scale the user’s dollar input into integer cents (e.g.value: Math.round(parseFloat(item.value) * 100)).
🧹 Nitpick comments (6)
packages/component-library/src/Input.tsx (1)
70-72: Remove redundant onKeyDown wrapper (no-op).This overrides the consumer’s onKeyDown only to immediately call it, adding an extra closure per render without behavior change.
Apply this diff:
- {...props} - onKeyDown={e => { - props.onKeyDown?.(e); - }} + {...props}packages/desktop-client/src/components/rules/RuleEditor.tsx (1)
824-832: When switching to “isbetween”, coerce to number to avoid leaking strings.cond.value can be stale or templated; ensure numeric fallback.
Apply this diff:
- return makeValue( - { - num1: cond.value, - num2: cond.value, - }, - { ...cond, op: value }, - ); + { + const base = + typeof cond.value === 'number' && Number.isFinite(cond.value) + ? cond.value + : 0; + return makeValue({ num1: base, num2: base }, { ...cond, op: value }); + }packages/loot-core/src/shared/rules.ts (1)
353-355: Defaulting null numeric values to 0 may create unintended rules.A newly switched-to-number field will silently become 0 instead of surfacing a validation error. Consider leaving null and letting validation raise 'no-null'/'not-number'.
Apply this diff:
- if (cond.type === 'number' && value == null) { - return { ...cond, error: null, value: 0 }; - } + if (cond.type === 'number' && value == null) { + return { ...cond, error: 'no-null', value: null }; + }If keeping 0 is intentional, please document the UX rationale in a comment.
packages/desktop-client/src/components/filters/FiltersMenu.jsx (1)
253-253: Scope currency formatting to amount fields for clarity.Passing
numberFormatType="currency"for all non-boolean/non-payee fields is harmless (ignored unlesstype === 'number'), but scoping tofield === 'amount'improves readability.- numberFormatType="currency" + numberFormatType={field === 'amount' ? 'currency' : undefined}packages/desktop-client/src/components/util/GenericInput.jsx (1)
64-73: ForwardonEnterfor percentage inputs as well (consistency).If
PercentInputsupports an Enter interaction, consider forwardingonEntersimilarly to currency for consistent UX.case 'percentage': return ( <PercentInput inputRef={ref} value={value} onUpdatePercent={onChange} + // If/when PercentInput supports it: + // onEnter={(e, updated) => onEnter?.(e, updated)} /> );packages/desktop-client/src/components/util/AmountInput.tsx (1)
106-120: Optional: Preserve typed sign during auto-decimal normalization.When
autoDecimalsis on, user-typed “-123” loses the sign. Consider syncingsymbolfrom the raw input’s leading sign.- function onInputTextChange(val) { + function onInputTextChange(val) { let newText = val; if (autoDecimals) { + if (val.startsWith('-')) setSymbol('-'); + else if (val.startsWith('+')) setSymbol('+'); const digits = val.replace(/\D/g, ''); if (digits === '') { newText = ''; } else { const intValue = parseInt(digits, 10); newText = format.forEdit(intValue); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
upcoming-release-notes/5639.mdis excluded by!**/*.md
📒 Files selected for processing (8)
packages/component-library/src/Input.tsx(1 hunks)packages/desktop-client/src/components/filters/FilterExpression.tsx(1 hunks)packages/desktop-client/src/components/filters/FiltersMenu.jsx(2 hunks)packages/desktop-client/src/components/rules/RuleEditor.tsx(4 hunks)packages/desktop-client/src/components/rules/Value.tsx(3 hunks)packages/desktop-client/src/components/util/AmountInput.tsx(6 hunks)packages/desktop-client/src/components/util/GenericInput.jsx(2 hunks)packages/loot-core/src/shared/rules.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Write concise, technical TypeScript code
Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoaded, hasError)
Structure files with sections in order: exported page/component, GraphQL queries, helpers, static content, types
Favor named exports for components and utilities
Prefer interfaces over types in TypeScript
Avoid enums; use objects or maps instead
Avoid using any or unknown unless absolutely necessary; prefer existing types
Avoid type assertions with as or non-null !; prefer satisfies
Use the function keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Files:
packages/desktop-client/src/components/filters/FilterExpression.tsxpackages/desktop-client/src/components/rules/Value.tsxpackages/loot-core/src/shared/rules.tspackages/component-library/src/Input.tsxpackages/desktop-client/src/components/util/AmountInput.tsxpackages/desktop-client/src/components/rules/RuleEditor.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Use declarative JSX, keeping JSX minimal and readable
Files:
packages/desktop-client/src/components/filters/FilterExpression.tsxpackages/desktop-client/src/components/rules/Value.tsxpackages/component-library/src/Input.tsxpackages/desktop-client/src/components/util/AmountInput.tsxpackages/desktop-client/src/components/rules/RuleEditor.tsx
**/*.{js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Use TypeScript for all code
Files:
packages/desktop-client/src/components/util/GenericInput.jsxpackages/desktop-client/src/components/filters/FiltersMenu.jsx
🧠 Learnings (15)
📓 Common learnings
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:41:34.452Z
Learning: The format function in useFormat.ts is widely used across the application and currently needs to accept both string and number inputs for financial format types. While strict type checking is preferred long-term, string parsing is maintained as a pragmatic compromise during the currency feature development phase.
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:60-66
Timestamp: 2025-06-16T17:15:16.326Z
Learning: The user prefers that financial format types in useFormat.ts should not accept string inputs, maintaining strict type checking. The expectation is that only numbers should be passed to financial format cases, and type checks may be unnecessary if this contract is enforced.
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.691Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:222-229
Timestamp: 2025-06-14T20:50:58.830Z
Learning: The `currencyToAmount` function in `loot-core/shared/util.ts` converts currency strings (like "3.14") directly to numeric amounts (3.14) without taking decimal places as a parameter. It only parses the string format and does not handle decimal place scaling - that happens downstream in functions like `amountToInteger` and `fromAmount`.
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:45:40.807Z
Learning: The user misu-dev prefers strict type checking for financial format types in useFormat.ts as a long-term goal, but acknowledges that creating follow-up issues for cleanup should wait until after the current PR is merged, not during the development phase.
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:161-166
Timestamp: 2025-06-14T21:20:46.938Z
Learning: In the useFormat hook's formatDisplay function, there's an intentional separation between displayDecimalPlaces (used for Intl.NumberFormat display formatting) and activeCurrency.decimalPlaces (used for integerToCurrency conversion logic). These should not be mixed as displayDecimalPlaces controls how many decimals to show while activeCurrency.decimalPlaces controls the mathematical conversion from integer amounts to decimal amounts.
📚 Learning: 2025-06-14T20:50:58.830Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:222-229
Timestamp: 2025-06-14T20:50:58.830Z
Learning: The `currencyToAmount` function in `loot-core/shared/util.ts` converts currency strings (like "3.14") directly to numeric amounts (3.14) without taking decimal places as a parameter. It only parses the string format and does not handle decimal place scaling - that happens downstream in functions like `amountToInteger` and `fromAmount`.
Applied to files:
packages/desktop-client/src/components/filters/FilterExpression.tsxpackages/desktop-client/src/components/rules/Value.tsxpackages/loot-core/src/shared/rules.tspackages/desktop-client/src/components/util/AmountInput.tsxpackages/desktop-client/src/components/rules/RuleEditor.tsx
📚 Learning: 2024-10-24T18:57:16.406Z
Learnt from: lelemm
PR: actualbudget/actual#3732
File: packages/desktop-client/src/components/util/AmountInput.tsx:102-108
Timestamp: 2024-10-24T18:57:16.406Z
Learning: In `packages/desktop-client/src/components/util/AmountInput.tsx`, zero amounts are handled in the `onSwitch` function, so zero amounts are intentionally ignored in the `fireUpdate` function.
Applied to files:
packages/desktop-client/src/components/filters/FilterExpression.tsxpackages/desktop-client/src/components/util/GenericInput.jsxpackages/desktop-client/src/components/util/AmountInput.tsx
📚 Learning: 2025-06-16T17:41:34.452Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:41:34.452Z
Learning: The format function in useFormat.ts is widely used across the application and currently needs to accept both string and number inputs for financial format types. While strict type checking is preferred long-term, string parsing is maintained as a pragmatic compromise during the currency feature development phase.
Applied to files:
packages/desktop-client/src/components/rules/Value.tsxpackages/loot-core/src/shared/rules.tspackages/desktop-client/src/components/util/AmountInput.tsxpackages/desktop-client/src/components/rules/RuleEditor.tsx
📚 Learning: 2025-06-14T21:20:46.938Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:161-166
Timestamp: 2025-06-14T21:20:46.938Z
Learning: In the useFormat hook's formatDisplay function, there's an intentional separation between displayDecimalPlaces (used for Intl.NumberFormat display formatting) and activeCurrency.decimalPlaces (used for integerToCurrency conversion logic). These should not be mixed as displayDecimalPlaces controls how many decimals to show while activeCurrency.decimalPlaces controls the mathematical conversion from integer amounts to decimal amounts.
Applied to files:
packages/desktop-client/src/components/rules/Value.tsxpackages/desktop-client/src/components/util/AmountInput.tsx
📚 Learning: 2025-08-16T07:09:15.691Z
Learnt from: sjones512
PR: actualbudget/actual#5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:375-387
Timestamp: 2025-08-16T07:09:15.691Z
Learning: When handling financial data consistency issues in the desktop client, prefer keeping the data layer (spreadsheets) in raw integer cents and use the useFormat hook for all presentation formatting, rather than converting to display amounts in the data layer.
Applied to files:
packages/desktop-client/src/components/rules/Value.tsx
📚 Learning: 2025-06-16T17:15:16.326Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:60-66
Timestamp: 2025-06-16T17:15:16.326Z
Learning: The user prefers that financial format types in useFormat.ts should not accept string inputs, maintaining strict type checking. The expectation is that only numbers should be passed to financial format cases, and type checks may be unnecessary if this contract is enforced.
Applied to files:
packages/desktop-client/src/components/rules/Value.tsxpackages/desktop-client/src/components/util/AmountInput.tsx
📚 Learning: 2025-06-16T17:45:40.807Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:45:40.807Z
Learning: The user misu-dev prefers strict type checking for financial format types in useFormat.ts as a long-term goal, but acknowledges that creating follow-up issues for cleanup should wait until after the current PR is merged, not during the development phase.
Applied to files:
packages/desktop-client/src/components/rules/Value.tsx
📚 Learning: 2025-06-14T20:48:50.619Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:51-58
Timestamp: 2025-06-14T20:48:50.619Z
Learning: In the useFormat.ts format function, strict type checking is preferred over implicit type coercion. The 'number' format type should only accept actual number values and throw an error for non-numbers to maintain clear separation between format types and prevent ambiguous usage.
Applied to files:
packages/desktop-client/src/components/rules/Value.tsx
📚 Learning: 2024-10-02T08:45:11.136Z
Learnt from: UnderKoen
PR: actualbudget/actual#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.
Applied to files:
packages/loot-core/src/shared/rules.ts
📚 Learning: 2025-01-16T14:30:36.518Z
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`.
Applied to files:
packages/loot-core/src/shared/rules.ts
📚 Learning: 2025-06-14T21:25:40.410Z
Learnt from: misu-dev
PR: actualbudget/actual#5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:68-88
Timestamp: 2025-06-14T21:25:40.410Z
Learning: The integerToCurrency function in packages/loot-core/src/shared/util.ts already includes built-in protection against non-integer inputs through the safeNumber function, which validates that the input is an integer using Number.isInteger() and throws an error if it's not. This makes additional integer validation checks redundant when calling integerToCurrency.
Applied to files:
packages/loot-core/src/shared/rules.tspackages/desktop-client/src/components/rules/RuleEditor.tsx
📚 Learning: 2024-11-26T13:07:02.794Z
Learnt from: lelemm
PR: actualbudget/actual#3891
File: packages/loot-core/src/shared/rules.ts:209-212
Timestamp: 2024-11-26T13:07:02.794Z
Learning: The file `packages/loot-core/src/shared/rules.ts` is not yet translated, so internationalization using the `t()` function is not required here.
Applied to files:
packages/loot-core/src/shared/rules.ts
📚 Learning: 2024-10-12T19:13:25.005Z
Learnt from: jfdoming
PR: actualbudget/actual#3641
File: packages/loot-core/src/server/accounts/rules.test.ts:524-536
Timestamp: 2024-10-12T19:13:25.005Z
Learning: In `packages/loot-core/src/server/accounts/rules.test.ts`, prefer explicit action definitions over refactoring similar actions into loops or helper functions, even when actions are similar.
Applied to files:
packages/loot-core/src/shared/rules.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/loot-core/src/shared/rules.ts
🧬 Code graph analysis (3)
packages/desktop-client/src/components/rules/Value.tsx (1)
packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)
packages/desktop-client/src/components/filters/FiltersMenu.jsx (1)
packages/desktop-client/src/components/util/GenericInput.jsx (1)
dispatch(41-41)
packages/desktop-client/src/components/rules/RuleEditor.tsx (3)
packages/desktop-client/src/hooks/useDateFormat.ts (1)
useDateFormat(3-6)packages/desktop-client/src/hooks/useFormat.ts (1)
useFormat(109-282)packages/loot-core/src/shared/rules.ts (1)
makeValue(346-358)
🔇 Additional comments (8)
packages/desktop-client/src/components/filters/FilterExpression.tsx (1)
139-145: Confirm FilterEditor expects raw numeric value for amounts.Passing raw value is consistent with the new formatter flow. Please verify FilterEditor/GenericInput parse currency correctly and no legacy string-formatted values sneak in from persisted rules.
Run:
packages/desktop-client/src/components/rules/Value.tsx (1)
16-16: Good shift to centralized financial formatting.Using useFormat for 'amount' keeps display consistent with prefs and symbol settings.
Also applies to: 41-41, 77-77
packages/desktop-client/src/components/rules/RuleEditor.tsx (1)
372-376: LGTM: useFormat injection for ScheduleDescription.This aligns schedule amount rendering with global currency settings.
packages/loot-core/src/shared/rules.ts (1)
286-297: parse(): numeric case safe — upstream supplies numbers
All numeric conditions in RuleEditor now useAmountInput/BetweenAmountInput(both emitnumberviaonUpdate), andGenericInputis no longer used for number types. No string-to-number parsing remains required inparse().packages/desktop-client/src/components/filters/FiltersMenu.jsx (1)
261-268: LGTM: Enter-to-apply with computed value.Using
updatedValue !== undefined ? updatedValue : valuecorrectly preserves 0-values and negative amounts.packages/desktop-client/src/components/util/GenericInput.jsx (1)
38-40: LGTM:onEnterprop addition is non-breaking.Optional callback with
(event, updatedValue)signature is clear and safe.packages/desktop-client/src/components/util/AmountInput.tsx (2)
67-75: LGTM: Centralized display/edit formatting via useFormat.
getDisplayValuecleanly separates edit vs. financial display and memoizes on the formatter.
199-203: NoonEnterconsumers detected—please manually verify call sites
Auto-search inpackages/**returned no<AmountInput onEnter=…>usages; ensure no external handlers rely on the old(event)signature before merging.
| function formatAmount(amount, format) { | ||
| if (!amount) { | ||
| return integerToCurrency(0); | ||
| return format(0, 'financial'); | ||
| } else if (typeof amount === 'number') { | ||
| return integerToCurrency(amount); | ||
| return format(amount, 'financial'); | ||
| } else { | ||
| return `${integerToCurrency(amount.num1)} to ${integerToCurrency( | ||
| return `${format(amount.num1, 'financial')} to ${format( | ||
| amount.num2, | ||
| 'financial', | ||
| )}`; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden formatAmount against nullish and partial objects.
Use explicit nullish checks; guard object fields to avoid formatting undefined.
Apply this diff:
-function formatAmount(amount, format) {
- if (!amount) {
- return format(0, 'financial');
- } else if (typeof amount === 'number') {
- return format(amount, 'financial');
- } else {
- return `${format(amount.num1, 'financial')} to ${format(
- amount.num2,
- 'financial',
- )}`;
- }
-}
+function formatAmount(amount: number | { num1: number; num2: number } | null | undefined, fmt: ReturnType<typeof useFormat>) {
+ if (amount == null) {
+ return fmt(0, 'financial');
+ }
+ if (typeof amount === 'number' && Number.isFinite(amount)) {
+ return fmt(amount, 'financial');
+ }
+ const { num1, num2 } = amount as { num1?: number; num2?: number };
+ const a = Number.isFinite(num1) ? (num1 as number) : 0;
+ const b = Number.isFinite(num2) ? (num2 as number) : 0;
+ return `${fmt(a, 'financial')} to ${fmt(b, 'financial')}`;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function formatAmount(amount, format) { | |
| if (!amount) { | |
| return integerToCurrency(0); | |
| return format(0, 'financial'); | |
| } else if (typeof amount === 'number') { | |
| return integerToCurrency(amount); | |
| return format(amount, 'financial'); | |
| } else { | |
| return `${integerToCurrency(amount.num1)} to ${integerToCurrency( | |
| return `${format(amount.num1, 'financial')} to ${format( | |
| amount.num2, | |
| 'financial', | |
| )}`; | |
| } | |
| } | |
| function formatAmount( | |
| amount: number | { num1: number; num2: number } | null | undefined, | |
| fmt: ReturnType<typeof useFormat>, | |
| ) { | |
| if (amount == null) { | |
| return fmt(0, 'financial'); | |
| } | |
| if (typeof amount === 'number' && Number.isFinite(amount)) { | |
| return fmt(amount, 'financial'); | |
| } | |
| const { num1, num2 } = amount as { num1?: number; num2?: number }; | |
| const a = Number.isFinite(num1) ? (num1 as number) : 0; | |
| const b = Number.isFinite(num2) ? (num2 as number) : 0; | |
| return `${fmt(a, 'financial')} to ${fmt(b, 'financial')}`; | |
| } |
🤖 Prompt for AI Agents
In packages/desktop-client/src/components/rules/RuleEditor.tsx around lines 359
to 370, formatAmount currently treats falsy values and object fields unsafely;
change the checks to use explicit nullish checks (amount == null) for the empty
case, keep the typeof number branch, and for the object branch guard amount.num1
and amount.num2 with nullish coalescing or optional chaining so you never pass
undefined into format (e.g. use amount?.num1 ?? 0 and amount?.num2 ?? 0); ensure
the function returns a single formatted value when only one bound exists and a
"min to max" string when both exist, always calling format with a defined
numeric value.
| function getAmount() { | ||
| const signedValued = symbol === '-' ? symbol + value : value; | ||
| return amountToInteger(evalArithmetic(signedValued)); | ||
| return format.fromEdit(signedValued, 0); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t force decimal places to 0 in fromEdit; risk of truncating cents.
format.fromEdit(signedValued, 0) likely overrides decimal handling and can drop fractional parts for currencies with decimals. Let useFormat infer from active currency.
- return format.fromEdit(signedValued, 0);
+ return format.fromEdit(signedValued);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getAmount() { | |
| const signedValued = symbol === '-' ? symbol + value : value; | |
| return amountToInteger(evalArithmetic(signedValued)); | |
| return format.fromEdit(signedValued, 0); | |
| } | |
| function getAmount() { | |
| const signedValued = symbol === '-' ? symbol + value : value; | |
| return format.fromEdit(signedValued); | |
| } |
🤖 Prompt for AI Agents
In packages/desktop-client/src/components/util/AmountInput.tsx around lines 101
to 104, the helper getAmount currently calls format.fromEdit(signedValued, 0)
which forces zero decimal places and can truncate cents; remove the hard-coded 0
so the call uses the format's inferred decimal configuration (i.e. call
format.fromEdit(signedValued) or omit the second argument), ensuring the active
currency's decimal precision is respected when parsing user input.
Created a new PR as I force pushed the other one before reopening (#5536).
Added other handling of the Filter as suggested by @MatissJanis. I hope the current implementation is less invasive.