Loading Pull Request List via GraphQL

src/github/githubRepository.ts

@@ -14,7 +14,7 @@ import { AuthenticationError } from '../common/authentication';
 import { QueryOptions, MutationOptions, ApolloQueryResult, NetworkStatus, FetchResult } from 'apollo-boost';
 import { PRDocumentCommentProvider } from '../view/prDocumentCommentProvider';
 import { convertRESTPullRequestToRawPullRequest, parseGraphQLPullRequest } from './utils';
-import { PullRequestResponse, MentionableUsersResponse } from './graphql';
+import { PullRequestResponse, MentionableUsersResponse, PullRequestListResponse } from './graphql';
 const queries = require('./queries.gql');
 
 export const PULL_REQUEST_PAGE_SIZE = 20;
@@ -205,103 +205,44 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 		};
 	}
 
-	async getPullRequests(prType: PRType, page?: number): Promise<PullRequestData | undefined> {
-		return prType === PRType.All ? this.getAllPullRequests(page) : this.getPullRequestsForCategory(prType, page);
-	}
+	async getPullRequestsGraphQL(type: PRType, nextCursor?: string|null):Promise<PullRequestListResponse|undefined> {
+		const { remote, query, octokit } = await this.ensure();
+		const currentUser = octokit && (octokit as any).currentUser;
+		const currentUserLogin: string = currentUser.login;
 
-	private async getAllPullRequests(page?: number): Promise<PullRequestData | undefined> {
-		try {
-			Logger.debug(`Fetch all pull requests - enter`, GitHubRepository.ID);
-			const { octokit, remote } = await this.ensure();
-			const result = await octokit.pullRequests.getAll({
-				owner: remote.owner,
-				repo: remote.repositoryName,
-				per_page: PULL_REQUEST_PAGE_SIZE,
-				page: page || 1
-			});
-
-			const hasMorePages = !!result.headers.link && result.headers.link.indexOf('rel="next"') > -1;
-			const pullRequests = result.data
-				.map(
-					pullRequest => {
-						if (!pullRequest.head.repo) {
-							Logger.appendLine(
-								'GitHubRepository> The remote branch for this PR was already deleted.'
-							);
-							return null;
-						}
-
-						const item = convertRESTPullRequestToRawPullRequest(pullRequest);
-						return new PullRequestModel(this, this.remote, item);
-					}
-				)
-				.filter(item => item !== null) as PullRequestModel[];
+		let filter = `type:pr is:open repo:${remote.owner}/${remote.repositoryName}`;
 
-			Logger.debug(`Fetch all pull requests - done`, GitHubRepository.ID);
-			return {
-				pullRequests,
-				hasMorePages
-			};
-		} catch (e) {
-			Logger.appendLine(`Fetching all pull requests failed: ${e}`, GitHubRepository.ID);
-			if (e.code === 404) {
-				// not found
-				vscode.window.showWarningMessage(`Fetching pull requests for remote '${this.remote.remoteName}' failed, please check if the url ${this.remote.url} is valid.`);
+		if (type !== PRType.All) {
+			if (type === PRType.Mine) {
+				filter += ` author:${currentUserLogin}`;
+			} else if (type === PRType.RequestReview) {
+				filter += ` review-requested:${currentUserLogin}`;
+			} else if (type === PRType.AssignedToMe) {
+				filter += ` assignee:${currentUserLogin}`;
 			} else {
-				throw e;
+				throw new Error('Unexpected pull request filter: ' + PRType[type]);
 			}
 		}
-	}
 
-	private async getPullRequestsForCategory(prType: PRType, page?: number): Promise<PullRequestData | undefined> {
-		try {
-			Logger.debug(`Fetch pull request catogory ${PRType[prType]} - enter`, GitHubRepository.ID);
-			const { octokit, remote } = await this.ensure();
-			const user = await octokit.users.get({});
-			// Search api will not try to resolve repo that redirects, so get full name first
-			const repo = await octokit.repos.get({ owner: this.remote.owner, repo: this.remote.repositoryName });
-			const { data, headers } = await octokit.search.issues({
-				q: this.getPRFetchQuery(repo.data.full_name, user.data.login, prType),
-				per_page: PULL_REQUEST_PAGE_SIZE,
-				page: page || 1
-			});
-			let promises: Promise<Octokit.Response<Octokit.PullRequestsGetResponse>>[] = [];
-			data.items.forEach((item: any /** unluckily Octokit.AnyResponse */) => {
-				promises.push(new Promise(async (resolve, reject) => {
-					let prData = await octokit.pullRequests.get({
-						owner: remote.owner,
-						repo: remote.repositoryName,
-						number: item.number
-					});
-					resolve(prData);
-				}));
-			});
-
-			const hasMorePages = !!headers.link && headers.link.indexOf('rel="next"') > -1;
-			const pullRequests = await Promise.all(promises).then(values => {
-				return values.map(item => {
-					if (!item.data.head.repo) {
-						Logger.appendLine('GitHubRepository> The remote branch for this PR was already deleted.');
-						return null;
-					}
-					return new PullRequestModel(this, this.remote, convertRESTPullRequestToRawPullRequest(item.data));
-				}).filter(item => item !== null) as PullRequestModel[];
-			});
-			Logger.debug(`Fetch pull request catogory ${PRType[prType]} - done`, GitHubRepository.ID);
+		const variables : {
+			query: string;
+			first: number;
+			after?: string;
+		} = {
+			query: filter,
+			first: 30
+		};
 
-			return {
-				pullRequests,
-				hasMorePages
-			};
-		} catch (e) {
-			Logger.appendLine(`GitHubRepository> Fetching all pull requests failed: ${e}`);
-			if (e.code === 404) {
-				// not found
-				vscode.window.showWarningMessage(`Fetching pull requests for remote ${this.remote.remoteName}, please check if the url ${this.remote.url} is valid.`);
-			} else {
-				throw e;
-			}
+		if(!!nextCursor) {
+			variables.after = nextCursor;
 		}
+
+		const { data } = await query<PullRequestListResponse>({
+			query: queries.GetPullRequests,
+			variables
+		});
+
+		return data;
 	}
 
 	async getPullRequest(id: number): Promise<PullRequestModel | undefined> {
@@ -387,23 +328,4 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable {
 
 		return [];
 	}
-
-	private getPRFetchQuery(repo: string, user: string, type: PRType) {
-		let filter = '';
-		switch (type) {
-			case PRType.RequestReview:
-				filter = `review-requested:${user}`;
-				break;
-			case PRType.AssignedToMe:
-				filter = `assignee:${user}`;
-				break;
-			case PRType.Mine:
-				filter = `author:${user}`;
-				break;
-			default:
-				break;
-		}
-
-		return `is:open ${filter} type:pr repo:${repo}`;
-	}
 }

src/github/graphql.ts

@@ -3,6 +3,8 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { IAccount } from './interface';
+
 export interface MergedEvent {
 	__typename: string;
 	id: string;
@@ -309,4 +311,31 @@ export interface RateLimit {
 	cost: number;
 	remaining: number;
 	resetAt: string;
+}
+
+export interface PullRequestListItem {
+	nodeId: string;
+	number: number;
+	title: string;
+	url: string;
+	author: IAccount;
+	state: string;
+	assignees: {
+		nodes: IAccount[];
+	};
+	createdAt: string;
+	updatedAt: string;
+	merged: boolean;
+	headRef: Ref;
+	baseRef: Ref;
+}
+
+export interface PullRequestListResponse {
+	search: {
+		nodes: PullRequestListItem[];
+		pageInfo: {
+			hasNextPage: boolean;
+			endCursor: string;
+		};
+	};
 }

src/github/pullRequestManager.ts

@@ -25,8 +25,8 @@ import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsRes
 const queries = require('./queries.gql');
 
 interface PageInformation {
-	pullRequestPage: number;
 	hasMorePages: boolean | null;
+	endCursor: string | null;
 }
 
 interface RestErrorResult {
@@ -136,7 +136,7 @@ export class PullRequestManager {
 			Logger.debug(`Displaying configured remotes: ${remotesSetting.join(', ')}`, PullRequestManager.ID);
 
 			return remotesSetting
-				.map(remote =>  allGitHubRemotes.find(repo => repo.remoteName === remote))
+				.map(remote => allGitHubRemotes.find(repo => repo.remoteName === remote))
 				.filter(repo => !!repo) as Remote[];
 		}
 
@@ -401,8 +401,8 @@ export class PullRequestManager {
 				const remoteId = repository.remote.url.toString();
 				if (!this._repositoryPageInformation.get(remoteId)) {
 					this._repositoryPageInformation.set(remoteId, {
-						pullRequestPage: 1,
-						hasMorePages: null
+						hasMorePages: null,
+						endCursor: null
 					});
 				}
 			}
@@ -566,8 +566,8 @@ export class PullRequestManager {
 		if (!options.fetchNextPage) {
 			for (let repository of this._githubRepositories) {
 				this._repositoryPageInformation.set(repository.remote.url.toString(), {
-					pullRequestPage: 1,
-					hasMorePages: null
+					hasMorePages: null,
+					endCursor: null
 				});
 			}
 		}
@@ -579,15 +579,60 @@ export class PullRequestManager {
 
 		for (let i = 0; i < githubRepositories.length; i++) {
 			const githubRepository = githubRepositories[i];
+
 			const pageInformation = this._repositoryPageInformation.get(githubRepository.remote.url.toString())!;
-			const pullRequestData = await githubRepository.getPullRequests(type, pageInformation.pullRequestPage);
+			const pullRequestResponseData = await githubRepository.getPullRequestsGraphQL(type, pageInformation.endCursor);
+			const { remote } = await githubRepository.ensure();
+
+			if(!!pullRequestResponseData) {
+				pageInformation.hasMorePages = pullRequestResponseData.search.pageInfo.hasNextPage;
+				pageInformation.endCursor = pullRequestResponseData.search.pageInfo.endCursor;
+			} else {
+				pageInformation.hasMorePages = false;
+				pageInformation.endCursor = null;
+			}
 
-			pageInformation.hasMorePages = !!pullRequestData && pullRequestData.hasMorePages;
-			pageInformation.pullRequestPage++;
+			if (pullRequestResponseData && pullRequestResponseData.search.nodes.length) {
+				let pullRequestItems = pullRequestResponseData.search.nodes;
+
+				const pullRequests: PullRequestModel[] = pullRequestItems.map(pullRequestItem => {
+						let assignee: IAccount | undefined;
+						if(!!pullRequestItem.assignees.nodes && pullRequestItem.assignees.nodes.length) {
+							assignee = pullRequestItem.assignees.nodes[0];
+						}
+
+						const pullRequest: PullRequest = {
+							url: pullRequestItem.url,
+							assignee,
+							base: {
+								label: `${pullRequestItem.baseRef.repository.owner}:${pullRequestItem.baseRef.name}`,
+								ref: pullRequestItem.baseRef.name,
+								repo: {cloneUrl: pullRequestItem.baseRef.repository.url },
+								sha: pullRequestItem.baseRef.target.oid
+							},
+							head: {
+								label: `${pullRequestItem.headRef.repository.owner}:${pullRequestItem.headRef.name}`,
+								ref: pullRequestItem.headRef.name,
+								repo: {cloneUrl: pullRequestItem.headRef.repository.url },
+								sha: pullRequestItem.baseRef.target.oid
+							},
+							labels: [],
+							body: '',
+							createdAt: pullRequestItem.createdAt,
+							updatedAt: pullRequestItem.updatedAt,
+							merged: pullRequestItem.merged,
+							nodeId: pullRequestItem.nodeId,
+							number: pullRequestItem.number,
+							state: pullRequestItem.state,
+							title: pullRequestItem.title,
+							user: pullRequestItem.author
+						};
+
+						return new PullRequestModel(githubRepository, remote, pullRequest);
+					});
 
-			if (pullRequestData && pullRequestData.pullRequests.length) {
 				return {
-					pullRequests: pullRequestData.pullRequests,
+					pullRequests: pullRequests,
 					hasMorePages: pageInformation.hasMorePages,
 					hasUnsearchedRepositories: i < githubRepositories.length - 1
 				};

src/github/queries.gql

@@ -112,6 +112,15 @@ fragment Reactable on Reactable {
 	}
 }
 
+fragment Account on Actor{
+    ... on User {
+        name
+    }
+    login
+    url
+    avatarUrl
+}
+
 query TimelineEvents($owner: String!, $name: String!, $number: Int!, $last: Int = 100) {
 	repository(owner: $owner, name: $name) {
 		pullRequest(number: $number) {
@@ -337,4 +346,39 @@ query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after:
 		remaining
 		resetAt
 	}
+}
+
+query GetPullRequests($query: String!, $first: Int!, $after: String) {
+  search(query: $query, first: $first, after:$after, type: ISSUE) {
+    nodes {
+        ... on PullRequest {
+            nodeId: id
+            number
+            title
+            url
+            author {
+                ...Account
+            }
+            state
+            assignees(first: 30) {
+                nodes {
+                    ...Account
+                }
+            }
+            createdAt
+            updatedAt
+            merged
+            headRef {
+            ...Ref
+            }
+            baseRef {
+            ...Ref
+            }
+        }
+    }
+    pageInfo {
+        hasNextPage
+        endCursor
+    }
+  }
 }