feat: add scan history analytics dashboard with Recharts#146
feat: add scan history analytics dashboard with Recharts#146arcgod-design wants to merge 2 commits into
Conversation
|
Someone is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
🎉 Thank you for your Pull Request! We're thrilled to have your contribution to FreshScan AI. Before we review, please make sure you have:
A maintainer will review your code as soon as possible! |
📝 WalkthroughWalkthroughAdds a new ChangesAnalytics Dashboard Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ResultsPage
participant AnalyticsPage
participant api
participant ScanHistory
User->>ResultsPage: view results (totalScans > 0)
ResultsPage->>User: show "View Analytics Dashboard" link
User->>AnalyticsPage: navigate to /analytics
AnalyticsPage->>api: getScanHistory(100, 0)
api->>ScanHistory: fetch stored scans
ScanHistory-->>api: scan records
api-->>AnalyticsPage: scans or error
AnalyticsPage->>AnalyticsPage: filter by timeRange and compute chart data
AnalyticsPage-->>User: render charts and summary stats
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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.
🧹 Nitpick comments (3)
src/pages/AnalyticsPage.tsx (3)
117-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftHardcoded English strings break the app's i18n convention.
ResultsPagetranslates every label viat('results.*'), but this page hardcodes nearly all UI copy ("Total Scans", "Loading analytics...", "Back to Results", chart titles, "Run a Scan", time-range labels, etc.) — onlyerrorKeyis translated. This will leave the Analytics page untranslated when the locale changes.Also applies to: 228-350
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AnalyticsPage.tsx` around lines 117 - 224, The AnalyticsPage UI copy is still hardcoded in English, which breaks the app’s i18n pattern used elsewhere like ResultsPage. Update the strings in AnalyticsPage and its related render blocks to use translation keys via t(...) for labels, buttons, status messages, chart titles, and time-range options, and keep the existing errorKey translation flow intact so the page fully localizes when the locale changes.
39-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated scan-history fetch/error logic with
ResultsPage.This load function mirrors
ResultsPage'suseEffect/load()almost line-for-line (same try/catch/error-key pattern). Worth extracting into a shared hook (e.g.,useScanHistory(limit, offset)) to avoid drift between the two pages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AnalyticsPage.tsx` around lines 39 - 56, The scan-history loading logic in AnalyticsPage is duplicated from ResultsPage, so extract the shared fetch/error handling into a reusable hook such as useScanHistory(limit, offset). Move the current load() behavior from the useEffect in AnalyticsPage (and the matching logic in ResultsPage) into that hook, preserving the setLoading/setScans/setErrorKey pattern and the error.message.startsWith('error.') handling. Then update both pages to consume the shared hook instead of keeping separate near-identical useEffect implementations.
254-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTooltip
contentStyleduplicated across three charts.Same style object is repeated verbatim for the Pie, Bar, and Line chart tooltips. Extract into a shared constant to avoid drift if the theme changes.
♻️ Example
+const TOOLTIP_STYLE = { + background: '`#1a1a1a`', + border: '1px solid rgba(255,255,255,0.1)', + borderRadius: 0, + fontFamily: 'var(--font-mono)', + fontSize: '0.625rem', +};Then reuse
contentStyle={TOOLTIP_STYLE}in each<Tooltip />.Also applies to: 294-302, 329-337
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AnalyticsPage.tsx` around lines 254 - 262, The Tooltip contentStyle is duplicated across the Pie, Bar, and Line chart sections in AnalyticsPage, so extract the shared style object into a single reusable constant and reference it from each Tooltip. Update the repeated Tooltip usages to use that shared constant (for example via the existing Tooltip components in AnalyticsPage) so the style stays consistent and only needs to change in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/pages/AnalyticsPage.tsx`:
- Around line 117-224: The AnalyticsPage UI copy is still hardcoded in English,
which breaks the app’s i18n pattern used elsewhere like ResultsPage. Update the
strings in AnalyticsPage and its related render blocks to use translation keys
via t(...) for labels, buttons, status messages, chart titles, and time-range
options, and keep the existing errorKey translation flow intact so the page
fully localizes when the locale changes.
- Around line 39-56: The scan-history loading logic in AnalyticsPage is
duplicated from ResultsPage, so extract the shared fetch/error handling into a
reusable hook such as useScanHistory(limit, offset). Move the current load()
behavior from the useEffect in AnalyticsPage (and the matching logic in
ResultsPage) into that hook, preserving the setLoading/setScans/setErrorKey
pattern and the error.message.startsWith('error.') handling. Then update both
pages to consume the shared hook instead of keeping separate near-identical
useEffect implementations.
- Around line 254-262: The Tooltip contentStyle is duplicated across the Pie,
Bar, and Line chart sections in AnalyticsPage, so extract the shared style
object into a single reusable constant and reference it from each Tooltip.
Update the repeated Tooltip usages to use that shared constant (for example via
the existing Tooltip components in AnalyticsPage) so the style stays consistent
and only needs to change in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 159e922f-1aca-4eda-a32b-4a34f03ca1dc
📒 Files selected for processing (4)
package.jsonsrc/App.tsxsrc/pages/AnalyticsPage.tsxsrc/pages/ResultsPage.tsx
|
Great work on this! The architecture is clean, scope is perfectly scoped, and the recharts implementation matches the design system well. A few things to fix before we merge: Light mode compatibility — The and XAxis/YAxis tick fill props have hardcoded dark colors (#1a1a1a, rgba(255,255,255,0.4)). Please replace them with CSS variables: var(--color-surface-mid), var(--color-on-surface), var(--color-on-surface-variant) so charts look correct in light mode too. i18n — The dashboard strings ("Total Scans", "Grade Distribution", "Scan Volume", etc.) are hardcoded English. Please wrap them with t('analytics.totalScans') etc. and add the keys to src/i18n/locales/en.json, hi.json, and bn.json. Date sort — In freshnessData, Object.entries(byDate) doesn't guarantee chronological order. Add .sort((a, b) => a[0].localeCompare(b[0])) before .reverse() to ensure the line chart plots correctly. Fix these three things and this is ready to merge! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/AnalyticsPage.tsx (3)
178-183: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHeader title/subtitle still hardcoded, contradicting the PR's i18n fix.
"Scan Analytics"and"Freshness trends and grade distribution across your scans"are plain string literals, not routed throught(), unlike every other user-facing string on this page. The addedanalytics.*keys inen.json/hi.json/bn.jsondon't include entries for these two strings either, so this is an incomplete i18n pass.Add the corresponding keys to `en.json`, `hi.json`, and `bn.json`.✏️ Proposed fix
<h1 className="text-3xl md:text-5xl font-bold tracking-tight font-[family-name:var(--font-display)]"> - Scan <span className="text-neon">Analytics</span> + {t('analytics.pageTitlePrefix')} <span className="text-neon">{t('analytics.pageTitleHighlight')}</span> </h1> <p className="text-on-surface-variant font-[family-name:var(--font-mono)] text-[0.625rem] tracking-widest mt-2"> - Freshness trends and grade distribution across your scans + {t('analytics.subtitle')} </p>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AnalyticsPage.tsx` around lines 178 - 183, The AnalyticsPage header still uses hardcoded strings, so replace the “Scan Analytics” title and subtitle in AnalyticsPage with t()-backed keys and add matching analytics.* entries for both strings in en.json, hi.json, and bn.json. Use the existing localization pattern on this page to locate the header block and ensure the new keys are wired into the same translation namespace as the other analytics text.
97-114: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSort the entries before mapping, and keep a sortable date key separate from the display label.
a[0]isundefinedafter.map(), so this throws as soon as there are 2+ date buckets. Even with the sort moved earlier,formatDate()returnsd/m, which won’t order chronologically across month/day boundaries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AnalyticsPage.tsx` around lines 97 - 114, The freshnessData memo in AnalyticsPage is sorting on the mapped objects incorrectly and using a display-only date string that cannot be ordered chronologically. Update the logic around useMemo and formatDate usage so you keep a separate sortable date key from the rendered label, sort the raw entries before mapping, and then map to the chart shape; this avoids the undefined sort access and preserves correct date order across month/day boundaries.
116-128: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the
sort()comparator in both time-series datasets —Object.entries(...).map(...)turns the entries into objects before sorting, soa[0]isundefinedand the comparator throws once there are multiple dates. Sort the raw entries first, then map to{ date, scans }and{ date, avgFreshness }.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/AnalyticsPage.tsx` around lines 116 - 128, The time-series sorting in AnalyticsPage’s volumeData (and the matching freshness dataset) is using a comparator on mapped objects, so a[0] and b[0] are undefined and sorting fails when multiple dates exist. Fix the logic by sorting the raw Object.entries results before mapping them into { date, scans } and { date, avgFreshness }, keeping the sort comparator on the tuple entries rather than the transformed objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/pages/AnalyticsPage.tsx`:
- Around line 178-183: The AnalyticsPage header still uses hardcoded strings, so
replace the “Scan Analytics” title and subtitle in AnalyticsPage with t()-backed
keys and add matching analytics.* entries for both strings in en.json, hi.json,
and bn.json. Use the existing localization pattern on this page to locate the
header block and ensure the new keys are wired into the same translation
namespace as the other analytics text.
- Around line 97-114: The freshnessData memo in AnalyticsPage is sorting on the
mapped objects incorrectly and using a display-only date string that cannot be
ordered chronologically. Update the logic around useMemo and formatDate usage so
you keep a separate sortable date key from the rendered label, sort the raw
entries before mapping, and then map to the chart shape; this avoids the
undefined sort access and preserves correct date order across month/day
boundaries.
- Around line 116-128: The time-series sorting in AnalyticsPage’s volumeData
(and the matching freshness dataset) is using a comparator on mapped objects, so
a[0] and b[0] are undefined and sorting fails when multiple dates exist. Fix the
logic by sorting the raw Object.entries results before mapping them into { date,
scans } and { date, avgFreshness }, keeping the sort comparator on the tuple
entries rather than the transformed objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bd0f81db-1af0-4dde-b3cc-cde556920c21
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/i18n/locales/bn.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/hi.jsonsrc/pages/AnalyticsPage.tsx
|
Closes #44 |
|
Hey @jpdevhub, this PR has been ready for review for a while -- offering this friendly bump. The implementation matches the issue's specified behavior and CI is passing. PR: #146 When you have a moment, a review would be much appreciated. I am happy to iterate on any feedback. If the timing is not right, no worries at all -- just a gentle nudge from a contributor who has been waiting patiently. � |
2 participants
Description
Adds a scan history analytics dashboard with interactive charts to visualize freshness trends, grade distribution, and scan volume.
Changes
AnalyticsPage.tsxwith three interactive charts using Recharts/analyticsroute to App.tsxDesign
GlassCardcomponents--font-display,--font-mono)lg:grid-cols-2for chartsTesting
Closes #44
Summary by CodeRabbit
/analyticsroute for direct access to the dashboard.