diff --git a/apps/web/src/components/pullRequest/PullRequestCommentForm.tsx b/apps/web/src/components/pullRequest/PullRequestCommentForm.tsx index f7aa2ece22a42..abbabd402b0ee 100644 --- a/apps/web/src/components/pullRequest/PullRequestCommentForm.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCommentForm.tsx @@ -17,6 +17,9 @@ import { import { pullRequestReviewKey } from "./pullRequestReviewStore"; import { useInlineActionConfirm } from "./useInlineActionConfirm"; +/** A fresh object each time, so quoting the same comment twice still lands. */ +export type PullRequestQuoteRequest = { readonly text: string }; + export function PullRequestCommentForm({ environmentId, active, @@ -24,9 +27,11 @@ export function PullRequestCommentForm({ detail, actionPending, textareaRef, + quoteRequest, onCommentAction, onCommented, onClose, + onQuoted, }: { environmentId: EnvironmentId; /** False while the form stays mounted but hidden; dictation is cancelled then. */ @@ -35,16 +40,45 @@ export function PullRequestCommentForm({ detail: PullRequestDetailView; actionPending: boolean; textareaRef: RefObject; + quoteRequest?: PullRequestQuoteRequest | undefined; onCommentAction: ( body: string, action: "close" | "reopen", ) => Promise<{ readonly commentPosted: boolean }>; onCommented: () => void; onClose: () => void; + /** A quote deferred past a successful post lands after `onClose`, so the form asks to be shown again. */ + onQuoted: () => void; }) { const [body, setBody] = useState(""); const [submitting, setSubmitting] = useState<"comment" | "close" | "reopen" | null>(null); const locked = submitting !== null || actionPending; + const [answeredQuoteRequest, setAnsweredQuoteRequest] = useState(quoteRequest); + const [pendingQuotes, setPendingQuotes] = useState>([]); + const [caretAfterQuote, setCaretAfterQuote] = useState(false); + let quotes = pendingQuotes; + if (quoteRequest !== answeredQuoteRequest) { + setAnsweredQuoteRequest(quoteRequest); + if (quoteRequest) quotes = [...quotes, quoteRequest.text]; + } + // A successful post clears the draft, so quotes wait until posting settles. + if (quotes.length > 0 && !locked) { + setPendingQuotes([]); + setBody((draft) => [draft.trimEnd(), ...quotes].filter((part) => part.length > 0).join("\n\n")); + setCaretAfterQuote(true); + } else if (quotes !== pendingQuotes) { + setPendingQuotes(quotes); + } + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- reveals the composer and moves the DOM caret after a quote lands + useEffect(() => { + const textarea = textareaRef.current; + if (!caretAfterQuote || !textarea) return; + setCaretAfterQuote(false); + onQuoted(); + textarea.focus({ preventScroll: true }); + textarea.setSelectionRange(textarea.value.length, textarea.value.length); + textarea.scrollTop = textarea.scrollHeight; + }, [caretAfterQuote, textareaRef, onQuoted]); const speech = useTextFieldSpeech({ environmentId, projectId: reference.projectId, diff --git a/apps/web/src/components/pullRequest/PullRequestComposer.tsx b/apps/web/src/components/pullRequest/PullRequestComposer.tsx index f954c2b262dee..f604a0695d3bd 100644 --- a/apps/web/src/components/pullRequest/PullRequestComposer.tsx +++ b/apps/web/src/components/pullRequest/PullRequestComposer.tsx @@ -9,7 +9,7 @@ import { useRef, useState } from "react"; import { Button } from "../ui/button"; import { Popover, PopoverClose, PopoverPopup, PopoverTitle, PopoverTrigger } from "../ui/popover"; import { Toggle, ToggleGroup } from "../ui/toggle-group"; -import { PullRequestCommentForm } from "./PullRequestCommentForm"; +import { PullRequestCommentForm, type PullRequestQuoteRequest } from "./PullRequestCommentForm"; import { PullRequestReviewForm } from "./PullRequestReviewForm"; import { pullRequestReviewKey, @@ -27,6 +27,7 @@ export function PullRequestComposer({ onCommented, onReviewSubmitted, openReviewRequest, + quoteRequest, }: { environmentId: EnvironmentId; reference: PullRequestRef; @@ -39,6 +40,7 @@ export function PullRequestComposer({ onCommented: () => void; onReviewSubmitted: () => void; openReviewRequest?: number | undefined; + quoteRequest?: PullRequestQuoteRequest | undefined; }) { const term = sourceControlClients.get(detail.provider).changeRequest.singular; const [open, setOpen] = useState(false); @@ -60,6 +62,12 @@ export function PullRequestComposer({ setRequestedMode("review"); setOpen(true); } + const [answeredQuoteRequest, setAnsweredQuoteRequest] = useState(quoteRequest); + if (quoteRequest !== answeredQuoteRequest) { + setAnsweredQuoteRequest(quoteRequest); + setRequestedMode("comment"); + setOpen(true); + } const canComment = detail.capabilities.comment && detail.viewerPermissions.comment; const verdicts = detail.capabilities.review.verdicts.filter((verdict) => @@ -180,9 +188,14 @@ export function PullRequestComposer({ detail={detail} actionPending={actionPending} textareaRef={commentRef} + quoteRequest={quoteRequest} onCommentAction={onCommentAction} onCommented={onCommented} onClose={() => setOpen(false)} + onQuoted={() => { + setRequestedMode("comment"); + setOpen(true); + }} /> ) : null} diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index a4ce4ae7431e0..80101339e30d1 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -114,6 +114,7 @@ import { openOnHostLabel } from "./pullRequestLinkContextMenu"; import { PullRequestMarkdownContext } from "./PullRequestMarkdown"; import { pullRequestMarkdownPlugins } from "./pullRequestMarkdown.logic"; import { PullRequestComposer } from "./PullRequestComposer"; +import type { PullRequestQuoteRequest } from "./PullRequestCommentForm"; import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; import { PullRequestTimelineTab } from "./PullRequestTimelineTab"; import { @@ -376,6 +377,7 @@ export function PullRequestDetailPanel({ useEnvironmentQuery(pullRequestEnvironment.changeBreakdown({ environmentId, input: reference })); const [threadPickerOpen, setThreadPickerOpen] = useState(false); const [openReviewRequest, setOpenReviewRequest] = useState(0); + const [quoteRequest, setQuoteRequest] = useState(); const { canWriteSourceControl, detailQuery, @@ -1887,6 +1889,9 @@ export function PullRequestDetailPanel({ fixCheckLabel={handoffLabels.fixCheck} {...(canFixFindings ? { onFixFinding: startFixFinding } : {})} {...(attachTarget === null ? {} : { onAddToChat: addCommentToChat })} + {...(detail.capabilities.comment && detail.viewerPermissions.comment + ? { onQuoteReply: (text: string) => setQuoteRequest({ text }) } + : {})} onRefresh={refreshDetail} onRefreshChecks={refreshFromHost} /> @@ -2003,6 +2008,7 @@ export function PullRequestDetailPanel({ onCommented={refreshDetail} onReviewSubmitted={refreshDetail} openReviewRequest={openReviewRequest} + quoteRequest={quoteRequest} /> ) : null} diff --git a/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx b/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx index 212da0d088fe3..258de0b037f22 100644 --- a/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx +++ b/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx @@ -36,7 +36,11 @@ import { PullRequestActorAvatar, PullRequestActorLabel } from "./pullRequestPres import { PullRequestMarkdown } from "./PullRequestMarkdown"; import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; import { PullRequestReactionBar } from "./PullRequestReactions"; -import type { PendingReviewComment } from "./pullRequestReviewStore"; +import { + pullRequestThreadReplyKey, + usePullRequestReviewStore, + type PendingReviewComment, +} from "./pullRequestReviewStore"; import { PullRequestMarkdownTextarea, type PullRequestCompletionScope, @@ -225,12 +229,8 @@ export function ReviewThreadCard({ hidden?: boolean; }) { const [expanded, setExpanded] = useState(hidden !== undefined || !thread.isResolved); - const [replying, setReplying] = useState(false); - const [reply, setReply] = useState(""); const [editingId, setEditingId] = useState(null); const [savingEdit, setSavingEdit] = useState(false); - const sendingRef = useRef(false); - const replyRef = useRef(null); const [loadedPage, setLoadedPage] = useState< (PullRequestThreadCommentsResult & { readonly threadId: string }) | null >(null); @@ -259,15 +259,8 @@ export function ReviewThreadCard({ } }; - const send = async (text: string) => { - const trimmed = text.trim(); - if (trimmed.length === 0 || pending || sendingRef.current) return; - sendingRef.current = true; - const result = await settlePromise(() => onReply(trimmed)); - sendingRef.current = false; - if (result._tag === "Failure") throw squashAtomCommandFailure(result); - if (!result.value) return; - // Replies return no comment; retain loaded data and reopen pagination for a later read. + // Replies return no comment; retain loaded data and reopen pagination for a later read. + const keepLoadedPage = () => setLoadedPage((previous) => previous?.threadId === thread.id ? { @@ -276,9 +269,6 @@ export function ReviewThreadCard({ } : previous, ); - setReply(""); - setReplying(false); - }; const loadMore = async () => { if (nextCommentsCursor === null || loadingMore) return; setLoadingMore(true); @@ -420,67 +410,131 @@ export function ReviewThreadCard({ ) : null} - - {canReply ? ( - replying ? ( - - {(speech) => ( -
- speech.submitAfterDictation((text) => void send(text)), - onCancel: () => - speech.state.phase === "idle" ? setReplying(false) : speech.cancel(), - })} - /> -
- - - -
-
- )} -
- ) : ( - - ) - ) : null} ) : null} + {/* Stays mounted while collapsed so an unsent reply survives. */} + {canReply ? ( + + ) : null} ); } + +export function PullRequestThreadReplyForm({ + environmentId, + reference, + threadId, + completionScope, + pending, + collapsed = false, + className, + onReply, + onSent, +}: { + environmentId: EnvironmentId; + reference: PullRequestRef; + threadId: string; + completionScope: PullRequestCompletionScope | null; + pending: boolean; + /** Stops dictation, which would keep recording behind hidden controls. */ + collapsed?: boolean; + className?: string; + onReply: (body: string) => Promise; + onSent?: () => void; +}) { + // Resolving, reopening, or filtering remounts the form, so the draft lives in the store. + const replyKey = pullRequestThreadReplyKey(reference, threadId); + const reply = usePullRequestReviewStore((store) => store.replies[replyKey] ?? ""); + const storeReply = usePullRequestReviewStore((store) => store.setReply); + const clearReply = usePullRequestReviewStore((store) => store.clearReply); + const setReply = (next: string) => storeReply(replyKey, next); + const [replying, setReplying] = useState(reply.length > 0); + const sendingRef = useRef(false); + const replyRef = useRef(null); + + const send = async (text: string) => { + const trimmed = text.trim(); + if (trimmed.length === 0 || pending || sendingRef.current) return; + sendingRef.current = true; + const result = await settlePromise(() => onReply(trimmed)); + sendingRef.current = false; + if (result._tag === "Failure") throw squashAtomCommandFailure(result); + if (!result.value) return; + onSent?.(); + clearReply(replyKey, trimmed); + setReplying(false); + }; + + if (!replying) { + return ( +
+ +
+ ); + } + return ( + + {(speech) => ( +
+ speech.submitAfterDictation((text) => void send(text)), + onCancel: () => + speech.state.phase === "idle" ? setReplying(false) : speech.cancel(), + })} + /> +
+ + + +
+
+ )} +
+ ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestReviewForm.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestReviewForm.browser.test.tsx index eae69143d31ea..6d79e7fe480df 100644 --- a/apps/web/src/components/pullRequest/PullRequestReviewForm.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestReviewForm.browser.test.tsx @@ -1,7 +1,13 @@ import { createRef } from "react"; -import { EnvironmentId, ProjectId } from "@t3tools/contracts"; +import { + EnvironmentId, + ProjectId, + SourceControlProviderKind, + type PullRequestDetailView, +} from "@t3tools/contracts"; import { expect, it, vi } from "vite-plus/test"; import { act, render } from "../../../test/render"; +import { PullRequestCommentForm, type PullRequestQuoteRequest } from "./PullRequestCommentForm"; import { PullRequestReviewForm } from "./PullRequestReviewForm"; import { pullRequestReviewKey, usePullRequestReviewStore } from "./pullRequestReviewStore"; @@ -11,12 +17,16 @@ const mocks = vi.hoisted(() => ({ })); vi.mock("~/speech/useTextFieldSpeech", () => ({ TextFieldSpeechControls: () => null, - useTextFieldSpeech: () => ({ blocksSubmission: true, submitAfterDictation: mocks.queue }), + useTextFieldSpeech: () => ({ + blocksSubmission: true, + submitAfterDictation: mocks.queue, + cancel: () => {}, + }), })); vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => mocks.submit })); vi.mock(import("~/state/pullRequests"), async (original) => ({ ...(await original()), - pullRequestEnvironment: { submitReview: {} } as never, + pullRequestEnvironment: { submitReview: {}, comment: {} } as never, })); it("submits only the line comments still in the draft when dictation finishes", async () => { @@ -56,3 +66,33 @@ it("submits only the line comments still in the draft when dictation finishes", }), ); }); + +const detail = { + provider: SourceControlProviderKind.make("github"), + state: "open", + capabilities: { actions: [] }, + viewerPermissions: { actions: [] }, +} as unknown as PullRequestDetailView; +const reference = { projectId: ProjectId.make("project"), repository: "owner/repo", number: 1 }; + +it("adds a quote that arrives as posting settles along with the ones queued before it", async () => { + const form = (actionPending: boolean, quoteRequest: PullRequestQuoteRequest) => ( + + ); + await using screen = await render(form(true, { text: "> first" })); + await screen.rerender(form(true, { text: "> second" })); + await screen.rerender(form(false, { text: "> third" })); + await expect.element(screen.getByRole("textbox")).toHaveValue("> second\n\n> third"); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx index 19c6cb75c5ac5..3adb39b7f4af5 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx @@ -186,6 +186,52 @@ it("leaves description checkboxes read-only without the right to edit it", async ); }); +it("keeps an unsent reply when a resolved conversation is collapsed and reopened", async () => { + setup(); + const note = { + author: { login: "reviewer", name: null, avatarUrl: null }, + createdAt: "2026-09-01T00:00:00Z", + url: null, + }; + const body = "Rename this helper"; + const filterKey = "t3code:pull-request-comment-filter:v1"; + localStorage.setItem(filterKey, JSON.stringify({ bots: false, resolved: true })); + using _filter = { [Symbol.dispose]: () => localStorage.removeItem(filterKey) }; + await using screen = await render({ + ...detail, + capabilities: { + ...detail.capabilities, + review: { ...detail.capabilities.review, reply: true }, + }, + comments: [ + { ...note, id: "10", kind: "review-comment", body, path: "src/a.ts", reviewState: null }, + ], + reviewThreads: [ + { + id: "thread", + path: "src/a.ts", + line: 3, + side: "right", + isResolved: true, + isOutdated: false, + comments: [{ ...note, id: "10", body }], + }, + ], + }); + const status = screen.getByRole("button", { name: "Resolved" }); + + await userEvent.click(status); + await userEvent.click(screen.getByRole("button", { name: "Reply" })); + await userEvent.type(screen.getByLabelText("Reply to this conversation"), "Unsent reply"); + await userEvent.click(status); + await expect.element(screen.getByLabelText("Reply to this conversation")).not.toBeVisible(); + await userEvent.click(status); + + await expect + .element(screen.getByLabelText("Reply to this conversation")) + .toHaveValue("Unsent reply"); +}); + it("fixes an overview thread as its opening remark, since it has no line to attach", async () => { setup(); const author = { login: "reviewer", name: null, avatarUrl: null }; diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index a78cdfbd6f06f..68fcd79adb551 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -22,6 +22,7 @@ import { HammerIcon, ListFilterIcon, MessageSquarePlusIcon, + ReplyIcon, TagIcon, UsersIcon, } from "lucide-react"; @@ -70,6 +71,7 @@ import { pullRequestConversationItemComments, pullRequestConversationItems, pullRequestFindingKey, + pullRequestQuoteReply, pullRequestReviewOutcome, visibleBody, type PullRequestChatSubject, @@ -95,6 +97,7 @@ import { } from "./PullRequestMarkdownTextarea"; import { PullRequestReactionBar } from "./PullRequestReactions"; import { PullRequestResolveThreadButton } from "./PullRequestResolveThreadButton"; +import { PullRequestThreadReplyForm } from "./PullRequestReviewAnnotation"; import { PullRequestConversationGhost } from "./PullRequestGhosts"; import { sectionCollapseAnchorScrollTop } from "./pullRequestSummaryScroll.logic"; @@ -105,9 +108,11 @@ function reviewerKey(login: string): string { function CommentIdentity({ comment, detail, + action, }: { - comment: PullRequestComment; + comment: Pick; detail: PullRequestDetailView; + action?: string; }) { const actor = comment.author; const profileUrl = actor @@ -116,6 +121,7 @@ function CommentIdentity({ return (
+ {action ? {action} : null} void }) { ); } +function countOf(count: number, one: string, many: string): string | null { + return count === 0 ? null : `${count} ${count === 1 ? one : many}`; +} + function CollapsedComment({ comment, editing, @@ -262,9 +272,10 @@ function CollapsedComment({ resolveButton, detail, thread, - replyCount = 0, + count = null, replies = null, moreReplies = null, + replyForm = null, onAddToChat, }: { comment: PullRequestComment; @@ -275,9 +286,11 @@ function CollapsedComment({ resolveButton: ReactNode; detail: PullRequestDetailView; thread: PullRequestConversationThread | undefined; - replyCount?: number; + /** What the collapsed card holds, such as "2 replies". */ + count?: string | null; replies?: ReactNode; moreReplies?: ReactNode; + replyForm?: ((collapsed: boolean) => ReactNode) | null; onAddToChat?: (() => void) | undefined; }) { const [open, setOpen] = useState(false); @@ -288,10 +301,8 @@ function CollapsedComment({
- {replyCount > 0 ? ( - - {replyCount} {replyCount === 1 ? "reply" : "replies"} - + {count ? ( + {count} ) : null} ) : null} + {/* Stays mounted while collapsed so an unsent reply survives. */} + {replyForm ? ( + + ) : null} {moreReplies ?
{moreReplies}
: null} @@ -457,6 +474,7 @@ export function PullRequestSummaryTab({ fixCheckLabel = "Fix", onFixFinding, onAddToChat, + onQuoteReply, onRefresh, onRefreshChecks = onRefresh, }: { @@ -477,6 +495,8 @@ export function PullRequestSummaryTab({ onFixFinding?: (finding: PullRequestFinding) => void; /** Absent where there is no composer beside the panel to add a remark to. */ onAddToChat?: (subject: PullRequestChatSubject) => void; + /** Absent when the viewer cannot comment; opens the composer with the comment quoted. */ + onQuoteReply?: (quote: string) => void; onRefresh: () => void; onRefreshChecks?: () => void; }) { @@ -667,6 +687,42 @@ export function PullRequestSummaryTab({ }; const completionScope = pullRequestCompletionScope(detail, environmentId, reference); + const replyToThread = useAtomCommand(pullRequestEnvironment.replyToThread, { + reportFailure: false, + }); + const [threadReplyPending, setThreadReplyPending] = useState(false); + const canReplyToThreads = detail.capabilities.review.reply && detail.viewerPermissions.comment; + const sendThreadReply = async (threadId: string, body: string) => { + if (threadReplyPending) return false; + setThreadReplyPending(true); + const result = await replyToThread({ environmentId, input: { ...reference, threadId, body } }); + setThreadReplyPending(false); + if (result._tag === "Failure") { + toastManager.add({ type: "error", title: "Reply could not be posted" }); + return false; + } + onRefresh(); + return true; + }; + const threadReplyForm = ( + thread: PullRequestConversationThread, + className?: string, + collapsed = false, + ) => + canReplyToThreads ? ( + sendThreadReply(thread.id, body)} + /> + ) : null; const commentEditing: CommentEditing = { cwd: detail.workspaceRoot, environmentId, @@ -736,6 +792,8 @@ export function PullRequestSummaryTab({ : { kind: "thread", thread }; // A reply goes with its conversation, from the card that heads it. const chat = reply ? null : chatSubject(comment, thread); + // Review threads take real replies; anything else can only be answered by quoting it. + const canQuote = onQuoteReply && !reply && thread === undefined && body !== null; return (
onAddToChat(chat)} /> ) : null} + {onQuoteReply && canQuote ? ( + + ) : null} {reactionBarFor(comment)}
{reply ? null : ( @@ -786,47 +869,95 @@ export function PullRequestSummaryTab({ ); }; - const renderItem = (item: PullRequestConversationItem) => { - const comment = item.kind === "comment" ? item.comment : item.root; - const thread = item.kind === "thread" ? item.thread : undefined; - const replies = - item.kind === "thread" && item.replies.length > 0 ? ( -
- {item.replies.map((reply) => renderCommentCard(reply, undefined, { reply: true }))} + const renderItem = (item: PullRequestConversationItem): ReactNode => { + if (item.kind === "review") { + const threads = ( +
+ {item.threads.map(renderItem)}
- ) : null; - const moreReplies = - item.kind === "thread" && item.hasMoreReplies ? : null; - if (isPullRequestCommentFinished(comment, thread)) { - const chat = chatSubject(comment, thread); + ); + const review = item.review; + // A dismissed review collapses together with the threads it posted. + if (review !== null && isPullRequestCommentFinished(review)) { + return renderCollapsed( + { kind: "comment", id: review.id, createdAt: review.createdAt, comment: review }, + threads, + countOf(item.threads.length, "thread", "threads"), + ); + } return ( - onAddToChat(chat) : undefined} - reactionBar={reactionBarFor(comment)} - resolveButton={resolveButtonFor(thread)} - /> +
+ {review === null ? ( +
+ +
+ ) : ( + renderCommentCard(review, undefined) + )} + {threads} +
); } - if (item.kind === "comment") return renderCommentCard(comment, undefined); + const thread = item.kind === "thread" ? item.thread : undefined; + if (isPullRequestCommentFinished(item.kind === "comment" ? item.comment : item.root, thread)) { + return renderCollapsed( + item, + threadReplies(item), + item.kind === "thread" + ? countOf(item.replies.length + item.moreReplies, "reply", "replies") + : null, + ); + } + if (item.kind === "comment") return renderCommentCard(item.comment, undefined); return (
- {renderCommentCard(comment, thread)} - {replies} - {moreReplies} + {renderCommentCard(item.root, item.thread)} + {threadReplies(item)} + {item.hasMoreReplies ? : null} + {threadReplyForm(item.thread, "mt-0")}
); }; + const threadReplies = (item: PullRequestConversationItem) => + item.kind === "thread" && item.replies.length > 0 ? ( +
+ {item.replies.map((reply) => renderCommentCard(reply, undefined, { reply: true }))} +
+ ) : null; + + const renderCollapsed = ( + item: Exclude, + replies: ReactNode, + count: string | null, + ) => { + const comment = item.kind === "comment" ? item.comment : item.root; + const thread = item.kind === "thread" ? item.thread : undefined; + const chat = chatSubject(comment, thread); + return ( + + ) : null + } + replyForm={thread ? (collapsed) => threadReplyForm(thread, "mt-0", collapsed) : null} + onAddToChat={onAddToChat && chat !== null ? () => onAddToChat(chat) : undefined} + reactionBar={reactionBarFor(comment)} + resolveButton={resolveButtonFor(thread)} + /> + ); + }; + return (
{ diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 6ea7d6a5b0e1a..bc458aac07b4f 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -53,6 +53,7 @@ import { pullRequestLinkSubject, writePullRequestDetailSnapshot, readableFailure, + pullRequestQuoteReply, } from "./pullRequestDetail.logic"; import type { ReviewCommentContext } from "~/reviewCommentContext"; @@ -537,6 +538,39 @@ describe("review thread conversations", () => { comments: [lastReply, { ...finding, body: "Stale finding" }, reply], }; + it("quotes a comment without its hidden markup and mentions a bot by its handle", () => { + expect( + pullRequestQuoteReply({ + ...finding, + body: "\r\nFirst line\r\n\r\nSecond line", + }), + ).toBe("> First line\n>\n> Second line\n\n@review "); + expect(pullRequestQuoteReply({ ...loose, author: null, body: "Hi" })).toBe("> Hi\n\n"); + expect(pullRequestQuoteReply({ ...loose, author: null }, "\n picked\n\tlines \n")).toBe( + "> \u00a0\u00a0picked\n> \u00a0\u00a0\u00a0\u00a0lines\n\n", + ); + expect(pullRequestQuoteReply({ ...loose, author: null }, "©")).toBe("> \\©\n\n"); + expect(pullRequestQuoteReply({ ...loose, author: null }, "")).toBe( + "> \\<\\!\\-\\- keep \\-\\-\\>\n\n", + ); + expect(pullRequestQuoteReply({ ...loose, author: null }, "# Heading")).toBe( + "> \\# Heading\n\n", + ); + expect( + pullRequestQuoteReply({ + ...loose, + author: null, + body: "Use ``:\n```html\n\n```", + }), + ).toBe("> Use ``:\n> ```html\n> \n> ```\n\n"); + expect( + pullRequestQuoteReply({ ...loose, author: null, body: "~~~html\n\n~~~" }), + ).toBe("> ~~~html\n> \n> ~~~\n\n"); + expect( + pullRequestQuoteReply({ ...loose, author: null, body: "Not code \\`\\`" }), + ).toBe("> Not code \\`\\`\n\n"); + }); + it("keeps a timeline finding and its replies in one row at the finding time", () => { const events = buildPullRequestTimeline({ ...TIMELINE_SOURCE, @@ -595,6 +629,125 @@ describe("review thread conversations", () => { ); }); + it("nests each thread under the review its first comment was posted in", () => { + const reviewUrl = (id: string) => `https://example.test/pull/1#pullrequestreview-${id}`; + const review = (id: string, day: number, body: string, reviewState: string) => ({ + ...comment(`review-${id}`, day), + kind: "review" as const, + body, + path: null, + reviewState, + url: reviewUrl(id), + }); + const posted = (id: string, day: number, inReview: string | null) => ({ + ...comment(id, day), + ...(inReview === null ? {} : { reviewUrl: reviewUrl(inReview) }), + }); + const threadOf = (id: string, ...members: ReturnType[]) => ({ + ...thread, + id, + comments: members, + }); + const summarized = review("1", 3, "Two notes", "COMMENTED"); + const verdict = review("2", 5, "", "CHANGES_REQUESTED"); + const first = posted("first", 2, "1"); + // The comment's copy of the link can carry another path; the anchor names the review. + const second = { + ...posted("second", 2, null), + reviewUrl: `https://example.test/pull/1/files#pullrequestreview-1`, + }; + // A reply posts as its own small review; the thread stays with its root's. + const laterReply = posted("later-reply", 4, "2"); + const blocker = posted("blocker", 5, "2"); + const bodiless = posted("bodiless", 7, "3"); + const unlinked = posted("unlinked", 8, null); + const items = pullRequestConversationItems( + [summarized, verdict], + [ + threadOf("t-second", second), + threadOf("t-first", first, laterReply), + threadOf("t-blocker", blocker), + threadOf("t-bodiless", bodiless), + threadOf("t-unlinked", unlinked), + ], + ); + expect( + items.map((item) => + item.kind === "review" + ? { id: item.id, review: item.review, threads: item.threads.map((entry) => entry.id) } + : item.id, + ), + ).toEqual([ + { id: "review-1", review: summarized, threads: ["t-second", "t-first"] }, + { id: "review-2", review: verdict, threads: ["t-blocker"] }, + { id: `review:${reviewUrl("3")}`, review: null, threads: ["t-bodiless"] }, + "t-unlinked", + ]); + expect(items[2]).toMatchObject({ createdAt: bodiless.createdAt, author: bodiless.author }); + expect(pullRequestConversationItemComments(items[0]!)).toEqual([ + summarized, + { ...second, kind: "review-comment", reviewState: null }, + { ...first, kind: "review-comment", reviewState: null }, + { ...laterReply, kind: "review-comment", reviewState: null }, + ]); + const resolvedFirst = pullRequestConversationItems( + [summarized], + [threadOf("t-second", second), { ...threadOf("t-first", first), isResolved: true }], + ); + expect( + filterPullRequestConversationItems(resolvedFirst, { bots: true, resolved: false }).map( + (item) => (item.kind === "review" ? item.threads.map((entry) => entry.id) : item.id), + ), + ).toEqual([["t-second"]]); + const allResolved = pullRequestConversationItems( + [], + [{ ...threadOf("t-bodiless", bodiless), isResolved: true }], + ); + expect( + filterPullRequestConversationItems(allResolved, { bots: true, resolved: false }), + ).toEqual([]); + // With every thread filtered out, a summary that still shows stands as a plain comment. + const summaryOnly = pullRequestConversationItems( + [summarized], + [{ ...threadOf("t-first", first), isResolved: true }], + ); + expect( + filterPullRequestConversationItems(summaryOnly, { bots: true, resolved: false }), + ).toEqual([ + { kind: "comment", id: summarized.id, createdAt: summarized.createdAt, comment: summarized }, + ]); + // A human reply keeps a bot's thread, but the bot's summary gives way to a bare header. + const botReview = { ...summarized, author: bot }; + const botThread = threadOf("t-first", { ...first, author: bot }, laterReply); + expect( + filterPullRequestConversationItems(pullRequestConversationItems([botReview], [botThread]), { + bots: false, + resolved: true, + }), + ).toMatchObject([ + { kind: "review", id: botReview.id, review: null, threads: [{ id: "t-first" }] }, + ]); + // A review counts each of its threads toward a page, and the page edge cuts its earliest. + const page = pagePullRequestConversationItems(items, 3); + expect(page.hidden).toBe(2); + expect( + page.shown.map((item) => + item.kind === "review" ? item.threads.map((entry) => entry.id) : item.id, + ), + ).toEqual([["t-blocker"], ["t-bodiless"], "t-unlinked"]); + expect(pagePullRequestConversationItems(items, 4).shown[0]).toMatchObject({ + id: "review-1", + threads: [{ id: "t-first" }], + }); + expect( + buildPullRequestTimeline({ + ...TIMELINE_SOURCE, + comments: [summarized], + reviewThreads: [threadOf("t-first", first)], + }).map((event) => event.id), + ).toEqual(["review-1", "first", "1baf7bdcafe", "created"]); + }); + it("filters whole conversations when any member matches, including resolved and dismissed reviews", () => { const botOnly = { ...comment("bot-only", 7), author: bot }; const dismissed = { diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 97c5919f3ef2e..93a1f0897bf81 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -528,6 +528,31 @@ export function visibleBody(body: string): string | null { return body.replace(//gu, "").trim().length === 0 ? null : body.trim(); } +/** Quotes `selection` when the reader picked part of the comment, else the whole body. */ +export function pullRequestQuoteReply(comment: PullRequestComment, selection?: string): string { + // Selected text is what the reader saw, so it is escaped to render literally. Blockquotes + // drop leading spaces, so its indentation becomes non-breaking spaces. + const selected = selection + ?.replace(/^\s*\n|\s+$/gu, "") + .replace(/[\\`*_[\]{}()#+.!|~<>&-]/gu, "\\$&") + .replace(/^[ \t]+/gmu, (indent) => indent.replace(/\t/gu, " ").replace(/ /gu, "\u00a0")); + const body = + selected || + comment.body + .replace( + /(\\[\s\S]|```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]*`)|/gu, + (_, kept?: string) => kept ?? "", + ) + .trim(); + const quote = body + .split(/\r?\n/u) + .map((line) => (line.trim().length === 0 ? ">" : `> ${line}`)) + .join("\n"); + // GitHub bot logins carry a "[bot]" suffix that does not resolve as a mention. + const login = comment.author?.login.replace(/\[bot\]$/u, ""); + return `${quote}\n\n${login ? `@${login} ` : ""}`; +} + function commentTimelineEvent(comment: PullRequestComment): PullRequestTimelineEvent { return { comment, @@ -548,6 +573,15 @@ function commentTimelineEvent(comment: PullRequestComment): PullRequestTimelineE }; } +export type PullRequestConversationThreadItem = { + readonly kind: "thread"; + readonly id: string; + readonly createdAt: string; + readonly root: PullRequestComment; + readonly replies: ReadonlyArray; + readonly thread: PullRequestConversationThread; +} & PullRequestReplyAvailability; + export type PullRequestConversationItem = | { readonly kind: "comment"; @@ -555,14 +589,17 @@ export type PullRequestConversationItem = readonly createdAt: string; readonly comment: PullRequestComment; } - | ({ - readonly kind: "thread"; + | PullRequestConversationThreadItem + | { + readonly kind: "review"; readonly id: string; readonly createdAt: string; - readonly root: PullRequestComment; - readonly replies: ReadonlyArray; - readonly thread: PullRequestConversationThread; - } & PullRequestReplyAvailability); + /** Null for a review the host leaves out of the conversation, such as GitHub's with no text. */ + readonly review: PullRequestComment | null; + readonly author: PullRequestActor | null; + readonly url: string; + readonly threads: ReadonlyArray; + }; export function pullRequestConversationItems( comments: ReadonlyArray, @@ -570,7 +607,16 @@ export function pullRequestConversationItems( ): ReadonlyArray { const byId = new Map(comments.map((comment) => [comment.id, comment])); const claimed = new Set(); - const threads: PullRequestConversationItem[] = []; + const reviewKeys = new Set( + comments.flatMap((comment) => + comment.kind === "review" && comment.url !== null ? [reviewKey(comment.url)] : [], + ), + ); + const loose: PullRequestConversationItem[] = []; + const byReview = new Map< + string, + { readonly url: string; readonly threads: PullRequestConversationThreadItem[] } + >(); for (const thread of reviewThreads) { // Keep the conversation's objects for editing/reactions; some hosts only include a // comment in the thread, so normalize that copy when the flat conversation lacks it. @@ -589,7 +635,7 @@ export function pullRequestConversationItems( if (root === undefined) continue; for (const comment of ordered) claimed.add(comment.id); const moreReplies = Math.max(0, (thread.commentCount ?? ordered.length) - ordered.length); - threads.push({ + const item: PullRequestConversationThreadItem = { kind: "thread", id: thread.id, createdAt: root.createdAt, @@ -599,30 +645,75 @@ export function pullRequestConversationItems( moreReplies, hasMoreReplies: moreReplies > 0 || thread.nextCommentsCursor !== undefined, url: ordered.find((comment) => comment.url !== null)?.url ?? null, + }; + // Only the root places a thread: each later reply posts as a review of its own. + const reviewUrl = thread.comments.find((comment) => comment.id === root.id)?.reviewUrl; + if (!reviewUrl) { + loose.push(item); + continue; + } + const key = reviewKey(reviewUrl); + const review = byReview.get(key); + if (review) review.threads.push(item); + else byReview.set(key, { url: reviewUrl, threads: [item] }); + } + for (const [key, { url, threads }] of byReview) { + threads.sort((left, right) => instant(left.createdAt) - instant(right.createdAt)); + if (reviewKeys.has(key)) continue; + const first = threads[0]!; + loose.push({ + kind: "review", + id: `review:${url}`, + createdAt: first.createdAt, + review: null, + author: first.root.author, + url, + threads, }); } - threads.sort((left, right) => instant(left.createdAt) - instant(right.createdAt)); + loose.sort((left, right) => instant(left.createdAt) - instant(right.createdAt)); const items: PullRequestConversationItem[] = []; let next = 0; // Preserve loose comments' existing order, inserting each thread at its finding's time. for (const comment of comments) { if (claimed.has(comment.id)) continue; - while ( - next < threads.length && - instant(threads[next]!.createdAt) < instant(comment.createdAt) - ) { - items.push(threads[next++]!); + while (next < loose.length && instant(loose[next]!.createdAt) < instant(comment.createdAt)) { + items.push(loose[next++]!); } - items.push({ kind: "comment", id: comment.id, createdAt: comment.createdAt, comment }); + const url = comment.url; + const threads = url === null ? undefined : byReview.get(reviewKey(url))?.threads; + items.push( + comment.kind === "review" && url !== null && threads !== undefined + ? { + kind: "review", + id: comment.id, + createdAt: comment.createdAt, + review: comment, + author: comment.author, + url, + threads, + } + : { kind: "comment", id: comment.id, createdAt: comment.createdAt, comment }, + ); } - items.push(...threads.slice(next)); + items.push(...loose.slice(next)); return items; } +/** GitHub review links differ only in their anchor between the review and its comments' copies. */ +function reviewKey(url: string): string { + return /#pullrequestreview-\d+$/i.exec(url)?.[0].toLowerCase() ?? url; +} + export function pullRequestConversationItemComments( item: PullRequestConversationItem, ): ReadonlyArray { - return item.kind === "comment" ? [item.comment] : [item.root, ...item.replies]; + if (item.kind === "comment") return [item.comment]; + if (item.kind === "thread") return [item.root, ...item.replies]; + return [ + ...(item.review === null ? [] : [item.review]), + ...item.threads.flatMap(pullRequestConversationItemComments), + ]; } export function isPullRequestCommentFinished( @@ -643,26 +734,55 @@ export function filterPullRequestConversationItems( items: ReadonlyArray, filter: { readonly bots: boolean; readonly resolved: boolean }, ): ReadonlyArray { - return items.filter((item) => - // Matching any member keeps the finding beside a human reply to a bot. - pullRequestConversationItemComments(item).some( - (comment) => - // A bot's approval or change request is a verdict, not chatter. - (filter.bots || - comment.author?.isBot !== true || - isPullRequestVerdict(comment.reviewState)) && - (filter.resolved || - !isPullRequestCommentFinished(comment, item.kind === "thread" ? item.thread : undefined)), - ), - ); + const keeps = (comment: PullRequestComment, thread?: PullRequestConversationThread) => + // A bot's approval or change request is a verdict, not chatter. + (filter.bots || comment.author?.isBot !== true || isPullRequestVerdict(comment.reviewState)) && + (filter.resolved || !isPullRequestCommentFinished(comment, thread)); + // Matching any member keeps the finding beside a human reply to a bot. + const matches = (item: PullRequestConversationItem) => + pullRequestConversationItemComments(item).some((comment) => + keeps(comment, item.kind === "thread" ? item.thread : undefined), + ); + return items.flatMap((item): ReadonlyArray => { + if (item.kind !== "review") return matches(item) ? [item] : []; + const threads = item.threads.filter(matches); + // A summary the filter hides gives way to a bare header over the threads it kept. + const review = item.review !== null && keeps(item.review) ? item.review : null; + if (threads.length === 0) { + return review === null + ? [] + : [{ kind: "comment", id: review.id, createdAt: review.createdAt, comment: review }]; + } + return [ + threads.length === item.threads.length && review === item.review + ? item + : { ...item, review, threads }, + ]; + }); } export function pagePullRequestConversationItems( items: ReadonlyArray, count: number, ): { readonly shown: ReadonlyArray; readonly hidden: number } { - const shown = items.slice(Math.max(0, items.length - count)); - return { shown, hidden: items.length - shown.length }; + // A review counts each of its threads, so one with dozens of findings does not fill a page. + const weight = (item: PullRequestConversationItem) => + item.kind === "review" ? item.threads.length : 1; + const shown: PullRequestConversationItem[] = []; + let left = count; + for (let index = items.length - 1; index >= 0 && left > 0; index--) { + const item = items[index]!; + if (item.kind === "review" && item.threads.length > left) { + // The page edge cuts the review's earliest threads; the next page brings them back. + shown.push({ ...item, threads: item.threads.slice(-left) }); + left = 0; + break; + } + shown.push(item); + left -= weight(item); + } + const hidden = items.reduce((total, item) => total + weight(item), 0) - (count - left); + return { shown: shown.reverse(), hidden }; } /** @@ -823,24 +943,31 @@ export function buildPullRequestTimeline( ...pullRequestConversationItems(detail.comments, [ ...(detail.reviewThreads ?? []), ...(detail.overviewThreads ?? []), - ]).map((item) => { - if (item.kind === "comment") return commentTimelineEvent(item.comment); - return { - ...commentTimelineEvent(item.root), - thread: { - id: item.thread.id, - ...(item.thread.canResolve === undefined ? {} : { canResolve: item.thread.canResolve }), - path: item.thread.path, - line: item.thread.line, - isResolved: item.thread.isResolved, - isOutdated: item.thread.isOutdated, - replies: item.replies.map(commentTimelineEvent), - moreReplies: item.moreReplies, - hasMoreReplies: item.hasMoreReplies, - url: item.url, - }, - }; - }), + ]) + // The Timeline tab keeps a review's threads as rows of their own. + .flatMap((item) => + item.kind === "review" + ? [...(item.review === null ? [] : [item.review]), ...item.threads] + : [item.kind === "comment" ? item.comment : item], + ) + .map((item) => { + if (item.kind !== "thread") return commentTimelineEvent(item); + return { + ...commentTimelineEvent(item.root), + thread: { + id: item.thread.id, + ...(item.thread.canResolve === undefined ? {} : { canResolve: item.thread.canResolve }), + path: item.thread.path, + line: item.thread.line, + isResolved: item.thread.isResolved, + isOutdated: item.thread.isOutdated, + replies: item.replies.map(commentTimelineEvent), + moreReplies: item.moreReplies, + hasMoreReplies: item.hasMoreReplies, + url: item.url, + }, + }; + }), ...(detail.mergedAt ? [ { diff --git a/apps/web/src/components/pullRequest/pullRequestReviewStore.test.ts b/apps/web/src/components/pullRequest/pullRequestReviewStore.test.ts index 2ba71f34eb080..9772ed785fdd2 100644 --- a/apps/web/src/components/pullRequest/pullRequestReviewStore.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestReviewStore.test.ts @@ -54,12 +54,18 @@ describe("pull request review drafts", () => { expect(usePullRequestReviewStore.getState().summaries[publicKey]).toBe("Public review"); }); - it("does not clear a summary revised while submission is in flight", () => { + it("does not clear a summary or reply revised while submission is in flight", () => { const store = usePullRequestReviewStore.getState(); store.setSummary("review-a", "Submitted body"); usePullRequestReviewStore.getState().setSummary("review-a", "Revised body"); usePullRequestReviewStore.getState().clearSummary("review-a", "Submitted body"); expect(usePullRequestReviewStore.getState().summaries["review-a"]).toBe("Revised body"); + + store.setReply("reply-a", "Revised reply"); + store.clearReply("reply-a", "Sent reply"); + expect(usePullRequestReviewStore.getState().replies["reply-a"]).toBe("Revised reply"); + store.clearReply("reply-a", "Revised reply"); + expect(usePullRequestReviewStore.getState().replies["reply-a"]).toBeUndefined(); }); }); diff --git a/apps/web/src/components/pullRequest/pullRequestReviewStore.ts b/apps/web/src/components/pullRequest/pullRequestReviewStore.ts index d48695ed8f5a2..03e0dc4ede877 100644 --- a/apps/web/src/components/pullRequest/pullRequestReviewStore.ts +++ b/apps/web/src/components/pullRequest/pullRequestReviewStore.ts @@ -23,12 +23,16 @@ export function pullRequestReviewKey(reference: PullRequestRef): string { interface PullRequestReviewStoreState { readonly drafts: Readonly>>; readonly summaries: Readonly>; + /** Unsent thread replies, so a reply form that remounts elsewhere reads the live draft. */ + readonly replies: Readonly>; readonly addComment: (key: string, comment: PendingReviewComment) => void; readonly removeComment: (key: string, commentId: string) => void; readonly removeComments: (key: string, commentIds: ReadonlyArray) => void; readonly clear: (key: string) => void; readonly setSummary: (key: string, body: string) => void; readonly clearSummary: (key: string, submittedBody: string) => void; + readonly setReply: (key: string, body: string) => void; + readonly clearReply: (key: string, submittedBody: string) => void; } const EMPTY: ReadonlyArray = []; @@ -36,6 +40,7 @@ const EMPTY: ReadonlyArray = []; export const usePullRequestReviewStore = create()((set) => ({ drafts: {}, summaries: {}, + replies: {}, addComment: (key, comment) => set((state) => ({ drafts: { ...state.drafts, [key]: [...(state.drafts[key] ?? EMPTY), comment] }, @@ -68,8 +73,19 @@ export const usePullRequestReviewStore = create()(( const { [key]: _removed, ...rest } = state.summaries; return { summaries: rest }; }), + setReply: (key, body) => set((state) => ({ replies: { ...state.replies, [key]: body } })), + clearReply: (key, submittedBody) => + set((state) => { + if (state.replies[key]?.trim() !== submittedBody) return state; + const { [key]: _removed, ...rest } = state.replies; + return { replies: rest }; + }), })); +export function pullRequestThreadReplyKey(reference: PullRequestRef, threadId: string): string { + return JSON.stringify([pullRequestReviewKey(reference), threadId]); +} + export function usePendingReviewComments( reference: PullRequestRef, ): ReadonlyArray { diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index 685afdc355c08..39827e8afe440 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -900,6 +900,28 @@ describe("review thread decoding", () => { createdAt: "2026-07-01T01:00:00Z", pullRequestReview: { url: "https://github.com/acme/web/pull/1#pullrequestreview-9", + state: "COMMENTED", + isMinimized: false, + }, + }, + { + id: "t3", + body: "draft", + createdAt: "2026-07-01T02:00:00Z", + pullRequestReview: { + url: "https://github.com/acme/web/pull/1#pullrequestreview-10", + state: "PENDING", + isMinimized: false, + }, + }, + { + id: "t4", + body: "hidden", + createdAt: "2026-07-01T03:00:00Z", + pullRequestReview: { + url: "https://github.com/acme/web/pull/1#pullrequestreview-11", + state: "COMMENTED", + isMinimized: true, }, }, ], @@ -909,11 +931,14 @@ describe("review thread decoding", () => { ), ); const comments = reviewThreadConversation(result.threads.map((entry) => entry.thread)); - expect(comments.map((comment) => comment.id)).toEqual(["t1", "t2"]); + expect(comments.map((comment) => comment.id)).toEqual(["t1", "t2", "t3", "t4"]); // A reply-only review has no text of its own; its link finds it through the reply it posted. + // A pending or hidden review is not in the conversation, so nothing links to it. expect(result.threads[0]?.thread.comments.map((comment) => comment.reviewUrl)).toEqual([ undefined, "https://github.com/acme/web/pull/1#pullrequestreview-9", + undefined, + undefined, ]); }); diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index b97454d1c74bb..47e9dce023084 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -1344,7 +1344,9 @@ export function toReviewDismissalsPage(result: ReviewDismissals.Result): { } function toThreadComment(comment: ThreadComment, viewer: string | null): PullRequestThreadComment { - const reviewUrl = trimmed(comment.pullRequestReview?.url); + const review = comment.pullRequestReview; + const reviewUrl = + review == null || review.isMinimized || review.state === "PENDING" ? null : trimmed(review.url); return { id: comment.id, author: toActor(comment.author), diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql b/packages/source-control-github/src/server/pullRequestReviews.graphql index d497615c24693..4c63bf736b2d2 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql @@ -39,8 +39,11 @@ fragment ThreadComment on PullRequestReviewComment { url isMinimized # A review link names the review, not its comments; this lets it find the comments it posted. + # A pending or hidden review is not in the conversation, so it gets no link. pullRequestReview { url + state + isMinimized } ...Reactions } diff --git a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts index bce1451b8c296..b1c3aa051c088 100644 --- a/packages/source-control-github/src/server/pullRequestReviews.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReviews.graphql.ts @@ -92,7 +92,11 @@ export interface Actor extends Schema.Struct.Type {} /** A review object for a given pull request. */ export const PullRequestReviewPullRequestReview = Schema.Struct({ /** The HTTP URL permalink for this PullRequestReview. */ - url: Shared.URI + url: Shared.URI, + /** Identifies the current state of the pull request review. */ + state: GraphQL.enumLiterals(Shared.PullRequestReviewState.literals), + /** Returns whether or not a comment has been minimized. */ + isMinimized: Schema.Boolean }) export interface PullRequestReviewPullRequestReview extends Schema.Struct.Type {} @@ -434,7 +438,7 @@ export const ReviewThreads = GraphQL.query("ReviewThreads", { fragments: [ { name: "ReviewThreadPage", body: ["on PullRequestReviewThreadConnection{totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}"] }, { name: "Reactions", body: ["on Reactable{__typename reactionGroups{content viewerHasReacted reactors(first:10){totalCount nodes{__typename...on User{login}...on Bot{login}...on Organization{login}...on Mannequin{login}}}}}"] }, - { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url isMinimized pullRequestReview{url}...Reactions}"] } + { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url isMinimized pullRequestReview{url state isMinimized}...Reactions}"] } ] }, variables: { @@ -484,7 +488,7 @@ export const ReviewThreadPages = GraphQL.query("ReviewThreadPages", { ], fragments: [ { name: "ReviewThreadPage", body: ["on PullRequestReviewThreadConnection{totalCount pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated path line diffSide comments(first:10){totalCount pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}"] }, - { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url isMinimized pullRequestReview{url}...Reactions}"] }, + { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url isMinimized pullRequestReview{url state isMinimized}...Reactions}"] }, { name: "Reactions", body: ["on Reactable{__typename reactionGroups{content viewerHasReacted reactors(first:10){totalCount nodes{__typename...on User{login}...on Bot{login}...on Organization{login}...on Mannequin{login}}}}}"] } ] }, @@ -561,7 +565,7 @@ export const ReviewThreadComments = GraphQL.query("ReviewThreadComments", { { name: "node", body: ["(id:$", "threadId", "){__typename...on PullRequestReviewThread{pullRequest{id}comments(first:100 after:$", "after", "){pageInfo{hasNextPage endCursor}nodes{...ThreadComment}}}}"] } ], fragments: [ - { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url isMinimized pullRequestReview{url}...Reactions}"] }, + { name: "ThreadComment", body: ["on PullRequestReviewComment{id author{__typename login avatarUrl}body createdAt lastEditedAt url isMinimized pullRequestReview{url state isMinimized}...Reactions}"] }, { name: "Reactions", body: ["on Reactable{__typename reactionGroups{content viewerHasReacted reactors(first:10){totalCount nodes{__typename...on User{login}...on Bot{login}...on Organization{login}...on Mannequin{login}}}}}"] } ] },