Skip to content
Draft
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
5 changes: 5 additions & 0 deletions src/common/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ export interface IComment {
reactions?: Reaction[];
isResolved?: boolean;
isOutdated?: boolean;
/**
* The GraphQL node ID of the review thread this comment belongs to, if any.
* Populated when the comment is parsed from a review thread.
*/
threadId?: string;
}

const COPILOT_AUTHOR = {
Expand Down
8 changes: 7 additions & 1 deletion src/common/timelineEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,13 @@ export type ReviewStateValue = 'COMMENTED' | 'APPROVED' | 'CHANGES_REQUESTED' |

export interface ReviewEvent {
id: number;
reviewThread?: ReviewResolveInfo
/**
* Map of review thread GraphQL node id to its resolve info, for all
* review threads belonging to this review. A single review can have
* comments on multiple threads, so resolve actions must look up the
* thread by the comment's `threadId`.
*/
reviewThreads?: { [threadId: string]: ReviewResolveInfo };
event: EventType.Reviewed;
comments: IComment[];
submittedAt: string;
Expand Down
13 changes: 10 additions & 3 deletions src/github/pullRequestModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ import Logger from '../common/logger';
import { Remote } from '../common/remote';
import { DEFAULT_MERGE_METHOD, PR_SETTINGS_NAMESPACE } from '../common/settingKeys';
import { ITelemetry } from '../common/telemetry';
import { ClosedEvent, EventType, ReviewEvent, TimelineEvent } from '../common/timelineEvent';
import { ClosedEvent, EventType, ReviewEvent, ReviewResolveInfo, TimelineEvent } from '../common/timelineEvent';
import { resolvePath, Schemes, toGitHubCommitUri, toPRUri, toReviewUri } from '../common/uri';
import { formatError, isDescendant } from '../common/utils';
import { InMemFileChangeModel, RemoteFileChangeModel } from '../view/fileChangeModel';
Expand Down Expand Up @@ -980,13 +980,20 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
return;
}
const prReviewThreadEvent = reviewEventsById[thread.prReviewDatabaseId];
prReviewThreadEvent.reviewThread = {
const resolveInfo: ReviewResolveInfo = {
threadId: thread.id,
canResolve: thread.viewerCanResolve,
canUnresolve: thread.viewerCanUnresolve,
isResolved: thread.isResolved
};

// A single review can have comments on multiple threads. Track each
// thread's resolve info by its id so the webview can resolve the
// correct thread when the user clicks resolve/unresolve on any of
// the comment groups within the review.
if (!prReviewThreadEvent.reviewThreads) {
prReviewThreadEvent.reviewThreads = {};
}
prReviewThreadEvent.reviewThreads[thread.id] = resolveInfo;
});

const pendingReview = reviewEvents.filter(r => r.state?.toLowerCase() === 'pending')[0];
Expand Down
7 changes: 4 additions & 3 deletions src/github/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,12 +596,12 @@ export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRep
originalEndLine: thread.originalLine,
diffSide: thread.diffSide,
isOutdated: thread.isOutdated,
comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository)),
comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository, thread.id)),
subjectType: thread.subjectType ?? SubjectType.LINE
};
}

export function parseGraphQLComment(comment: GraphQL.ReviewComment, isResolved: boolean, isOutdated: boolean, githubRepository: GitHubRepository): IComment {
export function parseGraphQLComment(comment: GraphQL.ReviewComment, isResolved: boolean, isOutdated: boolean, githubRepository: GitHubRepository, threadId?: string): IComment {
const specialAuthor = COPILOT_ACCOUNTS[comment.author?.login ?? ''];
const c: IComment = {
id: comment.databaseId,
Expand All @@ -626,7 +626,8 @@ export function parseGraphQLComment(comment: GraphQL.ReviewComment, isResolved:
inReplyToId: comment.replyTo && comment.replyTo.databaseId,
reactions: parseGraphQLReaction(comment.reactionGroups),
isResolved,
isOutdated
isOutdated,
threadId
};

const diffHunks = parseCommentDiffHunk(c);
Expand Down
28 changes: 20 additions & 8 deletions webviews/components/timeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,19 @@ const NewCommitsSinceReviewEventView = () => {
const positionKey = (comment: IComment) =>
comment.position !== null ? `pos:${comment.position}` : `ori:${comment.originalPosition}`;

const groupCommentsByPath = (comments: IComment[]) =>
groupBy(comments, comment => comment.path + ':' + positionKey(comment));
// Group review comments so that each rendered CommentThread contains exactly
// one review thread's comments. Prefer grouping by the thread's GraphQL id
// (set on each IComment when parsed from a review thread) so that resolve /
// unresolve actions target the right thread. Fall back to path+position for
// comments without a threadId (e.g. pending review comments not yet posted).
const groupCommentsByThread = (comments: IComment[]) =>
groupBy(comments, comment =>
comment.threadId
? `thread:${comment.threadId}`
: comment.path + ':' + positionKey(comment));

const ReviewEventView = (event: ReviewEvent) => {
const comments = groupCommentsByPath(event.comments);
const comments = groupCommentsByThread(event.comments);
const reviewIsPending = event.state === 'PENDING';
return (
<CommentView comment={event} allowEmpty={true}>
Expand Down Expand Up @@ -243,17 +251,21 @@ function CommentThread({ thread, event }: { thread: IComment[]; event: ReviewEve
setResolved(!!comment.isResolved);
setRevealed(!comment.isResolved);
}, [comment.isResolved]);
// Look up this group's own review thread info. A single ReviewEvent can
// have comments on multiple threads, so resolve actions must target the
// thread that owns this group's comments - looked up by `comment.threadId`.
const reviewThread = comment.threadId ? event.reviewThreads?.[comment.threadId] : undefined;
const resolvePermission =
event.reviewThread &&
((event.reviewThread.canResolve && !event.reviewThread.isResolved) ||
(event.reviewThread.canUnresolve && event.reviewThread.isResolved));
reviewThread &&
((reviewThread.canResolve && !reviewThread.isResolved) ||
(reviewThread.canUnresolve && reviewThread.isResolved));

const toggleResolve = () => {
if (event.reviewThread) {
if (reviewThread) {
const newResolved = !resolved;
setRevealed(!newResolved);
setResolved(newResolved);
toggleResolveComment(event.reviewThread.threadId, thread, newResolved);
toggleResolveComment(reviewThread.threadId, thread, newResolved);
}
};

Expand Down