From dcb915408c80b25ffffa0f2db9228c5d0e98e102 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Wed, 3 May 2023 15:49:13 +0200 Subject: [PATCH] GE Creating comment failed when comment on PR Fixes #4791 --- src/github/githubRepository.ts | 27 +++++++++-- src/github/graphql.ts | 2 +- src/github/pullRequestModel.ts | 14 +++--- src/github/queries.gql | 83 ++++++++++++++++++++++++++++++++++ src/github/utils.ts | 2 +- 5 files changed, 115 insertions(+), 13 deletions(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 9efffd2b56..33df4440a2 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ApolloQueryResult, FetchResult, MutationOptions, NetworkStatus, QueryOptions } from 'apollo-boost'; +import { ApolloQueryResult, DocumentNode, FetchResult, MutationOptions, NetworkStatus, QueryOptions } from 'apollo-boost'; import * as vscode from 'vscode'; import { AuthenticationError, AuthProvider, GitHubServerType, isSamlError } from '../common/authentication'; import Logger from '../common/logger'; @@ -197,7 +197,7 @@ export class GitHubRepository implements vscode.Disposable { } } - query = async (query: QueryOptions, ignoreSamlErrors: boolean = false): Promise> => { + query = async (query: QueryOptions, ignoreSamlErrors: boolean = false, legacyFallback?: { query: DocumentNode }): Promise> => { const gql = this.authMatchesServer && this.hub && this.hub.graphql; if (!gql) { Logger.debug(`Not available for query: ${query}`, GRAPHQL_COMPONENT_ID); @@ -214,6 +214,11 @@ export class GitHubRepository implements vscode.Disposable { try { rsp = await gql.query(query); } catch (e) { + if (legacyFallback) { + query.query = legacyFallback.query; + return this.query(query, ignoreSamlErrors); + } + // Some queries just result in SAML errors, and some queries we may not want to retry because it will be too disruptive. if (!ignoreSamlErrors && e.message?.startsWith('GraphQL error: Resource protected by organization SAML enforcement.')) { await this._credentialStore.recreate(); @@ -226,7 +231,7 @@ export class GitHubRepository implements vscode.Disposable { return rsp; }; - mutate = async (mutation: MutationOptions): Promise> => { + mutate = async (mutation: MutationOptions, legacyFallback?: { mutation: DocumentNode, deleteProps: string[] }): Promise> => { const gql = this.authMatchesServer && this.hub && this.hub.graphql; if (!gql) { Logger.debug(`Not available for query: ${mutation}`, GRAPHQL_COMPONENT_ID); @@ -239,7 +244,21 @@ export class GitHubRepository implements vscode.Disposable { } Logger.trace(`Request: ${JSON.stringify(mutation, null, 2)}`, GRAPHQL_COMPONENT_ID); - const rsp = await gql.mutate(mutation); + let rsp; + try { + rsp = await gql.mutate(mutation); + } catch { + if (legacyFallback) { + mutation.mutation = legacyFallback.mutation; + if (mutation.variables?.input) { + for (const prop of legacyFallback.deleteProps) { + delete mutation.variables.input[prop]; + } + } + } + + return this.mutate(mutation); + } Logger.trace(`Response: ${JSON.stringify(rsp, null, 2)}`, GRAPHQL_COMPONENT_ID); return rsp; }; diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 47b8e0e33d..fd4aa0ac00 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -194,7 +194,7 @@ export interface ReviewThread { originalStartLine: number | null; originalLine: number; isOutdated: boolean; - subjectType: SubjectType; + subjectType?: SubjectType; comments: { nodes: ReviewComment[]; edges: [{ diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index 48c3a557f8..8c139a12c8 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -581,9 +581,9 @@ export class PullRequestModel extends IssueModel implements IPullRe line: (endLine === undefined) ? 0 : endLine, side, subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE - }, - }, - }); + } + } + }, { mutation: schema.LegacyAddReviewThread, deleteProps: ['subjectType'] }); if (!data) { throw new Error('Creating review thread failed.'); @@ -892,7 +892,7 @@ export class PullRequestModel extends IssueModel implements IPullRe name: remote.repositoryName, number: this.number, }, - }); + }, false, { query: schema.LegacyPullRequestComments }); const reviewThreads = data.repository.pullRequest.reviewThreads.nodes.map(node => { return parseGraphQLReviewThread(node, this.githubRepository); @@ -921,7 +921,7 @@ export class PullRequestModel extends IssueModel implements IPullRe name: remote.repositoryName, number: this.number, }, - }); + }, false, { query: schema.LegacyPullRequestComments }); const comments = data.repository.pullRequest.reviewThreads.nodes .map(node => node.comments.nodes.map(comment => parseGraphQLComment(comment, node.isResolved, this.githubRepository), remote)) @@ -1465,7 +1465,7 @@ export class PullRequestModel extends IssueModel implements IPullRe threadId, }, }, - }); + }, { mutation: schema.LegacyResolveReviewThread, deleteProps: [] }); if (!data) { // Undo optimistic update @@ -1505,7 +1505,7 @@ export class PullRequestModel extends IssueModel implements IPullRe threadId, }, }, - }); + }, { mutation: schema.LegacyUnresolveReviewThread, deleteProps: [] }); if (!data) { // Undo optimistic update diff --git a/src/github/queries.gql b/src/github/queries.gql index 49ff7a7109..ffa45c1496 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -160,6 +160,25 @@ fragment ReviewThread on PullRequestReviewThread { } } +fragment LegacyReviewThread on PullRequestReviewThread { + id + isResolved + viewerCanResolve + viewerCanUnresolve + path + diffSide + line + startLine + originalStartLine + originalLine + isOutdated + comments(first: 100) { + nodes { + ...ReviewComment + } + } +} + fragment PullRequestFragment on PullRequest { number url @@ -541,6 +560,46 @@ query PullRequestComments($owner: String!, $name: String!, $number: Int!, $first } } +query LegacyPullRequestComments($owner: String!, $name: String!, $number: Int!, $first: Int = 100) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: $first) { + nodes { + id + isResolved + viewerCanResolve + viewerCanUnresolve + path + diffSide + startLine + line + originalStartLine + originalLine + isOutdated + comments(first: 100) { + edges { + node { + pullRequestReview { + databaseId + } + } + } + nodes { + ...ReviewComment + } + } + } + } + } + } + rateLimit { + limit + cost + remaining + resetAt + } +} + query Viewer { viewer { login @@ -792,6 +851,14 @@ mutation AddReviewThread($input: AddPullRequestReviewThreadInput!) { } } +mutation LegacyAddReviewThread($input: AddPullRequestReviewThreadInput!) { + addPullRequestReviewThread(input: $input) { + thread { + ...LegacyReviewThread + } + } +} + mutation AddReviewers($input: RequestReviewsInput!) { requestReviews(input: $input) { pullRequest { @@ -1427,6 +1494,14 @@ mutation ResolveReviewThread($input: ResolveReviewThreadInput!) { } } +mutation LegacyResolveReviewThread($input: ResolveReviewThreadInput!) { + resolveReviewThread(input: $input) { + thread { + ...LegacyReviewThread + } + } +} + mutation UnresolveReviewThread($input: UnresolveReviewThreadInput!) { unresolveReviewThread(input: $input) { thread { @@ -1435,6 +1510,14 @@ mutation UnresolveReviewThread($input: UnresolveReviewThreadInput!) { } } +mutation LegacyUnresolveReviewThread($input: UnresolveReviewThreadInput!) { + unresolveReviewThread(input: $input) { + thread { + ...LegacyReviewThread + } + } +} + mutation EnablePullRequestAutoMerge($input: EnablePullRequestAutoMergeInput!) { enablePullRequestAutoMerge(input: $input) { pullRequest { diff --git a/src/github/utils.ts b/src/github/utils.ts index f2f0ec7537..9bc16a41df 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -434,7 +434,7 @@ export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRep diffSide: thread.diffSide, isOutdated: thread.isOutdated, comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, githubRepository)), - subjectType: thread.subjectType + subjectType: thread.subjectType ?? SubjectType.LINE }; }