Address Greptile deleted-comment feedback

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
2026-06-05 03:43:12 +00:00
parent cf0ec9933a
commit 1afa337841
5 changed files with 91 additions and 35 deletions
@@ -230,4 +230,44 @@ describeEmbeddedPostgres("documentAnnotationService", () => {
expect(updatedThread?.status).toBe("resolved"); expect(updatedThread?.status).toBe("resolved");
expect(updatedThread?.resolvedByUserId).toBe("board-user"); expect(updatedThread?.resolvedByUserId).toBe("board-user");
}); });
it("rejects annotation comments linked to already-deleted issue comments", async () => {
const { companyId, issueId, document } = await createIssueWithDocument();
const [issueComment] = await db
.insert(issueComments)
.values({
companyId,
issueId,
authorType: "user",
authorUserId: "board-user",
body: "",
deletedAt: new Date("2026-06-05T03:00:00.000Z"),
deletedByType: "user",
deletedByUserId: "board-user",
})
.returning();
await expect(
annotations.createThread(
issueId,
"plan",
{
baseRevisionId: document.latestRevisionId!,
baseRevisionNumber: document.latestRevisionNumber,
selector: {
quote: { exact: "selected text", prefix: "Alpha ", suffix: " omega" },
position: { normalizedStart: 6, normalizedEnd: 19, markdownStart: 6, markdownEnd: 19 },
},
body: "Do not link this annotation to a deleted comment",
issueCommentId: issueComment.id,
},
{ actorType: "user", actorId: "board-user", userId: "board-user" },
),
).rejects.toMatchObject({
status: 422,
message: "Linked issue comment must belong to this issue",
});
await expect(db.select().from(documentAnnotationComments)).resolves.toHaveLength(0);
});
}); });
+2 -2
View File
@@ -1,4 +1,4 @@
import { and, asc, desc, eq, inArray, sql } from "drizzle-orm"; import { and, asc, desc, eq, inArray, isNull, sql } from "drizzle-orm";
import type { Db } from "@paperclipai/db"; import type { Db } from "@paperclipai/db";
import { import {
documentAnnotationAnchorSnapshots, documentAnnotationAnchorSnapshots,
@@ -155,7 +155,7 @@ export function documentAnnotationService(db: Db) {
issueId: issueComments.issueId, issueId: issueComments.issueId,
}) })
.from(issueComments) .from(issueComments)
.where(eq(issueComments.id, commentId)) .where(and(eq(issueComments.id, commentId), isNull(issueComments.deletedAt)))
.then((rows: Array<{ id: string; companyId: string; issueId: string }>) => rows[0] ?? null); .then((rows: Array<{ id: string; companyId: string; issueId: string }>) => rows[0] ?? null);
if (!comment || comment.issueId !== issueId) { if (!comment || comment.issueId !== issueId) {
throw unprocessable("Linked issue comment must belong to this issue"); throw unprocessable("Linked issue comment must belong to this issue");
+9 -3
View File
@@ -1270,7 +1270,6 @@ describe("IssueChatThread", () => {
it("confirms and invokes delete only for the current user's normal comments", async () => { it("confirms and invokes delete only for the current user's normal comments", async () => {
const root = createRoot(container); const root = createRoot(container);
const onDeleteComment = vi.fn(async () => {}); const onDeleteComment = vi.fn(async () => {});
const confirmSpy = vi.spyOn(window, "confirm").mockReturnValue(true);
act(() => { act(() => {
root.render( root.render(
@@ -1324,10 +1323,17 @@ describe("IssueChatThread", () => {
deleteButtons[0]?.dispatchEvent(new MouseEvent("click", { bubbles: true })); deleteButtons[0]?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
}); });
expect(confirmSpy).toHaveBeenCalled(); expect(document.body.textContent).toContain("Delete comment?");
const confirmButton = Array.from(document.body.querySelectorAll("button"))
.find((button) => button.textContent === "Delete comment");
expect(confirmButton).toBeTruthy();
await act(async () => {
confirmButton?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
expect(onDeleteComment).toHaveBeenCalledWith("comment-owned"); expect(onDeleteComment).toHaveBeenCalledWith("comment-owned");
confirmSpy.mockRestore();
act(() => { act(() => {
root.unmount(); root.unmount();
}); });
+40 -16
View File
@@ -1290,6 +1290,7 @@ function IssueChatUserMessage({
const deleted = Boolean(custom.deletedAt); const deleted = Boolean(custom.deletedAt);
const queueTargetRunId = typeof custom.queueTargetRunId === "string" ? custom.queueTargetRunId : null; const queueTargetRunId = typeof custom.queueTargetRunId === "string" ? custom.queueTargetRunId : null;
const [copied, setCopied] = useState(false); const [copied, setCopied] = useState(false);
const [deleteDialogOpen, setDeleteDialogOpen] = useState(false);
const { const {
isCurrentUser, isCurrentUser,
authorName: resolvedAuthorName, authorName: resolvedAuthorName,
@@ -1309,8 +1310,11 @@ function IssueChatUserMessage({
const canDeleteComment = Boolean(onDeleteComment && isCurrentUser && !queued && !pending && !deleted); const canDeleteComment = Boolean(onDeleteComment && isCurrentUser && !queued && !pending && !deleted);
const handleDeleteComment = () => { const handleDeleteComment = () => {
if (!canDeleteComment) return; if (!canDeleteComment) return;
const confirmed = window.confirm("Delete this comment? This will replace it with a deleted-comment marker."); setDeleteDialogOpen(true);
if (!confirmed) return; };
const confirmDeleteComment = () => {
if (!canDeleteComment) return;
setDeleteDialogOpen(false);
void onDeleteComment?.(commentId); void onDeleteComment?.(commentId);
}; };
const messageBody = ( const messageBody = (
@@ -1432,21 +1436,41 @@ function IssueChatUserMessage({
); );
return ( return (
<div id={anchorId}> <>
<div className={cn("group flex items-start gap-2.5", isCurrentUser && "justify-end")}> <div id={anchorId}>
{isCurrentUser ? ( <div className={cn("group flex items-start gap-2.5", isCurrentUser && "justify-end")}>
<> {isCurrentUser ? (
{messageBody} <>
{authorAvatar} {messageBody}
</> {authorAvatar}
) : ( </>
<> ) : (
{authorAvatar} <>
{messageBody} {authorAvatar}
</> {messageBody}
)} </>
)}
</div>
</div> </div>
</div> <Dialog open={deleteDialogOpen} onOpenChange={setDeleteDialogOpen}>
<DialogContent>
<DialogHeader>
<DialogTitle>Delete comment?</DialogTitle>
<DialogDescription>
This will replace the comment with a deleted-comment marker.
</DialogDescription>
</DialogHeader>
<DialogFooter>
<Button variant="outline" onClick={() => setDeleteDialogOpen(false)}>
Cancel
</Button>
<Button variant="destructive" onClick={confirmDeleteComment}>
Delete comment
</Button>
</DialogFooter>
</DialogContent>
</Dialog>
</>
); );
} }
-14
View File
@@ -2461,20 +2461,6 @@ export function IssueDetail() {
const cancelQueuedComment = useMutation({ const cancelQueuedComment = useMutation({
mutationFn: async ({ commentId }: { commentId: string }) => issuesApi.cancelComment(issueId!, commentId), mutationFn: async ({ commentId }: { commentId: string }) => issuesApi.cancelComment(issueId!, commentId),
onSuccess: (comment) => { onSuccess: (comment) => {
if (comment.deletedAt) {
upsertCommentInCache(comment);
clearCommentHashIfCurrent(comment.id);
invalidateIssueDetail();
invalidateIssueThreadLazily();
invalidateIssueCollections();
invalidateIssueDocumentAnnotationState();
pushToast({
title: "Comment deleted",
body: "The comment was replaced with a deleted marker.",
tone: "success",
});
return;
}
setLocallyQueuedCommentRunIds((current) => { setLocallyQueuedCommentRunIds((current) => {
if (!current.has(comment.id)) return current; if (!current.has(comment.id)) return current;
const next = new Map(current); const next = new Map(current);