Fix handling of GHE without merge queue support (#926)

Summary:
Fix handling of GHE without merge queue support
#830 and #906 reported sapling not working for old Github Enterprise versions since their graphql schema doesnt include merge queue.

This adds a query to detect if merge queue is supported in the graphql version. Then, it will issue a different "YourPullRequestsQuery" depending on if merge queue is supported.

I considered a few other approaches:
- Just detect the error message and retry without merge queue field - a little gross to extract from the error
- Use "include" instead of an entirely separate query - didn't seem to count as a valid query still via graphiql at least (unsure if the query client here would strip out the unused field though)

Pull Request resolved: #926

Test Plan: Haven't had a chance to test yet, will try and validate that both versions work and that the detection logic works.

Reviewed By: evangrayk

Differential Revision: D60215245

fbshipit-source-id: cb36a77fbe0665353740d7fa97bc21ee9c86e0e8

addons/isl-server/src/github/generated/graphql.ts

@@ -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 };
@@ -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
@@ -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) {
@@ -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
+              }
+            }
+          }
+        }
+      }
+    }
+  }
 }
     `;

addons/isl-server/src/github/githubCodeReviewProvider.ts

@@ -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,
@@ -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 = {
@@ -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,
@@ -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;

addons/isl-server/src/github/queries/MergeQueueSupportQuery.graphql

@@ -0,0 +1,5 @@
+query MergeQueueSupportQuery {
+  __type(name: "MergeQueueEntry") {
+    __typename
+  }
+}

addons/isl-server/src/github/queries/YourPullRequestsWithoutMergeQueueQuery.graphql

@@ -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
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}