feat: unified view logging – track article views on post open - #6380
Conversation
Articles previously only counted a view (for reading streaks/reading days) when the user clicked "Read article"; every other post type already logged a view on page/modal open. Track it unconditionally in PostContent and PostFocusCard so opening an article's post page or modal counts the same way, matching a companion daily-api change that dedups against the existing read-article click. Co-Authored-By: Claude Fable 5 <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.
Inline commentary for reviewers: where the actual behavior change lives, and which parts are just test fallout.
| }; | ||
|
|
||
| useTrackPostView({ post, shouldTrack: isVideoType }); | ||
| useTrackPostView({ post }); |
There was a problem hiding this comment.
This one-liner is the actual behavior change of the PR: PostContent renders article (and video/liveRoom) posts, and the shouldTrack: isVideoType gate meant articles never sent viewPost on open — they only got a view server-side when the user clicked "Read article" (which routes through api.daily.dev/r/:postId).
Now opening the page or modal counts, same as freeform/welcome/share/collection/poll/brief/digest already do. The click-through path still exists unchanged; the backend dedups the two within a one-week window per (user, post) — see dailydotdev/daily-api#4026.
| post, | ||
| shouldTrack: isVideoPost(post) || viewTrackedPostTypes.includes(post.type), | ||
| }); | ||
| useTrackPostView({ post }); |
There was a problem hiding this comment.
Same change for the layout-v2 focus card, which duplicated the article exclusion via its own viewTrackedPostTypes allowlist (deleted above — it's dead once tracking is unconditional). isVideoPost/PostType imports stay because both are still used elsewhere in this file.
| await waitFor(() => expect(viewPostCallCount).toBe(3)); | ||
| }); | ||
|
|
||
| it('should report irrelevant tags', async () => { |
There was a problem hiding this comment.
Reviewer note: this router-mock reset (and the one in the next test) fixes a pre-existing test-isolation gap rather than anything introduced here. The two preceding tests set a pmid router query via mockImplementation, which jest.clearAllMocks() doesn't reset, so these tests were silently rendering with a leftover pmid and auto-opening the post modal all along. That was harmless while articles didn't track views; once they do, the modal fires an unmocked viewPost and nock fails the test — which is how the latent bleed-over surfaced.
| }, | ||
| result: () => ({ data: { _: true } }), | ||
| }, | ||
| // Opening any post (including articles) now logs a view on mount; tests |
There was a problem hiding this comment.
Default catch-all mock so the ~40 existing article-page tests don't each need to care about the new on-mount mutation; the new should log post view on mount test below asserts the behavior explicitly. Registered last so any test-supplied mock for the same request takes precedence.
2 participants
Summary
api.daily.dev/r/:postIdlink). Every other post type (freeform, welcome, share, collection, brief, digest, poll, social:twitter) already logged a view as soon as the page/modal opened.viewPostmutation (useTrackPostView/useViewPost) every other post type already uses.PostContent.tsxandPostFocusCard.tsx(the layout_v2 focus card) now calluseTrackPostView({ post })unconditionally instead of gating onisVideoType/ a hardcodedviewTrackedPostTypeslist, which is now dead code and removed.Changes
packages/shared/src/components/post/PostContent.tsx:useTrackPostView({ post, shouldTrack: isVideoType })→useTrackPostView({ post }).isVideoTypeis still used elsewhere in the file (layout/metadata), so it's untouched otherwise.packages/shared/src/components/post/focus/PostFocusCard.tsx: same change, plus deletion of the now-unusedviewTrackedPostTypesarray.isVideoPostandPostTypeare still used elsewhere in the file, so their imports are unchanged.PostPage.tsxspec, sharedFeed.spec.tsx): added/adjustedviewPostmutation mocks now that opening an article's post page/modal fires that mutation. Added an explicit "should log post view on mount" test for articles inPostPage.tsx, mirroring the existing collection-type test. Also hardened twoFeed.spec.tsxtests that were silently inheriting a leftoverpmidrouter-mock query from a preceding test (a pre-existing test-isolation gap that only started mattering once articles began tracking views too).Test plan
node ./scripts/typecheck-strict-changed.js— passes on all changed files.NODE_ENV=test pnpm --filter shared test— 2001/2004 pass. The 3 failures (src/lib/numberFormat.spec.ts, locale-formatting comma-vs-period) are pre-existing onorigin/mainand unrelated to this change (verified viagit stash).NODE_ENV=test pnpm --filter webapp test— 298/298 pass.eslinton all changed files — clean.🤖 Generated with Claude Code
Preview domain
https://feat-unified-post-view-logging.preview.app.daily.dev