Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 5 additions & 28 deletions src/browser/components/pr-overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
import {
usePRReviewSelector,
usePRReviewStore,
useThreadActions,
getTimeAgo,
} from "../contexts/pr-review";
import { parseDiffCached, type ParsedDiff } from "../lib/diff";
Expand Down Expand Up @@ -794,34 +795,10 @@ export const PROverview = memo(function PROverview() {
[github, owner, repo, pr.number, store]
);

const handleResolveThread = useCallback(
async (threadId: string) => {
try {
await github.resolveThread(threadId);
// Update local state
store.updateReviewThread(threadId, (t) => ({ ...t, isResolved: true }));
} catch (error) {
console.error("Failed to resolve thread:", error);
}
},
[github, store]
);

const handleUnresolveThread = useCallback(
async (threadId: string) => {
try {
await github.unresolveThread(threadId);
// Update local state
store.updateReviewThread(threadId, (t) => ({
...t,
isResolved: false,
}));
} catch (error) {
console.error("Failed to unresolve thread:", error);
}
},
[github, store]
);
const {
resolveThread: handleResolveThread,
unresolveThread: handleUnresolveThread,
} = useThreadActions();

// Calculate check status
const checkStatus = calculateCheckStatus(
Expand Down
33 changes: 17 additions & 16 deletions src/browser/components/pr-review.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
useFileCopyActions,
useSkipBlockExpansion,
useThreadActions,
enrichCommentsWithThreads,
useCurrentFile,
useCurrentDiff,
useIsCurrentFileLoading,
Expand Down Expand Up @@ -227,7 +228,12 @@ export function PRReviewContent({

setPr(prData);
setFiles(filesData);
setComments(commentsData as ReviewComment[]);
setComments(
enrichCommentsWithThreads(
commentsData as ReviewComment[],
reviewThreadsResult.threads
)
);
setViewerPermission(reviewThreadsResult.viewerPermission);
setViewerCanMergeAsAdmin(reviewThreadsResult.viewerCanMergeAsAdmin);

Expand Down Expand Up @@ -2499,19 +2505,19 @@ const CommentThread = memo(function CommentThread({
const repo = usePRReviewSelector((s) => s.repo);
const { replyToComment, updateComment, deleteComment } = useCommentActions();
const { resolveThread, unresolveThread } = useThreadActions();
// Get resolution info from first comment (all comments in thread share same resolution status)
const firstComment = comments[0];
const isResolved = firstComment?.is_resolved ?? false;
const threadId = firstComment?.pull_request_review_thread_id;

const [replyText, setReplyText] = useState("");
const [submitting, setSubmitting] = useState(false);
const [isCollapsed, setIsCollapsed] = useState(false);
const [isCollapsed, setIsCollapsed] = useState(isResolved);
const [resolving, setResolving] = useState(false);

const replyingTo =
comments.find((c) => c.id === replyingToCommentId)?.id ?? null;

// Get resolution info from first comment (all comments in thread share same resolution status)
const firstComment = comments[0];
const isResolved = firstComment?.is_resolved ?? false;
const threadId = firstComment?.pull_request_review_thread_id;

const handleSubmitReply = useCallback(async () => {
if (!replyText.trim() || !replyingTo) return;

Expand Down Expand Up @@ -2549,8 +2555,6 @@ const CommentThread = memo(function CommentThread({
setResolving(true);
try {
await resolveThread(threadId);
// Auto-collapse when resolved
setIsCollapsed(true);
} finally {
setResolving(false);
}
Expand All @@ -2561,17 +2565,14 @@ const CommentThread = memo(function CommentThread({
setResolving(true);
try {
await unresolveThread(threadId);
setIsCollapsed(false);
} finally {
setResolving(false);
}
}, [threadId, unresolveThread]);

// Auto-collapse resolved threads
// Sync collapse state on resolution changes, including from other views
useEffect(() => {
if (isResolved) {
setIsCollapsed(true);
}
setIsCollapsed(isResolved);
}, [isResolved]);

return (
Expand Down Expand Up @@ -2602,9 +2603,9 @@ const CommentThread = memo(function CommentThread({
? "Resolved"
: `${comments.length} comment${comments.length !== 1 ? "s" : ""}`}
</span>
{isResolved && isCollapsed && (
{isResolved && isCollapsed && firstComment.resolved_by?.login && (
<span className="text-xs text-muted-foreground">
by {firstComment.user.login}
by {firstComment.resolved_by.login}
</span>
)}
</div>
Expand Down
80 changes: 80 additions & 0 deletions src/browser/contexts/pr-review/enrichComments.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { test, expect } from "bun:test";
import type { ReviewComment } from "@/api/types";
import type { ReviewThread } from "@/browser/contexts/github";
import { enrichCommentsWithThreads } from "./enrichComments";

function createComment(id: number): ReviewComment {
return { id, body: `comment ${id}` } as ReviewComment;
}

function createThread(
id: string,
commentIds: number[],
options?: {
isResolved?: boolean;
resolvedBy?: { login: string; avatarUrl: string } | null;
}
): ReviewThread {
return {
id,
isResolved: options?.isResolved ?? false,
resolvedBy: options?.resolvedBy ?? null,
pullRequestReview: null,
comments: {
nodes: commentIds.map((databaseId) => ({
id: `gql-${databaseId}`,
databaseId,
body: "",
path: "file.ts",
line: 1,
originalLine: 1,
startLine: null,
diffHunk: null,
author: null,
createdAt: "",
updatedAt: "",
replyTo: null,
})),
},
};
}

test("enrichCommentsWithThreads attaches resolution info to every comment in a thread", () => {
const comments = [createComment(1), createComment(2), createComment(3)];
const threads = [
createThread("THREAD_A", [1, 2], {
isResolved: true,
resolvedBy: { login: "alice", avatarUrl: "https://a.png" },
}),
createThread("THREAD_B", [3]),
];

const enriched = enrichCommentsWithThreads(comments, threads);

expect(enriched[0]?.pull_request_review_thread_id).toBe("THREAD_A");
expect(enriched[0]?.is_resolved).toBe(true);
expect(enriched[0]?.resolved_by).toEqual({
login: "alice",
avatar_url: "https://a.png",
});
expect(enriched[1]?.is_resolved).toBe(true);
expect(enriched[2]?.pull_request_review_thread_id).toBe("THREAD_B");
expect(enriched[2]?.is_resolved).toBe(false);
expect(enriched[2]?.resolved_by).toBeNull();
});

test("enrichCommentsWithThreads leaves comments without a matching thread untouched", () => {
const comments = [createComment(1), createComment(99)];
const threads = [createThread("THREAD_A", [1], { isResolved: true })];

const enriched = enrichCommentsWithThreads(comments, threads);

expect(enriched[0]?.is_resolved).toBe(true);
expect(enriched[1]).toBe(comments[1]!);
expect(enriched[1]?.is_resolved).toBeUndefined();
});

test("enrichCommentsWithThreads returns comments as-is when there are no threads", () => {
const comments = [createComment(1)];
expect(enrichCommentsWithThreads(comments, [])).toBe(comments);
});
37 changes: 37 additions & 0 deletions src/browser/contexts/pr-review/enrichComments.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import type { ReviewComment } from "@/api/types";
import type { ReviewThread } from "@/browser/contexts/github";

/**
* Join GraphQL review-thread resolution info onto REST review comments.
* REST comments carry no thread ID or resolution state, so without this
* enrichment resolved threads render as unresolved.
*/
export function enrichCommentsWithThreads(
comments: ReviewComment[],
threads: ReviewThread[]
): ReviewComment[] {
if (threads.length === 0) return comments;

const threadByCommentId = new Map<number, ReviewThread>();
for (const thread of threads) {
for (const comment of thread.comments.nodes) {
threadByCommentId.set(comment.databaseId, thread);
}
}

return comments.map((comment) => {
const thread = threadByCommentId.get(comment.id);
if (!thread) return comment;
return {
...comment,
pull_request_review_thread_id: thread.id,
is_resolved: thread.isResolved,
resolved_by: thread.resolvedBy
? {
login: thread.resolvedBy.login,
avatar_url: thread.resolvedBy.avatarUrl,
}
: null,
};
});
}
1 change: 1 addition & 0 deletions src/browser/contexts/pr-review/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2758,6 +2758,7 @@ export { useDiffLoader } from "./useDiffLoader";
export { useCurrentUserLoader } from "./useCurrentUserLoader";
export { usePendingReviewLoader } from "./usePendingReviewLoader";
export { useThreadActions } from "./useThreadActions";
export { enrichCommentsWithThreads } from "./enrichComments";
export { useCommentActions } from "./useCommentActions";
export { useReviewActions } from "./useReviewActions";
export { useSkipBlockExpansion } from "./useSkipBlockExpansion";
Expand Down
23 changes: 16 additions & 7 deletions src/browser/contexts/pr-review/useReviewActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { ReviewComment } from "@/api/types";
import { useGitHub, type Review } from "@/browser/contexts/github";
import { useTelemetry } from "@/browser/contexts/telemetry";
import { usePRReviewStore, usePRReviewSelector } from ".";
import { enrichCommentsWithThreads } from "./enrichComments";

export function useReviewActions() {
const store = usePRReviewStore();
Expand Down Expand Up @@ -65,13 +66,21 @@ export function useReviewActions() {
// Invalidate timeline cache so we get fresh data
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}:timeline`);

// Refresh comments, reviews, and timeline
const [newComments, reviews, timeline] = await Promise.all([
github.getPRComments(owner, repo, pr.number),
github.getPRReviews(owner, repo, pr.number),
github.getPRTimeline(owner, repo, pr.number),
]);
store.setComments(newComments as ReviewComment[]);
const [newComments, reviews, timeline, reviewThreadsResult] =
await Promise.all([
github.getPRComments(owner, repo, pr.number),
github.getPRReviews(owner, repo, pr.number),
github.getPRTimeline(owner, repo, pr.number),
github.getReviewThreads(owner, repo, pr.number).catch(() => null),
]);
const threads =
reviewThreadsResult?.threads ?? store.getSnapshot().reviewThreads;
store.setComments(
enrichCommentsWithThreads(newComments as ReviewComment[], threads)
);
if (reviewThreadsResult) {
store.setReviewThreads(reviewThreadsResult.threads);
}
store.setReviews(reviews);
store.setTimeline(timeline);

Expand Down
66 changes: 38 additions & 28 deletions src/browser/contexts/pr-review/useThreadActions.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,51 @@
import { useCallback } from "react";
import { useGitHub } from "@/browser/contexts/github";
import { usePRReviewStore } from ".";

export function useThreadActions() {
const store = usePRReviewStore();
const github = useGitHub();

const resolveThread = async (threadId: string) => {
try {
await github.resolveThread(threadId);
// Update local state - mark all comments in this thread as resolved
// Update both comments (diff view) and reviewThreads (overview) so
// resolution state stays in sync across views.
const setThreadResolved = useCallback(
(threadId: string, isResolved: boolean) => {
const state = store.getSnapshot();
const updatedComments = state.comments.map((c) =>
c.pull_request_review_thread_id === threadId
? { ...c, is_resolved: true }
: c
store.setComments(
state.comments.map((c) =>
c.pull_request_review_thread_id === threadId
? { ...c, is_resolved: isResolved }
: c
)
);
store.setComments(updatedComments);
} catch (error) {
console.error("Failed to resolve thread:", error);
}
};
store.updateReviewThread(threadId, (t) => ({ ...t, isResolved }));
},
[store]
);

const unresolveThread = async (threadId: string) => {
try {
await github.unresolveThread(threadId);
// Update local state - mark all comments in this thread as unresolved
const state = store.getSnapshot();
const updatedComments = state.comments.map((c) =>
c.pull_request_review_thread_id === threadId
? { ...c, is_resolved: false }
: c
);
store.setComments(updatedComments);
} catch (error) {
console.error("Failed to unresolve thread:", error);
}
};
const resolveThread = useCallback(
async (threadId: string) => {
try {
await github.resolveThread(threadId);
setThreadResolved(threadId, true);
} catch (error) {
console.error("Failed to resolve thread:", error);
}
},
[github, setThreadResolved]
);

const unresolveThread = useCallback(
async (threadId: string) => {
try {
await github.unresolveThread(threadId);
setThreadResolved(threadId, false);
} catch (error) {
console.error("Failed to unresolve thread:", error);
}
},
[github, setThreadResolved]
);

return { resolveThread, unresolveThread };
}
Loading