From f821fc0180415d96541c08f4e15868899a79be7a Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Thu, 13 Aug 2020 12:31:33 -0700 Subject: [PATCH] Use graphQL to fetch check runs and check suites, fixes #1105, fixes #790 --- preview-src/cache.ts | 5 +- preview-src/index.css | 1 + preview-src/merge.tsx | 10 ++-- src/github/credentials.ts | 2 +- src/github/folderRepositoryManager.ts | 59 +++++++++++++++---- src/github/graphql.ts | 46 +++++++++++++++ src/github/interface.ts | 13 ++++ src/github/queries.gql | 42 +++++++++++++ .../builders/managedPullRequestBuilder.ts | 4 +- .../builders/rest/combinedStatusBuilder.ts | 12 +--- 10 files changed, 162 insertions(+), 32 deletions(-) diff --git a/preview-src/cache.ts b/preview-src/cache.ts index f840c18d9..60a88cc2c 100644 --- a/preview-src/cache.ts +++ b/preview-src/cache.ts @@ -4,9 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { vscode } from './message'; -import { GithubItemStateEnum, IAccount, ReviewState, ILabel, MergeMethod, MergeMethodsAvailability, PullRequestMergeability } from '../src/github/interface'; +import { GithubItemStateEnum, IAccount, ReviewState, ILabel, MergeMethod, MergeMethodsAvailability, PullRequestMergeability, PullRequestChecks } from '../src/github/interface'; import { TimelineEvent } from '../src/common/timelineEvent'; -import { ReposGetCombinedStatusForRefResponseData } from '@octokit/types'; export interface PullRequest { number: number; @@ -35,7 +34,7 @@ export interface PullRequest { hasWritePermission: boolean; pendingCommentText?: string; pendingCommentDrafts?: { [key: string]: string; }; - status: ReposGetCombinedStatusForRefResponseData; + status: PullRequestChecks; mergeable: PullRequestMergeability; defaultMergeMethod: MergeMethod; mergeMethodsAvailability: MergeMethodsAvailability; diff --git a/preview-src/index.css b/preview-src/index.css index b5690644f..6faf11b35 100644 --- a/preview-src/index.css +++ b/preview-src/index.css @@ -171,6 +171,7 @@ body .comment-container .review-comment-header { .status-check { display: flex; align-items: center; + justify-content: space-between; margin-top: 5px; margin-left: 15px; } diff --git a/preview-src/merge.tsx b/preview-src/merge.tsx index 1a42a0c56..9e60d5d75 100644 --- a/preview-src/merge.tsx +++ b/preview-src/merge.tsx @@ -245,10 +245,12 @@ const StatusCheckDetails = ({ statuses }: Partial) =>
{ statuses.map(s =>
- - - {s.context} — {s.description} - Details +
+ + + {s.context} {s.description ? `— ${s.description}` : ''} +
+ { !!s.target_url ? Details : null }
) }
; diff --git a/src/github/credentials.ts b/src/github/credentials.ts index 5ad620d80..36810214e 100644 --- a/src/github/credentials.ts +++ b/src/github/credentials.ts @@ -198,7 +198,7 @@ const link = (url: string, token: string) => headers: { ...headers, authorization: token ? `Bearer ${token}` : '', - Accept: 'application/vnd.github.shadow-cat-preview+json' + Accept: 'application/vnd.github.shadow-cat-preview+json, application/vnd.github.antiope-preview+json' } }))).concat(createHttpLink({ uri: `${url}/graphql`, diff --git a/src/github/folderRepositoryManager.ts b/src/github/folderRepositoryManager.ts index e51807fc4..1e44912bd 100644 --- a/src/github/folderRepositoryManager.ts +++ b/src/github/folderRepositoryManager.ts @@ -11,7 +11,7 @@ import { IComment } from '../common/comment'; import { Remote, parseRepositoryRemotes } from '../common/remote'; import { TimelineEvent, EventType, ReviewEvent as CommonReviewEvent, isReviewEvent } from '../common/timelineEvent'; import { GitHubRepository, PullRequestData, ItemsData, ViewerPermission } from './githubRepository'; -import { IPullRequestsPagingOptions, PRType, ReviewEvent, IPullRequestEditData, PullRequest, IRawFileChange, IAccount, ILabel, RepoAccessAndMergeMethods, PullRequestMergeability, User } from './interface'; +import { IPullRequestsPagingOptions, PRType, ReviewEvent, IPullRequestEditData, PullRequest, IRawFileChange, IAccount, ILabel, RepoAccessAndMergeMethods, PullRequestMergeability, User, PullRequestChecks } from './interface'; import { PullRequestGitHelper, PullRequestMetadata } from './pullRequestGitHelper'; import { PullRequestModel, IResolvedPullRequestModel } from './pullRequestModel'; import { GitHubManager } from '../authentication/githubServer'; @@ -21,7 +21,7 @@ import Logger from '../common/logger'; import { EXTENSION_ID } from '../constants'; import { fromPRUri } from '../common/uri'; import { convertRESTPullRequestToRawPullRequest, parseGraphQLTimelineEvents, getRelatedUsersFromTimelineEvents, parseGraphQLComment, getReactionGroup, convertRESTUserToAccount, convertRESTReviewEvent, parseGraphQLReviewEvent, loginComparator, parseGraphQlIssueComment, convertPullRequestsGetCommentsResponseItemToComment, convertRESTIssueToRawPullRequest, parseGraphQLUser } from './utils'; -import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse, MarkPullRequestReadyForReviewResponse, PullRequestState, UpdatePullRequestResponse, EditIssueCommentResponse, AddIssueCommentResponse, UserResponse, StartReviewResponse } from './graphql'; +import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse, DeleteReactionResponse, AddReactionResponse, MarkPullRequestReadyForReviewResponse, PullRequestState, UpdatePullRequestResponse, EditIssueCommentResponse, AddIssueCommentResponse, UserResponse, StartReviewResponse, GetChecksResponse, isCheckRun } from './graphql'; import { ITelemetry } from '../common/telemetry'; import { ApiImpl } from '../api/api1'; import { Protocol } from '../common/protocol'; @@ -830,20 +830,53 @@ export class FolderRepositoryManager implements vscode.Disposable { return max; } - async getStatusChecks(pullRequest: PullRequestModel): Promise { - if (!pullRequest.isResolved()) { - return; - } + async getStatusChecks(pullRequest: PullRequestModel): Promise { + const { query, remote, schema } = await pullRequest.githubRepository.ensure(); + const result = await query({ + query: schema.GetChecks, + variables: { + owner: remote.owner, + name: remote.repositoryName, + number: pullRequest.number + } + }); - const { remote, octokit } = await pullRequest.githubRepository.ensure(); + // We always fetch the status checks for only the last commit, so there should only be one node present + const statusCheckRollup = result.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup; - const result = await octokit.repos.getCombinedStatusForRef({ - owner: remote.owner, - repo: remote.repositoryName, - ref: pullRequest.head.sha - }); + if (!statusCheckRollup) { + return { + state: 'pending', + statuses: [] + }; + } - return result.data; + return { + state: statusCheckRollup.state.toLowerCase(), + statuses: statusCheckRollup.contexts.nodes.map(context => { + if (isCheckRun(context)) { + return { + id: context.id, + url: context.checkSuite.app?.url, + avatar_url: context.checkSuite.app?.logoUrl, + state: context.conclusion?.toLowerCase() || 'pending', + description: context.title, + context: context.name, + target_url: context.detailsUrl + }; + } else { + return { + id: context.id, + url: context.targetUrl, + avatar_url: context.avatarUrl, + state: context.state.toLowerCase(), + description: context.description, + context: context.context, + target_url: context.targetUrl + }; + } + }) + }; } async getReviewRequests(pullRequest: PullRequestModel): Promise { diff --git a/src/github/graphql.ts b/src/github/graphql.ts index e6c8f6159..1d003bb7a 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -528,3 +528,49 @@ export interface StartReviewResponse { }; }; } + +export interface StatusContext { + id: string; + state: 'ERROR' | 'EXPECTED' | 'FAILURE' | 'PENDING' | 'SUCCESS'; + description?: string; + context: string; + targetUrl?: string; + avatarUrl?: string; +} + +export interface CheckRun { + id: string; + conclusion?: 'ACTION_REQUIRED' | 'CANCELLED' | 'FAILURE' | 'NEUTRAL' | 'SKIPPED' | 'STALE' | 'SUCCESS' | 'TIMED_OUT'; + name: string; + title?: string; + detailsUrl?: string; + checkSuite: { + app?: { + logoUrl: string; + url: string; + }; + }; +} + +export function isCheckRun(x: CheckRun | StatusContext): x is CheckRun { + return !!(x as CheckRun).conclusion; +} + +export interface GetChecksResponse { + repository: { + pullRequest: { + commits: { + nodes: { + commit: { + statusCheckRollup?: { + state: string; + contexts: { + nodes: (StatusContext | CheckRun)[] + } + } + } + }[] + } + } + }; +} diff --git a/src/github/interface.ts b/src/github/interface.ts index c4b758587..7c6be05c4 100644 --- a/src/github/interface.ts +++ b/src/github/interface.ts @@ -150,3 +150,16 @@ export interface User extends IAccount { repoNameWithOwner: string; }[]; } + +export interface PullRequestChecks { + state: string; + statuses: { + id: string; + url?: string; + avatar_url?: string; + state: string; + description?: string; + target_url?: string; + context: string; + }[]; +} \ No newline at end of file diff --git a/src/github/queries.gql b/src/github/queries.gql index 4ea0819fc..22bfda25e 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -749,3 +749,45 @@ query GetRepositoryForkDetails($owner: String!, $name: String!) { } } } + +query GetChecks($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + commits(last: 1) { + nodes { + commit { + statusCheckRollup { + state + contexts(first: 100) { + nodes { + ...on StatusContext { + id + state + targetUrl + description + context + avatarUrl + } + ...on CheckRun { + id + conclusion + title + detailsUrl + name + resourcePath + checkSuite { + app { + logoUrl + url + } + } + } + } + } + } + } + } + } + } + } +} diff --git a/src/test/builders/managedPullRequestBuilder.ts b/src/test/builders/managedPullRequestBuilder.ts index 25c0af15d..25878ca78 100644 --- a/src/test/builders/managedPullRequestBuilder.ts +++ b/src/test/builders/managedPullRequestBuilder.ts @@ -3,7 +3,6 @@ import { TimelineEventsResponse as TimelineEventsGraphQL } from '../../github/graphql'; import { - ReposGetCombinedStatusForRefResponseData as CombinedStatusREST, PullsListRequestedReviewersResponseData as ReviewRequestsREST, IssuesListEventsForTimelineResponseData as TimelineEventREST, } from '@octokit/types'; @@ -15,6 +14,7 @@ import { RepoUnion as RepositoryREST, RepositoryBuilder as RepositoryRESTBuilder import { CombinedStatusBuilder as CombinedStatusRESTBuilder } from './rest/combinedStatusBuilder'; import { ReviewRequestsBuilder as ReviewRequestsRESTBuilder } from './rest/reviewRequestsBuilder'; import { createBuilderClass } from './base'; +import { PullRequestChecks } from '../../github/interface'; type ResponseFlavor = APIFlavor extends 'graphql' ? GQL : RST; @@ -22,7 +22,7 @@ export interface ManagedPullRequest { pullRequest: ResponseFlavor; timelineEvents: ResponseFlavor; repositoryREST: RepositoryREST; - combinedStatusREST: CombinedStatusREST; + combinedStatusREST: PullRequestChecks; reviewRequestsREST: ReviewRequestsREST; } diff --git a/src/test/builders/rest/combinedStatusBuilder.ts b/src/test/builders/rest/combinedStatusBuilder.ts index d5fe9be01..e8bb15b67 100644 --- a/src/test/builders/rest/combinedStatusBuilder.ts +++ b/src/test/builders/rest/combinedStatusBuilder.ts @@ -1,8 +1,7 @@ -import * as OctokitTypes from '@octokit/types'; import { createBuilderClass } from '../base'; -import { RepositoryBuilder } from './repoBuilder'; import { OctokitCommon } from '../../../github/common'; +import { PullRequestChecks } from '../../../github/interface'; export const StatusItemBuilder = createBuilderClass()({ url: { default: 'https://api.github.com/repos/octocat/Hello-World/statuses/0000000000000000000000000000000000000000' }, @@ -19,14 +18,9 @@ export const StatusItemBuilder = createBuilderClass; -export const CombinedStatusBuilder = createBuilderClass()({ +export const CombinedStatusBuilder = createBuilderClass()({ state: { default: 'success' }, - statuses: { default: [] }, - sha: { default: '0000000000000000000000000000000000000000' }, - commit_url: { default: 'https://api.github.com/repos/octocat/Hello-World/commits/0000000000000000000000000000000000000000' }, - url: { default: 'https://api.github.com/repos/octocat/Hello-World/0000000000000000000000000000000000000000/status' }, - total_count: { default: 1 }, - repository: { linked: RepositoryBuilder }, + statuses: { default: [] } }); export type CombinedStatusBuilder = InstanceType; \ No newline at end of file