From 52ab24122a7644ad91103ddae7ae034a0caa38c7 Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Fri, 3 May 2024 20:27:36 -0300 Subject: [PATCH 1/8] chore(plugins/scaffolder-backend-module-githu): adding support to override default author commit for github PRs Signed-off-by: Matheus Castiglioni --- .changeset/nasty-papayas-heal.md | 5 ++ .../githubPullRequest.examples.test.ts | 86 ++++++++++++++++++- .../src/actions/githubPullRequest.examples.ts | 21 +++++ .../src/actions/githubPullRequest.test.ts | 86 +++++++++++++++++++ .../src/actions/githubPullRequest.ts | 21 +++++ 5 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 .changeset/nasty-papayas-heal.md diff --git a/.changeset/nasty-papayas-heal.md b/.changeset/nasty-papayas-heal.md new file mode 100644 index 0000000000..cfeacb3238 --- /dev/null +++ b/.changeset/nasty-papayas-heal.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend-module-github': minor +--- + +Adding support to change the default commit author for pull-request github action" diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts index dd07c88a70..e881ae3192 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts @@ -134,6 +134,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -174,6 +178,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -213,6 +221,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -252,6 +264,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -298,6 +314,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -338,6 +358,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -378,6 +402,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -424,6 +452,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -470,6 +502,54 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, + }, + ], + }); + + expect(fakeClient.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + expect(mockContext.output).toHaveBeenCalledTimes(3); + expect(mockContext.output).toHaveBeenCalledWith('targetBranchName', 'main'); + expect(mockContext.output).toHaveBeenCalledWith( + 'remoteUrl', + 'https://github.com/myorg/myrepo/pull/123', + ); + expect(mockContext.output).toHaveBeenCalledWith('pullRequestNumber', 123); + }); + + it('Create a pull request with a git author', async () => { + const input = yaml.parse(examples[9].example).steps[0].input; + + await action.handler({ + ...mockContext, + workspacePath, + input, + }); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + title: 'Create my new app', + body: 'This PR is really good', + head: 'new-app', + draft: undefined, + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'foo@bar.example', + name: 'Foo Bar', + }, }, ], }); @@ -491,7 +571,7 @@ describe('publish:github:pull-request examples', () => { irrelevant: { 'bar.txt': 'Nothing to see here' }, }, }); - const input = yaml.parse(examples[9].example).steps[0].input; + const input = yaml.parse(examples[10].example).steps[0].input; await action.handler({ ...mockContext, @@ -517,6 +597,10 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, + author: { + email: 'foo@bar.example', + name: 'Foo Bar', + }, }, ], }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts index d6138b262b..f3042a7e15 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts @@ -178,6 +178,25 @@ export const examples: TemplateExample[] = [ ], }), }, + { + description: 'Create a pull request with a git author', + example: yaml.stringify({ + steps: [ + { + action: 'publish:github:pull-request', + name: 'Create a pull reuqest', + input: { + repoUrl: 'github.com?repo=repo&owner=owner', + branchName: 'new-app', + title: 'Create my new app', + description: 'This PR is really good', + gitAuthorName: 'Foo Bar', + gitAuthorEmail: 'foo@bar.example', + }, + }, + ], + }), + }, { description: 'Create a pull request with all parameters', example: yaml.stringify({ @@ -198,6 +217,8 @@ export const examples: TemplateExample[] = [ reviewers: ['foobar'], teamReviewers: ['team-foo'], commitMessage: 'Commit for foo changes', + gitAuthorName: 'Foo Bar', + gitAuthorEmail: 'foo@bar.example', }, }, ], diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts index c9d1498187..a38b070bdf 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts @@ -155,6 +155,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -212,6 +216,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -271,6 +279,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -322,6 +334,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -449,6 +465,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '120000', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -498,6 +518,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100755', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -557,6 +581,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100755', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -612,6 +640,10 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], }); @@ -657,10 +689,64 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, + author: { + email: 'scaffolder@backstage.io', + name: 'Scaffolder', + }, }, ], forceFork: true, }); }); }); + + describe('with author', () => { + 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', + gitAuthorEmail: 'foo@bar.example', + gitAuthorName: 'Foo Bar', + }; + + mockDir.setContent({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = createMockActionContext({ 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', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'foo@bar.example', + name: 'Foo Bar', + }, + }, + ], + }); + }); + }); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts index b4aa03b602..0a256d36d5 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts @@ -136,6 +136,8 @@ export const createPublishGithubPullRequestAction = ( commitMessage?: string; update?: boolean; forceFork?: boolean; + gitAuthorName?: string; + gitAuthorEmail?: string; }>({ id: 'publish:github:pull-request', examples, @@ -223,6 +225,18 @@ export const createPublishGithubPullRequestAction = ( title: 'Force Fork', description: 'Create pull request from a fork', }, + gitAuthorName: { + type: 'string', + title: 'Default Author Name', + description: + "Sets the default author name for the commit. The default value is 'Scaffolder'", + }, + gitAuthorEmail: { + type: 'string', + title: 'Default Author Email', + description: + "Sets the default author email for the commit. The default value is 'scaffolder@backstage.io'", + }, }, }, output: { @@ -262,6 +276,8 @@ export const createPublishGithubPullRequestAction = ( commitMessage, update, forceFork, + gitAuthorEmail = 'scaffolder@backstage.io', + gitAuthorName = 'Scaffolder', } = ctx.input; const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); @@ -328,6 +344,10 @@ export const createPublishGithubPullRequestAction = ( { files, commit: commitMessage ?? title, + author: { + name: gitAuthorName, + email: gitAuthorEmail, + }, }, ], body: description, @@ -336,6 +356,7 @@ export const createPublishGithubPullRequestAction = ( update, forceFork, }; + if (targetBranchName) { createOptions.base = targetBranchName; } From 0be2c4d808cce132cf23cb4b0fb1ab23d9de75d6 Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Fri, 3 May 2024 20:46:41 -0300 Subject: [PATCH 2/8] fix(plugins/scaffolder-backend-module-githu): generating api report for changed plugin Signed-off-by: Matheus Castiglioni --- plugins/scaffolder-backend-module-github/api-report.md | 2 ++ plugins/scaffolder-backend/api-report.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index 0284c362eb..07f5146901 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -389,6 +389,8 @@ export const createPublishGithubPullRequestAction: ( commitMessage?: string | undefined; update?: boolean | undefined; forceFork?: boolean | undefined; + gitAuthorName?: string | undefined; + gitAuthorEmail?: string | undefined; }, JsonObject >; diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 6af1ac04a6..e02d6f09a9 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -285,6 +285,8 @@ export const createPublishGithubPullRequestAction: ( commitMessage?: string | undefined; update?: boolean | undefined; forceFork?: boolean | undefined; + gitAuthorName?: string | undefined; + gitAuthorEmail?: string | undefined; }, JsonObject >; From a35ef27eadeda1b47ded934c87709200254e30ce Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Sun, 5 May 2024 21:53:57 -0300 Subject: [PATCH 3/8] fix(plugins/scaffolder-backend-module-github): only overriding default comit author when provide it Signed-off-by: Matheus Castiglioni --- .../api-report.md | 2 + .../githubPullRequest.examples.test.ts | 129 +++++++---- .../src/actions/githubPullRequest.examples.ts | 42 +++- .../src/actions/githubPullRequest.test.ts | 215 ++++++++++++++---- .../src/actions/githubPullRequest.ts | 49 +++- .../src/module.ts | 1 + .../actions/builtin/createBuiltinActions.ts | 1 + 7 files changed, 351 insertions(+), 88 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index 07f5146901..cac2a3fee2 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -128,6 +128,8 @@ export interface CreateGithubPullRequestActionOptions { } | null>; } >; + // (undocumented) + config: Config; githubCredentialsProvider?: GithubCredentialsProvider; integrations: ScmIntegrationRegistry; } diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts index e881ae3192..26469dfcb4 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.test.ts @@ -103,6 +103,7 @@ describe('publish:github:pull-request examples', () => { integrations, githubCredentialsProvider, clientFactory, + config, }); }); @@ -134,10 +135,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -178,10 +175,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -221,10 +214,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -264,10 +253,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -314,10 +299,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -358,10 +339,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -402,10 +379,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -452,10 +425,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -502,10 +471,6 @@ describe('publish:github:pull-request examples', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -520,7 +485,7 @@ describe('publish:github:pull-request examples', () => { expect(mockContext.output).toHaveBeenCalledWith('pullRequestNumber', 123); }); - it('Create a pull request with a git author', async () => { + it('Create a pull request with a git author name and email', async () => { const input = yaml.parse(examples[9].example).steps[0].input; await action.handler({ @@ -564,6 +529,94 @@ describe('publish:github:pull-request examples', () => { expect(mockContext.output).toHaveBeenCalledWith('pullRequestNumber', 123); }); + it('Create a pull request with a git author name', async () => { + const input = yaml.parse(examples[10].example).steps[0].input; + + await action.handler({ + ...mockContext, + workspacePath, + input, + }); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + title: 'Create my new app', + body: 'This PR is really good', + head: 'new-app', + draft: undefined, + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'scaffolder@backstage.io', + name: 'Foo Bar', + }, + }, + ], + }); + + expect(fakeClient.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + expect(mockContext.output).toHaveBeenCalledTimes(3); + expect(mockContext.output).toHaveBeenCalledWith('targetBranchName', 'main'); + expect(mockContext.output).toHaveBeenCalledWith( + 'remoteUrl', + 'https://github.com/myorg/myrepo/pull/123', + ); + expect(mockContext.output).toHaveBeenCalledWith('pullRequestNumber', 123); + }); + + it('Create a pull request with a git author email', async () => { + const input = yaml.parse(examples[11].example).steps[0].input; + + await action.handler({ + ...mockContext, + workspacePath, + input, + }); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repo', + title: 'Create my new app', + body: 'This PR is really good', + head: 'new-app', + draft: undefined, + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'foo@bar.example', + name: 'Scaffolder', + }, + }, + ], + }); + + expect(fakeClient.rest.pulls.requestReviewers).not.toHaveBeenCalled(); + expect(mockContext.output).toHaveBeenCalledTimes(3); + expect(mockContext.output).toHaveBeenCalledWith('targetBranchName', 'main'); + expect(mockContext.output).toHaveBeenCalledWith( + 'remoteUrl', + 'https://github.com/myorg/myrepo/pull/123', + ); + expect(mockContext.output).toHaveBeenCalledWith('pullRequestNumber', 123); + }); + it('Create a pull request with all parameters', async () => { mockDir.setContent({ [workspacePath]: { @@ -571,7 +624,7 @@ describe('publish:github:pull-request examples', () => { irrelevant: { 'bar.txt': 'Nothing to see here' }, }, }); - const input = yaml.parse(examples[10].example).steps[0].input; + const input = yaml.parse(examples[12].example).steps[0].input; await action.handler({ ...mockContext, diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts index f3042a7e15..b6d31e732b 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.examples.ts @@ -179,7 +179,7 @@ export const examples: TemplateExample[] = [ }), }, { - description: 'Create a pull request with a git author', + description: 'Create a pull request with a git author name and email', example: yaml.stringify({ steps: [ { @@ -197,6 +197,46 @@ export const examples: TemplateExample[] = [ ], }), }, + { + description: 'Create a pull request with a git author name', + example: yaml.stringify({ + steps: [ + { + action: 'publish:github:pull-request', + name: 'Create a pull reuqest', + input: { + repoUrl: 'github.com?repo=repo&owner=owner', + branchName: 'new-app', + title: 'Create my new app', + description: 'This PR is really good', + // gitAuthorEmail will be 'scaffolder@backstage.io' + // once one author attribute has been set we need to set both + gitAuthorName: 'Foo Bar', + }, + }, + ], + }), + }, + { + description: 'Create a pull request with a git author email', + example: yaml.stringify({ + steps: [ + { + action: 'publish:github:pull-request', + name: 'Create a pull reuqest', + input: { + repoUrl: 'github.com?repo=repo&owner=owner', + branchName: 'new-app', + title: 'Create my new app', + description: 'This PR is really good', + // gitAuthorName will be 'Scaffolder' + // once one author attribute has been set we need to set both + gitAuthorEmail: 'foo@bar.example', + }, + }, + ], + }), + }, { description: 'Create a pull request with all parameters', example: yaml.stringify({ diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts index a38b070bdf..6f730fabcf 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts @@ -15,7 +15,7 @@ */ import { createRootLogger } from '@backstage/backend-common'; -import { ConfigReader } from '@backstage/config'; +import { Config, ConfigReader } from '@backstage/config'; import { GithubCredentialsProvider, ScmIntegrations, @@ -46,6 +46,8 @@ describe('createPublishGithubPullRequestAction', () => { pulls: { requestReviewers: jest.Mock }; }; }; + let config: Config; + let integrations: ScmIntegrations; const mockDir = createMockDirectory(); const workspacePath = mockDir.resolve('workspace'); @@ -53,7 +55,8 @@ describe('createPublishGithubPullRequestAction', () => { beforeEach(() => { mockDir.clear(); - const integrations = ScmIntegrations.fromConfig(new ConfigReader({})); + config = new ConfigReader({}); + integrations = ScmIntegrations.fromConfig(config); fakeClient = { createPullRequest: jest.fn(async (_: any) => { return { @@ -84,6 +87,7 @@ describe('createPublishGithubPullRequestAction', () => { integrations, githubCredentialsProvider, clientFactory, + config, }); }); @@ -155,10 +159,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -216,10 +216,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -279,10 +275,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -334,10 +326,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -465,10 +453,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '120000', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -518,10 +502,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100755', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -581,10 +561,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100755', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -640,10 +616,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], }); @@ -689,10 +661,6 @@ describe('createPublishGithubPullRequestAction', () => { mode: '100644', }, }, - author: { - email: 'scaffolder@backstage.io', - name: 'Scaffolder', - }, }, ], forceFork: true, @@ -700,7 +668,7 @@ describe('createPublishGithubPullRequestAction', () => { }); }); - describe('with author', () => { + describe('with author name and email', () => { let input: GithubPullRequestActionInput; let ctx: ActionContext; @@ -721,7 +689,7 @@ describe('createPublishGithubPullRequestAction', () => { ctx = createMockActionContext({ input, workspacePath }); }); - it('creates a pull request', async () => { + it('creates a pull request with author name', async () => { await instance.handler(ctx); expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ @@ -749,4 +717,171 @@ describe('createPublishGithubPullRequestAction', () => { }); }); }); + + describe('with author name', () => { + 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', + gitAuthorName: 'Foo Bar', + }; + + mockDir.setContent({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = createMockActionContext({ input, workspacePath }); + }); + + it('creates a pull request with author name', async () => { + await instance.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'scaffolder@backstage.io', + name: 'Foo Bar', + }, + }, + ], + }); + }); + }); + + describe('with author email', () => { + 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', + gitAuthorEmail: 'foo@bar.example', + }; + + mockDir.setContent({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = createMockActionContext({ input, workspacePath }); + }); + + it('creates a pull request with author name', async () => { + await instance.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'foo@bar.example', + name: 'Scaffolder', + }, + }, + ], + }); + }); + }); + + describe('with author from config file', () => { + 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', + }; + + mockDir.setContent({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = createMockActionContext({ input, workspacePath }); + }); + + it('creates a pull request with author name', async () => { + config = new ConfigReader({ + scaffolder: { + defaultAuthor: { + name: 'Config', + email: 'config@file.example', + }, + }, + }); + + const clientFactory = jest.fn(async () => fakeClient as any); + const githubCredentialsProvider: GithubCredentialsProvider = { + getCredentials: jest.fn(), + }; + + const instanceWithConfig = createPublishGithubPullRequestAction({ + integrations, + githubCredentialsProvider, + clientFactory, + config, + }); + + await instanceWithConfig.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'config@file.example', + name: 'Config', + }, + }, + ], + }); + }); + }); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts index 0a256d36d5..3c40361af3 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts @@ -32,6 +32,7 @@ import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { getOctokitOptions } from './helpers'; import { examples } from './githubPullRequest.examples'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { Config } from '@backstage/config'; export type Encoding = 'utf-8' | 'base64'; @@ -100,6 +101,7 @@ export interface CreateGithubPullRequestActionOptions { } | null>; } >; + config: Config; } type GithubPullRequest = { @@ -119,6 +121,7 @@ export const createPublishGithubPullRequestAction = ( integrations, githubCredentialsProvider, clientFactory = defaultClientFactory, + config, } = options; return createTemplateAction<{ @@ -229,13 +232,13 @@ export const createPublishGithubPullRequestAction = ( type: 'string', title: 'Default Author Name', description: - "Sets the default author name for the commit. The default value is 'Scaffolder'", + "Sets the default author name for the commit. The default value is the authenticated user or 'Scaffolder'", }, gitAuthorEmail: { type: 'string', title: 'Default Author Email', description: - "Sets the default author email for the commit. The default value is 'scaffolder@backstage.io'", + "Sets the default author email for the commit. The default value is the authenticated user or 'scaffolder@backstage.io'", }, }, }, @@ -276,8 +279,8 @@ export const createPublishGithubPullRequestAction = ( commitMessage, update, forceFork, - gitAuthorEmail = 'scaffolder@backstage.io', - gitAuthorName = 'Scaffolder', + gitAuthorEmail, + gitAuthorName, } = ctx.input; const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); @@ -343,11 +346,10 @@ export const createPublishGithubPullRequestAction = ( changes: [ { files, - commit: commitMessage ?? title, - author: { - name: gitAuthorName, - email: gitAuthorEmail, - }, + commit: + commitMessage ?? + config.getOptionalString('scaffolder.defaultCommitMessage') ?? + title, }, ], body: description, @@ -357,6 +359,35 @@ export const createPublishGithubPullRequestAction = ( forceFork, }; + const gitAuthorInfo = { + name: + gitAuthorName ?? + config.getOptionalString('scaffolder.defaultAuthor.name'), + email: + gitAuthorEmail ?? + config.getOptionalString('scaffolder.defaultAuthor.email'), + }; + + if (gitAuthorInfo.name || gitAuthorInfo.email) { + if (Array.isArray(createOptions.changes)) { + createOptions.changes = createOptions.changes.map(change => ({ + ...change, + author: { + name: gitAuthorInfo.name || 'Scaffolder', + email: gitAuthorInfo.email || 'scaffolder@backstage.io', + }, + })); + } else { + createOptions.changes = { + ...createOptions.changes, + author: { + name: gitAuthorInfo.name || 'Scaffolder', + email: gitAuthorInfo.email || 'scaffolder@backstage.io', + }, + }; + } + } + if (targetBranchName) { createOptions.base = targetBranchName; } diff --git a/plugins/scaffolder-backend-module-github/src/module.ts b/plugins/scaffolder-backend-module-github/src/module.ts index 79c954a6dd..1817a68bfb 100644 --- a/plugins/scaffolder-backend-module-github/src/module.ts +++ b/plugins/scaffolder-backend-module-github/src/module.ts @@ -89,6 +89,7 @@ export const githubModule = createBackendModule({ createPublishGithubPullRequestAction({ integrations, githubCredentialsProvider, + config, }), ); }, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index a017c2e224..673776bec9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -178,6 +178,7 @@ export const createBuiltinActions = ( createPublishGithubPullRequestAction({ integrations, githubCredentialsProvider, + config, }), createPublishGitlabAction({ integrations, From b000ee6fa2f28cbcf0edcf9ae15703b352fc59ac Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Sun, 5 May 2024 22:12:18 -0300 Subject: [PATCH 4/8] fix(plugins/scaffolder-backend-module-github): api report config undocumented Signed-off-by: Matheus Castiglioni --- plugins/scaffolder-backend-module-github/api-report.md | 1 - .../src/actions/githubPullRequest.ts | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index cac2a3fee2..4f3cd819ad 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -128,7 +128,6 @@ export interface CreateGithubPullRequestActionOptions { } | null>; } >; - // (undocumented) config: Config; githubCredentialsProvider?: GithubCredentialsProvider; integrations: ScmIntegrationRegistry; diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts index 3c40361af3..cc5e1280ef 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts @@ -101,6 +101,9 @@ export interface CreateGithubPullRequestActionOptions { } | null>; } >; + /** + * An instance of {@link @backstage/config#Config} that will be used in the action. + */ config: Config; } From e5486988c06a6757eb918ce972f4767c22cfa487 Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Sun, 5 May 2024 22:39:44 -0300 Subject: [PATCH 5/8] test(plugins/scaffolder-backend-module-github): adding missing author data priority test Signed-off-by: Matheus Castiglioni --- .../src/actions/githubPullRequest.test.ts | 79 ++++++++++++++++++- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts index 6f730fabcf..dd7641ce4c 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts @@ -689,7 +689,7 @@ describe('createPublishGithubPullRequestAction', () => { ctx = createMockActionContext({ input, workspacePath }); }); - it('creates a pull request with author name', async () => { + it('creates a pull request', async () => { await instance.handler(ctx); expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ @@ -738,7 +738,7 @@ describe('createPublishGithubPullRequestAction', () => { ctx = createMockActionContext({ input, workspacePath }); }); - it('creates a pull request with author name', async () => { + it('creates a pull request', async () => { await instance.handler(ctx); expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ @@ -787,7 +787,7 @@ describe('createPublishGithubPullRequestAction', () => { ctx = createMockActionContext({ input, workspacePath }); }); - it('creates a pull request with author name', async () => { + it('creates a pull request', async () => { await instance.handler(ctx); expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ @@ -835,7 +835,7 @@ describe('createPublishGithubPullRequestAction', () => { ctx = createMockActionContext({ input, workspacePath }); }); - it('creates a pull request with author name', async () => { + it('creates a pull request with default config attributes', async () => { config = new ConfigReader({ scaffolder: { defaultAuthor: { @@ -884,4 +884,75 @@ describe('createPublishGithubPullRequestAction', () => { }); }); }); + + describe('with author attributes and config file', () => { + 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', + gitAuthorEmail: 'foo@bar.example', + gitAuthorName: 'Foo Bar', + }; + + mockDir.setContent({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = createMockActionContext({ input, workspacePath }); + }); + + it('creates a pull request with using author name and email from input', async () => { + config = new ConfigReader({ + scaffolder: { + defaultAuthor: { + name: 'Config', + email: 'config@file.example', + }, + }, + }); + + const clientFactory = jest.fn(async () => fakeClient as any); + const githubCredentialsProvider: GithubCredentialsProvider = { + getCredentials: jest.fn(), + }; + + const instanceWithConfig = createPublishGithubPullRequestAction({ + integrations, + githubCredentialsProvider, + clientFactory, + config, + }); + + await instanceWithConfig.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'foo@bar.example', + name: 'Foo Bar', + }, + }, + ], + }); + }); + }); }); From b7cc1e8cc8547e00c2946fa03b8a9f1ffcc5c690 Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Tue, 7 May 2024 08:05:43 -0300 Subject: [PATCH 6/8] Update .changeset/nasty-papayas-heal.md Co-authored-by: Ben Lambert Signed-off-by: Matheus Castiglioni --- .changeset/nasty-papayas-heal.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/nasty-papayas-heal.md b/.changeset/nasty-papayas-heal.md index cfeacb3238..371eba66a7 100644 --- a/.changeset/nasty-papayas-heal.md +++ b/.changeset/nasty-papayas-heal.md @@ -2,4 +2,4 @@ '@backstage/plugin-scaffolder-backend-module-github': minor --- -Adding support to change the default commit author for pull-request github action" +Adding support to change the default commit author for `publish:github:pull-request` From 13bd8ef5ba96ac194e4481a755288f41b08bbb86 Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Tue, 7 May 2024 08:05:56 -0300 Subject: [PATCH 7/8] Update .changeset/nasty-papayas-heal.md Co-authored-by: Ben Lambert Signed-off-by: Matheus Castiglioni --- .changeset/nasty-papayas-heal.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/nasty-papayas-heal.md b/.changeset/nasty-papayas-heal.md index 371eba66a7..0e8e3f7ccc 100644 --- a/.changeset/nasty-papayas-heal.md +++ b/.changeset/nasty-papayas-heal.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder-backend-module-github': minor +'@backstage/plugin-scaffolder-backend-module-github': patch --- Adding support to change the default commit author for `publish:github:pull-request` From 18cd46471f2936027cd2492523892f27bbe4bf3b Mon Sep 17 00:00:00 2001 From: Matheus Castiglioni Date: Tue, 7 May 2024 15:01:16 -0300 Subject: [PATCH 8/8] chore(plugins/scaffolder-backend-module-github): making config optional Signed-off-by: Matheus Castiglioni --- .../api-report.md | 2 +- .../src/actions/githubPullRequest.test.ts | 60 +++++++++++++++++++ .../src/actions/githubPullRequest.ts | 8 +-- 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index 4f3cd819ad..b1a1c80af5 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -128,7 +128,7 @@ export interface CreateGithubPullRequestActionOptions { } | null>; } >; - config: Config; + config?: Config; githubCredentialsProvider?: GithubCredentialsProvider; integrations: ScmIntegrationRegistry; } diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts index dd7641ce4c..20508494b8 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.test.ts @@ -955,4 +955,64 @@ describe('createPublishGithubPullRequestAction', () => { }); }); }); + + describe('with author fallback and no config', () => { + 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', + gitAuthorName: 'Foo Bar', + }; + + mockDir.setContent({ + [workspacePath]: { 'file.txt': 'Hello there!' }, + }); + + ctx = createMockActionContext({ input, workspacePath }); + }); + + it('creates a pull request with using author name and email fallback when have no config', async () => { + const clientFactory = jest.fn(async () => fakeClient as any); + const githubCredentialsProvider: GithubCredentialsProvider = { + getCredentials: jest.fn(), + }; + + const instanceWithConfig = createPublishGithubPullRequestAction({ + integrations, + githubCredentialsProvider, + clientFactory, + }); + + await instanceWithConfig.handler(ctx); + + expect(fakeClient.createPullRequest).toHaveBeenCalledWith({ + owner: 'myorg', + repo: 'myrepo', + title: 'Create my new app', + head: 'new-app', + body: 'This PR is really good', + changes: [ + { + commit: 'Create my new app', + files: { + 'file.txt': { + content: Buffer.from('Hello there!').toString('base64'), + encoding: 'base64', + mode: '100644', + }, + }, + author: { + email: 'scaffolder@backstage.io', + name: 'Foo Bar', + }, + }, + ], + }); + }); + }); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts index cc5e1280ef..a72ec8345b 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubPullRequest.ts @@ -104,7 +104,7 @@ export interface CreateGithubPullRequestActionOptions { /** * An instance of {@link @backstage/config#Config} that will be used in the action. */ - config: Config; + config?: Config; } type GithubPullRequest = { @@ -351,7 +351,7 @@ export const createPublishGithubPullRequestAction = ( files, commit: commitMessage ?? - config.getOptionalString('scaffolder.defaultCommitMessage') ?? + config?.getOptionalString('scaffolder.defaultCommitMessage') ?? title, }, ], @@ -365,10 +365,10 @@ export const createPublishGithubPullRequestAction = ( const gitAuthorInfo = { name: gitAuthorName ?? - config.getOptionalString('scaffolder.defaultAuthor.name'), + config?.getOptionalString('scaffolder.defaultAuthor.name'), email: gitAuthorEmail ?? - config.getOptionalString('scaffolder.defaultAuthor.email'), + config?.getOptionalString('scaffolder.defaultAuthor.email'), }; if (gitAuthorInfo.name || gitAuthorInfo.email) {