diff --git a/.changeset/tricky-ligers-move.md b/.changeset/tricky-ligers-move.md new file mode 100644 index 0000000000..21aa7757ba --- /dev/null +++ b/.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 diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 615d6aa8fb..de9a9e677f 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/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; // @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 { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts index d030430924..1bc768e4bd 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts +++ b/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; - let fakeClient: OctokitWithPullRequestPluginClient; - - let clientFactory: ( - input: CreateGithubPullRequestClientFactoryInput, - ) => Promise; + 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; @@ -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; + + 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; + + 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(); - }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index f8f9b22d61..51049e4357 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/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; } +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, + ); + } + } };