diff --git a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts index b5db1b4811..e95856658e 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts @@ -67,13 +67,29 @@ const mockGitlabClient = { }, Users: { all: jest.fn(async (userOptions: { username: string }) => { - if (userOptions.username !== 'John Smith') - throw new Error('user does not exist'); - return [ - { - id: 123, - }, - ]; + switch (userOptions.username) { + case 'John Smith': + return [ + { + id: 123, + }, + ]; + case 'Jane Doe': + return [ + { + id: 456, + }, + ]; + default: + throw new Error('user does not exist'); + } + // if (userOptions.username !== 'John Smith') + // throw new Error('user does not exist'); + // return [ + // { + // id: 123, + // }, + // ]; }), }, Repositories: { @@ -517,6 +533,169 @@ describe('createGitLabMergeRequest', () => { }); }); + describe('createGitlabMergeRequestWithReviewers', () => { + it('no reviewers are set when a no reviewer are 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, + targetPath: 'Subdirectory', + assignee: 'John Smith', + }; + mockDir.setContent({ + [workspacePath]: { + source: { 'foo.txt': 'Hello there!' }, + irrelevant: { 'bar.txt': 'Nothing to see here' }, + }, + }); + + const ctx = createMockActionContext({ input, workspacePath }); + await instance.handler(ctx); + + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + ); + expect(mockGitlabClient.Commits.create).not.toHaveBeenCalled(); + 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('reviewer is set correcly when a valid reviewer 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, + targetPath: 'Subdirectory', + assignee: 'John Smith', + reviewers: ['Jane Doe'], + }; + mockDir.setContent({ + [workspacePath]: { + source: { 'foo.txt': 'Hello there!' }, + irrelevant: { 'bar.txt': 'Nothing to see here' }, + }, + }); + + const ctx = createMockActionContext({ input, workspacePath }); + await instance.handler(ctx); + + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + ); + expect(mockGitlabClient.Commits.create).not.toHaveBeenCalled(); + expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + assigneeId: 123, + reviewerIds: [456], + }, + ); + }); + + it('reviewers are set correcly when valid reviewers username are 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, + targetPath: 'Subdirectory', + assignee: 'John Smith', + reviewers: ['Jane Doe', 'John Smith'], + }; + mockDir.setContent({ + [workspacePath]: { + source: { 'foo.txt': 'Hello there!' }, + irrelevant: { 'bar.txt': 'Nothing to see here' }, + }, + }); + + const ctx = createMockActionContext({ input, workspacePath }); + await instance.handler(ctx); + + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + ); + expect(mockGitlabClient.Commits.create).not.toHaveBeenCalled(); + expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + assigneeId: 123, + reviewerIds: [456, 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, + targetPath: 'Subdirectory', + reviewers: ['John Doe'], + }; + mockDir.setContent({ + [workspacePath]: { + source: { 'foo.txt': 'Hello there!' }, + irrelevant: { 'bar.txt': 'Nothing to see here' }, + }, + }); + + const ctx = createMockActionContext({ input, workspacePath }); + await instance.handler(ctx); + + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + ); + expect(mockGitlabClient.Commits.create).not.toHaveBeenCalled(); + expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + assigneeId: undefined, + reviewerIds: [], + }, + ); + }); + }); + describe('createGitLabMergeRequestWithoutCommitAction', () => { it('default commitAction is auto', async () => { const input = { diff --git a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts index 502b9988aa..e72f4831a9 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts @@ -252,21 +252,25 @@ which uses additional API calls in order to detect whether to 'create', 'update' } } - let reviewerIds: number[] | undefined; + let reviewerIds: number[] | undefined = undefined; // Explicitly set to undefined. Strangely, passing an empty array to the API will result the other options being undefined also being explicity passed to the Gitlab API call (e.g. assigneeId) if (reviewers !== undefined) { - reviewerIds = await Promise.all( - reviewers.map(async reviewer => { - try { - const reviewerUser = await api.Users.username(reviewer); - return reviewerUser[0].id; - } catch (e) { - ctx.logger.warn( - `Failed to find gitlab user id for ${reviewer}: ${e}. Proceeding with MR creation without reviewer.`, - ); - return undefined; - } - }), - ); + reviewerIds = ( + await Promise.all( + reviewers.map(async reviewer => { + try { + const reviewerUser = await api.Users.all({ + username: reviewer, + }); + return reviewerUser[0].id; + } catch (e) { + ctx.logger.warn( + `Failed to find gitlab user id for ${reviewer}: ${e}. Proceeding with MR creation without reviewer.`, + ); + return undefined; + } + }), + ) + ).filter(id => id !== undefined) as number[]; } let fileRoot: string;