Skip to content
Merged
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
27 changes: 23 additions & 4 deletions src/github/githubRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -197,7 +197,7 @@ export class GitHubRepository implements vscode.Disposable {
}
}

query = async <T>(query: QueryOptions, ignoreSamlErrors: boolean = false): Promise<ApolloQueryResult<T>> => {
query = async <T>(query: QueryOptions, ignoreSamlErrors: boolean = false, legacyFallback?: { query: DocumentNode }): Promise<ApolloQueryResult<T>> => {
const gql = this.authMatchesServer && this.hub && this.hub.graphql;
if (!gql) {
Logger.debug(`Not available for query: ${query}`, GRAPHQL_COMPONENT_ID);
Expand All @@ -214,6 +214,11 @@ export class GitHubRepository implements vscode.Disposable {
try {
rsp = await gql.query<T>(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();
Expand All @@ -226,7 +231,7 @@ export class GitHubRepository implements vscode.Disposable {
return rsp;
};

mutate = async <T>(mutation: MutationOptions<T>): Promise<FetchResult<T>> => {
mutate = async <T>(mutation: MutationOptions<T>, legacyFallback?: { mutation: DocumentNode, deleteProps: string[] }): Promise<FetchResult<T>> => {
const gql = this.authMatchesServer && this.hub && this.hub.graphql;
if (!gql) {
Logger.debug(`Not available for query: ${mutation}`, GRAPHQL_COMPONENT_ID);
Expand All @@ -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<T>(mutation);
let rsp;
try {
rsp = await gql.mutate<T>(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;
};
Expand Down
2 changes: 1 addition & 1 deletion src/github/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export interface ReviewThread {
originalStartLine: number | null;
originalLine: number;
isOutdated: boolean;
subjectType: SubjectType;
subjectType?: SubjectType;
comments: {
nodes: ReviewComment[];
edges: [{
Expand Down
14 changes: 7 additions & 7 deletions src/github/pullRequestModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,9 @@ export class PullRequestModel extends IssueModel<PullRequest> 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.');
Expand Down Expand Up @@ -892,7 +892,7 @@ export class PullRequestModel extends IssueModel<PullRequest> 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);
Expand Down Expand Up @@ -921,7 +921,7 @@ export class PullRequestModel extends IssueModel<PullRequest> 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))
Expand Down Expand Up @@ -1465,7 +1465,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
threadId,
},
},
});
}, { mutation: schema.LegacyResolveReviewThread, deleteProps: [] });

if (!data) {
// Undo optimistic update
Expand Down Expand Up @@ -1505,7 +1505,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
threadId,
},
},
});
}, { mutation: schema.LegacyUnresolveReviewThread, deleteProps: [] });

if (!data) {
// Undo optimistic update
Expand Down
83 changes: 83 additions & 0 deletions src/github/queries.gql
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,25 @@ fragment ReviewThread on PullRequestReviewThread {
}
}

fragment LegacyReviewThread on PullRequestReviewThread {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to use the @skip directive instead of having a whole new fragment, but it never worked. Commenting here in case someone sees this and has some idea why.

id
isResolved
viewerCanResolve
viewerCanUnresolve
path
diffSide
line
startLine
originalStartLine
originalLine
isOutdated
comments(first: 100) {
nodes {
...ReviewComment
}
}
}

fragment PullRequestFragment on PullRequest {
number
url
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/github/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}

Expand Down