Conversation
📝 WalkthroughWalkthroughAdds optimistic voting and draft state to MyStreamWaveMyVoteInput with live vote/max calculations and query-cache invalidation; introduces availableVotes propagation to MyStreamWaveMyVotesReset and expands tests to cover optimistic flows, UI text, draft handling, and react-query mocks. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Component as MyStreamWaveMyVoteInput
participant QueryClient as QueryClient
participant Server as Server
User->>Component: Enter value & submit
activate Component
Component->>Component: Clamp to liveMaxRating\nUpdate optimistic state & draft
Component->>User: Show optimistic UI (Available / Max updated)
Component->>Server: mutateAsync POST vote
deactivate Component
activate Server
Server->>Server: Process vote, build response (may include context_profile_context)
Server-->>Component: Return response
deactivate Server
activate Component
Component->>QueryClient: invalidateQueries(DROPS_LEADERBOARD)
Component->>Component: Apply server context to optimistic state\nClear or adjust draft
Component->>User: Reflect final server-backed UI
deactivate Component
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx (1)
152-168:⚠️ Potential issue | 🟡 MinorNo early return when clamped value equals current vote — submits a no-op API call.
When
clampedValue === liveCurrentVoteValue(line 164), the draft is cleared but execution continues intosetIsProcessing(true)→requestAuth→mutateAsync. The button is disabled via!isEditing, buthandleSubmitis also reachable via the Enter key handler (line 202), bypassing that guard. This results in an unnecessary API round-trip submitting the same vote value.Proposed fix
if (clampedValue === liveCurrentVoteValue) { setVoteDraftState(null); + return; - } else { - setVoteDraftValue(String(clampedValue)); } + setVoteDraftValue(String(clampedValue));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx` around lines 152 - 168, The submit handler handleSubmit currently clears the draft when clampedValue === liveCurrentVoteValue but continues execution and triggers setIsProcessing/requestAuth/mutateAsync, causing a no-op API call; change handleSubmit so that after setting setVoteDraftState(null) it returns immediately (early exit) to prevent further processing and the mutateAsync call when liveCurrentVoteValue equals the clamped parsedVoteValue; keep existing guards for isProcessing/isResetting and ensure functions referenced (setVoteDraftState, setVoteDraftValue, setIsProcessing, requestAuth, mutateAsync, liveCurrentVoteValue, parsedVoteValue) remain unchanged otherwise.
🧹 Nitpick comments (2)
__tests__/components/brain/my-stream/votes/MyStreamWaveMyVoteInput.test.tsx (1)
8-11: Full module mock replaces all react-query exports.This mock replaces the entire
@tanstack/react-querymodule, so any export besidesuseMutationanduseQueryClientused transitively would beundefined. Currently the component only imports those two, so this is fine, but it's fragile if additional hooks are added later. Consider usingjest.requireActualto preserve the rest:Optional improvement
jest.mock("@tanstack/react-query", () => ({ + ...jest.requireActual("@tanstack/react-query"), useMutation: jest.fn(), useQueryClient: jest.fn(), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/brain/my-stream/votes/MyStreamWaveMyVoteInput.test.tsx` around lines 8 - 11, The jest.mock call currently replaces the entire `@tanstack/react-query` module which can unintentionally stub other exports; update the mock in MyStreamWaveMyVoteInput.test.tsx to call jest.requireActual('@tanstack/react-query') and only override useMutation and useQueryClient so other exports remain intact. Locate the existing jest.mock(...) and change it to merge the real module with mocked implementations for useMutation and useQueryClient (keeping their jest.fn() behavior) so future added hooks still work.components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx (1)
113-150: Consider potential double error toast whenmutateAsyncrejects.The
onErrorcallback (pre-existing, line 143) callssetToastand re-throws. SincehandleSubmitusesmutateAsync, the rejection also propagates to thecatchblock at line 186, which callssetToastagain. This results in two error toasts for a single failure. Thethrow errorinonErrorcould also cause an unhandled rejection depending on the TanStack Query version.If you're touching this mutation anyway, consider either removing the
onErrorhandler (lettinghandleSubmit'scatchbe the sole handler) or removing the duplicate toast fromhandleSubmit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx` around lines 113 - 150, The mutation's onError currently both sets a toast and re-throws, causing duplicate toasts and possible unhandled rejections; remove the onError handler from the rateChangeMutation (the useMutation call) so errors propagate to handleSubmit's mutateAsync catch block which is the single place to set the error toast; locate rateChangeMutation (useMutation) and delete the entire onError: (error) => { ... } block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx`:
- Around line 47-68: The computed availableVotes can exceed liveMaxRating when
liveCurrentVoteValue is negative; modify the calculation of availableVotes
(current variable in MyStreamWaveMyVoteInput.tsx) to clamp the result between 0
and liveMaxRating (e.g., use Math.min(liveMaxRating, Math.max(0, liveMaxRating -
liveCurrentVoteValue))) so "available" never exceeds the configured max; update
any dependent UI/labels if they rely on the previous unbounded value.
- Around line 238-256: The spinner SVG uses absolute positioning (tw-absolute)
but its parent <button> in MyStreamWaveMyVoteInput.tsx lacks a positioned
ancestor, so add the positioning class (e.g., "tw-relative") to the button
element that wraps the spinner (the button rendered alongside
isProcessing/isResetting) so the svg is positioned correctly; update the
button's className to include "tw-relative" (preserve existing classes) so the
spinner stays inside the button bounds.
---
Outside diff comments:
In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx`:
- Around line 152-168: The submit handler handleSubmit currently clears the
draft when clampedValue === liveCurrentVoteValue but continues execution and
triggers setIsProcessing/requestAuth/mutateAsync, causing a no-op API call;
change handleSubmit so that after setting setVoteDraftState(null) it returns
immediately (early exit) to prevent further processing and the mutateAsync call
when liveCurrentVoteValue equals the clamped parsedVoteValue; keep existing
guards for isProcessing/isResetting and ensure functions referenced
(setVoteDraftState, setVoteDraftValue, setIsProcessing, requestAuth,
mutateAsync, liveCurrentVoteValue, parsedVoteValue) remain unchanged otherwise.
---
Nitpick comments:
In `@__tests__/components/brain/my-stream/votes/MyStreamWaveMyVoteInput.test.tsx`:
- Around line 8-11: The jest.mock call currently replaces the entire
`@tanstack/react-query` module which can unintentionally stub other exports;
update the mock in MyStreamWaveMyVoteInput.test.tsx to call
jest.requireActual('@tanstack/react-query') and only override useMutation and
useQueryClient so other exports remain intact. Locate the existing
jest.mock(...) and change it to merge the real module with mocked
implementations for useMutation and useQueryClient (keeping their jest.fn()
behavior) so future added hooks still work.
In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx`:
- Around line 113-150: The mutation's onError currently both sets a toast and
re-throws, causing duplicate toasts and possible unhandled rejections; remove
the onError handler from the rateChangeMutation (the useMutation call) so errors
propagate to handleSubmit's mutateAsync catch block which is the single place to
set the error toast; locate rateChangeMutation (useMutation) and delete the
entire onError: (error) => { ... } block.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx (1)
151-167:⚠️ Potential issue | 🟡 Minor
handleSubmitproceeds to API call even when the value hasn't changed.When
clampedValue === liveCurrentVoteValue(lines 163-164), the draft is cleared but execution falls through tosetIsProcessing(true)→requestAuth→mutateAsync. This fires a redundant API call and shows a "Vote updated" toast for a no-op. The button'sdisabledstate guards against click, but the Enter-key path (handleKeyDown) doesn't checkisEditing.Proposed fix — early-return when nothing changed
const clampedValue = Math.min( Math.max(parsedVoteValue, minRating), liveMaxRating ); if (clampedValue === liveCurrentVoteValue) { setVoteDraftState(null); + return; } else { setVoteDraftValue(String(clampedValue)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx` around lines 151 - 167, The handleSubmit function currently clears the draft when clampedValue === liveCurrentVoteValue but continues and triggers the API flow (setIsProcessing, requestAuth, mutateAsync); change handleSubmit (in MyStreamWaveMyVoteInput) to return early after setting setVoteDraftState(null) when clampedValue equals liveCurrentVoteValue so it does not call setIsProcessing/requestAuth/mutateAsync for a no-op vote update; ensure the early-return covers both click and Enter-key paths (handleSubmit called from handleKeyDown) so redundant API calls and "Vote updated" toasts are prevented.
🧹 Nitpick comments (2)
__tests__/components/brain/my-stream/votes/MyStreamWaveMyVoteInput.test.tsx (1)
79-88: Consider adding a test for Enter-key submission.The component supports submitting via Enter (
handleKeyDown), which bypasses the button'sdisabledguard onisEditing. A test verifying that pressing Enter with an unchanged value doesn't fire a mutation would strengthen coverage — especially given the missing early-return noted in the production code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/brain/my-stream/votes/MyStreamWaveMyVoteInput.test.tsx` around lines 79 - 88, Add a test that exercises the component's Enter-key submission path (handleKeyDown) in MyStreamWaveMyVoteInput: render the component (same setup as the existing test), focus the textbox and fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) when the input value is unchanged and isEditing is true, then assert that auth.requestAuth and mutateAsync were NOT called; also add a complementary assertion that when the value is changed (e.g., fireEvent.change to a clamped value) pressing Enter does call auth.requestAuth and mutateAsync with the expected { rate } to verify correct behavior.components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx (1)
140-148: Double toast on mutation error.
onError(line 142-146) callssetToastand thenthrow errorre-throws into thecatchblock (line 185-193), which callssetToastagain with a potentially different message. The second call overwrites the first, so the user sees only thecatchblock's message. This is pre-existing behavior, but now that the component is being reworked, consider removing thethrow errorfromonErroror removing the duplicatesetToastin thecatchblock.Option: let onError handle the toast, remove re-throw
onError: (error) => { setToast({ message: error as unknown as string, type: "error", }); - throw error; },Then in
handleSubmit, thecatchwould only handle truly unexpected errors (e.g., network failures before the mutation fires).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx` around lines 140 - 148, The mutation's onError callback (onError in the mutation options) is calling setToast then re-throwing the error, which causes the catch in handleSubmit to show a second toast; remove the throw error from onError so onError fully handles user-visible errors and the catch block in handleSubmit only handles unexpected failures (or alternatively remove the duplicate setToast in handleSubmit's catch if you prefer to surface errors there), ensuring references to onError, setToast, and handleSubmit are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx`:
- Around line 151-167: The handleSubmit function currently clears the draft when
clampedValue === liveCurrentVoteValue but continues and triggers the API flow
(setIsProcessing, requestAuth, mutateAsync); change handleSubmit (in
MyStreamWaveMyVoteInput) to return early after setting setVoteDraftState(null)
when clampedValue equals liveCurrentVoteValue so it does not call
setIsProcessing/requestAuth/mutateAsync for a no-op vote update; ensure the
early-return covers both click and Enter-key paths (handleSubmit called from
handleKeyDown) so redundant API calls and "Vote updated" toasts are prevented.
---
Nitpick comments:
In `@__tests__/components/brain/my-stream/votes/MyStreamWaveMyVoteInput.test.tsx`:
- Around line 79-88: Add a test that exercises the component's Enter-key
submission path (handleKeyDown) in MyStreamWaveMyVoteInput: render the component
(same setup as the existing test), focus the textbox and
fireEvent.keyDown(input, { key: "Enter", code: "Enter" }) when the input value
is unchanged and isEditing is true, then assert that auth.requestAuth and
mutateAsync were NOT called; also add a complementary assertion that when the
value is changed (e.g., fireEvent.change to a clamped value) pressing Enter does
call auth.requestAuth and mutateAsync with the expected { rate } to verify
correct behavior.
In `@components/brain/my-stream/votes/MyStreamWaveMyVoteInput.tsx`:
- Around line 140-148: The mutation's onError callback (onError in the mutation
options) is calling setToast then re-throwing the error, which causes the catch
in handleSubmit to show a second toast; remove the throw error from onError so
onError fully handles user-visible errors and the catch block in handleSubmit
only handles unexpected failures (or alternatively remove the duplicate setToast
in handleSubmit's catch if you prefer to surface errors there), ensuring
references to onError, setToast, and handleSubmit are updated accordingly.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests