fix(frontend): featured posts navigate to post detail (RC1-004) #355 - #433
Conversation
Wire featured post cards to post detail routes and add unit/E2E coverage. Stop tracking .env.local so machine-specific API overrides stay local. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Use String! for GetActionReactions to match the hosted API, request activity _id in the feed query, and key activity rows by stable ids. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep Cursor rules and agent config machine-local and out of the repo. Co-authored-by: Cursor <cursoragent@cursor.com>
motirebuma
left a comment
motirebuma
left a comment
There was a problem hiding this comment.
hi @niamao nice work all-around, clean PR. Looks good to me overall.
My thoughts on your changes:
-
Featured post card nav (RC1-004): wrapping the card in a when a post.url is present, and a otherwise, is the right approach. Using toAppPostUrl is good for consistency, and I also like that you've added the necessary aria-label and focus-visible styling. Using the shared cardContent fragment to avoid duplication between the link and div cases is a good touch as well.
-
actionReactions type: changing actionId: ID! to String! in both the backend and the frontend query is the correct fix here. ID is a scalar type that can be serialized differently on different clients, but the hosted API (and presumably legacy APIs) expect a string. The change makes sense, and the note in the commit message explaining why it's String! is a good addition.
-
Stable activity list keys: using item._id along with the postId fallback, activity type, vote id, etc. is the right approach. Adding _id to the GET_USER_ACTIVITY query (and the corresponding activity type and vote id fields) ensures that we have the unique key available in all cases. I think this is sufficient, but in the rare case that those fields are somehow missing, it'll fall back to the less reliable combination of postId + activityType + voteId, which should still be better than using the array index. This pattern has been applied to both ActivityList.tsx and PaginatedActivityList.tsx.
-
.env.local -> .env.example: unstaging the .env.local file and instead adding a more informative .env.example is the right thing to do. Looks like the .gitignore patterns are also correct (ignoring .env.local, but not .env.example and .env.e2e.example). README has been updated as well.
-
.cursor/ in .gitignore: no problem, that's a sensible addition.
Testing: I've reviewed both the unit and E2E tests, and they both look good to me. The unit tests verify that the card is rendered as an tag with the expected href, and confirm that data-testid selectors are present. The E2E test mocks GraphGL and fires off a click event, verifying that the card navigation works and then the back button works as well. The note about clearing storageState to emulate a logged-out session is a good touch.
My general impression: looks good to me, approved for merging.
Thanks @niamao
2 participants
Summary
toAppPostUrl..env.local(add.env.example+ gitignore) so API overrides stay machine-local.GetActionReactionsvariable type (String!) to match the hosted API and avoid 400s._idand use stable keys in activity lists (no index-based keys)..cursor/so IDE/agent rules stay machine-specific.Commits
fix(frontend): navigate featured posts to detail pages (RC1-004).env.local; add.env.exampleand frontend gitignore/README updatesfix: align actionReactions types and stabilize activity list keysGetActionReactions($actionId: String!)(frontend + backend schema)GET_USER_ACTIVITYrequests entity_idActivityList/PaginatedActivityListchore: ignore local .cursor directory.gitignoreentry for.cursor/Test plan
/dashboard/post/...for that post.GetActionReactions400 in the console.pnpm test -- src/__tests__/app/page.test.tsx(featured posts navigation cases)pnpm test:e2e featured-posts.spec.ts.env.localis ignored and new clones usecp .env.example .env.local.cursor/is ignored and not present in the PR diff