Require variables for legacy GraphQL queries

src/github/githubRepository.ts

@@ -328,7 +328,7 @@ export class GitHubRepository extends Disposable {
 		}
 	}
 
-	query = async <T>(query: QueryOptions, ignoreSamlErrors: boolean = false, legacyFallback?: { query: DocumentNode, variables?: OperationVariables }): Promise<ApolloQueryResult<T>> => {
+	query = async <T>(query: QueryOptions, ignoreSamlErrors: boolean = false, legacyFallback?: { query: DocumentNode, variables: OperationVariables }): Promise<ApolloQueryResult<T>> => {
 		const gql = this.authMatchesServer && this.hub && this.hub.graphql;
 		if (!gql) {
 			const logValue = (query.query.definitions[0] as { name: { value: string } | undefined }).name?.value;

src/github/pullRequestModel.ts

@@ -1497,16 +1497,20 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 		const reviewThreads: ReviewThread[] = [];
 		try {
 			do {
+				const variables = {
+					owner: remote.owner,
+					name: remote.repositoryName,
+					number: this.number,
+					after,
+				};
 				const { data } = await query<PullRequestCommentsResponse>({
 					query: schema.PullRequestComments,
-					variables: {
-						owner: remote.owner,
-						name: remote.repositoryName,
-						number: this.number,
-						after
-					},
-				}, false, { query: schema.LegacyPullRequestComments });
+					variables,
+				}, false, { query: schema.LegacyPullRequestComments, variables });
 
+				if (!data?.repository) {
+					throw new Error('Review comments response did not include a repository.');
+				}
 				reviewThreads.push(...data.repository.pullRequest.reviewThreads.nodes);
 
 				hasNextPage = data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage;

src/test/github/githubRepository.test.ts

@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { default as assert } from 'assert';
+import { NetworkStatus } from 'apollo-boost';
 import { SinonSandbox, createSandbox } from 'sinon';
 import { CredentialStore } from '../../github/credentials';
 import { MockCommandRegistry } from '../mocks/mockCommandRegistry';
@@ -18,6 +19,7 @@ import { GitHubServerType } from '../../common/authentication';
 import { CheckState, PullRequestCheckStatus } from '../../github/interface';
 import { PullRequestBuilder as GraphQLPullRequestBuilder } from '../builders/graphql/pullRequestBuilder';
 import Logger from '../../common/logger';
+import { LoggingApolloClient, LoggingOctokit } from '../../github/loggingOctokit';
 
 describe('GitHubRepository', function () {
 	let sinon: SinonSandbox;
@@ -38,6 +40,36 @@ describe('GitHubRepository', function () {
 		sinon.restore();
 	});
 
+	describe('query', function () {
+		it('replaces variables for a legacy query with different arguments', async function () {
+			const url = 'https://github.com/some/repo';
+			const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom);
+			const repo = new GitHubRepository(1, remote, Uri.file('/workspaces/repo'), credentialStore, telemetry, true);
+			const graphql = sinon.createStubInstance(LoggingApolloClient);
+			sinon.stub(credentialStore, 'isAuthenticated').returns(true);
+			sinon.stub(repo, 'hub').get(() => ({ graphql, octokit: sinon.createStubInstance(LoggingOctokit) }));
+			const variables = { owner: 'some', name: 'repo', first: 100, after: 'cursor' };
+			const response = { data: {}, loading: false, stale: false, networkStatus: NetworkStatus.ready };
+			graphql.query.onFirstCall().rejects(new Error('Unsupported query'));
+			graphql.query.onSecondCall().resolves(response);
+
+			try {
+				const result = await repo.query({
+					query: repo.schema.GetSuggestedActors,
+					variables: { ...variables, capabilities: ['CAN_BE_ASSIGNED'] },
+				}, false, { query: repo.schema.GetAssignableUsers, variables });
+
+				assert.strictEqual(result, response);
+				assert.strictEqual(graphql.query.callCount, 2);
+				const [fallback] = graphql.query.secondCall.args;
+				assert.strictEqual(fallback.query, repo.schema.GetAssignableUsers);
+				assert.deepStrictEqual(fallback.variables, variables);
+			} finally {
+				repo.dispose();
+			}
+		});
+	});
+
 	describe('isGitHubDotCom', function () {
 		it('detects when the remote is pointing to github.com', function () {
 			const url = 'https://github.com/some/repo';

src/test/github/pullRequestModel.test.ts

@@ -19,6 +19,9 @@ import { NetworkStatus } from 'apollo-client';
 import { MockExtensionContext } from '../mocks/mockExtensionContext';
 import { GitHubServerType } from '../../common/authentication';
 import { mergeQuerySchemaWithShared } from '../../github/common';
+import { GitHubRepository } from '../../github/githubRepository';
+import { LoggingApolloClient, LoggingOctokit } from '../../github/loggingOctokit';
+import Logger from '../../common/logger';
 const queries = mergeQuerySchemaWithShared(require('../../github/queries.gql'), require('../../github/queriesShared.gql')) as any;
 
 const telemetry = new MockTelemetry();
@@ -96,6 +99,67 @@ describe('PullRequestModel', function () {
 	});
 
 	describe('reviewThreadCache', function () {
+		function page(id: string, endCursor: string | null) {
+			return {
+				data: {
+					repository: {
+						pullRequest: {
+							reviewThreads: {
+								nodes: [{ ...reviewThreadResponse, id }],
+								pageInfo: { hasNextPage: endCursor !== null, endCursor },
+							},
+						},
+					},
+				},
+				loading: false,
+				stale: false,
+				networkStatus: NetworkStatus.ready,
+			};
+		}
+
+		it('passes review comment variables to every legacy page', async function () {
+			const repository = new GitHubRepository(1, remote, repo.rootUri, credentials, telemetry, true);
+			const graphql = sinon.createStubInstance(LoggingApolloClient);
+			sinon.stub(credentials, 'isAuthenticated').returns(true);
+			sinon.stub(repository, 'hub').get(() => ({ graphql, octokit: sinon.createStubInstance(LoggingOctokit) }));
+			sinon.stub(repository, 'ensure').resolves(repository);
+			graphql.query.onCall(0).rejects(new Error('Unsupported query'));
+			graphql.query.onCall(1).resolves(page('1', 'first'));
+			graphql.query.onCall(2).rejects(new Error('Unsupported query'));
+			graphql.query.onCall(3).resolves(page('2', null));
+
+			try {
+				const pr = new PullRequestBuilder().build();
+				const model = new PullRequestModel(credentials, telemetry, repository, remote, convertRESTPullRequestToRawPullRequest(pr, repository));
+				const threads = await model.getReviewThreads();
+
+				assert.deepStrictEqual(threads.map(thread => thread.id), ['1', '2']);
+				assert.strictEqual(graphql.query.callCount, 4);
+				for (const [call, after] of [[graphql.query.secondCall, null], [graphql.query.lastCall, 'first']] as const) {
+					const [fallback] = call.args;
+					assert.strictEqual(fallback.query, repository.schema.LegacyPullRequestComments);
+					assert.deepStrictEqual(fallback.variables, {
+						owner: remote.owner, name: remote.repositoryName, number: pr.number, after,
+					});
+				}
+			} finally {
+				repository.dispose();
+			}
+		});
+
+		it('reports missing review data without retrying', async function () {
+			const pr = new PullRequestBuilder().build();
+			const model = new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo));
+			const query = sinon.stub(repo, 'query').resolves({
+				data: null, loading: false, stale: false, networkStatus: NetworkStatus.error,
+			});
+			const error = sinon.stub(Logger, 'error');
+
+			assert.deepStrictEqual(await model.getReviewThreads(), []);
+			assert.strictEqual(query.callCount, 1);
+			assert.strictEqual(error.lastCall.args[0], 'Failed to get pull request review comments: Error: Review comments response did not include a repository.');
+		});
+
 		it('should update the cache when then cache is initialized', async function () {
 			const pr = new PullRequestBuilder().build();
 			const model = new PullRequestModel(