-
Notifications
You must be signed in to change notification settings - Fork 0
Add currency formatting and rate retrieval functions with unit tests #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Introduced `formatCurrency` function to format numbers as USD currency, ensuring proper localization and rounding. - Added `getRateByDate` function to retrieve rates from a historical dataset based on a specified date, with handling for undefined inputs and edge cases. - Created unit tests for both functions to validate their functionality and accuracy, covering various scenarios for currency formatting and rate retrieval. These additions enhance the utility functions for financial calculations and improve test coverage for the application.
📝 WalkthroughWalkthroughA new utility module was added with two exported functions for currency formatting and historical rate lookup. Corresponding unit tests were introduced to validate their behavior under various scenarios. Additionally, an import path in a type definition file was updated to a more specific module location. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant formatUtils
Caller->>formatUtils: formatCurrency(value)
formatUtils-->>Caller: formatted USD string
Caller->>formatUtils: getRateByDate(rateHistory, date)
formatUtils-->>Caller: rate for given date or undefined
📜 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 (7)
✨ Finishing Touches
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. 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 (
|
🔍 Vulnerabilities of
|
digest | sha256:22aec63caf9094cc00504497ac276e02af6382fdfe21456991ff66e82521c98d |
vulnerabilities | |
platform | linux/amd64 |
size | 243 MB |
packages | 1628 |
📦 Base Image node:20-alpine
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
| ||||||||||||
Description
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
workers/main/src/common/formatUtils.ts (2)
1-3
: Expose locale / fraction-digit options or document rounding semantics
toLocaleString
rounds to the nearest integer; the exported name implies simple “format” but not how rounding happens.
Consider:-export function formatCurrency(value: number): string { - return `$${value.toLocaleString('en-US', { minimumFractionDigits: 0, maximumFractionDigits: 0 })}`; -} +export function formatCurrency( + value: number, + opts: Intl.NumberFormatOptions = { minimumFractionDigits: 0, maximumFractionDigits: 0 }, + locale = 'en-US', +): string { + return `$${value.toLocaleString(locale, opts)}`; +}This keeps current behaviour but makes it explicit/customisable, and prevents surprises with negative or non-USD localisation later.
13-24
: Avoid O(n log n) sort on every call when rate history is large
Object.keys(rateHistory).sort()
reallocates and sorts each time.
IfgetRateByDate
is called per record (e.g. once per timesheet line), this can dominate runtime.Two quick wins:
- Store
const sortedDates = Object.keys(rateHistory).sort()
once outside hot paths (e.g. where the history is loaded).- Or convert to an array of
[date, rate]
tuples sorted once and binary-search it (O(log n) look-ups).Not blocking, but worth profiling if histories grow.
workers/main/src/common/formatUtils.test.ts (1)
12-15
: Test description doesn’t match expectationsThe block is titled “rounds down decimal values” but the first assertion (
1234.56 → $1,235
) verifies rounding up.
Rename to avoid confusion:- it('rounds down decimal values', () => { + it('rounds decimal values to the nearest integer', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
workers/main/src/common/formatUtils.test.ts
(1 hunks)workers/main/src/common/formatUtils.ts
(1 hunks)workers/main/src/common/types.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
workers/main/src/common/formatUtils.test.ts (1)
workers/main/src/common/formatUtils.ts (2)
formatCurrency
(1-3)getRateByDate
(5-25)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Docker Security Scanning (temporal, Dockerfile.temporal, temporal-test:latest)
- GitHub Check: Service Availability Check
- GitHub Check: SonarQube
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
workers/main/src/common/types.ts (1)
1-1
: Confirm new import path actually resolves at build time
'../configs/weeklyFinancialReport'
needs to exportGroupNameEnum
(either directly or via anindex.ts
). If other files still import the enum from the old barrel (../configs
), you may end up with two distinct instances of the enum at runtime, breaking===
comparisons.Please verify compilation succeeds and that there is exactly one canonical export path for this enum across the codebase.
- Updated the `getRateByDate` function to sort the rate history dates chronologically using a custom sorting function. This ensures that the dates are processed in the correct order, improving the accuracy of rate retrieval based on date. These changes enhance the functionality of the rate retrieval process, ensuring more reliable data handling.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good!
formatCurrency
function to format numbers as USD currency, ensuring proper localization and rounding.getRateByDate
function to retrieve rates from a historical dataset based on a specified date, with handling for undefined inputs and edge cases.These additions enhance the utility functions for financial calculations and improve test coverage for the application.