diff --git a/apps/web/src/components/app/diff-feedback-annotation.tsx b/apps/web/src/components/app/diff-feedback-annotation.tsx index 9f868cee..025635ee 100644 --- a/apps/web/src/components/app/diff-feedback-annotation.tsx +++ b/apps/web/src/components/app/diff-feedback-annotation.tsx @@ -1,13 +1,9 @@ import { useEffect, useRef, useState } from "react"; import { CheckCircle2, ChevronRight, Circle, XCircle } from "lucide-react"; import { AnimatePresence, motion } from "framer-motion"; -import { toast } from "sonner"; import { type ReviewFeedbackItem } from "@/hooks/use-agent-reviews"; -import { - useAddReviewThreadMessage, - useSetReviewFeedbackResolution, -} from "@/hooks/use-agent-reviews"; +import { useFeedbackItemActions } from "@/components/app/use-feedback-item-actions"; import { cn } from "@/lib/utils"; import { Markdown } from "@/components/ui/markdown"; import { stickyAnnotationStyle } from "@/components/app/diff-annotation-style"; @@ -40,10 +36,18 @@ export function InlineFeedbackAnnotation({ }): JSX.Element { const [expanded, setExpanded] = useState(false); const annotationRef = useRef(null); - const [reply, setReply] = useState(""); - const [replying, setReplying] = useState(false); - const addMessage = useAddReviewThreadMessage(agentId); - const setResolution = useSetReviewFeedbackResolution(agentId); + const { + reply, + setReply, + replying, + startReply, + cancelReply, + submitReply, + isSendingReply, + updateResolution, + isUpdatingResolution, + pendingResolution, + } = useFeedbackItemActions(agentId, feedbackItem.id); const state = !isResolved ? "open" @@ -89,35 +93,6 @@ export function InlineFeedbackAnnotation({ }; }, [feedbackItem.id, focused, onFocusComplete]); - const submitReply = async (event: React.FormEvent) => { - event.preventDefault(); - const body = reply.trim(); - if (!body) return; - try { - await addMessage.mutateAsync({ itemId: feedbackItem.id, body }); - setReply(""); - setReplying(false); - } catch { - toast.error("Couldn't send the reply. Try again."); - } - }; - - const cancelReply = () => { - setReply(""); - setReplying(false); - }; - - const updateResolution = async (resolution: "fixed" | "dismissed" | null) => { - try { - await setResolution.mutateAsync({ - itemId: feedbackItem.id, - resolution, - }); - } catch { - toast.error("Couldn't update the feedback state. Try again."); - } - }; - return (
setReplying(true)} + onStartReply={startReply} onCancelReply={cancelReply} onSubmit={submitReply} /> @@ -211,8 +186,8 @@ export function InlineFeedbackAnnotation({ state={state} resolution={feedbackItem.resolution} resolutionNote={feedbackItem.resolutionNote} - isPending={setResolution.isPending} - pendingResolution={setResolution.variables?.resolution} + isPending={isUpdatingResolution} + pendingResolution={pendingResolution} variant="inline" onUpdateResolution={(resolution) => void updateResolution(resolution) diff --git a/apps/web/src/components/app/reviews-feedback-item.tsx b/apps/web/src/components/app/reviews-feedback-item.tsx index cafb59c6..d94c1da6 100644 --- a/apps/web/src/components/app/reviews-feedback-item.tsx +++ b/apps/web/src/components/app/reviews-feedback-item.tsx @@ -1,13 +1,9 @@ import { useCallback, useState } from "react"; import { CheckCircle2, ChevronRight, Circle, XCircle } from "lucide-react"; import { AnimatePresence, motion } from "framer-motion"; -import { toast } from "sonner"; -import { - useAddReviewThreadMessage, - useSetReviewFeedbackResolution, - type ReviewFeedbackItem, -} from "@/hooks/use-agent-reviews"; +import { type ReviewFeedbackItem } from "@/hooks/use-agent-reviews"; +import { useFeedbackItemActions } from "@/components/app/use-feedback-item-actions"; import { Tooltip, TooltipContent, @@ -68,10 +64,18 @@ export function FeedbackItemRow({ }): JSX.Element { const state = feedbackState(item); const [expanded, setExpanded] = useState(false); - const [reply, setReply] = useState(""); - const [replying, setReplying] = useState(false); - const addMessage = useAddReviewThreadMessage(agentId); - const setResolution = useSetReviewFeedbackResolution(agentId); + const { + reply, + setReply, + replying, + startReply, + cancelReply, + submitReply, + isSendingReply, + updateResolution, + isUpdatingResolution, + pendingResolution, + } = useFeedbackItemActions(agentId, item.id); const fileInDiff = !diffFilePaths || !item.filePath || diffFilePaths.has(item.filePath); @@ -105,32 +109,6 @@ export function FeedbackItemRow({ const stateLabel = state === "fixed" ? "Fixed" : state === "dismissed" ? "Dismissed" : "Open"; - const submitReply = async (event: React.FormEvent) => { - event.preventDefault(); - const body = reply.trim(); - if (!body) return; - try { - await addMessage.mutateAsync({ itemId: item.id, body }); - setReply(""); - setReplying(false); - } catch { - toast.error("Couldn't send the reply. Try again."); - } - }; - - const cancelReply = () => { - setReply(""); - setReplying(false); - }; - - const updateResolution = async (resolution: "fixed" | "dismissed" | null) => { - try { - await setResolution.mutateAsync({ itemId: item.id, resolution }); - } catch { - toast.error("Couldn't update the feedback state. Try again."); - } - }; - return (
setReplying(true)} + onStartReply={startReply} onCancelReply={cancelReply} onSubmit={submitReply} /> @@ -252,8 +230,8 @@ export function FeedbackItemRow({ state={state} resolution={item.resolution} resolutionNote={item.resolutionNote} - isPending={setResolution.isPending} - pendingResolution={setResolution.variables?.resolution} + isPending={isUpdatingResolution} + pendingResolution={pendingResolution} variant="sidebar" onUpdateResolution={(resolution) => void updateResolution(resolution) diff --git a/apps/web/src/components/app/use-feedback-item-actions.test.ts b/apps/web/src/components/app/use-feedback-item-actions.test.ts new file mode 100644 index 00000000..f871fd32 --- /dev/null +++ b/apps/web/src/components/app/use-feedback-item-actions.test.ts @@ -0,0 +1,153 @@ +// @vitest-environment jsdom +import { act, cleanup, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { useFeedbackItemActions } from "./use-feedback-item-actions"; + +const addMessage = { + mutateAsync: vi.fn(), + isPending: false, +}; +const setResolution = { + mutateAsync: vi.fn(), + isPending: false, + variables: undefined as + | { resolution: "fixed" | "dismissed" | null } + | undefined, +}; + +vi.mock("@/hooks/use-agent-reviews", () => ({ + useAddReviewThreadMessage: () => addMessage, + useSetReviewFeedbackResolution: () => setResolution, +})); +vi.mock("sonner", () => ({ toast: { error: vi.fn() } })); + +const { toast } = await import("sonner"); +const toastError = vi.mocked(toast.error); + +/** A submit event stand-in that records preventDefault. */ +function formEvent() { + return { preventDefault: vi.fn() } as unknown as React.FormEvent & { + preventDefault: ReturnType; + }; +} + +beforeEach(() => { + addMessage.mutateAsync = vi.fn(async () => undefined); + addMessage.isPending = false; + setResolution.mutateAsync = vi.fn(async () => undefined); + setResolution.isPending = false; + setResolution.variables = undefined; + toastError.mockClear(); +}); + +afterEach(cleanup); + +describe("useFeedbackItemActions", () => { + it("sends the trimmed reply for the given item and resets the draft", async () => { + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 42)); + + act(() => result.current.startReply()); + act(() => result.current.setReply(" looks good ")); + expect(result.current.replying).toBe(true); + expect(result.current.reply).toBe(" looks good "); + + await act(async () => { + await result.current.submitReply(formEvent()); + }); + + expect(addMessage.mutateAsync).toHaveBeenCalledWith({ + itemId: 42, + body: "looks good", + }); + expect(result.current.reply).toBe(""); + expect(result.current.replying).toBe(false); + }); + + it("does not send a blank reply and leaves the form open", async () => { + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 42)); + + act(() => result.current.startReply()); + act(() => result.current.setReply(" ")); + const event = formEvent(); + await act(async () => { + await result.current.submitReply(event); + }); + + expect(event.preventDefault).toHaveBeenCalled(); + expect(addMessage.mutateAsync).not.toHaveBeenCalled(); + expect(result.current.replying).toBe(true); + expect(result.current.reply).toBe(" "); + }); + + it("keeps the draft and toasts when sending the reply fails", async () => { + addMessage.mutateAsync = vi.fn(async () => { + throw new Error("boom"); + }); + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 42)); + + act(() => result.current.startReply()); + act(() => result.current.setReply("retry me")); + await act(async () => { + await result.current.submitReply(formEvent()); + }); + + expect(toastError).toHaveBeenCalledWith( + "Couldn't send the reply. Try again." + ); + expect(result.current.reply).toBe("retry me"); + expect(result.current.replying).toBe(true); + }); + + it("clears the draft on cancel", () => { + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 42)); + + act(() => result.current.startReply()); + act(() => result.current.setReply("never mind")); + act(() => result.current.cancelReply()); + + expect(result.current.reply).toBe(""); + expect(result.current.replying).toBe(false); + }); + + it("updates the resolution for the given item", async () => { + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 7)); + + await act(async () => { + await result.current.updateResolution("fixed"); + }); + + expect(setResolution.mutateAsync).toHaveBeenCalledWith({ + itemId: 7, + resolution: "fixed", + }); + expect(toastError).not.toHaveBeenCalled(); + }); + + it("toasts when updating the resolution fails", async () => { + setResolution.mutateAsync = vi.fn(async () => { + throw new Error("boom"); + }); + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 7)); + + await act(async () => { + await result.current.updateResolution(null); + }); + + expect(toastError).toHaveBeenCalledWith( + "Couldn't update the feedback state. Try again." + ); + }); + + it("surfaces the mutation pending flags and the in-flight resolution", () => { + addMessage.isPending = true; + setResolution.isPending = true; + setResolution.variables = { resolution: "dismissed" }; + + const { result } = renderHook(() => useFeedbackItemActions("agt_1", 7)); + + expect(result.current.isSendingReply).toBe(true); + expect(result.current.isUpdatingResolution).toBe(true); + expect(result.current.pendingResolution).toBe("dismissed"); + }); +}); diff --git a/apps/web/src/components/app/use-feedback-item-actions.ts b/apps/web/src/components/app/use-feedback-item-actions.ts new file mode 100644 index 00000000..03ce70eb --- /dev/null +++ b/apps/web/src/components/app/use-feedback-item-actions.ts @@ -0,0 +1,72 @@ +import { useState, type FormEvent } from "react"; +import { toast } from "sonner"; + +import { + useAddReviewThreadMessage, + useSetReviewFeedbackResolution, +} from "@/hooks/use-agent-reviews"; + +export type FeedbackResolution = "fixed" | "dismissed" | null; + +// Reply-draft state and the two review-feedback mutations, shared by the +// in-diff annotation and the reviews sidebar row. Only the behaviour is +// shared — the two cards render completely different markup. +export function useFeedbackItemActions( + agentId: string | null, + itemId: number +): { + reply: string; + setReply: (value: string) => void; + replying: boolean; + startReply: () => void; + cancelReply: () => void; + submitReply: (event: FormEvent) => Promise; + isSendingReply: boolean; + updateResolution: (resolution: FeedbackResolution) => Promise; + isUpdatingResolution: boolean; + pendingResolution: FeedbackResolution | undefined; +} { + const [reply, setReply] = useState(""); + const [replying, setReplying] = useState(false); + const addMessage = useAddReviewThreadMessage(agentId); + const setResolution = useSetReviewFeedbackResolution(agentId); + + const submitReply = async (event: FormEvent) => { + event.preventDefault(); + const body = reply.trim(); + if (!body) return; + try { + await addMessage.mutateAsync({ itemId, body }); + setReply(""); + setReplying(false); + } catch { + toast.error("Couldn't send the reply. Try again."); + } + }; + + const cancelReply = () => { + setReply(""); + setReplying(false); + }; + + const updateResolution = async (resolution: FeedbackResolution) => { + try { + await setResolution.mutateAsync({ itemId, resolution }); + } catch { + toast.error("Couldn't update the feedback state. Try again."); + } + }; + + return { + reply, + setReply, + replying, + startReply: () => setReplying(true), + cancelReply, + submitReply, + isSendingReply: addMessage.isPending, + updateResolution, + isUpdatingResolution: setResolution.isPending, + pendingResolution: setResolution.variables?.resolution, + }; +}