Merge pull request #13008 from fabuloso/master

Add `reviewers` and `teamReviewers` params to `publish:github:pull-request` action

.changeset/tricky-ligers-move.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': minor
+---
+
+Added `reviewers` and `teamReviewers` parameters to `publish:github:pull-request` action to add reviewers on the pull request created by the action

plugins/scaffolder-backend/api-report.md

@@ -20,6 +20,7 @@ import { Knex } from 'knex';
 import { LocationSpec } from '@backstage/plugin-catalog-backend';
 import { Logger } from 'winston';
 import { Observable } from '@backstage/types';
+import { Octokit } from 'octokit';
 import { PluginDatabaseManager } from '@backstage/backend-common';
 import { Schema } from 'jsonschema';
 import { ScmIntegrationRegistry } from '@backstage/integration';
@@ -391,6 +392,8 @@ export const createPublishGithubPullRequestAction: ({
   targetPath?: string | undefined;
   sourcePath?: string | undefined;
   token?: string | undefined;
+  reviewers?: string[] | undefined;
+  teamReviewers?: string[] | undefined;
 }>;
 
 // @public
@@ -523,15 +526,14 @@ export function fetchContents({
 }): Promise<void>;
 
 // @public (undocumented)
-export interface OctokitWithPullRequestPluginClient {
-  // (undocumented)
+export type OctokitWithPullRequestPluginClient = Octokit & {
   createPullRequest(options: createPullRequest.Options): Promise<{
     data: {
       html_url: string;
       number: number;
     };
   } | null>;
-}
+};
 
 // @public
 export interface RouterOptions {

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts

@@ -26,7 +26,6 @@ import { resolve as resolvePath } from 'path';
 import { Writable } from 'stream';
 import { ActionContext, TemplateAction } from '../../types';
 import {
-  CreateGithubPullRequestClientFactoryInput,
   createPublishGithubPullRequestAction,
   OctokitWithPullRequestPluginClient,
 } from './githubPullRequest';
@@ -42,11 +41,12 @@ type GithubPullRequestActionInput = ReturnType<
 
 describe('createPublishGithubPullRequestAction', () => {
   let instance: TemplateAction<GithubPullRequestActionInput>;
-  let fakeClient: OctokitWithPullRequestPluginClient;
-
-  let clientFactory: (
-    input: CreateGithubPullRequestClientFactoryInput,
-  ) => Promise<OctokitWithPullRequestPluginClient>;
+  let fakeClient: {
+    createPullRequest: jest.Mock;
+    rest: {
+      pulls: { requestReviewers: jest.Mock };
+    };
+  };
 
   beforeEach(() => {
     const integrations = ScmIntegrations.fromConfig(new ConfigReader({}));
@@ -62,8 +62,15 @@ describe('createPublishGithubPullRequestAction', () => {
           },
         };
       }),
+      rest: {
+        pulls: {
+          requestReviewers: jest.fn(async (_: any) => ({ data: {} })),
+        },
+      },
     };
-    clientFactory = jest.fn(async () => fakeClient);
+    const clientFactory = jest.fn(
+      async () => fakeClient as unknown as OctokitWithPullRequestPluginClient,
+    );
     const githubCredentialsProvider: GithubCredentialsProvider = {
       getCredentials: jest.fn(),
     };
@@ -75,6 +82,11 @@ describe('createPublishGithubPullRequestAction', () => {
     });
   });
 
+  afterEach(() => {
+    mockFs.restore();
+    jest.resetAllMocks();
+  });
+
   describe('with no sourcePath', () => {
     let input: GithubPullRequestActionInput;
     let ctx: ActionContext<GithubPullRequestActionInput>;
@@ -101,6 +113,7 @@ describe('createPublishGithubPullRequestAction', () => {
         workspacePath,
       };
     });
+
     it('creates a pull request', async () => {
       await instance.handler(ctx);
 
@@ -135,10 +148,6 @@ describe('createPublishGithubPullRequestAction', () => {
       );
       expect(ctx.output).toHaveBeenCalledWith('pullRequestNumber', 123);
     });
-    afterEach(() => {
-      mockFs.restore();
-      jest.resetAllMocks();
-    });
   });
 
   describe('with sourcePath', () => {
@@ -171,11 +180,6 @@ describe('createPublishGithubPullRequestAction', () => {
       };
     });
 
-    afterEach(() => {
-      mockFs.restore();
-      jest.resetAllMocks();
-    });
-
     it('creates a pull request with only relevant files', async () => {
       await instance.handler(ctx);
 
@@ -267,9 +271,91 @@ describe('createPublishGithubPullRequestAction', () => {
       );
       expect(ctx.output).toHaveBeenCalledWith('pullRequestNumber', 123);
     });
-    afterEach(() => {
-      mockFs.restore();
-      jest.resetAllMocks();
+  });
+
+  describe('with reviewers and teamReviewers', () => {
+    let input: GithubPullRequestActionInput;
+    let ctx: ActionContext<GithubPullRequestActionInput>;
+
+    beforeEach(() => {
+      input = {
+        repoUrl: 'github.com?owner=myorg&repo=myrepo',
+        title: 'Create my new app',
+        branchName: 'new-app',
+        description: 'This PR is really good',
+        reviewers: ['foobar'],
+        teamReviewers: ['team-foo'],
+      };
+
+      mockFs({ [workspacePath]: {} });
+
+      ctx = {
+        createTemporaryDirectory: jest.fn(),
+        output: jest.fn(),
+        logger: getRootLogger(),
+        logStream: new Writable(),
+        input,
+        workspacePath,
+      };
+    });
+
+    it('creates a pull request and requests a review from the given reviewers', async () => {
+      await instance.handler(ctx);
+
+      expect(fakeClient.createPullRequest).toBeCalled();
+      expect(fakeClient.rest.pulls.requestReviewers).toBeCalledWith({
+        owner: 'myorg',
+        repo: 'myrepo',
+        pull_number: 123,
+        reviewers: ['foobar'],
+        team_reviewers: ['team-foo'],
+      });
+    });
+
+    it('creates outputs for the pull request url and number even if requesting reviewers fails', async () => {
+      fakeClient.rest.pulls.requestReviewers.mockImplementation(() => {
+        throw new Error('a random error');
+      });
+
+      await instance.handler(ctx);
+
+      expect(ctx.output).toHaveBeenCalledWith(
+        'remoteUrl',
+        'https://github.com/myorg/myrepo/pull/123',
+      );
+      expect(ctx.output).toHaveBeenCalledWith('pullRequestNumber', 123);
+    });
+  });
+
+  describe('with no reviewers and teamReviewers', () => {
+    let input: GithubPullRequestActionInput;
+    let ctx: ActionContext<GithubPullRequestActionInput>;
+
+    beforeEach(() => {
+      input = {
+        repoUrl: 'github.com?owner=myorg&repo=myrepo',
+        title: 'Create my new app',
+        branchName: 'new-app',
+        description: 'This PR is really good',
+      };
+
+      mockFs({ [workspacePath]: {} });
+
+      ctx = {
+        createTemporaryDirectory: jest.fn(),
+        output: jest.fn(),
+        logger: getRootLogger(),
+        logStream: new Writable(),
+        input,
+        workspacePath,
+      };
+    });
+
+    it('does not call the API endpoint for requesting reviewers', async () => {
+      await instance.handler(ctx);
+
+      expect(fakeClient.createPullRequest).toBeCalled();
+      expect(fakeClient.rest.pulls.requestReviewers).not.toBeCalled();
     });
   });
 
@@ -336,10 +422,6 @@ describe('createPublishGithubPullRequestAction', () => {
       );
       expect(ctx.output).toHaveBeenCalledWith('pullRequestNumber', 123);
     });
-    afterEach(() => {
-      mockFs.restore();
-      jest.resetAllMocks();
-    });
   });
 
   describe('with executable file mode 775', () => {
@@ -405,9 +487,5 @@ describe('createPublishGithubPullRequestAction', () => {
       );
       expect(ctx.output).toHaveBeenCalledWith('pullRequestNumber', 123);
     });
-    afterEach(() => {
-      mockFs.restore();
-      jest.resetAllMocks();
-    });
   });
 });

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts

@@ -27,20 +27,21 @@ import { createPullRequest } from 'octokit-plugin-create-pull-request';
 import { resolveSafeChildPath } from '@backstage/backend-common';
 import { getOctokitOptions } from '../github/helpers';
 import { serializeDirectoryContents } from '../../../../lib/files';
+import { Logger } from 'winston';
 
 export type Encoding = 'utf-8' | 'base64';
 
 class GithubResponseError extends CustomErrorBase {}
 
 /** @public */
-export interface OctokitWithPullRequestPluginClient {
+export type OctokitWithPullRequestPluginClient = Octokit & {
   createPullRequest(options: createPullRequest.Options): Promise<{
     data: {
       html_url: string;
       number: number;
     };
   } | null>;
-}
+};
 
 /**
  * The options passed to the client factory function.
@@ -99,6 +100,12 @@ export interface CreateGithubPullRequestActionOptions {
   ) => Promise<OctokitWithPullRequestPluginClient>;
 }
 
+type GithubPullRequest = {
+  owner: string;
+  repo: string;
+  number: number;
+};
+
 /**
  * Creates a Github Pull Request action.
  * @public
@@ -117,6 +124,8 @@ export const createPublishGithubPullRequestAction = ({
     targetPath?: string;
     sourcePath?: string;
     token?: string;
+    reviewers?: string[];
+    teamReviewers?: string[];
   }>({
     id: 'publish:github:pull-request',
     schema: {
@@ -165,6 +174,24 @@ export const createPublishGithubPullRequestAction = ({
             type: 'string',
             description: 'The token to use for authorization to GitHub',
           },
+          reviewers: {
+            title: 'Pull Request Reviewers',
+            type: 'array',
+            items: {
+              type: 'string',
+            },
+            description:
+              'The users that will be added as reviewers to the pull request',
+          },
+          teamReviewers: {
+            title: 'Pull Request Team Reviewers',
+            type: 'array',
+            items: {
+              type: 'string',
+            },
+            description:
+              'The teams that will be added as reviewers to the pull request',
+          },
         },
       },
       output: {
@@ -194,6 +221,8 @@ export const createPublishGithubPullRequestAction = ({
         targetPath,
         sourcePath,
         token: providedToken,
+        reviewers,
+        teamReviewers,
       } = ctx.input;
 
       const { owner, repo, host } = parseRepoUrl(repoUrl, integrations);
@@ -259,11 +288,51 @@ export const createPublishGithubPullRequestAction = ({
           throw new GithubResponseError('null response from Github');
         }
 
+        const pullRequestNumber = response.data.number;
+        if (reviewers || teamReviewers) {
+          const pullRequest = { owner, repo, number: pullRequestNumber };
+          await requestReviewersOnPullRequest(
+            pullRequest,
+            reviewers,
+            teamReviewers,
+            client,
+            ctx.logger,
+          );
+        }
+
         ctx.output('remoteUrl', response.data.html_url);
-        ctx.output('pullRequestNumber', response.data.number);
+        ctx.output('pullRequestNumber', pullRequestNumber);
       } catch (e) {
         throw new GithubResponseError('Pull request creation failed', e);
       }
     },
   });
+
+  async function requestReviewersOnPullRequest(
+    pr: GithubPullRequest,
+    reviewers: string[] | undefined,
+    teamReviewers: string[] | undefined,
+    client: Octokit,
+    logger: Logger,
+  ) {
+    try {
+      const result = await client.rest.pulls.requestReviewers({
+        owner: pr.owner,
+        repo: pr.repo,
+        pull_number: pr.number,
+        reviewers,
+        team_reviewers: teamReviewers,
+      });
+      const addedUsers = result.data.requested_reviewers?.join(', ') ?? '';
+      const addedTeams = result.data.requested_teams?.join(', ') ?? '';
+      logger.info(
+        `Added users [${addedUsers}] and teams [${addedTeams}] as reviewers to Pull request ${pr.number}`,
+      );
+    } catch (e) {
+      logger.error(
+        `Failure when adding reviewers to Pull request ${pr.number}`,
+        e,
+      );
+    }
+  }
 };