Connect Overview stats to local storage cache & implement real chart data#18
Connect Overview stats to local storage cache & implement real chart data#18r9jdp wants to merge 3 commits into
Conversation
WalkthroughOverview now reads loading/hasFetched from transactions, computes safe totals and formatted revenue, passes transactions/hasFetched/loading into DashboardChart, generates chart data from real transactions (week/month/year), and updates banners, stat labels, and rendering based on loading and fetch state. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Overview Page (page.tsx)
participant Hook as useTransactions
participant Service as transaction-service
participant Blockchain as Block RPC
participant Chart as DashboardChart
UI->>Hook: mount → request transactions (start loading)
Hook->>Service: fetchStableCoinPurchases()
Service->>Blockchain: fetch event logs / block for each event
Blockchain-->>Service: block data (timestamp)
Service-->>Hook: transactions[] (numeric timestamps)
Hook-->>UI: set transactions, hasFetched=true, loading=false
UI->>Chart: render(transactions, hasFetched, loading)
Chart-->>UI: render chart data (or empty-state if !hasFetched)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)dashboard/app/page.tsx (2)
dashboard/components/dashboard/chart/index.tsx (3)
🔇 Additional comments (4)
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: 4
🧹 Nitpick comments (2)
dashboard/app/page.tsx (1)
22-22: Consider handling the error state.The
useTransactionshook returns anerrorfield (per the hook implementation), but it's not destructured or handled here. While the current implementation works for the happy path, consider adding error handling to improve user experience when fetches fail.Example:
- const { transactions, loading, hasFetched } = useTransactions(); + const { transactions, loading, hasFetched, error } = useTransactions();Then display an error banner when
erroris truthy.dashboard/components/dashboard/chart/index.tsx (1)
103-103: Document the hardcoded fee assumption.The 1% fee calculation is a hardcoded assumption that should be documented or made configurable. This could mislead users if actual fees differ.
Add a comment explaining this is an assumption:
- fees: Math.round(data.revenue * 0.01 * 100) / 100, // Assume 1% fees + fees: Math.round(data.revenue * 0.01 * 100) / 100, // TODO: Replace with actual fee data (currently assumes 1%)Or consider extracting to a constant:
const ESTIMATED_FEE_PERCENTAGE = 0.01; // 1% - placeholder until real fee data is available
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dashboard/app/page.tsx(3 hunks)dashboard/components/dashboard/chart/index.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
dashboard/components/dashboard/chart/index.tsx (3)
dashboard/lib/transaction-service.ts (1)
TransactionEvent(7-15)dashboard/types/dashboard.ts (2)
TimePeriod(30-30)ChartDataPoint(11-16)dashboard/components/ui/tabs.tsx (1)
TabsContent(66-66)
dashboard/app/page.tsx (2)
dashboard/hooks/use-transactions.ts (1)
useTransactions(13-85)dashboard/components/dashboard/chart/index.tsx (1)
DashboardChart(137-333)
🔇 Additional comments (12)
dashboard/app/page.tsx (6)
10-10: LGTM!The LockIcon import and mapping addition is correct and properly integrates with the icon system.
Also applies to: 18-18
32-34: LGTM!The revenue formatting function correctly formats currency with proper locale settings and decimal places.
37-75: LGTM!The stats array properly implements the stale-while-revalidate pattern, showing cached values immediately and falling back to "T/A" only when data has never been fetched.
81-85: LGTM!The conditional header description logic correctly distinguishes between loading, fetched, and initial states, providing clear user feedback.
97-104: LGTM!The loading banner implementation provides excellent UX by showing when cached data is being refreshed. The spinner animation and messaging are clear.
121-121: LGTM!The DashboardChart component is invoked with the correct props matching its updated signature.
dashboard/components/dashboard/chart/index.tsx (6)
14-15: LGTM!The type imports and props interface are well-defined and properly integrate with the component's new data-driven design.
Also applies to: 24-28
46-50: LGTM!The empty transaction check with fallback to placeholder data is a good defensive pattern.
52-82: LGTM!The date bucket initialization logic correctly handles different time periods and formats dates appropriately for each view.
137-145: LGTM!The memoization correctly optimizes chart data generation and properly declares
transactionsas the dependency.
303-306: LGTM!The loading indicator provides good visual feedback when data is being refreshed.
309-330: LGTM!The conditional rendering based on
hasFetchedproperly handles the empty state and shows a helpful message directing users to fetch transactions.
| function getEmptyChartData(period: TimePeriod): ChartDataPoint[] { | ||
| const now = new Date(); | ||
| const data: ChartDataPoint[] = []; | ||
|
|
||
| const daysToShow = period === "week" ? 7 : period === "month" ? 30 : 12; | ||
|
|
||
| for (let i = daysToShow - 1; i >= 0; i--) { | ||
| const date = new Date(now); | ||
| if (period === "year") { | ||
| date.setMonth(date.getMonth() - i); | ||
| data.push({ | ||
| date: date.toLocaleString("en-US", { month: "short" }), | ||
| revenue: 0, | ||
| transactions: 0, | ||
| fees: 0, | ||
| }); | ||
| } else { | ||
| date.setDate(date.getDate() - i); | ||
| data.push({ | ||
| date: `${String(date.getDate()).padStart(2, "0")}/${String(date.getMonth() + 1).padStart(2, "0")}`, | ||
| revenue: 0, | ||
| transactions: 0, | ||
| fees: 0, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return data; | ||
| } |
There was a problem hiding this comment.
Fix inconsistency in year period handling.
getEmptyChartData uses 12 data points for the year period (line 111), while generateChartData uses 365 days (line 69). This inconsistency means the empty state will show a different x-axis structure than populated data.
Apply this diff to align both functions:
function getEmptyChartData(period: TimePeriod): ChartDataPoint[] {
const now = new Date();
const data: ChartDataPoint[] = [];
- const daysToShow = period === "week" ? 7 : period === "month" ? 30 : 12;
+ const daysToShow = period === "week" ? 7 : period === "month" ? 30 : 365;
for (let i = daysToShow - 1; i >= 0; i--) {
const date = new Date(now);
if (period === "year") {
- date.setMonth(date.getMonth() - i);
+ date.setDate(date.getDate() - i);
data.push({
- date: date.toLocaleString("en-US", { month: "short" }),
+ date: date.toLocaleString("en-US", { month: "short" }),
revenue: 0,
transactions: 0,
fees: 0,
});However, note that 365 daily points may be too dense for a year view. Consider using monthly buckets (12 points) in both functions instead.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In dashboard/components/dashboard/chart/index.tsx around lines 107–135,
getEmptyChartData currently emits 12 monthly points for "year" while
generateChartData produces 365 daily points; make them consistent by switching
both functions to use monthly buckets for the year view: change the
daysToShow/iteration logic so "year" uses 12 iterations, use
date.setMonth(date.getMonth() - i) when building each point, format the x-axis
as the short month label (e.g., toLocaleString("en-US",{month:"short"})), and
update generateChartData to aggregate its daily data into 12 monthly buckets
(summing revenue, transactions, fees) so the populated data and empty-state
share the same x-axis structure.
|
Working of the frontend pr4.mp4 |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
dashboard/components/dashboard/chart/index.tsx (1)
125-153: Year period handling is now consistent.Both
generateChartDataandgetEmptyChartDatanow use 12 monthly buckets for the year view, addressing the inconsistency noted in the previous review.
🧹 Nitpick comments (3)
dashboard/lib/transaction-service.ts (2)
58-79: Performance concern: N+1 network calls for block timestamps.Each transaction event now triggers an individual
getBlockcall. For large transaction sets, this creates N additional RPC requests which may:
- Hit rate limits on the public node
- Significantly slow down data loading
Consider batching unique block numbers and fetching them once:
+// Collect unique block numbers +const uniqueBlockNumbers = [...new Set(allEvents.map(e => e.blockNumber))]; +const blockMap = new Map<bigint, bigint>(); + +// Batch fetch blocks (or use Promise.all with deduplication) +await Promise.all(uniqueBlockNumbers.map(async (blockNum) => { + const block = await this.publicClient.getBlock({ blockNumber: blockNum }); + blockMap.set(blockNum, block.timestamp); +})); + const formattedEvents: TransactionEvent[] = await Promise.all(allEvents.map(async event => { // ... existing code ... - const block = await this.publicClient.getBlock({ blockNumber: event.blockNumber }); + const timestamp = blockMap.get(event.blockNumber); return { // ... - timestamp: Number(block.timestamp) + timestamp: timestamp ? Number(timestamp) : undefined }; }));This deduplicates block fetches when multiple transactions share the same block.
68-68: Add error handling for individual block fetches.If
getBlockfails for a specific event (network issue, RPC error), the entirePromise.allwill reject, losing all successfully processed events.-const block = await this.publicClient.getBlock({ blockNumber: event.blockNumber }); +let timestamp: number | undefined; +try { + const block = await this.publicClient.getBlock({ blockNumber: event.blockNumber }); + timestamp = Number(block.timestamp); +} catch (err) { + console.warn(`Failed to fetch block ${event.blockNumber}:`, err); +}Then use
timestampinstead ofNumber(block.timestamp)on line 77.dashboard/components/dashboard/chart/index.tsx (1)
104-104: Potential off-by-one in day boundary calculation.
daysDiffusesMath.floorwhich may cause edge cases around midnight due to timezone differences betweennowandtxDate. A transaction at 11:59 PM today could show as "1 day ago" ifnowis early morning.Consider normalizing both dates to midnight:
+const normalizeToMidnight = (d: Date) => new Date(d.getFullYear(), d.getMonth(), d.getDate()); +const normalizedNow = normalizeToMidnight(now); +const normalizedTxDate = normalizeToMidnight(txDate); -const daysDiff = Math.floor((now.getTime() - txDate.getTime()) / (1000 * 60 * 60 * 24)); +const daysDiff = Math.floor((normalizedNow.getTime() - normalizedTxDate.getTime()) / (1000 * 60 * 60 * 24));This is a minor edge case but worth noting for accuracy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dashboard/app/page.tsx(3 hunks)dashboard/components/dashboard/chart/index.tsx(4 hunks)dashboard/lib/transaction-service.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
dashboard/components/dashboard/chart/index.tsx (2)
dashboard/lib/transaction-service.ts (1)
TransactionEvent(7-15)dashboard/types/dashboard.ts (2)
TimePeriod(30-30)ChartDataPoint(11-16)
dashboard/app/page.tsx (3)
dashboard/hooks/use-transactions.ts (1)
useTransactions(13-85)dashboard/components/dashboard/layout/index.tsx (1)
DashboardPageLayout(13-37)dashboard/components/dashboard/chart/index.tsx (1)
DashboardChart(155-351)
🔇 Additional comments (5)
dashboard/app/page.tsx (2)
96-103: Good UX: loading banner for stale-while-revalidate pattern.Clean implementation showing cached values while refreshing. The spinner and message clearly communicate the state to users.
119-121: Props correctly wired to DashboardChart.The transactions, hasFetched, and loading props are properly passed, enabling the chart to react to loading states and display real data.
dashboard/components/dashboard/chart/index.tsx (3)
46-50: Well-structured chart data generation.Good defensive handling with early return to
getEmptyChartDatawhen no transactions exist, preventing crashes and ensuring the chart always renders properly.
158-163: Good use of useMemo for chart data.Memoizing chart data generation with
[transactions]dependency prevents unnecessary recalculations when toggling tabs or during re-renders unrelated to transaction changes.
321-323: Nice loading indicator integration.The spinner in the legend area provides subtle feedback without disrupting the chart layout.
|
Hey @r9jdp Can you resolve merge conflicts so that i could merge this PR. |
2 participants
This PR addresses the requirement to show cached statistics immediately on the Overview tab while fetching fresh data in the background. It also replaces the static mock data in the main chart with real transaction data.
Fixes #5
Changes made:
Technical Implementation:
page.tsx: Wired up the useTransactions loading state. The UI now gracefully handles the "stale-while-revalidate" pattern showing cached data while loading is true.
index.tsx (stat): Wrapped the data generation in useMemo to prevent unnecessary re-renders when toggling between timeframes. Added handling for empty states (e.g., if a user has 0 transactions, the graph renders a flat line instead of crashing).
pr3.mp4
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.