From 84b0e47373db5b51b99862bd29f5ecef159a0c61 Mon Sep 17 00:00:00 2001 From: mingfu Date: Mon, 5 Jun 2023 15:08:19 +0800 Subject: [PATCH] chore: Add `TargetBranchName` variable and output for the `publish:gitlab:merge-request` and `publish:github:pull-request` s'cascaffolder actions. Signed-off-by: mingfu --- .changeset/fifty-shirts-brush.md | 5 + plugins/scaffolder-backend/api-report.md | 5 + .../builtin/publish/githubPullRequest.test.ts | 92 +++++++++++++++++++ .../builtin/publish/githubPullRequest.ts | 24 ++++- .../publish/gitlabMergeRequest.test.ts | 51 ++++++++++ .../builtin/publish/gitlabMergeRequest.ts | 28 ++++-- 6 files changed, 197 insertions(+), 8 deletions(-) create mode 100644 .changeset/fifty-shirts-brush.md diff --git a/.changeset/fifty-shirts-brush.md b/.changeset/fifty-shirts-brush.md new file mode 100644 index 0000000000..98cd7b885c --- /dev/null +++ b/.changeset/fifty-shirts-brush.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Add `TargetBranchName` variable and output for the `publish:gitlab:merge-request` and `publish:github:pull-request` s'cascaffolder actions. diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 5e18ad15bf..0e686c31f3 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -584,6 +584,7 @@ export const createPublishGithubPullRequestAction: ( { title: string; branchName: string; + targetBranchName?: string | undefined; description: string; repoUrl: string; draft?: boolean | undefined; @@ -626,6 +627,7 @@ export const createPublishGitlabMergeRequestAction: (options: { title: string; description: string; branchName: string; + targetBranchName?: string | undefined; sourcePath?: string | undefined; targetPath?: string | undefined; token?: string | undefined; @@ -772,6 +774,9 @@ export type OctokitWithPullRequestPluginClient = Octokit & { data: { html_url: string; number: number; + base: { + ref: string; + }; }; } | null>; }; 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 7750277728..82f8d22aea 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 @@ -62,6 +62,9 @@ describe('createPublishGithubPullRequestAction', () => { data: { html_url: 'https://github.com/myorg/myrepo/pull/123', number: 123, + base: { + ref: 'main', + }, }, }; }), @@ -90,6 +93,94 @@ describe('createPublishGithubPullRequestAction', () => { jest.resetAllMocks(); }); + describe('with targetBranchName', () => { + let input: GithubPullRequestActionInput; + let ctx: ActionContext; + + beforeEach(() => { + fakeClient = { + createPullRequest: jest.fn(async (_: any) => { + return { + url: 'https://api.github.com/myorg/myrepo/pull/123', + headers: {}, + status: 201, + data: { + html_url: 'https://github.com/myorg/myrepo/pull/123', + number: 123, + base: { + ref: 'test', + }, + }, + }; + }), + rest: { + pulls: { + requestReviewers: jest.fn(async (_: any) => ({ data: {} })), + }, + }, + }; + + input = { + repoUrl: 'github.com?owner=myorg&repo=myrepo', + title: 'Create my new app', + branchName: 'new-app', + targetBranchName: 'test', + description: 'This PR is really good', + draft: true, + }; + + mockFs({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = { + createTemporaryDirectory: jest.fn(), + output: jest.fn(), + logger: getRootLogger(), + logStream: new Writable(), + input, + workspacePath, + }; + }); + + it('creates a pull request', async () => { + await instance.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + base: 'test', + body: 'This PR is really good', + draft: true, + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + }, + ], + }); + }); + + it('creates outputs for the pull request url and number', async () => { + await instance.handler(ctx); + + expect(ctx.output).toHaveBeenCalledWith('targetBranchName', 'test'); + expect(ctx.output).toHaveBeenCalledWith( + 'remoteUrl', + 'https://github.com/myorg/myrepo/pull/123', + ); + expect(ctx.output).toHaveBeenCalledWith('pullRequestNumber', 123); + }); + }); + describe('with no sourcePath', () => { let input: GithubPullRequestActionInput; let ctx: ActionContext; @@ -145,6 +236,7 @@ describe('createPublishGithubPullRequestAction', () => { it('creates outputs for the pull request url and number', async () => { await instance.handler(ctx); + expect(ctx.output).toHaveBeenCalledWith('targetBranchName', 'main'); expect(ctx.output).toHaveBeenCalledWith( 'remoteUrl', 'https://github.com/myorg/myrepo/pull/123', 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 6706ad9c06..167fedc88e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts @@ -42,6 +42,9 @@ export type OctokitWithPullRequestPluginClient = Octokit & { data: { html_url: string; number: number; + base: { + ref: string; + }; }; } | null>; }; @@ -128,6 +131,7 @@ export const createPublishGithubPullRequestAction = ( return createTemplateAction<{ title: string; branchName: string; + targetBranchName?: string; description: string; repoUrl: string; draft?: boolean; @@ -154,6 +158,11 @@ export const createPublishGithubPullRequestAction = ( title: 'Branch Name', description: 'The name for the branch', }, + targetBranchName: { + type: 'string', + title: 'Target Branch Name', + description: 'The target branch name of the merge request', + }, title: { type: 'string', title: 'Pull Request Name', @@ -214,6 +223,10 @@ export const createPublishGithubPullRequestAction = ( required: ['remoteUrl'], type: 'object', properties: { + targetBranchName: { + title: 'Target branch name of the merge request', + type: 'string', + }, remoteUrl: { type: 'string', title: 'Pull Request URL', @@ -231,6 +244,7 @@ export const createPublishGithubPullRequestAction = ( const { repoUrl, branchName, + targetBranchName, title, description, draft, @@ -298,7 +312,7 @@ export const createPublishGithubPullRequestAction = ( ); try { - const response = await client.createPullRequest({ + const createOptions: createPullRequest.Options = { owner, repo, title, @@ -311,7 +325,11 @@ export const createPublishGithubPullRequestAction = ( body: description, head: branchName, draft, - }); + }; + if (targetBranchName) { + createOptions.base = targetBranchName; + } + const response = await client.createPullRequest(createOptions); if (!response) { throw new GithubResponseError('null response from Github'); @@ -329,6 +347,8 @@ export const createPublishGithubPullRequestAction = ( ); } + const targetBranch = response.data.base.ref; + ctx.output('targetBranchName', targetBranch); ctx.output('remoteUrl', response.data.html_url); ctx.output('pullRequestNumber', pullRequestNumber); } catch (e) { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts index c3d0fe58f1..afb93b348a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.test.ts @@ -101,6 +101,49 @@ describe('createGitLabMergeRequest', () => { mockFs.restore(); }); + describe('createGitLabMergeRequestWithSpecifiedTargetBranch', () => { + it('removeSourceBranch is false by default when not passed in options', async () => { + const input = { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + title: 'Create my new MR', + branchName: 'new-mr', + targetBranchName: 'test', + description: 'This MR is really good', + targetPath: 'Subdirectory', + }; + mockFs({ + [workspacePath]: { + source: { 'foo.txt': 'Hello there!' }, + irrelevant: { 'bar.txt': 'Nothing to see here' }, + }, + }); + const ctx = { + createTemporaryDirectory: jest.fn(), + output: jest.fn(), + logger: getRootLogger(), + logStream: new Writable(), + input, + workspacePath, + }; + await instance.handler(ctx); + + expect(mockGitlabClient.Projects.show).not.toHaveBeenCalled(); + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'test', + ); + expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'test', + 'Create my new MR', + { description: 'This MR is really good', removeSourceBranch: false }, + ); + expect(ctx.output).toHaveBeenCalledWith('targetBranchName', 'test'); + }); + }); + describe('createGitLabMergeRequestWithoutRemoveBranch', () => { it('removeSourceBranch is false by default when not passed in options', async () => { const input = { @@ -126,6 +169,12 @@ describe('createGitLabMergeRequest', () => { }; await instance.handler(ctx); + expect(mockGitlabClient.Projects.show).toHaveBeenCalledWith('owner/repo'); + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + ); expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( 'owner/repo', 'new-mr', @@ -133,6 +182,8 @@ describe('createGitLabMergeRequest', () => { 'Create my new MR', { description: 'This MR is really good', removeSourceBranch: false }, ); + + expect(ctx.output).toHaveBeenCalledWith('targetBranchName', 'main'); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts index 3dfe0cff5c..8abb53288c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts @@ -39,6 +39,7 @@ export const createPublishGitlabMergeRequestAction = (options: { title: string; description: string; branchName: string; + targetBranchName?: string; sourcePath?: string; targetPath?: string; token?: string; @@ -77,8 +78,13 @@ export const createPublishGitlabMergeRequestAction = (options: { }, branchName: { type: 'string', - title: 'Destination Branch name', - description: 'The description of the merge request', + title: 'Source Branch Name', + description: 'The source branch name of the merge request', + }, + targetBranchName: { + type: 'string', + title: 'Target Branch Name', + description: 'The target branch name of the merge request', }, sourcePath: { type: 'string', @@ -119,6 +125,10 @@ export const createPublishGitlabMergeRequestAction = (options: { output: { type: 'object', properties: { + targetBranchName: { + title: 'Target branch name of the merge request', + type: 'string', + }, projectid: { title: 'Gitlab Project id/Name(slug)', type: 'string', @@ -139,6 +149,7 @@ export const createPublishGitlabMergeRequestAction = (options: { const { assignee, branchName, + targetBranchName, description, repoUrl, removeSourceBranch, @@ -211,12 +222,16 @@ export const createPublishGitlabMergeRequestAction = (options: { execute_filemode: file.executable, })); - const projects = await api.Projects.show(repoID); + let targetBranch = targetBranchName; + if (!targetBranch) { + const projects = await api.Projects.show(repoID); - const { default_branch: defaultBranch } = projects; + const { default_branch: defaultBranch } = projects; + targetBranch = defaultBranch!; + } try { - await api.Branches.create(repoID, branchName, String(defaultBranch)); + await api.Branches.create(repoID, branchName, String(targetBranch)); } catch (e) { throw new InputError( `The branch creation failed. Please check that your repo does not already contain a branch named '${branchName}'. ${e}`, @@ -235,7 +250,7 @@ export const createPublishGitlabMergeRequestAction = (options: { const mergeRequestUrl = await api.MergeRequests.create( repoID, branchName, - String(defaultBranch), + String(targetBranch), title, { description, @@ -246,6 +261,7 @@ export const createPublishGitlabMergeRequestAction = (options: { return mergeRequest.web_url; }); ctx.output('projectid', repoID); + ctx.output('targetBranchName', targetBranch); ctx.output('projectPath', repoID); ctx.output('mergeRequestUrl', mergeRequestUrl); } catch (e) {