Claude/add quick overview #711
Conversation
…rypoint.sh is executable Prevents 'exec /docker-entrypoint.sh: no such file or directory' on Windows checkouts by stripping CRLF (sed) and chmod +x in the runtime image.
… stats This commit implements a comprehensive 30,000-foot view of all monitored domains, enabling users to see all site metrics at a glance. ## Backend Changes - Enhanced `getSitesFromOrg` API endpoint to support optional detailed metrics - Added query parameters: `includeMetrics` (boolean) and `timePeriod` (24h/7d/30d) - Implemented comprehensive analytics aggregation across all sites including: - Users, Sessions, Pageviews - Bounce Rate, Session Duration, Pages per Session - Period-over-period comparison calculations - Optimized queries to only fetch detailed metrics when requested ## Frontend Changes - Created `SitesOverviewTable` component with: - Sortable columns for all key metrics - Period-over-period change indicators with color coding - Totals row showing aggregate statistics - Responsive design with horizontal scroll - Created `SitesSummaryStats` component displaying: - Total sites, users, sessions, pageviews - Average bounce rate and session duration - Trend indicators comparing to previous period - Enhanced home page with: - View toggle between Cards and Table modes - Time period selector (24h, 7d, 30d) - LocalStorage persistence for user preferences - Conditional data fetching based on view mode ## Type Updates - Added `SiteMetrics` type with all metric fields and change percentages - Updated `GetSitesFromOrgResponse` to include optional metrics - Modified `useGetSitesFromOrg` hook to accept options for metrics and time period ## Features - One-click switching between card and table views - Flexible time period selection with period-over-period comparison - Sortable table columns for easy data analysis - Visual indicators for metric trends (up/down arrows with color coding) - Total aggregation across all sites for quick overview - Persistent user preferences across sessions
|
@claude is attempting to deploy a commit to the goldflag's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR adds metrics retrieval and multi-view display for sites. The backend now supports optional query parameters ( Changes
Sequence DiagramsequenceDiagram
participant User
participant Page as page.tsx
participant Hook as useGetSitesFromOrg
participant Backend as getSitesFromOrg
participant DB as Database
User->>Page: Select time period / view mode
Note over Page: Persist state to localStorage
Page->>Hook: Call with includeMetrics & timePeriod
Hook->>Backend: GET /sites?includeMetrics=true&timePeriod=7d
Backend->>DB: Fetch sites & ownership
alt includeMetrics = true
Backend->>DB: Fetch metrics (current period)
Backend->>DB: Fetch metrics (previous period)
Note over Backend: Calculate deltas (usersChange, etc.)
end
Backend-->>Hook: Return sites with metrics
Hook-->>Page: Update query cache
alt viewMode = "table"
Page->>SitesOverviewTable: Render with metrics
Page->>SitesSummaryStats: Render aggregated stats
else viewMode = "cards"
Page->>SiteCard: Render site cards
end
SitesOverviewTable-->>User: Sortable table + totals
SitesSummaryStats-->>User: Summary stats grid
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/api/sites/getSitesFromOrg.ts (1)
62-223: Session-level metrics (bounce rate, pages/session, session duration) are computed incorrectly over events instead of sessionsIn both the current and previous period metrics queries, you:
- Build
AllSessionPageviewsandSessionStats(one row per site+session withtotal_pageviews_in_session,start_time,end_time).- Then do
FROM events e LEFT JOIN SessionStats ss USING (site_id, session_id)and aggregate:AVG(total_pageviews_in_session) AS pages_per_session, sumIf(1, total_pageviews_in_session = 1) / COUNT(DISTINCT session_id) * 100 AS bounce_rate, AVG(end_time - start_time) AS session_durationBecause the join duplicates each
SessionStatsrow once per event in that session:
AVG(total_pageviews_in_session)andAVG(end_time - start_time)become event‑weighted, not session‑weighted averages.sumIf(1, total_pageviews_in_session = 1)counts one per event in a “bounce” session (not one per session), so if a bounce session has multiple non‑pageview events, the numerator can exceedCOUNT(DISTINCT session_id)andbounce_ratecan go over 100%.This significantly distorts the metrics the UI relies on.
A safer pattern is to:
- Aggregate once per session in
SessionStats.- Compute session‑level aggregates (sessions count, pages/session, bounce rate, session duration) in a separate CTE over
SessionStats.- Join that with an events‑level aggregate only for things that must be per‑event (users, pageviews).
Sketch:
- const metricsResult = await clickhouse.query({ - query: ` - WITH - AllSessionPageviews AS (...), - SessionStats AS (...) - SELECT - site_id, - COUNT(DISTINCT session_id) AS sessions, - COUNT(DISTINCT CASE WHEN type = 'pageview' THEN session_id END) AS pageview_sessions, - uniqExact(user_id) AS users, - countIf(type = 'pageview') AS pageviews, - AVG(total_pageviews_in_session) AS pages_per_session, - sumIf(1, total_pageviews_in_session = 1) / COUNT(DISTINCT session_id) * 100 AS bounce_rate, - AVG(end_time - start_time) AS session_duration - FROM events e - LEFT JOIN SessionStats ss USING (site_id, session_id) - WHERE - site_id IN (${siteIds.join(",")}) - AND timestamp >= now() - INTERVAL ${currentInterval} - GROUP BY site_id - `, - format: "JSONEachRow", - }); + const metricsResult = await clickhouse.query({ + query: ` + WITH + AllSessionPageviews AS (...), + SessionStats AS (...), + SessionAgg AS ( + SELECT + site_id, + count() AS sessions, + avg(total_pageviews_in_session) AS pages_per_session, + sumIf(total_pageviews_in_session = 1, 1) / count() * 100 AS bounce_rate, + avg(end_time - start_time) AS session_duration + FROM SessionStats + GROUP BY site_id + ), + EventAgg AS ( + SELECT + site_id, + uniqExact(user_id) AS users, + countIf(type = 'pageview') AS pageviews + FROM events + WHERE + site_id IN (${siteIds.join(",")}) + AND timestamp >= now() - INTERVAL ${currentInterval} + GROUP BY site_id + ) + SELECT + ea.site_id, + sa.sessions, + ea.users, + ea.pageviews, + sa.pages_per_session, + sa.bounce_rate, + sa.session_duration + FROM SessionAgg sa + LEFT JOIN EventAgg ea USING (site_id) + `, + format: "JSONEachRow", + });You’d apply the same pattern to the previous‑period query. Exact SQL may need tweaking, but the key point is: do session‑level metrics purely from one‑row‑per‑session data, and only join in events for distinct users/pageviews.
🧹 Nitpick comments (4)
client/src/components/SitesSummaryStats.tsx (2)
15-69: Averages and % changes are biased by including sites without metrics in the denominator
totals.countis incremented for every site, while metrics fields use(site.metrics?....) ?? 0. For sites without metrics this effectively treats “no data” as0, so:
avgBounceRate,avgSessionDuration,avgPagesPerSessionusersChange,sessionsChange,pageviewsChangeare all pulled down if some sites have no metrics yet.
If the intent is “averages across sites with data”, a more accurate approach is to only increment the counter when
site.metricsis present.Example change:
- const totals = sites.reduce( - (acc, site) => ({ - users: acc.users + (site.metrics?.users ?? 0), - sessions: acc.sessions + (site.metrics?.sessions ?? 0), - pageviews: acc.pageviews + (site.metrics?.pageviews ?? 0), - bounceRate: acc.bounceRate + (site.metrics?.bounceRate ?? 0), - sessionDuration: acc.sessionDuration + (site.metrics?.sessionDuration ?? 0), - pagesPerSession: acc.pagesPerSession + (site.metrics?.pagesPerSession ?? 0), - usersChange: acc.usersChange + (site.metrics?.usersChange ?? 0), - sessionsChange: acc.sessionsChange + (site.metrics?.sessionsChange ?? 0), - pageviewsChange: acc.pageviewsChange + (site.metrics?.pageviewsChange ?? 0), - count: acc.count + 1, - }), + const totals = sites.reduce( + (acc, site) => { + const m = site.metrics; + if (!m) return acc; + return { + users: acc.users + m.users, + sessions: acc.sessions + m.sessions, + pageviews: acc.pageviews + m.pageviews, + bounceRate: acc.bounceRate + m.bounceRate, + sessionDuration: acc.sessionDuration + m.sessionDuration, + pagesPerSession: acc.pagesPerSession + m.pagesPerSession, + usersChange: acc.usersChange + m.usersChange, + sessionsChange: acc.sessionsChange + m.sessionsChange, + pageviewsChange: acc.pageviewsChange + m.pageviewsChange, + count: acc.count + 1, + }; + }, { users: 0, sessions: 0, pageviews: 0, bounceRate: 0, sessionDuration: 0, pagesPerSession: 0, usersChange: 0, sessionsChange: 0, pageviewsChange: 0, count: 0, } );You’d keep
totalSites: sites.lengthas-is so the card still shows total sites irrespective of metrics.
72-84: TightenStatCardicon typing for better safety and DX
iconis currently typed asany, which loses type safety and editor help. You can narrow this to the Lucide/React component shape you actually use:- const StatCard = ({ - icon: Icon, + const StatCard = ({ + icon: Icon, @@ - }: { - icon: any; + }: { + icon: React.ComponentType<{ size?: number; className?: string }>; label: string; value: string | number; change?: number; suffix?: string; }) => ((With a corresponding
import type React from "react";or using the existing React import pattern in the project.)client/src/components/SitesOverviewTable.tsx (1)
102-133: Totals row averages are skewed by including sites without metricsIn the totals calculation,
countis incremented for every site, while metrics use(site.metrics?....) ?? 0. This means sites with no metrics are effectively treated as zeroes when computing:
avgSessionDurationavgPagesPerSessionavgBounceRateIf you want “averages across sites with data”, the denominator should be the number of sites that have
metrics, not the total number of sites.You can mirror the pattern suggested for
SitesSummaryStats:- const total = sites.reduce( - (acc, site) => ({ - users: acc.users + (site.metrics?.users ?? 0), - sessions: acc.sessions + (site.metrics?.sessions ?? 0), - pageviews: acc.pageviews + (site.metrics?.pageviews ?? 0), - sessionDuration: acc.sessionDuration + (site.metrics?.sessionDuration ?? 0), - pagesPerSession: acc.pagesPerSession + (site.metrics?.pagesPerSession ?? 0), - bounceRate: acc.bounceRate + (site.metrics?.bounceRate ?? 0), - count: acc.count + 1, - }), + const total = sites.reduce( + (acc, site) => { + const m = site.metrics; + if (!m) return acc; + return { + users: acc.users + m.users, + sessions: acc.sessions + m.sessions, + pageviews: acc.pageviews + m.pageviews, + sessionDuration: acc.sessionDuration + m.sessionDuration, + pagesPerSession: acc.pagesPerSession + m.pagesPerSession, + bounceRate: acc.bounceRate + m.bounceRate, + count: acc.count + 1, + }; + }, { users: 0, sessions: 0, pageviews: 0, sessionDuration: 0, pagesPerSession: 0, bounceRate: 0, count: 0, } );This keeps the totals row aligned with how you compute averages in
SitesSummaryStats.client/src/app/page.tsx (1)
28-39: Consider validating persistedviewMode/timePeriodvalues from localStorage
localStorage.getItem(...)values are cast toViewMode/TimePeriodand applied directly if truthy. If those keys ever get corrupted (e.g. manual edits, old values), state could end up with an invalid string and the UI would still try to use it.You could defensively guard the assignments:
useEffect(() => { setIsClient(true); - const savedViewMode = localStorage.getItem("sitesViewMode") as ViewMode; - const savedTimePeriod = localStorage.getItem("sitesTimePeriod") as TimePeriod; - if (savedViewMode) setViewMode(savedViewMode); - if (savedTimePeriod) setTimePeriod(savedTimePeriod); + const savedViewMode = localStorage.getItem("sitesViewMode"); + if (savedViewMode === "cards" || savedViewMode === "table") { + setViewMode(savedViewMode); + } + const savedTimePeriod = localStorage.getItem("sitesTimePeriod"); + if (savedTimePeriod === "24h" || savedTimePeriod === "7d" || savedTimePeriod === "30d") { + setTimePeriod(savedTimePeriod as TimePeriod); + } }, []);Not critical, but it makes the state more resilient to bad persisted values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/src/api/admin/sites.ts(3 hunks)client/src/app/page.tsx(2 hunks)client/src/components/SitesOverviewTable.tsx(1 hunks)client/src/components/SitesSummaryStats.tsx(1 hunks)server/Dockerfile(1 hunks)server/src/api/sites/getSitesFromOrg.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
client/src/api/admin/sites.ts (1)
client/src/api/utils.ts (1)
authedFetch(54-96)
client/src/components/SitesOverviewTable.tsx (3)
client/src/api/admin/sites.ts (1)
GetSitesFromOrgResponse(46-82)client/src/components/Favicon.tsx (1)
Favicon(4-29)client/src/lib/dateTimeUtils.ts (1)
formatDuration(75-97)
client/src/components/SitesSummaryStats.tsx (2)
client/src/api/admin/sites.ts (1)
GetSitesFromOrgResponse(46-82)client/src/lib/dateTimeUtils.ts (1)
formatDuration(75-97)
client/src/app/page.tsx (3)
client/src/api/admin/sites.ts (1)
useGetSitesFromOrg(84-103)client/src/components/SitesSummaryStats.tsx (1)
SitesSummaryStats(14-151)client/src/components/SitesOverviewTable.tsx (1)
SitesOverviewTable(34-333)
server/src/api/sites/getSitesFromOrg.ts (2)
server/src/db/clickhouse/clickhouse.ts (1)
clickhouse(4-8)server/src/api/analytics/utils.ts (1)
processResults(67-85)
🔇 Additional comments (2)
server/Dockerfile (1)
43-45: Excellent defensive fix for cross-platform line endings.Normalizing CRLF to LF before execution is a solid best practice. This ensures the entrypoint script executes reliably on Alpine Linux containers regardless of the platform where the code was checked out. The sed command is clean, the && chaining is correct, and the updated comment clearly documents the intent.
client/src/api/admin/sites.ts (1)
31-44: Client metrics types and query options look consistent with the server
SiteMetricsmatches the fields populated in the API,metrics?: SiteMetricsis correctly optional on each site, anduseGetSitesFromOrg’sincludeMetrics/timePeriodoptions are wired into both thequeryKeyand the URL params with sensible defaults, so existing call sites keep 24h behavior unless they opt in.Also applies to: 72-99
Vermino
deleted the
claude/add-quick-overview-01RpzwWPCZku5FLy4mdhRvPV
branch
2 participants
Backend Enhancements (server/src/api/sites/getSitesFromOrg.ts)
Frontend Components
Summary by CodeRabbit