From 41cec438353a25fdf6ae6e2dcf54a33e252b6c06 Mon Sep 17 00:00:00 2001 From: Lilly Holden Date: Fri, 15 Jul 2022 15:41:10 +0100 Subject: [PATCH 1/6] Scaffolder Gitlab MR - added optional assignee Signed-off-by: Lilly Holden --- plugins/scaffolder-backend/api-report.md | 1 + .../publish/gitlabMergeRequest.test.ts | 133 ++++++++++++++++++ .../builtin/publish/gitlabMergeRequest.ts | 22 +++ 3 files changed, 156 insertions(+) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index bcdcfd7546..69777631b7 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -406,6 +406,7 @@ export const createPublishGitlabMergeRequestAction: (options: { token?: string | undefined; projectid?: string | undefined; removeSourceBranch?: boolean | undefined; + assignee?: string | undefined; }>; // @public 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 987e360617..486e5b4ac1 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 @@ -53,6 +53,15 @@ const mockGitlabClient = { }, Users: { current: jest.fn(), + username: jest.fn(async (user: string) => { + if (user !== 'John Smith') throw new Error('user does not exist'); + else + return [ + { + id: 123, + }, + ]; + }), }, }; @@ -204,4 +213,128 @@ describe('createGitLabMergeRequest', () => { ); }); }); + + describe('createGitLabMergeRequestWithAssignee', () => { + it('assignee is set correcly when a valid assignee username is passed in options', async () => { + const input = { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + title: 'Create my new MR', + branchName: 'new-mr', + description: 'This is an important change', + removeSourceBranch: false, + draft: true, + targetPath: 'Subdirectory', + assignee: 'John Smith', + }; + 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.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + assigneeId: 123, + }, + ); + }); + + it('assignee is not set when a valid assignee username is not passed in options', async () => { + const input = { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + title: 'Create my new MR', + branchName: 'new-mr', + description: 'This is an important change', + removeSourceBranch: false, + draft: true, + targetPath: 'Subdirectory', + assingnee: 'John Doe', + }; + 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.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + assigneeId: undefined, + }, + ); + }); + }); + describe('createGitLabMergeRequestWithoutAssignee', () => { + it('merge request is successfully created without an assignee when assignee username is not passed in options', async () => { + const input = { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + title: 'Create my new MR', + branchName: 'new-mr', + description: 'This is an important change', + removeSourceBranch: false, + draft: true, + 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.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + }, + ); + }); + }); }); 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 b1d5b35fa3..4dfeb9c2b4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts @@ -43,6 +43,7 @@ export const createPublishGitlabMergeRequestAction = (options: { /** @deprecated Use projectPath instead */ projectid?: string; removeSourceBranch?: boolean; + assignee?: string; }>({ id: 'publish:gitlab:merge-request', schema: { @@ -92,6 +93,11 @@ export const createPublishGitlabMergeRequestAction = (options: { description: 'Option to delete source branch once the MR has been merged. Default: false', }, + assignee: { + title: 'Merge Request Assignee', + type: 'string', + description: 'User this merge request will be assigned to', + }, }, }, output: { @@ -146,6 +152,21 @@ export const createPublishGitlabMergeRequestAction = (options: { [tokenType]: token, }); + const assignee = ctx.input.assignee; + + let assigneeId = undefined; + + if (assignee !== undefined) { + try { + const assigneeUser = await api.Users.username(assignee); + assigneeId = assigneeUser[0].id; + } catch (e) { + throw new InputError( + `Failed to find gitlab user id for ${assignee}: ${e}`, + ); + } + } + const targetPath = resolveSafeChildPath( ctx.workspacePath, ctx.input.targetPath, @@ -199,6 +220,7 @@ export const createPublishGitlabMergeRequestAction = (options: { removeSourceBranch: ctx.input.removeSourceBranch ? ctx.input.removeSourceBranch : false, + assigneeId: assigneeId, }, ).then((mergeRequest: { web_url: string }) => { return mergeRequest.web_url; From e1a08d872c86ede7beb5311479da2e8cc5ebb6ab Mon Sep 17 00:00:00 2001 From: Lilly Holden Date: Fri, 15 Jul 2022 16:00:01 +0100 Subject: [PATCH 2/6] Scaffolder Gitlab MR - added changeset Signed-off-by: Lilly Holden --- .changeset/itchy-needles-think.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/itchy-needles-think.md diff --git a/.changeset/itchy-needles-think.md b/.changeset/itchy-needles-think.md new file mode 100644 index 0000000000..9dbcc64d84 --- /dev/null +++ b/.changeset/itchy-needles-think.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Added optional assignee parameter for Gitlab Merge Request action From 67f5e019da5941f22f9aeb4765da8eb2f4d1982f Mon Sep 17 00:00:00 2001 From: Lilly Holden Date: Mon, 18 Jul 2022 13:42:25 +0100 Subject: [PATCH 3/6] Looking Gitlab user search failure and proceeding with MR creation Signed-off-by: Lilly Holden --- .../publish/gitlabMergeRequest.test.ts | 40 +++++++++++++++++++ .../builtin/publish/gitlabMergeRequest.ts | 4 +- 2 files changed, 42 insertions(+), 2 deletions(-) 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 486e5b4ac1..69adbcb968 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 @@ -336,5 +336,45 @@ describe('createGitLabMergeRequest', () => { }, ); }); + + it('merge request is successfully created without an assignee when assignee is not found in Gitlab', async () => { + const input = { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + title: 'Create my new MR', + branchName: 'new-mr', + description: 'This is an important change', + removeSourceBranch: false, + draft: true, + targetPath: 'Subdirectory', + assignee: 'Unknown', + }; + 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.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + }, + ); + }); }); }); 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 4dfeb9c2b4..dc066a31bf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts @@ -161,8 +161,8 @@ export const createPublishGitlabMergeRequestAction = (options: { const assigneeUser = await api.Users.username(assignee); assigneeId = assigneeUser[0].id; } catch (e) { - throw new InputError( - `Failed to find gitlab user id for ${assignee}: ${e}`, + console.warn( + `Failed to find gitlab user id for ${assignee}: ${e}. Proceeding with MR creation without an assignee.`, ); } } From b658e8f8c62cc654b666d4879395a2969b194f92 Mon Sep 17 00:00:00 2001 From: Lilly Holden Date: Mon, 18 Jul 2022 13:45:10 +0100 Subject: [PATCH 4/6] added changeset Signed-off-by: Lilly Holden --- .changeset/selfish-moles-compete.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/selfish-moles-compete.md diff --git a/.changeset/selfish-moles-compete.md b/.changeset/selfish-moles-compete.md new file mode 100644 index 0000000000..80c94c36a4 --- /dev/null +++ b/.changeset/selfish-moles-compete.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Gitlab MR assignee - log Gitlab user search failures and proceed with MR creation From 75a5de9d738b9f5c8caed014a64f1f91b80783e9 Mon Sep 17 00:00:00 2001 From: Lilly Holden Date: Mon, 18 Jul 2022 15:51:37 +0100 Subject: [PATCH 5/6] using context logger Signed-off-by: Lilly Holden --- .../scaffolder/actions/builtin/publish/gitlabMergeRequest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dc066a31bf..8404f262cc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlabMergeRequest.ts @@ -161,7 +161,7 @@ export const createPublishGitlabMergeRequestAction = (options: { const assigneeUser = await api.Users.username(assignee); assigneeId = assigneeUser[0].id; } catch (e) { - console.warn( + ctx.logger.warn( `Failed to find gitlab user id for ${assignee}: ${e}. Proceeding with MR creation without an assignee.`, ); } From cc946aeefc682234df6e588a6b2bfba588622632 Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Mon, 18 Jul 2022 17:48:31 +0200 Subject: [PATCH 6/6] Remove additional changeset Signed-off-by: Ben Lambert --- .changeset/selfish-moles-compete.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/selfish-moles-compete.md diff --git a/.changeset/selfish-moles-compete.md b/.changeset/selfish-moles-compete.md deleted file mode 100644 index 80c94c36a4..0000000000 --- a/.changeset/selfish-moles-compete.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-scaffolder-backend': patch ---- - -Gitlab MR assignee - log Gitlab user search failures and proceed with MR creation