-
Notifications
You must be signed in to change notification settings - Fork 0
finish API design for historical data #20 #21
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
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Pull Request Overview
This PR adds historical exchange rate functionality to fetch and transform past currency data from Open Exchange Rates. The implementation includes a new time utilities module, historical data fetching with retry logic and parallel processing, and an API endpoint to expose this data.
- Implements historical rate fetching with concurrent workers and retry mechanism
- Adds time utilities for building UTC date ranges
- Creates a new
/api/rates/historyendpoint with configurable day ranges and output formats
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/time_utils.ts | New utility module for ISO date formatting and generating past UTC date arrays |
| lib/server/oxr_history.ts | Core historical data fetching logic with worker pool, retry handling, and data transformation functions |
| lib/server/oxr.ts | Adds HistoricalResponse type and getHistorical function for fetching historical rates |
| lib/server/oxr_convert.ts | Removes as const from DEFAULTS_CURRENCY to allow runtime mutation |
| lib/server/tests/oxr_history.test.ts | Comprehensive unit tests for historical rate functions with mocking |
| lib/server/tests/oxr.integration.test.ts | Adds integration test for historical endpoint and fixes formatting |
| app/api/rates/history/route.ts | New API route handler for historical rates with caching and orientation options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
|
|
||
| /** * Shape of Open Exchange Rates historical payload. |
Copilot
AI
Oct 29, 2025
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.
The JSDoc comment has an extra space after the opening /**. It should be /** followed by a single space.
| /** * Shape of Open Exchange Rates historical payload. | |
| /** Shape of Open Exchange Rates historical payload. |
|
|
||
|
|
||
|
|
||
| /** * Fetches historical exchange rates for a specific date from Open Exchange Rates. |
Copilot
AI
Oct 29, 2025
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.
The JSDoc comment has an extra space after the opening /**. It should be /** followed by a single space.
| /** * Fetches historical exchange rates for a specific date from Open Exchange Rates. | |
| /** | |
| * Fetches historical exchange rates for a specific date from Open Exchange Rates. |
| * @returns A promise resolving to the `HistoricalResponse` payload. | ||
| */ | ||
| export async function getHistorical(dateISO: string): Promise<HistoricalResponse> { | ||
| // dateISO = YYYY-MM-DD(UTC) |
Copilot
AI
Oct 29, 2025
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.
The comment contains Chinese/full-width parentheses '(' and ')' which should be replaced with standard ASCII parentheses '(' and ')'.
| // dateISO = YYYY-MM-DD(UTC) | |
| // dateISO = YYYY-MM-DD (UTC) |
| for (const t of targets) series[t] = []; | ||
|
|
||
| for (const row of points) { | ||
| const d = row.date; // 已是 string |
Copilot
AI
Oct 29, 2025
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.
The comment is in Chinese ('已是 string' means 'already is string'). Comments should be in English for consistency with the rest of the codebase.
| const d = row.date; // 已是 string | |
| const d = row.date; // already a string |
| vi.mock("../../time_utils.ts", () => ({ | ||
| buildPastDatesUTC: (...args: unknown[]) => buildPastDatesUTCMock(...args), | ||
| })); |
Copilot
AI
Oct 29, 2025
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.
Duplicate mock for ../../time_utils.ts on lines 19-21 and 23-25. The second mock (with .ts extension) will override the first one. Remove the redundant mock or consolidate them.
| vi.mock("../../time_utils.ts", () => ({ | |
| buildPastDatesUTC: (...args: unknown[]) => buildPastDatesUTCMock(...args), | |
| })); |
| * Default currency codes returned when no targets are provided. | ||
| */ | ||
| export const DEFAULTS_CURRENCY = ["USD", "EUR", "JPY", "GBP", "CNY"] as const; | ||
| export const DEFAULTS_CURRENCY = ["USD", "EUR", "JPY", "GBP", "CNY"]; |
Copilot
AI
Oct 29, 2025
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.
Removing as const makes this array mutable, which could lead to unintended modifications at runtime. If immutability is no longer required, consider adding a comment explaining why this change was necessary, or use Object.freeze() to prevent mutation while maintaining assignability.
| export const DEFAULTS_CURRENCY = ["USD", "EUR", "JPY", "GBP", "CNY"]; | |
| export const DEFAULTS_CURRENCY = ["USD", "EUR", "JPY", "GBP", "CNY"] as const; |
|
|
||
| vi.mock("../oxr_convert", () => ({ | ||
| rebaseToAUD: (...args: unknown[]) => rebaseToAUDMock(...args), | ||
| DEFAULTS_CURRENCY: ["USD", "EUR", "JPY", "GBP", "CNY"] as const, |
Copilot
AI
Oct 29, 2025
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.
The mocked DEFAULTS_CURRENCY uses as const while the actual implementation was changed to remove it (line 27 in oxr_convert.ts). This inconsistency could mask issues in tests. Update the mock to match the actual implementation.
| DEFAULTS_CURRENCY: ["USD", "EUR", "JPY", "GBP", "CNY"] as const, | |
| DEFAULTS_CURRENCY: ["USD", "EUR", "JPY", "GBP", "CNY"], |
| const audRates = rebaseToAUD(usdRates); | ||
| const row: Record<string, number | string> = { date }; | ||
| for (const t of targets) if (audRates[t] != null) row[t] = audRates[t]; | ||
| return row as { date: string; [code: string]: number | string }; |
Copilot
AI
Oct 29, 2025
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.
[nitpick] The type assertion is unnecessary since row is already typed as Record<string, number | string> which is compatible with the return type. The explicit cast can be removed for cleaner code.
| return row as { date: string; [code: string]: number | string }; | |
| return row; |
This pull request introduces a new, robust backend for fetching and transforming historical exchange rate data, including an API endpoint, supporting library, and comprehensive tests. The main focus is on providing AUD-based historical exchange rates, handling retries for unreliable upstream data, and enabling flexible data orientations for consumers.
Key changes:
New Historical Exchange Rate API
app/api/rates/history/route.tsthat serves historical exchange rate data, supports orientation by date or by currency, and applies caching headers for performance.Core Library for Historical Data
lib/server/oxr_history.ts, which implements:lib/time_utils.tsto generate lists of past UTC dates in ISO format for historical lookups.Support and Integration
lib/server/oxr.tsto support fetching historical rates for a specific date, and defined theHistoricalResponsetype.lib/server/oxr_convert.tsto be mutable for compatibility with new utilities.Testing
lib/server/__tests__/oxr_history.test.tsto validate historical data fetching, error handling, and data transformation logic.lib/server/__tests__/oxr.integration.test.tsto cover the new historical endpoint and ensure correct data fetching from upstream. [1] [2]References:
[1] [2] [3] [4] [5] [6] [7] [8]