Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of GHE without merge queue support #926

Closed
wants to merge 1 commit into from
Closed
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
157 changes: 157 additions & 0 deletions addons/isl-server/src/github/generated/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29334,6 +29334,118 @@ export enum WorkflowState {
DisabledManually = 'DISABLED_MANUALLY'
}

/** One possible value for a given Enum. Enum values are unique values, not a placeholder for a string or numeric value. However an Enum value is returned in a JSON response as a string. */
export type __EnumValue = {
__typename?: '__EnumValue';
name: Scalars['String'];
description?: Maybe<Scalars['String']>;
isDeprecated: Scalars['Boolean'];
deprecationReason?: Maybe<Scalars['String']>;
};

/** Object and Interface types are described by a list of Fields, each of which has a name, potentially a list of arguments, and a return type. */
export type __Field = {
__typename?: '__Field';
name: Scalars['String'];
description?: Maybe<Scalars['String']>;
args: Array<__InputValue>;
type: __Type;
isDeprecated: Scalars['Boolean'];
deprecationReason?: Maybe<Scalars['String']>;
};


/** Object and Interface types are described by a list of Fields, each of which has a name, potentially a list of arguments, and a return type. */
export type __FieldArgsArgs = {
includeDeprecated?: InputMaybe<Scalars['Boolean']>;
};

/** Arguments provided to Fields or Directives and the input fields of an InputObject are represented as Input Values which describe their type and optionally a default value. */
export type __InputValue = {
__typename?: '__InputValue';
name: Scalars['String'];
description?: Maybe<Scalars['String']>;
type: __Type;
/** A GraphQL-formatted string representing the default value for this input value. */
defaultValue?: Maybe<Scalars['String']>;
isDeprecated: Scalars['Boolean'];
deprecationReason?: Maybe<Scalars['String']>;
};

/**
* The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum.
*
* Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name, description and optional `specifiedByURL`, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible at runtime. List and NonNull types compose other types.
*/
export type __Type = {
__typename?: '__Type';
kind: __TypeKind;
name?: Maybe<Scalars['String']>;
description?: Maybe<Scalars['String']>;
specifiedByURL?: Maybe<Scalars['String']>;
fields?: Maybe<Array<__Field>>;
interfaces?: Maybe<Array<__Type>>;
possibleTypes?: Maybe<Array<__Type>>;
enumValues?: Maybe<Array<__EnumValue>>;
inputFields?: Maybe<Array<__InputValue>>;
ofType?: Maybe<__Type>;
};


/**
* The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum.
*
* Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name, description and optional `specifiedByURL`, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible at runtime. List and NonNull types compose other types.
*/
export type __TypeFieldsArgs = {
includeDeprecated?: InputMaybe<Scalars['Boolean']>;
};


/**
* The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum.
*
* Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name, description and optional `specifiedByURL`, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible at runtime. List and NonNull types compose other types.
*/
export type __TypeEnumValuesArgs = {
includeDeprecated?: InputMaybe<Scalars['Boolean']>;
};


/**
* The fundamental unit of any GraphQL Schema is the type. There are many kinds of types in GraphQL as represented by the `__TypeKind` enum.
*
* Depending on the kind of a type, certain fields describe information about that type. Scalar types provide no information beyond a name, description and optional `specifiedByURL`, while Enum types provide their values. Object and Interface types provide the fields they describe. Abstract types, Union and Interface, provide the Object types possible at runtime. List and NonNull types compose other types.
*/
export type __TypeInputFieldsArgs = {
includeDeprecated?: InputMaybe<Scalars['Boolean']>;
};

/** An enum describing what kind of type a given `__Type` is. */
export enum __TypeKind {
/** Indicates this type is a scalar. */
Scalar = 'SCALAR',
/** Indicates this type is an object. `fields` and `interfaces` are valid fields. */
Object = 'OBJECT',
/** Indicates this type is an interface. `fields`, `interfaces`, and `possibleTypes` are valid fields. */
Interface = 'INTERFACE',
/** Indicates this type is a union. `possibleTypes` is a valid field. */
Union = 'UNION',
/** Indicates this type is an enum. `enumValues` is a valid field. */
Enum = 'ENUM',
/** Indicates this type is an input object. `inputFields` is a valid field. */
InputObject = 'INPUT_OBJECT',
/** Indicates this type is a list. `ofType` is a valid field. */
List = 'LIST',
/** Indicates this type is a non-null. `ofType` is a valid field. */
NonNull = 'NON_NULL'
}

export type MergeQueueSupportQueryVariables = Exact<{ [key: string]: never; }>;


export type MergeQueueSupportQueryData = { __typename?: 'Query', __type?: { __typename: '__Type' } | null };

export type CommentParts_CommitComment_ = { __typename?: 'CommitComment', bodyHTML: string, createdAt: string, author?: { __typename?: 'Bot', login: string, avatarUrl: string } | { __typename?: 'EnterpriseUserAccount', login: string, avatarUrl: string } | { __typename?: 'Mannequin', login: string, avatarUrl: string } | { __typename?: 'Organization', login: string, avatarUrl: string } | { __typename?: 'User', login: string, avatarUrl: string } | null };

export type CommentParts_Discussion_ = { __typename?: 'Discussion', bodyHTML: string, createdAt: string, author?: { __typename?: 'Bot', login: string, avatarUrl: string } | { __typename?: 'EnterpriseUserAccount', login: string, avatarUrl: string } | { __typename?: 'Mannequin', login: string, avatarUrl: string } | { __typename?: 'Organization', login: string, avatarUrl: string } | { __typename?: 'User', login: string, avatarUrl: string } | null };
Expand Down Expand Up @@ -29398,6 +29510,14 @@ export type YourPullRequestsQueryVariables = Exact<{

export type YourPullRequestsQueryData = { __typename?: 'Query', search: { __typename?: 'SearchResultItemConnection', nodes?: Array<{ __typename?: 'App' } | { __typename?: 'Discussion' } | { __typename?: 'Issue' } | { __typename?: 'MarketplaceListing' } | { __typename?: 'Organization' } | { __typename: 'PullRequest', number: number, title: string, body: string, state: PullRequestState, isDraft: boolean, url: string, reviewDecision?: PullRequestReviewDecision | null, comments: { __typename?: 'IssueCommentConnection', totalCount: number }, mergeQueueEntry?: { __typename?: 'MergeQueueEntry', estimatedTimeToMerge?: number | null } | null, commits: { __typename?: 'PullRequestCommitConnection', nodes?: Array<{ __typename?: 'PullRequestCommit', commit: { __typename?: 'Commit', statusCheckRollup?: { __typename?: 'StatusCheckRollup', state: StatusState } | null } } | null> | null } } | { __typename?: 'Repository' } | { __typename?: 'User' } | null> | null } };

export type YourPullRequestsWithoutMergeQueueQueryVariables = Exact<{
searchQuery: Scalars['String'];
numToFetch: Scalars['Int'];
}>;


export type YourPullRequestsWithoutMergeQueueQueryData = { __typename?: 'Query', search: { __typename?: 'SearchResultItemConnection', nodes?: Array<{ __typename?: 'App' } | { __typename?: 'Discussion' } | { __typename?: 'Issue' } | { __typename?: 'MarketplaceListing' } | { __typename?: 'Organization' } | { __typename: 'PullRequest', number: number, title: string, body: string, state: PullRequestState, isDraft: boolean, url: string, reviewDecision?: PullRequestReviewDecision | null, comments: { __typename?: 'IssueCommentConnection', totalCount: number }, commits: { __typename?: 'PullRequestCommitConnection', nodes?: Array<{ __typename?: 'PullRequestCommit', commit: { __typename?: 'Commit', statusCheckRollup?: { __typename?: 'StatusCheckRollup', state: StatusState } | null } } | null> | null } } | { __typename?: 'Repository' } | { __typename?: 'User' } | null> | null } };

export const CommentParts = `
fragment CommentParts on Comment {
bodyHTML
Expand All @@ -29420,6 +29540,13 @@ export const ReactionParts = `
}
}
`;
export const MergeQueueSupportQuery = `
query MergeQueueSupportQuery {
__type(name: "MergeQueueEntry") {
__typename
}
}
`;
export const PullRequestCommentsQuery = `
query PullRequestCommentsQuery($url: URI!, $numToFetch: Int!) {
resource(url: $url) {
Expand Down Expand Up @@ -29480,5 +29607,35 @@ export const YourPullRequestsQuery = `
}
}
}
}
`;
export const YourPullRequestsWithoutMergeQueueQuery = `
query YourPullRequestsWithoutMergeQueueQuery($searchQuery: String!, $numToFetch: Int!) {
search(query: $searchQuery, type: ISSUE, first: $numToFetch) {
nodes {
... on PullRequest {
__typename
number
title
body
state
isDraft
url
reviewDecision
comments {
totalCount
}
commits(last: 1) {
nodes {
commit {
statusCheckRollup {
state
}
}
}
}
}
}
}
}
`;
56 changes: 44 additions & 12 deletions addons/isl-server/src/github/githubCodeReviewProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@
import type {CodeReviewProvider} from '../CodeReviewProvider';
import type {Logger} from '../logger';
import type {
MergeQueueSupportQueryData,
MergeQueueSupportQueryVariables,
PullRequestCommentsQueryData,
PullRequestCommentsQueryVariables,
PullRequestReviewComment,
PullRequestReviewDecision,
ReactionContent,
YourPullRequestsQueryData,
YourPullRequestsQueryVariables,
YourPullRequestsWithoutMergeQueueQueryData,
YourPullRequestsWithoutMergeQueueQueryVariables,
} from './generated/graphql';
import type {
CodeReviewSystem,
Expand All @@ -24,18 +28,18 @@ import type {
Result,
DiffComment,
} from 'isl/src/types';
import type {ParsedDiff} from 'shared/patch/parse';

import {
MergeQueueSupportQuery,
PullRequestCommentsQuery,
PullRequestState,
StatusState,
YourPullRequestsQuery,
YourPullRequestsWithoutMergeQueueQuery,
} from './generated/graphql';
import queryGraphQL from './queryGraphQL';
import {TypedEventEmitter} from 'shared/TypedEventEmitter';
import {debounce} from 'shared/debounce';
import {parsePatch} from 'shared/patch/parse';
import {notEmpty} from 'shared/utils';

export type GitHubDiffSummary = {
Expand All @@ -55,6 +59,7 @@ type GitHubCodeReviewSystem = CodeReviewSystem & {type: 'github'};
export class GitHubCodeReviewProvider implements CodeReviewProvider {
constructor(private codeReviewSystem: GitHubCodeReviewSystem, private logger: Logger) {}
private diffSummaries = new TypedEventEmitter<'data', Map<DiffId, GitHubDiffSummary>>();
private hasMergeQueueSupport: boolean | null = null;

onChangeDiffSummaries(
callback: (result: Result<Map<DiffId, GitHubDiffSummary>>) => unknown,
Expand All @@ -71,20 +76,47 @@ export class GitHubCodeReviewProvider implements CodeReviewProvider {
};
}

private async detectMergeQueueSupport(): Promise<boolean> {
const data = await this.query<MergeQueueSupportQueryData, MergeQueueSupportQueryVariables>(
MergeQueueSupportQuery,
{},
);
return data?.__type != null;
}

private fetchYourPullRequestsGraphQL(
includeMergeQueue: boolean,
): Promise<YourPullRequestsQueryData | undefined> {
const variables = {
// TODO: somehow base this query on the list of DiffIds
// This is not very easy with github's graphql API, which doesn't allow more than 5 "OR"s in a search query.
// But if we used one-query-per-diff we would reach rate limiting too quickly.
searchQuery: `repo:${this.codeReviewSystem.owner}/${this.codeReviewSystem.repo} is:pr author:@me`,
numToFetch: 50,
};
if (includeMergeQueue) {
return this.query<YourPullRequestsQueryData, YourPullRequestsQueryVariables>(
YourPullRequestsQuery,
variables,
);
} else {
return this.query<
YourPullRequestsWithoutMergeQueueQueryData,
YourPullRequestsWithoutMergeQueueQueryVariables
>(YourPullRequestsWithoutMergeQueueQuery, variables);
}
}

triggerDiffSummariesFetch = debounce(
async () => {
try {
if (this.hasMergeQueueSupport == null) {
this.logger.info('detecting if merge queue is supported');
this.hasMergeQueueSupport = (await this.detectMergeQueueSupport()) ?? false;
this.logger.info('set merge queue support to ' + this.hasMergeQueueSupport);
}
this.logger.info('fetching github PR summaries');
const allSummaries = await this.query<
YourPullRequestsQueryData,
YourPullRequestsQueryVariables
>(YourPullRequestsQuery, {
// TODO: somehow base this query on the list of DiffIds
// This is not very easy with github's graphql API, which doesn't allow more than 5 "OR"s in a search query.
// But if we used one-query-per-diff we would reach rate limiting too quickly.
searchQuery: `repo:${this.codeReviewSystem.owner}/${this.codeReviewSystem.repo} is:pr author:@me`,
numToFetch: 50,
});
const allSummaries = await this.fetchYourPullRequestsGraphQL(this.hasMergeQueueSupport);
if (allSummaries?.search.nodes == null) {
this.diffSummaries.emit('data', new Map());
return;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
query MergeQueueSupportQuery {
__type(name: "MergeQueueEntry") {
__typename
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
query YourPullRequestsWithoutMergeQueueQuery($searchQuery: String!, $numToFetch: Int!) {
search(query: $searchQuery, type: ISSUE, first: $numToFetch) {
nodes {
... on PullRequest {
__typename
number
title
body
state
isDraft
url
reviewDecision
comments {
totalCount
}
commits(last: 1) {
nodes {
commit {
statusCheckRollup {
state
}
}
}
}
}
}
}
}
Loading