fix(plugins/scaffolder-backend-module-github): only overriding default comit author when provide it

Signed-off-by: Matheus Castiglioni <mahenrique94@users.noreply.github.com>
This commit is contained in:
Matheus Castiglioni
2024-05-05 21:53:57 -03:00
parent 0be2c4d808
commit a35ef27ead
7 changed files with 351 additions and 88 deletions
@@ -128,6 +128,8 @@ export interface CreateGithubPullRequestActionOptions {
} | null>;
}
>;
// (undocumented)
config: Config;
githubCredentialsProvider?: GithubCredentialsProvider;
integrations: ScmIntegrationRegistry;
}
@@ -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,
@@ -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({
@@ -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<GithubPullRequestActionInput>;
@@ -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<GithubPullRequestActionInput>;
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<GithubPullRequestActionInput>;
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<GithubPullRequestActionInput>;
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',
},
},
],
});
});
});
});
@@ -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;
}
@@ -89,6 +89,7 @@ export const githubModule = createBackendModule({
createPublishGithubPullRequestAction({
integrations,
githubCredentialsProvider,
config,
}),
);
},
@@ -178,6 +178,7 @@ export const createBuiltinActions = (
createPublishGithubPullRequestAction({
integrations,
githubCredentialsProvider,
config,
}),
createPublishGitlabAction({
integrations,