feat(analytics): rebuild the creator dashboard on the authorized query - #6694
Conversation
The `/analytics` page read lifetime totals under an "Overview (last 45 days)" heading, re-bucketed UTC history rows in the viewer's timezone, and carried three separate copies of the number 45. Every number on it was either mislabelled or shifted by a day for anyone far from UTC. Rebuilds it on `creatorPerformance` and `creatorPostPerformance`, which report their own period semantics and coverage: - 30/90-day selection drives the overview, the chart and the table - Impressions, upvotes and comments are captioned with the selected window; outbound visits is captioned "All time", because no daily breakdown exists to scope clicks and calling it 30 days would be a lie - A null value renders an em dash and the reason, never a zero - A comparison is drawn only when the prior window is fully covered; a zero baseline renders "New" rather than a percentage off nothing - The chart's axis starts where measurement starts, so partial history reads as a shorter chart with a sentence, not a collapse to zero - The article table is a semantic table with sortable headers carrying `aria-sort`, and the comment count links into the discussion - Overview and table fail and retry independently, so a broken table leaves the headline numbers on screen Followers and reputation are dropped: the authorized query does not cover them, and sourcing them from the old lifetime query would put uncaptioned all-time numbers back under a period heading. Adds an achievements section that ENG-2127 can populate without moving anything else, showing an empty state until then rather than sample badges nobody earned. `UserPostsAnalyticsTable` went with the old page; it had no other caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are counters the product already has and already showed; dropping them when the page moved to the period-scoped contract lost real information for no reason other than that the new query does not carry them. They come back from the existing `userPostsAnalytics` row, which needs no API change, and render through the same tile as everything else: a null value shows the reason rather than a zero, so a failed lookup never reads as "nobody follows you". Neither has a daily grain, so the tiles are grouped under an explicit "All time" heading rather than mixed into the period grid. Outbound visits moves there too — it was always a lifetime number and only sat in the period row because that is where the old four-tile layout put it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rebelchris
left a comment
rebelchris
left a comment
There was a problem hiding this comment.
Summary
The rebuild reads well: the period/lifetime split, null-vs-zero handling, coverage-driven chart axis and the semantic sortable table are all traced and tested, and the new components are dumb with policy kept in the page. Link is legacyBehavior so the <a> children are fine, DataTile.value already accepts a ReactNode, and the query keys carry period/sort so cache identity is correct. I could not verify the API-side semantics of creatorPostPerformance beyond reading authoredPostsWhere (posts are not filtered by period, so the empty state is only reached for a creator with no posts, which is the intended meaning).
One merge-ordering blocker and a handful of non-blocking items are inline. Not approving here; the live checks the PR lists as "Not verified" (mobile/desktop, keyboard, partial-history creator) still need a pass on the preview and a screenshot or two would let a reviewer close them out.
Blocking
packages/shared/src/graphql/creatorAnalytics.ts:147—impressionsSeriesmakes the wholecreatorPerformancequery fail until daily-api#4287 (still open) is deployed. See inline.
Non-blocking
- Empty Achievements section shipped to every creator before ENG-2127 has data.
Last refreshedrenders in the viewer's local timezone right after a sentence that says UTC.- Dead shared exports left behind (
USER_POSTS_ANALYTICS_HISTORY_QUERY,USER_POSTS_WITH_ANALYTICS_QUERY, their node types andRequestKeys). - Period switch blanks the overview and chart to skeletons and disables the trigger;
placeholderData: keepPreviousDatawould keep the numbers on screen while the new window loads. - Small a11y/copy nits on the em-dash
aria-label,aria-currenton a plain menu item, a staleColumn.numericdocblock and the outbound-visits unknown reason.
Verification
- Read the review guide and repo conventions
- Scope matches ENG-2126;
UserPostsAnalyticsTabledeletion confirmed callerless - Loading/empty/error/refetch paths traced for the three queries
- Shared consumers:
creatorAnalytics.tsadditions are additive;DataTile/Link/DropdownMenuused within their existing contracts - CI green (lint, typecheck_strict_changed, test_webapp, test_shared, build_extension)
- Manual check on the preview at phone width (table
min-w-[42rem]insideoverflow-x-auto, tile grid at 2 cols) and keyboard sort still pending, as the PR says
Reviewed by AI.
| comments { | ||
| ...CreatorMetricFragment | ||
| } | ||
| impressionsSeries { |
There was a problem hiding this comment.
Blocking (merge order, not code): daily-api#4287 is still open. Once this lands and deploys ahead of the API, creatorPerformance fails GraphQL validation on the unknown impressionsSeries field, so for every creator the overview tiles go to CreatorDashboardError, the chart section is hidden, and only the article table and the followers/reputation tiles survive. The PR description already calls this out; please hold the merge until #4287 is deployed, or split the impressionsSeries selection into a follow-up that lands after it.
Reviewed by AI.
| {periodLabel[period]}, through {formatCoverageDate(coverage.endDate)}{' '} | ||
| (UTC). Today is still being counted and is not included. | ||
| {updatedAt && | ||
| ` Last refreshed ${new Date(updatedAt).toLocaleString()}.`} |
There was a problem hiding this comment.
Non-blocking: the sentence just before this says (UTC), and this appends Last refreshed <local time> via toLocaleString() with no zone marker and in the browser's default long format (e.g. 9/21/2026, 3:04:11 PM). That reintroduces the mixed-timezone reading the PR set out to remove. Either format it in UTC to match the coverage date (there is already formatCoverageDate here, or formatDate from lib/dateFormat), label the zone, or drop the clause.
Reviewed by AI.
| <Divider className={dividerClassName} /> | ||
| <SectionContainer> | ||
| <SectionHeader>Achievements</SectionHeader> | ||
| <CreatorAchievementsSection isEmpty /> |
There was a problem hiding this comment.
Non-blocking: with isEmpty hardcoded, every creator now gets a divider plus a permanent "No achievements yet" box until ENG-2127 ships. The scaffolded component is fine to keep in the tree, but I would not mount it on the page before it can ever show something; an always-empty section reads as a broken feature rather than a placeholder. Mount it in the ENG-2127 PR (or behind that flag) instead.
Reviewed by AI.
| <CreatorPeriodSelect | ||
| period={period} | ||
| onChange={setPeriod} | ||
| disabled={isOverviewPending} |
There was a problem hiding this comment.
Non-blocking (UX): every period switch produces a new query key with no cached data, so isPerformancePending flips true, the whole overview grid and the chart swap to skeletons and this trigger disables. Passing placeholderData: keepPreviousData to the performance query (and using isPlaceholderData for a subtle loading treatment) would keep the current numbers on screen while the new window loads. Also isLifetimePending gates the period tiles even though followers/reputation do not depend on the period; if the lifetime query is slow the impressions/upvotes/comments tiles wait on it for no reason.
Reviewed by AI.
| * `creatorPerformance`, so nothing in the period contract has to pretend they | ||
| * belong to the selected window. | ||
| */ | ||
| export const creatorLifetimeTotalsQueryOptions = ({ |
There was a problem hiding this comment.
Non-blocking: with UserPostsAnalyticsTable and the old page gone, USER_POSTS_ANALYTICS_HISTORY_QUERY, USER_POSTS_WITH_ANALYTICS_QUERY, UserPostsAnalyticsHistoryNode, UserPostWithAnalytics and the RequestKey.UserPostsAnalyticsHistory / UserPostsWithAnalytics entries have no callers in the monorepo (the PR notes this). Convention here is to remove dead exports in the same PR rather than leave a follow-up; it is a pure deletion and keeps graphql/users.ts honest about what the client actually asks for. Separately, this options builder pulls the full userPostsAnalytics row for two fields; small enough to accept, just noting it.
Reviewed by AI.
| onClick={() => onChange(option)} | ||
| // Radix exposes the checked state to assistive tech; the visual | ||
| // affordance is the trigger label, which already names the choice. | ||
| aria-current={option === period} |
There was a problem hiding this comment.
Non-blocking (a11y/comment accuracy): DropdownMenuItem is a plain Radix Item, which carries no checked state, so the comment above is not accurate and aria-current="false" is emitted on the non-selected rows (screen readers read aria-current as a truthy token; false should be omitted). Either set aria-current={option === period || undefined}, or use DropdownMenuOptions from the same module, which is the established pattern for a small option list on this repo.
Reviewed by AI.
| <span | ||
| className="text-text-tertiary" | ||
| // The dash is decorative; the reason is the actual content. | ||
| aria-label={unknownReason} |
There was a problem hiding this comment.
Non-blocking (a11y): aria-label on a <span> with no role is not reliably announced (the same pattern is in MetricCell in the table). If the reason is meant to be the accessible content, render it as visually-hidden text (sr-only) next to the dash, or give the span a role. The tests pass because getByLabelText reads the attribute, not because AT would.
Reviewed by AI.
| type Column = { | ||
| key: CreatorPostSortBy; | ||
| label: string; | ||
| /** Screen-reader description of what sorting this column means. */ |
There was a problem hiding this comment.
Nit: this docblock describes a field that does not exist; numeric only drives alignment. Worth dropping (the repo preference is to keep rationale in the PR description rather than in comments, and several of the inline comments in this PR are already drifting from the code they sit on).
Reviewed by AI.
| icon={<LinkIcon size={IconSize.Small} className={iconClassName} />} | ||
| metric={performance.outboundVisits} | ||
| period={period} | ||
| unknownReason="No click data has been recorded for your posts yet." |
There was a problem hiding this comment.
Nit (copy): null here means unknown, and the PR's own rule is that unknown and zero are different answers, but this reason reads as "you have zero clicks". Something like "Click data could not be loaded for your posts" keeps the distinction.
Reviewed by AI.
1 participant
Closes ENG-2126.
impressionsSeries. Merge and deploy the API first or the chart query fails.Why
/analyticsread lifetime totals under an "Overview (last 45 days)" heading, re-bucketed UTC history rows in the viewer's timezone, and carried three separate copies of the number 45 (a local constant, an exported one ingraphql/posts.ts, and the server default). Every number on it was either mislabelled or shifted by a day for anyone far from UTC. All three defects were flagged in the ENG-2124 spec.What
Rebuilt on
creatorPerformanceandcreatorPostPerformance, which report their own period semantics and coverage rather than leaving the page to guess.Route and authorization unchanged — same
/analytics, sameProtectedPage, samegetLayout.Overview
30/90-day selection drives the overview, the chart and the table. Tiles are split by what they are actually scoped to:
Outbound visits has no daily grain, so it is captioned "All time" and grouped with the other running totals rather than sitting under a period heading. Followers and reputation come from the existing
userPostsAnalyticsrow — no API change — because they are counters we already have and the page already showed.Honesty rules
nullvalue renders an em dash and the reason, never a zero. "Unknown" and "zero" are different answers.Table
Now a semantic
<table>with sortable headers carryingaria-sort, operable from the keyboard. Sorting a new column starts descending; re-picking the active one flips it. The comment count links straight into the discussion viacommentsPermalink; the title still links to the per-post analytics page.States
Loading skeletons, empty, and independent error/retry for the overview and the table — a broken table leaves the headline numbers on screen. An achievements section is scaffolded for ENG-2127 with an empty state; no sample badges, since a fabricated award is worse than none.
UserPostsAnalyticsTablewas deleted with the old page — it had no other caller.Tests
27 passing across three files: the delta rules (withheld comparison, zero baseline, unknown value), chart padding vs. coverage truncation, UTC labelling, null-vs-zero rendering in both tile and table, and sort + keyboard behaviour.
Not verified
No live app inspection. The responsive and keyboard checks the ticket asks for, across a creator with content / newly joined / empty / partial history, still need doing against a running app.
next buildcompiled and passed TypeScript; it then failed prerendering/gearon a local certificate error unrelated to this change.Follow-ups
USER_POSTS_ANALYTICS_HISTORY_QUERYandUSER_POSTS_WITH_ANALYTICS_QUERYinpackages/shared/src/graphql/users.tsnow have no callers anywhere in the monorepo. Left in place as removing shared exports is outside dashboard integration.🤖 Generated with Claude Code
Preview domain
https://eng-2126-creator-dashboard.preview.app.daily.dev