Skip to content

refactor(web): extract useFeedbackItemActions from the two feedback cards (#tech-debt) #1078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
selfcontained merged 1 commit into from
Sep 8, 2026
Merged

refactor(web): extract useFeedbackItemActions from the two feedback cards (#tech-debt) #1078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 17 additions & 42 deletions apps/web/src/components/app/diff-feedback-annotation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -40,10 +36,18 @@ export function InlineFeedbackAnnotation({
}): JSX.Element {
const [expanded, setExpanded] = useState(false);
const annotationRef = useRef<HTMLDivElement>(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"
Expand Down Expand Up @@ -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 (
<TooltipProvider delayDuration={200}>
<div
Expand Down Expand Up @@ -199,10 +174,10 @@ export function InlineFeedbackAnnotation({
<FeedbackReplyForm
replying={replying}
reply={reply}
isPending={addMessage.isPending}
isPending={isSendingReply}
variant="inline"
onReplyChange={setReply}
onStartReply={() => setReplying(true)}
onStartReply={startReply}
onCancelReply={cancelReply}
onSubmit={submitReply}
/>
Expand All @@ -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)
Expand Down
58 changes: 18 additions & 40 deletions apps/web/src/components/app/reviews-feedback-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 (
<div className="ml-1 pb-2 last:pb-0">
<div
Expand Down Expand Up @@ -240,10 +218,10 @@ export function FeedbackItemRow({
<FeedbackReplyForm
replying={replying}
reply={reply}
isPending={addMessage.isPending}
isPending={isSendingReply}
variant="sidebar"
onReplyChange={setReply}
onStartReply={() => setReplying(true)}
onStartReply={startReply}
onCancelReply={cancelReply}
onSubmit={submitReply}
/>
Expand All @@ -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)
Expand Down
153 changes: 153 additions & 0 deletions apps/web/src/components/app/use-feedback-item-actions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<typeof vi.fn>;
};
}

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");
});
});
Loading
Loading