feat: date range filter #1060
feat: date range filter #1060
Conversation
WalkthroughAdds optional startDate/endDate filtering across payments listing, count, and download. API endpoints parse and forward date range with timezone. Frontend introduces date-range UI, integrates into fetch, count, and export URLs, and coordinates with year filters. Service layer adds date-range utilities and extends query methods to apply date or year filters. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as Payments Page (UI)
participant API as /api/payments (GET)
participant S as transactionService
participant DB as Database
U->>P: Set filters (years or start/end date)
P->>API: GET with userId, paging, years, startDate, endDate, timezone
API->>S: fetchAllPaymentsByUserIdWithPagination(..., years, tz, startDate, endDate)
alt startDate & endDate provided
S->>S: buildDateRange(startDate, endDate, tz)
S->>DB: Query with date-range filter
else years provided
S->>S: getYearFilters(years, tz)
S->>DB: Query with year-based filter
end
DB-->>S: Results
S-->>API: Payments page data
API-->>P: JSON response
P-->>U: Render payments
sequenceDiagram
autonumber
actor U as User
participant P as Payments Page (UI)
participant C as /api/payments/count
participant D as /api/payments/download
participant S as transactionService
participant DB as Database
P->>C: GET count with years/startDate/endDate/tz
C->>S: getFilteredTransactionCount(..., years, tz, startDate, endDate)
S->>DB: Count with appropriate filter
DB-->>S: Count
S-->>C: Count
C-->>P: Count JSON
U->>P: Export CSV
P->>D: GET download with years/startDate/endDate/tz
D->>S: fetchAllPaymentsByUserId(..., tz, startDate, endDate)
S->>DB: Query with filter
DB-->>S: Rows
S-->>D: Data
D-->>P: CSV stream
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pages/payments/index.tsx (1)
140-142: Consider adding startDate to useEffect dependencies.The useEffect only includes
endDatein its dependencies, which means changingstartDatealone won't trigger a data refresh. While users typically set startDate first and then endDate (which would trigger the fetch), a user who modifies only startDate after setting both dates would see stale data.Apply this diff to include startDate in the dependencies:
useEffect(() => { setRefreshCount(prev => prev + 1) - }, [selectedButtonIds, selectedTransactionYears, endDate]) + }, [selectedButtonIds, selectedTransactionYears, startDate, endDate])services/transactionService.ts (1)
752-763: Consider consistent parameter ordering across functions.The parameter order differs between functions:
fetchAllPaymentsByUserIdWithPagination(lines 752-763):..., timezone?, startDate?, endDate?fetchAllPaymentsByUserId(lines 889-897):..., startDate?, endDate?, timezone?getFilteredTransactionCount(lines 949-956):..., timezone?, startDate?, endDate?While this doesn't cause bugs (calls pass parameters correctly), consistent ordering improves maintainability.
Consider standardizing the order (e.g., always place
timezonebeforestartDateandendDate) in a follow-up refactor:export async function fetchAllPaymentsByUserId ( userId: string, networkIds?: number[], buttonIds?: string[], years?: string[], + timezone?: string, startDate?: string, - endDate?: string, - timezone?: string + endDate?: string ): Promise<TransactionsWithPaybuttonsAndPrices[]> {Also applies to: 889-897, 949-956
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pages/api/payments/count/index.ts(1 hunks)pages/api/payments/download/index.ts(1 hunks)pages/api/payments/index.ts(2 hunks)pages/payments/index.tsx(8 hunks)services/transactionService.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
pages/api/payments/count/index.ts (3)
pages/api/payments/download/index.ts (1)
req(13-80)pages/api/payments/index.ts (1)
req(5-48)services/transactionService.ts (1)
getFilteredTransactionCount(949-982)
pages/api/payments/index.ts (2)
pages/api/payments/count/index.ts (1)
req(6-43)pages/api/payments/download/index.ts (1)
req(13-80)
pages/api/payments/download/index.ts (3)
pages/api/payments/count/index.ts (1)
req(6-43)pages/api/payments/index.ts (1)
req(5-48)services/transactionService.ts (1)
fetchAllPaymentsByUserId(889-935)
⏰ 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). (1)
- GitHub Check: Run Tests
🔇 Additional comments (3)
pages/api/payments/count/index.ts (1)
23-35: LGTM!The startDate and endDate parameter parsing follows the established pattern for buttonIds and years. The filter condition correctly requires both dates to be present and non-empty before enabling date-range filtering, and the function call properly passes timezone along with the date range parameters.
pages/payments/index.tsx (1)
553-603: Date range and year filter coordination looks good.The mutual exclusion between date range and year filters is correctly implemented:
- Selecting years clears the date range (lines 558-561)
- Setting startDate clears years and resets endDate (lines 582-584)
- Setting endDate clears years (lines 595-597)
The
minattribute on the endDate input (line 599) properly prevents selecting an end date before the start date.pages/api/payments/index.ts (1)
22-45: LGTM!The startDate and endDate parameters are correctly parsed and passed to
fetchAllPaymentsByUserIdWithPaginationwith proper timezone handling.
| } | ||
|
|
||
| const transactions = await fetchAllPaymentsByUserId(userId, networkIdArray, buttonIds, years) | ||
| const transactions = await fetchAllPaymentsByUserId(userId, networkIdArray, buttonIds, years, startDate, endDate) |
There was a problem hiding this comment.
Critical: Missing timezone parameter in fetchAllPaymentsByUserId call.
The timezone variable is calculated on line 33 but not passed to fetchAllPaymentsByUserId. This causes date range filtering (and existing year filtering) to ignore the user's timezone preference, potentially returning incorrect data.
Apply this diff to pass the timezone parameter:
- const transactions = await fetchAllPaymentsByUserId(userId, networkIdArray, buttonIds, years, startDate, endDate)
+ const transactions = await fetchAllPaymentsByUserId(userId, networkIdArray, buttonIds, years, startDate, endDate, timezone)Note: This bug also affects the existing years filtering functionality, not just the new date range feature.
📝 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.
| const transactions = await fetchAllPaymentsByUserId(userId, networkIdArray, buttonIds, years, startDate, endDate) | |
| const transactions = await fetchAllPaymentsByUserId( | |
| userId, | |
| networkIdArray, | |
| buttonIds, | |
| years, | |
| startDate, | |
| endDate, | |
| timezone | |
| ) |
🤖 Prompt for AI Agents
In pages/api/payments/download/index.ts around line 63, the call to
fetchAllPaymentsByUserId omits the timezone parameter (computed on line 33), so
date range and year filtering ignore user timezone; update the function
invocation to include the timezone argument (passing the timezone variable
computed earlier in the same scope) in the correct position according to the
function signature so that fetchAllPaymentsByUserId receives timezone and
performs timezone-aware filtering.
| const buildDateRange = ( | ||
| startDate: string | Date, | ||
| endDate: string | Date, | ||
| timezone?: string | ||
| ): { gte: number, lt: number } => { | ||
| let start: number | ||
| let end: number | ||
|
|
||
| const startObj = new Date(startDate) | ||
| const endObj = new Date(endDate) | ||
|
|
||
| if (timezone !== undefined && timezone !== null && timezone !== '') { | ||
| const startInTimezone = new Date(startObj.toLocaleString('en-US', { timeZone: timezone })) | ||
| const endInTimezone = new Date(endObj.toLocaleString('en-US', { timeZone: timezone })) | ||
|
|
||
| const startOffset = startObj.getTime() - startInTimezone.getTime() | ||
| const endOffset = endObj.getTime() - endInTimezone.getTime() | ||
|
|
||
| start = (startObj.getTime() + startOffset) / 1000 | ||
| end = (endObj.getTime() + endOffset) / 1000 | ||
| } else { | ||
| start = startObj.getTime() / 1000 | ||
| end = endObj.getTime() / 1000 | ||
| } | ||
|
|
||
| return { | ||
| gte: Math.floor(start), | ||
| lt: Math.floor(end) | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Timezone handling in buildDateRange is incorrect.
The timezone adjustment logic has a fundamental flaw:
new Date(startDate)interprets "2024-01-15" as midnight UTCstartObj.toLocaleString('en-US', { timeZone: timezone })converts that UTC time to a string in the target timezone (e.g., "1/14/2024, 7:00:00 PM" for NYC)new Date(startInTimezone)parses that string in the server's system timezone (which is UTC), not the target timezone- The resulting offset calculation is meaningless
Example:
- User in NYC (UTC-5) selects "2024-01-15"
startObj= 2024-01-15T00:00:00Z (midnight UTC)toLocaleString= "1/14/2024, 7:00:00 PM" (7 PM previous day in NYC)new Date("1/14/2024, 7:00:00 PM")on server (UTC) produces wrong result- The offset and final timestamp are incorrect
Impact:
Date range filters will return incorrect results for users in non-UTC timezones. Transactions may be included or excluded incorrectly based on timezone.
The file already uses Prisma, which likely has moment or date-fns available. Use a proper timezone library:
+import moment from 'moment-timezone'
+
const buildDateRange = (
startDate: string | Date,
endDate: string | Date,
timezone?: string
): { gte: number, lt: number } => {
- let start: number
- let end: number
-
- const startObj = new Date(startDate)
- const endObj = new Date(endDate)
-
- if (timezone !== undefined && timezone !== null && timezone !== '') {
- const startInTimezone = new Date(startObj.toLocaleString('en-US', { timeZone: timezone }))
- const endInTimezone = new Date(endObj.toLocaleString('en-US', { timeZone: timezone }))
-
- const startOffset = startObj.getTime() - startInTimezone.getTime()
- const endOffset = endObj.getTime() - endInTimezone.getTime()
-
- start = (startObj.getTime() + startOffset) / 1000
- end = (endObj.getTime() + endOffset) / 1000
- } else {
- start = startObj.getTime() / 1000
- end = endObj.getTime() / 1000
- }
-
+ let start: number
+ let end: number
+
+ if (timezone !== undefined && timezone !== null && timezone !== '') {
+ // Parse date in the given timezone
+ start = moment.tz(startDate, timezone).unix()
+ end = moment.tz(endDate, timezone).unix()
+ } else {
+ // No timezone: interpret as UTC
+ start = moment.utc(startDate).unix()
+ end = moment.utc(endDate).unix()
+ }
+
return {
gte: Math.floor(start),
lt: Math.floor(end)
}
}📝 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.
| const buildDateRange = ( | |
| startDate: string | Date, | |
| endDate: string | Date, | |
| timezone?: string | |
| ): { gte: number, lt: number } => { | |
| let start: number | |
| let end: number | |
| const startObj = new Date(startDate) | |
| const endObj = new Date(endDate) | |
| if (timezone !== undefined && timezone !== null && timezone !== '') { | |
| const startInTimezone = new Date(startObj.toLocaleString('en-US', { timeZone: timezone })) | |
| const endInTimezone = new Date(endObj.toLocaleString('en-US', { timeZone: timezone })) | |
| const startOffset = startObj.getTime() - startInTimezone.getTime() | |
| const endOffset = endObj.getTime() - endInTimezone.getTime() | |
| start = (startObj.getTime() + startOffset) / 1000 | |
| end = (endObj.getTime() + endOffset) / 1000 | |
| } else { | |
| start = startObj.getTime() / 1000 | |
| end = endObj.getTime() / 1000 | |
| } | |
| return { | |
| gte: Math.floor(start), | |
| lt: Math.floor(end) | |
| } | |
| } | |
| import moment from 'moment-timezone' | |
| const buildDateRange = ( | |
| startDate: string | Date, | |
| endDate: string | Date, | |
| timezone?: string | |
| ): { gte: number, lt: number } => { | |
| let start: number | |
| let end: number | |
| if (timezone !== undefined && timezone !== null && timezone !== '') { | |
| // Parse date in the given timezone | |
| start = moment.tz(startDate, timezone).unix() | |
| end = moment.tz(endDate, timezone).unix() | |
| } else { | |
| // No timezone: interpret as UTC | |
| start = moment.utc(startDate).unix() | |
| end = moment.utc(endDate).unix() | |
| } | |
| return { | |
| gte: Math.floor(start), | |
| lt: Math.floor(end) | |
| } | |
| } |
🤖 Prompt for AI Agents
In services/transactionService.ts around lines 837–866, the current timezone
adjustment uses toLocaleString and re-parses strings which yields incorrect
offsets; replace this logic with a timezone-aware library (e.g., date-fns-tz or
moment-timezone) to: 1) interpret the input date as a date in the target
timezone (get the timezone's startOfDay for startDate and endOfDay for endDate
or compute startOfNextDay for exclusive upper bound), 2) convert those zoned
datetimes to UTC instants (zonedTimeToUtc or moment.tz(...).utc()), and 3)
return gte as Math.floor(utcStart/1000) and lt as
Math.floor(utcEndExclusive/1000); ensure imports are added and that the upper
bound is exclusive (use start of next day or endOfDay+1ms converted to UTC) so
date filters behave correctly across timezones.
There was a problem hiding this comment.
I would expect the end date to be included, if nothing is said. Right now the initial date is included and the end date is not.
One option is to add a toggle in the rightside of the selectors saying "Include end date", but if you want just to make it include it and that's it, would be fine too
Related to #818
Description
Added date range filter to payments page
the filter also applies to csv
Test plan
Go to payments page and make sure payments filter works as expected also test csv file
Summary by CodeRabbit