From c8ec117e0c0f7f3f2e512d6dd66e70fe6d78464f Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 4 Mar 2019 06:54:04 -0500 Subject: [PATCH 1/8] Adding graphql query to retrieve pull request list --- src/github/queries.gql | 71 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/src/github/queries.gql b/src/github/queries.gql index 5591fb747a..54c55d47a2 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -336,4 +336,73 @@ query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after: remaining resetAt } -} \ No newline at end of file +} + +fragment ActorParts on Actor { + login + url + avatarUrl + ... on User { + name + } +} +fragment PageInfoParts on PageInfo { + hasNextPage + endCursor +} +fragment RefParts on Ref { + name + prefix + target { + oid + commitResourcePath + } + repository { + url + owner { + login + } + name + } +} +query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after: String){ + repository(owner: $owner, name: $name) { + pullRequests(first: $first, after: $after, states: OPEN) { + nodes { + number + title + bodyHTML + url + author { + ...ActorParts + } + labels(first: 100) { + nodes { + name + } + pageInfo { + ...PageInfoParts + } + } + state + assignees(first: 100) { + nodes { + ...ActorParts + } + } + createdAt + updatedAt + merged + headRef { + ...RefParts + } + baseRef { + ...RefParts + } + } + pageInfo { + ...PageInfoParts + } + } + } +} From 6a2c6f01df973a0ed6042a98c29a9deb132da9de Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 4 Mar 2019 10:34:40 -0500 Subject: [PATCH 2/8] Requesting the pull request list over graphql --- src/github/githubRepository.ts | 134 +++++------------------------ src/github/graphql.ts | 64 ++++++++++++++ src/github/pullRequestManager.ts | 84 +++++++++++++++--- src/github/queries.gql | 28 +++--- src/view/treeNodes/categoryNode.ts | 10 +-- 5 files changed, 172 insertions(+), 148 deletions(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index ff2a9628db..836cff9e98 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -7,14 +7,14 @@ import * as vscode from 'vscode'; import * as Octokit from '@octokit/rest'; import Logger from '../common/logger'; import { Remote, parseRemote } from '../common/remote'; -import { PRType, IGitHubRepository, IAccount } from './interface'; +import { IGitHubRepository, IAccount } from './interface'; import { PullRequestModel } from './pullRequestModel'; import { CredentialStore, GitHub } from './credentials'; import { AuthenticationError } from '../common/authentication'; import { QueryOptions, MutationOptions, ApolloQueryResult, NetworkStatus, FetchResult } from 'apollo-boost'; import { PRDocumentCommentProvider, PRDocumentCommentProviderGraphQL } 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; @@ -177,103 +177,30 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable { return 'master'; } - async getPullRequests(prType: PRType, page?: number): Promise { - return prType === PRType.All ? this.getAllPullRequests(page) : this.getPullRequestsForCategory(prType, page); - } + async getPullRequestsGraphQL(nextCursor?: string|null):Promise { + const { remote, query } = await this.ensure(); - private async getAllPullRequests(page?: number): Promise { - 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[]; + const variables : { + owner: string; + name: string; + first: number; + after?: string + } = { + owner: remote.owner, + name: remote.repositoryName, + first: 30 + }; - 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.`); - } else { - throw e; - } + if(!!nextCursor) { + variables.after = nextCursor; } - } - - private async getPullRequestsForCategory(prType: PRType, page?: number): Promise { - 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>[] = []; - 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 { data } = await query({ + query: queries.GetPullRequests, + variables + }); - 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; - } - } + return data; } async getPullRequest(id: number): Promise { @@ -359,23 +286,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}`; - } } diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 2e93f40a8f..1c674f7393 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -1,3 +1,5 @@ +import { IAccount } from "./interface"; + /*--------------------------------------------------------------------------------------------- * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. See License.txt in the project root for license information. @@ -300,4 +302,66 @@ export interface RateLimit { cost: number; remaining: number; resetAt: string; +} + +export interface PullRequestListItem { + nodeId: string; + number: number; + title: string; + url: string; + author: { + login: string; + url: string; + avatarUrl: string; + }; + state: string; + assignees: { + nodes: IAccount[]; + }; + reviewRequests: { + nodes: [{ + requestedReviewer: IAccount; + }]; + }; + createdAt: string; + updatedAt: string; + merged: boolean; + headRef: { + name: string; + target: { + sha: string; + }; + repo: { + url: string; + owner: { + login: string; + }; + name: string; + }; + }; + baseRef: { + name: string; + target: { + sha: string; + }; + repo: { + url: string; + owner: { + login: string; + }; + name: string; + }; + }; +} + +export interface PullRequestListResponse { + repository: { + pullRequests: { + nodes: PullRequestListItem[]; + pageInfo: { + hasNextPage: boolean; + endCursor: string; + }; + }; + }; } \ No newline at end of file diff --git a/src/github/pullRequestManager.ts b/src/github/pullRequestManager.ts index bd8f7de5e8..bf0280cd75 100644 --- a/src/github/pullRequestManager.ts +++ b/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 { @@ -139,7 +139,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[]; } @@ -404,8 +404,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 }); } } @@ -541,8 +541,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 }); } } @@ -554,15 +554,77 @@ 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(pageInformation.endCursor); + const { remote, octokit } = await githubRepository.ensure(); + const currentUser = octokit && (octokit as any).currentUser; + const currentUserLogin: string = currentUser.login; + + if(!!pullRequestResponseData) { + pageInformation.hasMorePages = pullRequestResponseData.repository.pullRequests.pageInfo.hasNextPage; + pageInformation.endCursor = pullRequestResponseData.repository.pullRequests.pageInfo.endCursor; + } else { + pageInformation.hasMorePages = false; + pageInformation.endCursor = null; + } - pageInformation.hasMorePages = !!pullRequestData && pullRequestData.hasMorePages; - pageInformation.pullRequestPage++; + if (pullRequestResponseData && pullRequestResponseData.repository.pullRequests.nodes.length) { + let pullRequestItems = pullRequestResponseData.repository.pullRequests.nodes; + + if (type !== PRType.All) { + if (type === PRType.Mine) { + pullRequestItems = pullRequestItems.filter(pr => pr.author.login === currentUserLogin); + } else if (type === PRType.RequestReview) { + pullRequestItems = pullRequestItems + .filter(pr => pr.reviewRequests.nodes + .findIndex(reviewRequest => reviewRequest.requestedReviewer.login === currentUserLogin) !== -1); + } else if (type === PRType.AssignedToMe) { + pullRequestItems = pullRequestItems + .filter(pr => pr.assignees.nodes + .findIndex(asignee => asignee.login === currentUserLogin) !== -1); + } else { + throw new Error('Unexpected pull request filter'); + } + } + + 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.repo.owner}:${pullRequestItem.baseRef.name}`, + ref: pullRequestItem.baseRef.name, + repo: {cloneUrl: pullRequestItem.baseRef.repo.url }, + sha: pullRequestItem.baseRef.target.sha + }, + head: { + label: `${pullRequestItem.headRef.repo.owner}:${pullRequestItem.headRef.name}`, + ref: pullRequestItem.headRef.name, + repo: {cloneUrl: pullRequestItem.headRef.repo.url }, + sha: pullRequestItem.baseRef.target.sha + }, + 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 }; diff --git a/src/github/queries.gql b/src/github/queries.gql index 54c55d47a2..9b4b7768e4 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -342,9 +342,6 @@ fragment ActorParts on Actor { login url avatarUrl - ... on User { - name - } } fragment PageInfoParts on PageInfo { hasNextPage @@ -352,12 +349,10 @@ fragment PageInfoParts on PageInfo { } fragment RefParts on Ref { name - prefix target { - oid - commitResourcePath + sha: oid } - repository { + repo: repository { url owner { login @@ -365,29 +360,28 @@ fragment RefParts on Ref { name } } -query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after: String){ +query GetPullRequests($owner: String!, $name: String!, $first: Int!, $after: String){ repository(owner: $owner, name: $name) { pullRequests(first: $first, after: $after, states: OPEN) { nodes { + nodeId: id number title - bodyHTML url author { ...ActorParts } - labels(first: 100) { + state + assignees(first: 30) { nodes { - name - } - pageInfo { - ...PageInfoParts + ...ActorParts } } - state - assignees(first: 100) { + reviewRequests(first: 30) { nodes { - ...ActorParts + requestedReviewer { + ...ActorParts + } } } createdAt diff --git a/src/view/treeNodes/categoryNode.ts b/src/view/treeNodes/categoryNode.ts index 41efe8191a..f479fb30db 100644 --- a/src/view/treeNodes/categoryNode.ts +++ b/src/view/treeNodes/categoryNode.ts @@ -73,17 +73,11 @@ export class PRCategoryActionNode extends TreeNode implements vscode.TreeItem { } } -interface PageInformation { - pullRequestPage: number; - hasMorePages: boolean; -} - export class CategoryTreeNode extends TreeNode implements vscode.TreeItem { public readonly label: string; public collapsibleState: vscode.TreeItemCollapsibleState; public prs: PullRequestModel[]; public fetchNextPage: boolean = false; - public repositoryPageInformation: Map = new Map(); constructor( public parent: TreeNode | vscode.TreeView, @@ -170,12 +164,14 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem { } } - if (this.prs && this.prs.length) { + if (this.prs) { let nodes: TreeNode[] = this.prs.map(prItem => new PRNode(this, this._prManager, prItem, this._type === PRType.LocalPullRequest)); if (hasMorePages) { nodes.push(new PRCategoryActionNode(this, PRCategoryActionType.More, this)); } else if (hasUnsearchedRepositories) { nodes.push(new PRCategoryActionNode(this, PRCategoryActionType.TryOtherRemotes, this)); + } else if(this.prs.length === 0) { + nodes.push(new PRCategoryActionNode(this, PRCategoryActionType.Empty, this)); } this.childrenDisposables = nodes; From 3b28d7a8d6c826961cda7de35f9c821b118a2d89 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 4 Mar 2019 11:10:04 -0500 Subject: [PATCH 3/8] Fixing compilation error after merge --- src/github/githubRepository.ts | 2 +- src/github/pullRequestManager.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 4e03054bb9..51dcc9272b 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -7,7 +7,7 @@ import * as vscode from 'vscode'; import * as Octokit from '@octokit/rest'; import Logger from '../common/logger'; import { Remote, parseRemote } from '../common/remote'; -import { PRType, IGitHubRepository, IAccount, MergeMethodsAvailability } from './interface'; +import { IGitHubRepository, IAccount, MergeMethodsAvailability } from './interface'; import { PullRequestModel } from './pullRequestModel'; import { CredentialStore, GitHub } from './credentials'; import { AuthenticationError } from '../common/authentication'; diff --git a/src/github/pullRequestManager.ts b/src/github/pullRequestManager.ts index ad9b0ac49c..ba1da69923 100644 --- a/src/github/pullRequestManager.ts +++ b/src/github/pullRequestManager.ts @@ -637,6 +637,7 @@ export class PullRequestManager { repo: {cloneUrl: pullRequestItem.headRef.repo.url }, sha: pullRequestItem.baseRef.target.sha }, + labels: [], body: '', createdAt: pullRequestItem.createdAt, updatedAt: pullRequestItem.updatedAt, From c7c3dede3ec9c05b433db96fdc9a30c5aa7e4eaf Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 4 Mar 2019 11:10:49 -0500 Subject: [PATCH 4/8] Formatting graphql queries --- src/github/queries.gql | 106 ++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/src/github/queries.gql b/src/github/queries.gql index 24b64e4656..9f8c15ae2d 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -340,64 +340,64 @@ query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after: } fragment ActorParts on Actor { - login - url - avatarUrl + login + url + avatarUrl } fragment PageInfoParts on PageInfo { - hasNextPage - endCursor + hasNextPage + endCursor } fragment RefParts on Ref { - name - target { - sha: oid - } - repo: repository { - url - owner { - login - } - name - } + name + target { + sha: oid + } + repo: repository { + url + owner { + login + } + name + } } query GetPullRequests($owner: String!, $name: String!, $first: Int!, $after: String){ - repository(owner: $owner, name: $name) { - pullRequests(first: $first, after: $after, states: OPEN) { - nodes { - nodeId: id - number - title - url - author { - ...ActorParts - } - state - assignees(first: 30) { - nodes { - ...ActorParts - } - } + repository(owner: $owner, name: $name) { + pullRequests(first: $first, after: $after, states: OPEN) { + nodes { + nodeId: id + number + title + url + author { + ...ActorParts + } + state + assignees(first: 30) { + nodes { + ...ActorParts + } + } reviewRequests(first: 30) { - nodes { - requestedReviewer { - ...ActorParts - } - } - } - createdAt - updatedAt - merged - headRef { - ...RefParts - } - baseRef { - ...RefParts - } - } - pageInfo { - ...PageInfoParts - } - } - } + nodes { + requestedReviewer { + ...ActorParts + } + } + } + createdAt + updatedAt + merged + headRef { + ...RefParts + } + baseRef { + ...RefParts + } + } + pageInfo { + ...PageInfoParts + } + } + } } From fd8a0231d6ccb899805e6f678e1cbc6b113a303a Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Mon, 4 Mar 2019 11:20:07 -0500 Subject: [PATCH 5/8] Cleaning up the filter code --- src/github/pullRequestManager.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/github/pullRequestManager.ts b/src/github/pullRequestManager.ts index ba1da69923..18c93de48d 100644 --- a/src/github/pullRequestManager.ts +++ b/src/github/pullRequestManager.ts @@ -21,7 +21,7 @@ import Logger from '../common/logger'; import { EXTENSION_ID } from '../constants'; import { fromPRUri } from '../common/uri'; import { convertRESTPullRequestToRawPullRequest, convertPullRequestsGetCommentsResponseItemToComment, convertIssuesCreateCommentResponseToComment, parseGraphQLTimelineEvents, convertRESTTimelineEvents, getRelatedUsersFromTimelineEvents, parseGraphQLComment, getReactionGroup, convertRESTUserToAccount, convertRESTReviewEvent, parseGraphQLReviewEvent } from './utils'; -import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse } from './graphql'; +import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse, PullRequestListItem } from './graphql'; const queries = require('./queries.gql'); interface PageInformation { @@ -601,19 +601,22 @@ export class PullRequestManager { let pullRequestItems = pullRequestResponseData.repository.pullRequests.nodes; if (type !== PRType.All) { + + let pullRequestFilter: (item: PullRequestListItem) => boolean; + if (type === PRType.Mine) { - pullRequestItems = pullRequestItems.filter(pr => pr.author.login === currentUserLogin); + pullRequestFilter = pr => pr.author.login === currentUserLogin; } else if (type === PRType.RequestReview) { - pullRequestItems = pullRequestItems - .filter(pr => pr.reviewRequests.nodes - .findIndex(reviewRequest => reviewRequest.requestedReviewer.login === currentUserLogin) !== -1); + pullRequestFilter = pr => pr.reviewRequests.nodes + .findIndex(reviewRequest => reviewRequest.requestedReviewer.login === currentUserLogin) !== -1; } else if (type === PRType.AssignedToMe) { - pullRequestItems = pullRequestItems - .filter(pr => pr.assignees.nodes - .findIndex(asignee => asignee.login === currentUserLogin) !== -1); + pullRequestFilter = pr => pr.assignees.nodes + .findIndex(asignee => asignee.login === currentUserLogin) !== -1; } else { throw new Error('Unexpected pull request filter'); } + + pullRequestItems = pullRequestItems.filter(pullRequestFilter); } const pullRequests: PullRequestModel[] = pullRequestItems.map(pullRequestItem => { From 0841a659b06e1ec749c48a1ce63d8aba94e046e8 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Tue, 5 Mar 2019 08:56:24 -0500 Subject: [PATCH 6/8] Correcting the graphql query to use the search connection --- src/github/githubRepository.ts | 30 ++++++--- src/github/graphql.ts | 53 +++------------- src/github/pullRequestManager.ts | 49 +++++---------- src/github/queries.gql | 105 +++++++++++++------------------ 4 files changed, 88 insertions(+), 149 deletions(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 51dcc9272b..3406cb13a5 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -7,7 +7,7 @@ import * as vscode from 'vscode'; import * as Octokit from '@octokit/rest'; import Logger from '../common/logger'; import { Remote, parseRemote } from '../common/remote'; -import { IGitHubRepository, IAccount, MergeMethodsAvailability } from './interface'; +import { IGitHubRepository, IAccount, MergeMethodsAvailability, PRType } from './interface'; import { PullRequestModel } from './pullRequestModel'; import { CredentialStore, GitHub } from './credentials'; import { AuthenticationError } from '../common/authentication'; @@ -203,17 +203,31 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable { }; } - async getPullRequestsGraphQL(nextCursor?: string|null):Promise { - const { remote, query } = await this.ensure(); + async getPullRequestsGraphQL(type: PRType, nextCursor?: string|null):Promise { + const { remote, query, octokit } = await this.ensure(); + const currentUser = octokit && (octokit as any).currentUser; + const currentUserLogin: string = currentUser.login; + + let filter = `type:pr is:open repo:${remote.owner}/${remote.repositoryName}`; + + 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 new Error('Unexpected pull request filter'); + } + } const variables : { - owner: string; - name: string; + query: string; first: number; - after?: string + after?: string; } = { - owner: remote.owner, - name: remote.repositoryName, + query: filter, first: 30 }; diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 97c0be1b89..96ea84e433 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -1,4 +1,4 @@ -import { IAccount } from "./interface"; +import { IAccount } from './interface'; /*--------------------------------------------------------------------------------------------- * Copyright (c) Microsoft Corporation. All rights reserved. @@ -318,59 +318,24 @@ export interface PullRequestListItem { number: number; title: string; url: string; - author: { - login: string; - url: string; - avatarUrl: string; - }; + author: IAccount; state: string; assignees: { nodes: IAccount[]; }; - reviewRequests: { - nodes: [{ - requestedReviewer: IAccount; - }]; - }; createdAt: string; updatedAt: string; merged: boolean; - headRef: { - name: string; - target: { - sha: string; - }; - repo: { - url: string; - owner: { - login: string; - }; - name: string; - }; - }; - baseRef: { - name: string; - target: { - sha: string; - }; - repo: { - url: string; - owner: { - login: string; - }; - name: string; - }; - }; + headRef: Ref; + baseRef: Ref; } export interface PullRequestListResponse { - repository: { - pullRequests: { - nodes: PullRequestListItem[]; - pageInfo: { - hasNextPage: boolean; - endCursor: string; - }; + search: { + nodes: PullRequestListItem[]; + pageInfo: { + hasNextPage: boolean; + endCursor: string; }; }; } \ No newline at end of file diff --git a/src/github/pullRequestManager.ts b/src/github/pullRequestManager.ts index 18c93de48d..f2382af2de 100644 --- a/src/github/pullRequestManager.ts +++ b/src/github/pullRequestManager.ts @@ -21,7 +21,7 @@ import Logger from '../common/logger'; import { EXTENSION_ID } from '../constants'; import { fromPRUri } from '../common/uri'; import { convertRESTPullRequestToRawPullRequest, convertPullRequestsGetCommentsResponseItemToComment, convertIssuesCreateCommentResponseToComment, parseGraphQLTimelineEvents, convertRESTTimelineEvents, getRelatedUsersFromTimelineEvents, parseGraphQLComment, getReactionGroup, convertRESTUserToAccount, convertRESTReviewEvent, parseGraphQLReviewEvent } from './utils'; -import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse, PullRequestListItem } from './graphql'; +import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse } from './graphql'; const queries = require('./queries.gql'); interface PageInformation { @@ -584,44 +584,23 @@ export class PullRequestManager { const githubRepository = githubRepositories[i]; const pageInformation = this._repositoryPageInformation.get(githubRepository.remote.url.toString())!; - const pullRequestResponseData = await githubRepository.getPullRequestsGraphQL(pageInformation.endCursor); - const { remote, octokit } = await githubRepository.ensure(); - const currentUser = octokit && (octokit as any).currentUser; - const currentUserLogin: string = currentUser.login; + const pullRequestResponseData = await githubRepository.getPullRequestsGraphQL(type, pageInformation.endCursor); + const { remote } = await githubRepository.ensure(); if(!!pullRequestResponseData) { - pageInformation.hasMorePages = pullRequestResponseData.repository.pullRequests.pageInfo.hasNextPage; - pageInformation.endCursor = pullRequestResponseData.repository.pullRequests.pageInfo.endCursor; + pageInformation.hasMorePages = pullRequestResponseData.search.pageInfo.hasNextPage; + pageInformation.endCursor = pullRequestResponseData.search.pageInfo.endCursor; } else { pageInformation.hasMorePages = false; pageInformation.endCursor = null; } - if (pullRequestResponseData && pullRequestResponseData.repository.pullRequests.nodes.length) { - let pullRequestItems = pullRequestResponseData.repository.pullRequests.nodes; - - if (type !== PRType.All) { - - let pullRequestFilter: (item: PullRequestListItem) => boolean; - - if (type === PRType.Mine) { - pullRequestFilter = pr => pr.author.login === currentUserLogin; - } else if (type === PRType.RequestReview) { - pullRequestFilter = pr => pr.reviewRequests.nodes - .findIndex(reviewRequest => reviewRequest.requestedReviewer.login === currentUserLogin) !== -1; - } else if (type === PRType.AssignedToMe) { - pullRequestFilter = pr => pr.assignees.nodes - .findIndex(asignee => asignee.login === currentUserLogin) !== -1; - } else { - throw new Error('Unexpected pull request filter'); - } - - pullRequestItems = pullRequestItems.filter(pullRequestFilter); - } + 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) { + if(!!pullRequestItem.assignees.nodes && pullRequestItem.assignees.nodes.length) { assignee = pullRequestItem.assignees.nodes[0]; } @@ -629,16 +608,16 @@ export class PullRequestManager { url: pullRequestItem.url, assignee, base: { - label: `${pullRequestItem.baseRef.repo.owner}:${pullRequestItem.baseRef.name}`, + label: `${pullRequestItem.baseRef.repository.owner}:${pullRequestItem.baseRef.name}`, ref: pullRequestItem.baseRef.name, - repo: {cloneUrl: pullRequestItem.baseRef.repo.url }, - sha: pullRequestItem.baseRef.target.sha + repo: {cloneUrl: pullRequestItem.baseRef.repository.url }, + sha: pullRequestItem.baseRef.target.oid }, head: { - label: `${pullRequestItem.headRef.repo.owner}:${pullRequestItem.headRef.name}`, + label: `${pullRequestItem.headRef.repository.owner}:${pullRequestItem.headRef.name}`, ref: pullRequestItem.headRef.name, - repo: {cloneUrl: pullRequestItem.headRef.repo.url }, - sha: pullRequestItem.baseRef.target.sha + repo: {cloneUrl: pullRequestItem.headRef.repository.url }, + sha: pullRequestItem.baseRef.target.oid }, labels: [], body: '', diff --git a/src/github/queries.gql b/src/github/queries.gql index 9f8c15ae2d..dcfbb6cace 100644 --- a/src/github/queries.gql +++ b/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) { @@ -339,65 +348,37 @@ query GetMentionableUsers($owner: String!, $name: String!, $first: Int!, $after: } } -fragment ActorParts on Actor { - login - url - avatarUrl -} -fragment PageInfoParts on PageInfo { - hasNextPage - endCursor -} -fragment RefParts on Ref { - name - target { - sha: oid - } - repo: repository { - url - owner { - login - } - name - } -} -query GetPullRequests($owner: String!, $name: String!, $first: Int!, $after: String){ - repository(owner: $owner, name: $name) { - pullRequests(first: $first, after: $after, states: OPEN) { - nodes { - nodeId: id - number - title - url - author { - ...ActorParts - } - state - assignees(first: 30) { - nodes { - ...ActorParts - } - } - reviewRequests(first: 30) { - nodes { - requestedReviewer { - ...ActorParts - } - } - } - createdAt - updatedAt - merged - headRef { - ...RefParts - } - baseRef { - ...RefParts - } - } - pageInfo { - ...PageInfoParts - } - } - } -} +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 + } + } +} \ No newline at end of file From 97e4513ef2c9eba040754a6427ab5c4d3c4e83c5 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Tue, 5 Mar 2019 09:02:18 -0500 Subject: [PATCH 7/8] Code nits --- src/github/githubRepository.ts | 2 +- src/github/graphql.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index 3406cb13a5..c9fbbd463e 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -7,7 +7,7 @@ import * as vscode from 'vscode'; import * as Octokit from '@octokit/rest'; import Logger from '../common/logger'; import { Remote, parseRemote } from '../common/remote'; -import { IGitHubRepository, IAccount, MergeMethodsAvailability, PRType } from './interface'; +import { PRType, IGitHubRepository, IAccount, MergeMethodsAvailability } from './interface'; import { PullRequestModel } from './pullRequestModel'; import { CredentialStore, GitHub } from './credentials'; import { AuthenticationError } from '../common/authentication'; diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 96ea84e433..f24e76c7ff 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -1,10 +1,10 @@ -import { IAccount } from './interface'; - /*--------------------------------------------------------------------------------------------- * Copyright (c) Microsoft Corporation. All rights reserved. * 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; From ce35d35eff1cc4c8d30a0fcec84a7b6f3eaf67ad Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Tue, 5 Mar 2019 13:54:28 -0500 Subject: [PATCH 8/8] Logging exception type --- src/github/githubRepository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/github/githubRepository.ts b/src/github/githubRepository.ts index c9fbbd463e..e08be6c4e7 100644 --- a/src/github/githubRepository.ts +++ b/src/github/githubRepository.ts @@ -218,7 +218,7 @@ export class GitHubRepository implements IGitHubRepository, vscode.Disposable { } else if (type === PRType.AssignedToMe) { filter += ` assignee:${currentUserLogin}`; } else { - throw new Error('Unexpected pull request filter'); + throw new Error('Unexpected pull request filter: ' + PRType[type]); } }