Add date range filter to request logs, hide DELETE body - #3742
Conversation
- Update getApiLogsDateRange to accept start/end/interval params with plan-based retention clamping - Add date range picker to logs table filter bar with plan-appropriate presets - Wire date params through API schemas, routes, and frontend query hooks - Add fromDate support to SimpleDateRangePicker to disable dates outside retention - Hide request body section on log detail page for DELETE requests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHandlers, schemas, hooks, and UI now accept optional Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Logs Dashboard
participant Hook as useLogFilters / useApiLogsCount
participant API as /api/logs Route
participant Range as getApiLogsDateRange
participant Service as getApiLogs / getApiLogsCount
participant DB as Database
User->>UI: Pick date range & filters (start,end,interval)
UI->>Hook: Update search params
Hook->>API: GET /api/logs?start=...&end=...&interval=...&filters...
API->>Range: getApiLogsDateRange({plan, start, end, interval})
Range-->>API: {startDate, endDate}
API->>Service: getApiLogs({ start: startDate, end: endDate, filters, workspaceId })
Service->>DB: Query logs within date range + filters
DB-->>Service: Return results
Service-->>API: Logs/count or list
API-->>UI: JSON response
UI-->>User: Render table / counts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/lib/api-logs/api-log-retention.ts`:
- Around line 7-17: The helper getApiLogsDateRange signature/return shape
changed but the [logId] route still calls getApiLogsDateRange(workspace.plan)
and destructures { start }, causing a mismatch; update the caller in the [logId]
route to call getApiLogsDateRange({ plan: workspace.plan, start?:..., end?:...,
interval?:... }) (or pass the appropriate properties you expect) and destructure
the returned shape that the function now returns, or alternatively restore a
backward-compatible overload in getApiLogsDateRange that accepts a PlanProps
single-arg and returns the previous shape so the existing destructuring of {
start } continues to work. Ensure you reference getApiLogsDateRange and the
[logId] route when making the change.
- Around line 29-36: The current logic only clamps startDate (clampedStart) to
retentionBoundary, which can yield startDate > endDate when endDate is also
older than retention; update the function to clamp both startDate and endDate
against retentionBoundary (e.g., compute clampedStart = max(startDate,
retentionBoundary) and clampedEnd = max(endDate, retentionBoundary)), then
ensure the returned range uses clampedStart and clampedEnd and is normalized so
clampedStart <= clampedEnd before calling formatUTCDateTimeClickhouse.
In `@apps/web/lib/api-logs/schemas.ts`:
- Around line 88-90: Tighten validation for the date-range schema fields by
replacing the loose z.string().optional() entries for start, end, and interval
with strict checks: ensure start and end are valid ISO/RFC date strings (use
z.string().refine or z.string().datetime to reject malformed dates) and validate
interval against the exact set of supported interval values used by
getIntervalData (use z.enum or z.union of literal values) so malformed dates or
unsupported intervals are rejected at the schema boundary (update the schema
symbols start, end, and interval accordingly).
In `@apps/web/ui/logs/logs-table.tsx`:
- Around line 360-376: The parent container currently forces a single-row layout
and causes the two full-width children (Filter.Select and SimpleDateRangePicker)
to clash on small screens; change the wrapper div's classes to use a responsive
column-to-row layout (e.g., replace "flex items-center gap-2" with "flex
flex-col md:flex-row items-start md:items-center gap-2") so the Filter.Select
and SimpleDateRangePicker stack vertically on mobile and return to a row on md+
screens while preserving existing "w-full md:w-fit" behavior on each component.
- Around line 372-374: The defaultInterval prop is hardcoded to "30d" which may
not exist for some plans; change the initialization in logs-table.tsx so
defaultInterval is derived from the available presets (use presets to select a
plan-aware default, e.g., prefer a configured plan default if present, otherwise
fall back to the first valid entry in presets) and ensure the chosen value is
valid before passing it to the DateRangePicker; keep the fromDate logic using
subDays(new Date(), retentionDays) as-is and guard against an empty presets
array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0dbec214-fa0d-4d72-a351-d9658c77e43d
📒 Files selected for processing (10)
apps/web/app/api/logs/count/route.tsapps/web/app/api/logs/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/logs/[logId]/page-client.tsxapps/web/lib/api-logs/api-log-retention.tsapps/web/lib/api-logs/constants.tsapps/web/lib/api-logs/schemas.tsapps/web/lib/swr/use-api-logs-count.tsapps/web/ui/logs/logs-table.tsxapps/web/ui/logs/use-log-filters.tsapps/web/ui/shared/simple-date-range-picker.tsx
|
@coderabbitai full review please |
🧩 Analysis chainI'll gather the file contents for a thorough review. 🏁 Script executed: Length of output: 264380 The PR description image shows the Logs page UI with a dual-month calendar date picker (April–May 2026) open, April 13 highlighted as the current date, and a quick-presets panel on the right listing "Last 24 hours", "Last 7 days", "Last 30 days", and "Last 3 months". The UI matches the described implementation well.
SummaryThe PR is well-structured and the core logic (date range clamping to plan retention, hiding DELETE request bodies, schema/hook extensions) is sound. A few issues are worth addressing before merge. Issues Found1. 🐛 Missing
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/ui/logs/logs-table.tsx (1)
369-375: Consider defining a stricter type for presets to avoidas any.The
presets as anycast works at runtime becauseSimpleDateRangePickerinternally handles string arrays (per the component's filtering logic). However, the component's prop type expects preset objects, causing the mismatch. A dedicated type or updatingSimpleDateRangePicker's type to acceptstring[]would improve type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/logs/logs-table.tsx` around lines 369 - 375, The current cast presets as any hides a type mismatch between the presets value and the SimpleDateRangePicker prop type; update the presets definition to a stricter type (e.g., declare a Preset type matching the actual values you pass—string[] or an array of {label: string, value: string} objects) and remove the as any cast, or alternatively update SimpleDateRangePicker's prop type to accept string[] if it truly only needs strings; adjust the variable named presets and/or the SimpleDateRangePicker prop signature so the types align and the explicit cast is no longer needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/ui/logs/logs-table.tsx`:
- Around line 369-375: The current cast presets as any hides a type mismatch
between the presets value and the SimpleDateRangePicker prop type; update the
presets definition to a stricter type (e.g., declare a Preset type matching the
actual values you pass—string[] or an array of {label: string, value: string}
objects) and remove the as any cast, or alternatively update
SimpleDateRangePicker's prop type to accept string[] if it truly only needs
strings; adjust the variable named presets and/or the SimpleDateRangePicker prop
signature so the types align and the explicit cast is no longer needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84d7445f-5830-4155-af0b-12ef3f4eef2c
📒 Files selected for processing (3)
apps/web/lib/api-logs/api-log-retention.tsapps/web/lib/api-logs/schemas.tsapps/web/ui/logs/logs-table.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/lib/api-logs/schemas.ts
…reset dates on clear - Add 60d/90d presets per retention tier and validate interval from presets\n- Use DEFAULT_RETENTION_DAYS for preset fallback in logs table\n- Clear start, end, and interval when removing all log filters
…on queries - getApiLogsDateRange start/end accept string | Date | null\n- get-api-logs and get-api-logs-count params omit schema start/end and require resolved strings
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/web/ui/logs/use-log-filters.ts (1)
135-141:onRemoveAlldoes not clear date range parameters.The
onRemoveAllcallback clears filter params (method,statusCode, etc.) but omitsstart,end, andinterval. If "Remove All" is intended to reset all filters, consider adding these keys. If the date range is meant to persist separately, this is fine as-is.♻️ Optional: Include date params in onRemoveAll
const onRemoveAll = useCallback( () => queryParams({ - del: ["method", "statusCode", "routePattern", "tokenId", "requestType"], + del: ["method", "statusCode", "routePattern", "tokenId", "requestType", "start", "end", "interval"], }), [queryParams], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/logs/use-log-filters.ts` around lines 135 - 141, The onRemoveAll callback currently calls queryParams with del set to ["method","statusCode","routePattern","tokenId","requestType"] but omits date range keys; update the del array in onRemoveAll (the function using queryParams) to also include "start", "end", and "interval" so the date range is cleared when Remove All is invoked (unless you intentionally want date range persistence).apps/web/lib/api-logs/api-log-retention.ts (1)
22-22: Partial date inputs silently fall back to full retention window.When only
startor onlyendis provided (but not both), the condition(start && end)is false, so the function returns the default full retention window. While the UI always provides both or neither, this could be surprising if the API is called directly with partial params.Consider documenting this behavior or enforcing both-or-none at the schema level with a
.refine().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/api-logs/api-log-retention.ts` at line 22, The condition if (interval || (start && end)) silently treats partial date inputs as no-dates; update validation to enforce both-or-none for start and end: in apps/web/lib/api-logs/api-log-retention.ts, add a guard that detects a partial date pair (start && !end) || (!start && end) and return/throw a validation error (or return a 400) instead of falling back to full retention, or enforce this at the input schema using a .refine() rule that requires both start and end together.apps/web/lib/api-logs/constants.ts (1)
85-89: Consider whether 60-day plans should have a "60d" preset option.Business plans have 60-day retention but the presets for
60only include["24h", "7d", "30d"]. Users on these plans can still select custom date ranges spanning their full retention, but might expect a "Last 60 days" preset. If this is intentional (to keep the UI simpler), this is fine. Otherwise, add"60d"to both the presets and the schema enum.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/api-logs/constants.ts` around lines 85 - 89, API_LOGS_PRESETS_BY_RETENTION currently omits a "60d" preset for 60-day retention plans; update the presets to include "60d" for the key 60 (i.e., add "60d" to the array in API_LOGS_PRESETS_BY_RETENTION) and also add "60d" to the corresponding schema enum that validates preset values so the new preset is accepted by validation (update the enum where preset strings like "24h","7d","30d","90d" are defined).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/lib/api-logs/api-log-retention.ts`:
- Line 22: The condition if (interval || (start && end)) silently treats partial
date inputs as no-dates; update validation to enforce both-or-none for start and
end: in apps/web/lib/api-logs/api-log-retention.ts, add a guard that detects a
partial date pair (start && !end) || (!start && end) and return/throw a
validation error (or return a 400) instead of falling back to full retention, or
enforce this at the input schema using a .refine() rule that requires both start
and end together.
In `@apps/web/lib/api-logs/constants.ts`:
- Around line 85-89: API_LOGS_PRESETS_BY_RETENTION currently omits a "60d"
preset for 60-day retention plans; update the presets to include "60d" for the
key 60 (i.e., add "60d" to the array in API_LOGS_PRESETS_BY_RETENTION) and also
add "60d" to the corresponding schema enum that validates preset values so the
new preset is accepted by validation (update the enum where preset strings like
"24h","7d","30d","90d" are defined).
In `@apps/web/ui/logs/use-log-filters.ts`:
- Around line 135-141: The onRemoveAll callback currently calls queryParams with
del set to ["method","statusCode","routePattern","tokenId","requestType"] but
omits date range keys; update the del array in onRemoveAll (the function using
queryParams) to also include "start", "end", and "interval" so the date range is
cleared when Remove All is invoked (unless you intentionally want date range
persistence).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f0b7e6f-3edc-48df-9776-689a561b0d36
📒 Files selected for processing (11)
apps/web/app/api/logs/[logId]/route.tsapps/web/app/api/logs/count/route.tsapps/web/app/api/logs/route.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/settings/logs/[logId]/page-client.tsxapps/web/lib/api-logs/api-log-retention.tsapps/web/lib/api-logs/constants.tsapps/web/lib/api-logs/schemas.tsapps/web/lib/swr/use-api-logs-count.tsapps/web/ui/logs/logs-table.tsxapps/web/ui/logs/use-log-filters.tsapps/web/ui/shared/simple-date-range-picker.tsx
|
@coderabbitai made some changes mentioned in review, please do another full review |
🧩 Analysis chainI'll fetch all the relevant files to check what's been updated. 🏁 Script executed: Length of output: 39249 The PR description image shows the Logs dashboard with the
✅ FixedIssue
|
| Area | Status |
|---|---|
DELETE body hidden (log.method !== "DELETE") |
✅ Correct |
| Retention clamping + normalization (inverted ranges) | ✅ Correct |
fromDate propagation to DateRangePicker |
✅ Correct |
plan flowing from useWorkspace() → LogsFilters → presets/fromDate |
✅ Correct |
SWR hooks include start/end/interval |
✅ Correct |
[logId]/route.ts retention check uses startDate key |
✅ Correct |
✅ Actions performed
Full review triggered.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/ui/logs/logs-table.tsx (1)
374-374: Remove theas anycast by properly typing the presets constant.
presets={presets as any}bypasses compile-time type checking.API_LOGS_PRESETS_BY_RETENTIONis typed asRecord<number, string[]>, butSimpleDateRangePickerexpectspresets?: (typeof DATE_RANGE_INTERVAL_PRESETS)[number][]. Either typeAPI_LOGS_PRESETS_BY_RETENTIONwith the specific preset union type (instead of genericstring[]), or explicitly type thepresetsvariable in logs-table.tsx to match the picker's expected type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/ui/logs/logs-table.tsx` at line 374, The prop is using an any cast; fix by giving the presets the exact union type expected by SimpleDateRangePicker: change the type of API_LOGS_PRESETS_BY_RETENTION (or the local presets variable in logs-table.tsx) from string[] to (typeof DATE_RANGE_INTERVAL_PRESETS)[number][], e.g. make API_LOGS_PRESETS_BY_RETENTION: Record<number, (typeof DATE_RANGE_INTERVAL_PRESETS)[number][]> or declare const presets: (typeof DATE_RANGE_INTERVAL_PRESETS)[number][] = API_LOGS_PRESETS_BY_RETENTION[retention]; then remove the "as any" on presets when passing to SimpleDateRangePicker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/ui/logs/logs-table.tsx`:
- Line 374: The prop is using an any cast; fix by giving the presets the exact
union type expected by SimpleDateRangePicker: change the type of
API_LOGS_PRESETS_BY_RETENTION (or the local presets variable in logs-table.tsx)
from string[] to (typeof DATE_RANGE_INTERVAL_PRESETS)[number][], e.g. make
API_LOGS_PRESETS_BY_RETENTION: Record<number, (typeof
DATE_RANGE_INTERVAL_PRESETS)[number][]> or declare const presets: (typeof
DATE_RANGE_INTERVAL_PRESETS)[number][] =
API_LOGS_PRESETS_BY_RETENTION[retention]; then remove the "as any" on presets
when passing to SimpleDateRangePicker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db5ea65a-1fac-462f-aa80-bab1b93247bc
📒 Files selected for processing (10)
apps/web/lib/api-logs/api-log-retention.tsapps/web/lib/api-logs/constants.tsapps/web/lib/api-logs/get-api-logs-count.tsapps/web/lib/api-logs/get-api-logs.tsapps/web/lib/api-logs/schemas.tsapps/web/lib/api/payouts/get-eligible-payouts.tsapps/web/ui/logs/logs-table.tsxapps/web/ui/logs/use-log-filters.tsapps/web/ui/partners/confirm-payouts-sheet.tsxapps/web/ui/shared/inline-badge-popover.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/web/lib/api-logs/get-api-logs-count.ts
- apps/web/ui/shared/inline-badge-popover.tsx
- apps/web/lib/api/payouts/get-eligible-payouts.ts
- apps/web/ui/partners/confirm-payouts-sheet.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/ui/logs/use-log-filters.ts
- apps/web/lib/api-logs/schemas.ts
- apps/web/lib/api-logs/constants.ts
- apps/web/lib/api-logs/api-log-retention.ts
2 participants
Changes
/api/logsand/api/logs/countendpoints to accept and processstart,end, andintervalquery parametersSimpleDateRangePickercomponent to logs filters with dynamic preset options based on workspace plan retention daysSummary by CodeRabbit
New Features
Bug Fixes / Improvements