fix(discussion): drop the frozen sort snapshot, serialise votes per comment - #1343
fix(discussion): drop the frozen sort snapshot, serialise votes per comment#1343NiallJoeMaher wants to merge 1 commit into
Conversation
…omment Follow-up to #1341. Review found the score-freezing in that change was both more than the fix needed and wrong on its own terms. Displayed counts still updated on refetch while the frozen ordering did not, so a thread could render a 42-point comment sitting below a 3-point one. The documented "re-pick the sort to get the live ranking" escape hatch never fired, because selecting the already-selected option is a no-op. And the tiebreak it introduced made Top identical to New on the common all-zero thread. Not refetching after a successful vote is the whole fix on its own: the new count lives in VoteControl, `discussions` is untouched, so nothing re-ranks under the reader. Ordering goes back to being derived from the data on screen, so it can never contradict the counts beside it. Votes are now serialised per comment, newest click replacing any queued one. #1341 removed the in-flight guard without replacing it, so overlapping requests could land in either order and leave the stored vote disagreeing with the UI; eight rapid clicks now collapse to two requests rather than eight against the rate limit. The remount that resyncs a failed vote is per comment too, so one failure no longer discards the optimistic state of votes on other comments or of one still in flight, and server messages (rate limiting) reach the toast instead of a generic string.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
GitHub Actions
Failure
|
Uh oh! @vercel[bot], the image you shared is missing helpful alt text. Check #1343 (comment). Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs. |
1 participant
Follow-up to #1341. A high-effort review of that PR (run after it had already merged) found the score-freezing was both more than the fix needed and wrong on its own terms.
What #1341 got wrong
setSortOrder("top")when already ontopis a no-op, React bails out, and the snapshot is never cleared.Array#sortstability to keep the server's order.voteResetKeywas global, so an error remounted all controls and discarded the state of votes on other comments, including ones still in flight.What this does
Not refetching after a successful vote is the whole fix on its own. The new count lives in
VoteControl;discussionsis untouched, so nothing re-ranks under the reader. Ordering goes back tob.score - a.scorederived from the data on screen, so it can never contradict what is displayed — and the stable-sort behaviour for ties is preserved.Votes are serialised per comment, newest click replacing any queued one. One request in flight per comment, so writes cannot land out of order.
The resync remount is per comment, so one failure no longer discards other comments' optimistic state. Server messages (rate limiting) now reach the toast instead of a generic string.
Net: 55 insertions, 75 deletions — this is mostly deleting the machinery #1341 added.
Verified locally
up,upvotes_count = 1).npm run lint,npm run prettier,npm run test:unit,npm run build.