[Dashboard] Fix date handling in analytics components #7885
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7885 +/- ##
=======================================
Coverage 56.54% 56.54%
=======================================
Files 904 904
Lines 58592 58592
Branches 4143 4143
=======================================
Hits 33131 33131
Misses 25355 25355
Partials 106 106
🚀 New features to boost your workflow:
|
size-limit report 📦
|
WalkthroughAdjusted date range end boundary to start of next day in date-range selector. Updated highlights card to aggregate time series by date-only keys across datasets and compute metrics per day with status filtering. Changed default analytics range for wallets chart from last-120 to last-30 days. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Dashboard UI
participant DRS as DateRangeSelector
participant HC as Highlights Card
participant WA as Wallets Analytics Chart
User->>UI: Open analytics views
UI->>DRS: Request default range
DRS-->>UI: { from, to } (to = next day 00:00)
UI->>HC: Fetch/process userStats, volumeStats with {from,to}
HC->>HC: processTimeSeriesData()
note right of HC: Build union of YYYY-MM-DD dates<br/>Aggregate metrics per date<br/>Filter volume by status=completed
HC-->>UI: Time series metrics
UI->>WA: Initialize chart range
alt No explicit props.range
WA->>WA: Use fallback "last-30"
else Explicit range
WA->>WA: Use provided range
end
WA-->>UI: Stats for rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/wallets/analytics/chart/index.tsx (4)
31-38: Type mismatch prevents the new fallback from ever triggering; makerangeoptional here
props.rangeis treated as optional (?? getLastNDaysRange("last-30")) but the type declares it as required viaInAppWalletAnalyticsProps. This will cause TS errors upstream or render the fallback unreachable in strict code. Makerangeoptional specifically on the async props type.Apply this diff:
-type AsyncInAppWalletAnalyticsProps = Omit< - InAppWalletAnalyticsProps, - "stats" | "isPending" -> & { +type AsyncInAppWalletAnalyticsProps = Omit< + InAppWalletAnalyticsProps, + "stats" | "isPending" | "range" +> & { teamId: string; projectId: string; authToken: string; -}; + range?: Range; +};
17-29: Do not over-type UI props; avoid leaking unrelated async props into the UI component
InAppWalletAnalyticsUIdoesn’t useintervalorrange, yet its props type requires them, and it’s invoked with{...props}, which includes unknown keys (teamId,projectId,authToken). This causes JSX prop type errors. Narrow the UI component’s props to what it actually consumes.Apply this diff:
-type InAppWalletAnalyticsProps = { - interval: "day" | "week"; - range: Range; - stats: InAppWalletStats[]; - isPending: boolean; -}; +type InAppWalletAnalyticsProps = { + interval: "day" | "week"; + range: Range; + stats: InAppWalletStats[]; + isPending: boolean; +}; -function InAppWalletAnalyticsUI({ - stats, - isPending, -}: InAppWalletAnalyticsProps) { +function InAppWalletAnalyticsUI({ + stats, + isPending, +}: { + stats: InAppWalletStats[]; + isPending: boolean; +}) {Then, update the call sites (see next comments) to pass only
statsandisPending.
59-66: Pass only the props the UI needs; remove spread of async propsAvoid spreading
AsyncInAppWalletAnalyticsPropsinto the UI (props leakage and type errors). Pass onlystatsandisPending.Apply this diff:
return ( <InAppWalletAnalyticsUI - {...props} - isPending={false} - range={range} - stats={stats} + isPending={false} + stats={stats} /> );
73-75: Fix fallback props to match the narrowed UI prop typeThe fallback should only pass
statsandisPendingnow.Apply this diff:
fallback={ - <InAppWalletAnalyticsUI {...props} isPending={true} stats={[]} /> + <InAppWalletAnalyticsUI isPending={true} stats={[]} /> }
🧹 Nitpick comments (6)
apps/dashboard/src/@/components/analytics/date-range-selector.tsx (2)
89-91: Compute “to” as start of next day using date-fns to avoid DST/locale pitfallsManually mutating Date with setDate/setHours can misbehave across DST transitions. Use date-fns helpers for clarity and correctness.
Apply this diff:
- const todayDate = new Date(); - todayDate.setDate(todayDate.getDate() + 1); // Move to next day - todayDate.setHours(0, 0, 0, 0); // Set to start of next day (00:00) + const todayDate = startOfDay(addDays(new Date(), 1)); // start of next day (local)Outside the selected lines, update the import to include addDays and startOfDay:
import { differenceInCalendarDays, format, subDays, addDays, startOfDay } from "date-fns";
22-25: Preset detection should compare against start of next day (new “to” semantics)Since getLastNDaysRange now sets
toto next day 00:00, the preset matching logic should compare to that same reference instead of “today at 00:00”. Otherwise, custom ranges equivalent to a preset won’t be recognized as such.Apply this diff:
- const matchingRange = - normalizeTime(range.to).getTime() === normalizeTime(new Date()).getTime() - ? durationPresets.find((preset) => preset.days === daysDiff) - : undefined; + const matchingRange = + normalizeTime(range.to).getTime() === normalizeTime(addDays(new Date(), 1)).getTime() + ? durationPresets.find((preset) => preset.days === daysDiff) + : undefined;Note: Ensure
addDaysis imported fromdate-fns(see previous comment). Please verify in the UI that selecting “Last 30 Days” (or similar) correctly highlights the preset rather than falling back to “custom”.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/overview/highlights-card.tsx (4)
108-114: Stabilize ordering and avoid repeated date parsing by introducing a date key helper
- The union of dates is currently unsorted; downstream logic like
trendFnimplicitly relies on chronological order.- You also parse and slice ISO strings repeatedly; introduce a small helper and reuse it.
Apply this diff:
- const dates = [ - ...new Set([ - ...userStats.map((a) => new Date(a.date).toISOString().slice(0, 10)), - ...volumeStats.map((a) => new Date(a.date).toISOString().slice(0, 10)), - ]), - ]; + const toDateKey = (d: string | Date) => new Date(d).toISOString().slice(0, 10); + const dates = Array.from( + new Set([ + ...userStats.map((a) => toDateKey(a.date)), + ...volumeStats.map((a) => toDateKey(a.date)), + ]), + ).sort();
117-117: Deduplicate date normalization using the helperUse the
toDateKeyhelper for consistency and fewer object creations.Apply this diff:
- const activeUsers = userStats - .filter((u) => new Date(u.date).toISOString().slice(0, 10) === date) + const activeUsers = userStats + .filter((u) => toDateKey(u.date) === date) .reduce((acc, curr) => acc + curr.uniqueWalletsConnected, 0); - const newUsers = userStats - .filter((u) => new Date(u.date).toISOString().slice(0, 10) === date) + const newUsers = userStats + .filter((u) => toDateKey(u.date) === date) .reduce((acc, curr) => acc + curr.newUsers, 0); - const volume = volumeStats - .filter( - (v) => - new Date(v.date).toISOString().slice(0, 10) === date && + const volume = volumeStats + .filter( + (v) => + toDateKey(v.date) === date && v.status === "completed", ) .reduce((acc, curr) => acc + curr.amountUsdCents / 100, 0); - const fees = volumeStats - .filter( - (v) => - new Date(v.date).toISOString().slice(0, 10) === date && + const fees = volumeStats + .filter( + (v) => + toDateKey(v.date) === date && v.status === "completed", ) .reduce((acc, curr) => acc + curr.developerFeeUsdCents / 100, 0);Also applies to: 121-121, 127-129, 135-137
85-91: Guard against divide-by-zero in trend calculationIf the first data point is 0,
((data[data.length - 2] / data[0]) - 1)yieldsInfinity. Returnundefined(or 0) to avoid misleading UI.Apply this diff:
- trendFn={(data, key) => - data.filter((d) => (d[key] as number) > 0).length >= 2 - ? ((data[data.length - 2]?.[key] as number) ?? 0) / - ((data[0]?.[key] as number) ?? 0) - - 1 - : undefined - } + trendFn={(data, key) => { + const hasEnough = data.filter((d) => (d[key] as number) > 0).length >= 2; + if (!hasEnough) return undefined; + const numerator = (data[data.length - 2]?.[key] as number) ?? 0; + const denominator = (data[0]?.[key] as number) ?? 0; + if (denominator === 0) return undefined; + return numerator / denominator - 1; + }}
124-139: Optional: pre-index by date to drop repeated scans (O(n) instead of O(n·d))You filter and reduce over
userStats/volumeStatsfour times per date. Consider pre-grouping both arrays bydateKeyonce, then reading sums in O(1) per date. This will materially help when datasets grow.If you want, I can provide a concise refactor using Maps to aggregate active/new users and volume/fees in one pass.
📜 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 selected for processing (3)
apps/dashboard/src/@/components/analytics/date-range-selector.tsx(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/overview/highlights-card.tsx(1 hunks)apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/wallets/analytics/chart/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/wallets/analytics/chart/index.tsxapps/dashboard/src/@/components/analytics/date-range-selector.tsxapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/overview/highlights-card.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/wallets/analytics/chart/index.tsxapps/dashboard/src/@/components/analytics/date-range-selector.tsxapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/overview/highlights-card.tsx
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/wallets/analytics/chart/index.tsxapps/dashboard/src/@/components/analytics/date-range-selector.tsxapps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/overview/highlights-card.tsx
🔇 Additional comments (3)
apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/wallets/analytics/chart/index.tsx (2)
43-43: LGTM: Narrowed default range to last-30Defaulting to a shorter range improves load and relevance for most views.
45-57: Confirm exclusive upper-bound semantics forto
- We normalize
tovianormalizedParams➔normalizeTime, then pass it straight throughbuildSearchParamsinto thev2/wallet/connectsquery.getLastNDaysRange("last-30")(and any customrange.to) is set to the start of the next day.- If the backend treats
toas inclusive, the final day’s data will be duplicated.Please verify in the API contract whether
tois meant as an exclusive upper bound and, if not, adjust either the client or server logic to prevent double-counting.apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/(sidebar)/overview/highlights-card.tsx (1)
108-114: Timezone confirmationYou normalize to UTC day keys via
toISOString().slice(0, 10). This aligns with API timestamps if they are UTC-based. If product expectations are to show metrics in the viewer’s local timezone, this will shift counts on day boundaries.Would you like a follow-up patch to toggle between UTC vs local-day grouping via a single strategy function?
joaquim-verges
deleted the
_Dashboard_Fix_date_handling_in_analytics_components
branch
PR-Codex overview
This PR focuses on modifying date handling and analytics calculations in the
walletsandhighlights-cardcomponents to improve data accuracy and adjust the date range for analytics.Detailed summary
"last-120"to"last-30"inindex.tsx.todayDateto set it to the start of the next day indate-range-selector.tsx.highlights-card.tsxto ensure consistency by usingslice(0, 10)for date comparisons.Summary by CodeRabbit