diff --git a/src/github/credentials.ts b/src/github/credentials.ts index d4422d3607..a5ccbfb126 100644 --- a/src/github/credentials.ts +++ b/src/github/credentials.ts @@ -576,7 +576,8 @@ const link = (url: string, token: string) => headers: { ...headers, authorization: token ? `Bearer ${token}` : '', - Accept: 'application/vnd.github.merge-info-preview' + Accept: 'application/vnd.github.merge-info-preview', + 'GraphQL-Features': 'graphql_pr_comment_positioning' }, })).concat( createHttpLink({ diff --git a/src/github/graphql.ts b/src/github/graphql.ts index ea05342343..a7a965d145 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -273,6 +273,12 @@ export interface ReviewThread { } }] }; + positioning?: { + __typename: string; + startLine?: number; + endLine?: number; + line?: number; + } } export interface TimelineEventsResponse { diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index f37f2feda8..de92421652 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -742,6 +742,26 @@ export class PullRequestModel extends IssueModel implements IPullRe const pendingReviewId = await this.getPendingReviewId(); const { mutate, schema } = await this.githubRepository.ensure(); + + let linePositioning: any; + let multilinePositioning: any; + if (startLine === endLine && startLine !== undefined) { + linePositioning = { + line: startLine, + path: commentPath, + commitOid: this.head?.sha + }; + } else if (startLine !== undefined && endLine !== undefined) { + multilinePositioning = { + startLine, + endLine, + startPath: commentPath, + endPath: commentPath, + startCommitOid: this.head?.sha, + endCommitOid: this.head?.sha + }; + } + const { data } = await mutate({ mutation: schema.AddReviewThread, variables: { @@ -753,7 +773,9 @@ export class PullRequestModel extends IssueModel implements IPullRe startLine: startLine === endLine ? undefined : startLine, line: (endLine === undefined) ? 0 : endLine, side, - subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE + subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE, + linePositioning, + multilinePositioning } } }, { mutation: schema.LegacyAddReviewThread, deleteProps: ['subjectType'] }); @@ -772,7 +794,7 @@ export class PullRequestModel extends IssueModel implements IPullRe } const thread = data.addPullRequestReviewThread.thread; - const newThread = parseGraphQLReviewThread(thread, this.githubRepository); + const newThread = parseGraphQLReviewThread(thread, this.githubRepository, this.head?.sha); if (!this._reviewThreadsCache) { this._reviewThreadsCache = []; } @@ -1453,7 +1475,7 @@ export class PullRequestModel extends IssueModel implements IPullRe } private setReviewThreadCacheFromRaw(raw: ReviewThread[]): IReviewThread[] { - const reviewThreads: IReviewThread[] = raw.map(thread => parseGraphQLReviewThread(thread, this.githubRepository)); + const reviewThreads: IReviewThread[] = raw.map(thread => parseGraphQLReviewThread(thread, this.githubRepository, this.head?.sha)); const oldReviewThreads = this._reviewThreadsCache ?? []; this._reviewThreadsCache = reviewThreads; this.diffThreads(oldReviewThreads, reviewThreads); @@ -2105,7 +2127,7 @@ export class PullRequestModel extends IssueModel implements IPullRe const index = this._reviewThreadsCache?.findIndex(thread => thread.id === threadId) ?? -1; if (index > -1) { - const thread = parseGraphQLReviewThread(data.resolveReviewThread.thread, this.githubRepository); + const thread = parseGraphQLReviewThread(data.resolveReviewThread.thread, this.githubRepository, this.head?.sha); this._reviewThreadsCache?.splice(index, 1, thread); this._onDidChangeReviewThreads.fire({ added: [], changed: [thread], removed: [] }); } @@ -2148,7 +2170,7 @@ export class PullRequestModel extends IssueModel implements IPullRe const index = this._reviewThreadsCache?.findIndex(thread => thread.id === threadId) ?? -1; if (index > -1) { - const thread = parseGraphQLReviewThread(data.unresolveReviewThread.thread, this.githubRepository); + const thread = parseGraphQLReviewThread(data.unresolveReviewThread.thread, this.githubRepository, this.head?.sha); this._reviewThreadsCache?.splice(index, 1, thread); this._onDidChangeReviewThreads.fire({ added: [], changed: [thread], removed: [] }); } diff --git a/src/github/queriesShared.gql b/src/github/queriesShared.gql index 478893e5cc..b454944b84 100644 --- a/src/github/queriesShared.gql +++ b/src/github/queriesShared.gql @@ -576,6 +576,16 @@ query PullRequestComments($owner: String!, $name: String!, $number: Int!, $after ...ReviewComment } } + positioning { + __typename + ... on MultilineComment { + startLine + endLine + } + ... on LineComment { + line + } + } } pageInfo { hasNextPage diff --git a/src/github/utils.ts b/src/github/utils.ts index 24c42a4508..5fb6794db2 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -510,7 +510,29 @@ export function convertGraphQLEventType(text: string) { } } -export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRepository: GitHubRepository): IReviewThread { +export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRepository: GitHubRepository, headCommitOid?: string): IReviewThread { + let startLine: number = 0; + let endLine: number = 0; + if (thread.positioning) { + if (thread.positioning.__typename === 'MultilineComment') { + startLine = thread.positioning.startLine!; + endLine = thread.positioning.endLine!; + } else if (thread.positioning.__typename === 'LineComment') { + startLine = thread.positioning.line!; + endLine = thread.positioning.line!; + } + } else { + startLine = thread.startLine ?? thread.line; + endLine = thread.line; + } + + // There's a bug with comments outside the diff where they always show as outdated. Try to work around that. + let isOutdated = thread.isOutdated; + if (isOutdated && headCommitOid && thread.comments.nodes.length > 0 && + thread.comments.nodes.every(comment => comment.commit.oid === headCommitOid)) { + isOutdated = false; + } + return { id: thread.id, prReviewDatabaseId: thread.comments.edges && thread.comments.edges.length ? @@ -520,13 +542,13 @@ export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRep viewerCanResolve: thread.viewerCanResolve, viewerCanUnresolve: thread.viewerCanUnresolve, path: thread.path, - startLine: thread.startLine ?? thread.line, - endLine: thread.line, + startLine, + endLine, originalStartLine: thread.originalStartLine ?? thread.originalLine, originalEndLine: thread.originalLine, diffSide: thread.diffSide, - isOutdated: thread.isOutdated, - comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository)), + isOutdated, + comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, isOutdated, githubRepository)), subjectType: thread.subjectType ?? SubjectType.LINE }; } diff --git a/src/test/view/reviewCommentController.test.ts b/src/test/view/reviewCommentController.test.ts index 0dead8909e..7c19096b06 100644 --- a/src/test/view/reviewCommentController.test.ts +++ b/src/test/view/reviewCommentController.test.ts @@ -291,7 +291,13 @@ describe('ReviewCommentController', function () { startLine: undefined, line: 22, side: 'RIGHT', - subjectType: 'LINE' + subjectType: 'LINE', + linePositioning: { + line: 22, + path: fileName, + commitOid: activePullRequest.head?.sha + }, + multilinePositioning: undefined } } }, @@ -322,6 +328,10 @@ describe('ReviewCommentController', function () { author: {} } ] + }, + positioning: { + __typename: 'LineComment', + line: 22, } } } diff --git a/src/view/reviewCommentController.ts b/src/view/reviewCommentController.ts index 820f4e163c..50d509f066 100644 --- a/src/view/reviewCommentController.ts +++ b/src/view/reviewCommentController.ts @@ -31,6 +31,7 @@ import { CommentReactionHandler, createVSCodeCommentThreadForReviewThread, getRepositoryForFile, + isEnterprise, isFileInRepo, setReplyAuthor, threadRange, @@ -522,25 +523,29 @@ export class ReviewCommentController extends CommentControllerBase implements Co const ranges: vscode.Range[] = []; if (matchedFile) { - const diffHunks = await matchedFile.changeModel.diffHunks(); - if ((matchedFile.status === GitChangeType.RENAME) && (diffHunks.length === 0)) { - Logger.debug('No commenting ranges: File was renamed with no diffs.', ReviewCommentController.ID); - return { ranges: [], enableFileComments: true }; - } + if (!isEnterprise(this._folderRepoManager.activePullRequest.remote.authProviderId)) { + ranges.push(new vscode.Range(0, 0, document.lineCount - 1, document.lineAt(document.lineCount - 1).text.length - 1)); + } else { + const diffHunks = await matchedFile.changeModel.diffHunks(); + if ((matchedFile.status === GitChangeType.RENAME) && (diffHunks.length === 0)) { + Logger.debug('No commenting ranges: File was renamed with no diffs.', ReviewCommentController.ID); + return { ranges: [], enableFileComments: true }; + } - const contentDiff = await this.getContentDiff(document.uri, matchedFile.fileName); + const contentDiff = await this.getContentDiff(document.uri, matchedFile.fileName); - for (let i = 0; i < diffHunks.length; i++) { - const diffHunk = diffHunks[i]; - const start = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber, document.lineCount); - const end = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber + diffHunk.newLength - 1, document.lineCount); - if (start > 0 && end > 0) { - ranges.push(new vscode.Range(start - 1, 0, end - 1, 0)); + for (let i = 0; i < diffHunks.length; i++) { + const diffHunk = diffHunks[i]; + const start = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber, document.lineCount); + const end = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber + diffHunk.newLength - 1, document.lineCount); + if (start > 0 && end > 0) { + ranges.push(new vscode.Range(start - 1, 0, end - 1, 0)); + } } - } - if (ranges.length === 0) { - Logger.debug('No commenting ranges: File has diffs, but they could not be mapped to current lines.', ReviewCommentController.ID); + if (ranges.length === 0) { + Logger.debug('No commenting ranges: File has diffs, but they could not be mapped to current lines.', ReviewCommentController.ID); + } } } else { Logger.debug('No commenting ranges: File does not match any of the files in the review.', ReviewCommentController.ID); diff --git a/src/view/treeNodes/pullRequestNode.ts b/src/view/treeNodes/pullRequestNode.ts index 7bedc0cc03..d5034fbe12 100644 --- a/src/view/treeNodes/pullRequestNode.ts +++ b/src/view/treeNodes/pullRequestNode.ts @@ -22,6 +22,7 @@ import { getIconForeground, getListErrorForeground, getListWarningForeground, ge import { DirectoryTreeNode } from './directoryTreeNode'; import { InMemFileChangeNode, RemoteFileChangeNode } from './fileChangeNode'; import { TreeNode, TreeNodeParent } from './treeNode'; +import { isEnterprise } from '../../github/utils'; import { NotificationsManager } from '../../notifications/notificationsManager'; import { PrsTreeModel } from '../prsTreeModel'; @@ -411,7 +412,11 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2 return undefined; } - return { ranges: getCommentingRanges(await fileChange.changeModel.diffHunks(), params.isBase, PRNode.ID), enableFileComments: true }; + if (isEnterprise(this.pullRequestModel.remote.authProviderId)) { + return { ranges: getCommentingRanges(await fileChange.changeModel.diffHunks(), params.isBase, PRNode.ID), enableFileComments: true }; + } else { + return { ranges: [(new vscode.Range(0, 0, document.lineCount - 1, document.lineAt(document.lineCount).text.length))], enableFileComments: true }; + } } return undefined;