diff --git a/.changeset/soft-readers-move.md b/.changeset/soft-readers-move.md new file mode 100644 index 0000000000..6440f0e49f --- /dev/null +++ b/.changeset/soft-readers-move.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend-module-gitlab': patch +--- + +Making publish:gitlab:merge-request idempotent. diff --git a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.examples.test.ts b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.examples.test.ts index e6aea81285..2f14412b84 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.examples.test.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.examples.test.ts @@ -34,7 +34,7 @@ const mockGitlabClient = { create: jest.fn(), }, Commits: { - create: jest.fn(), + create: jest.fn(() => ({ id: 'mockId' })), }, MergeRequests: { create: jest.fn(async (_: any) => { 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 08194b52cc..662a84b077 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts @@ -48,7 +48,7 @@ const mockGitlabClient = { }), }, Commits: { - create: jest.fn(), + create: jest.fn(() => ({ id: 'mockId' })), }, MergeRequests: { create: jest.fn(async (repoId: string) => { @@ -287,7 +287,10 @@ describe('createGitLabMergeRequest', () => { irrelevant: { 'bar.txt': 'Nothing to see here' }, }, }); - const ctx = createMockActionContext({ input, workspacePath }); + const ctx = createMockActionContext({ + input, + workspacePath, + }); await instance.handler(ctx); expect(mockGitlabClient.Projects.show).not.toHaveBeenCalled(); diff --git a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts index 26f7eff4ce..89ae1bddf2 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts @@ -21,12 +21,12 @@ import { serializeDirectoryContents, } from '@backstage/plugin-scaffolder-node'; import { + Camelize, + CommitAction, + ExpandedMergeRequestSchema, Gitlab, RepositoryTreeSchema, - CommitAction, SimpleUserSchema, - ExpandedMergeRequestSchema, - Camelize, } from '@gitbeaker/rest'; import path from 'path'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -302,7 +302,7 @@ which uses additional API calls in order to detect whether to 'create', 'update' repoUrl, }); - let assigneeId = undefined; + let assigneeId: number | undefined = undefined; if (assignee !== undefined) { try { @@ -439,74 +439,100 @@ which uses additional API calls in order to detect whether to 'create', 'update' ); } } - if (actions.length) { - try { - await api.Commits.create(repoID, branchName, title, actions); - } catch (e) { - throw new InputError( - `Committing the changes to ${branchName} failed. Please check that none of the files created by the template already exists. ${getErrorMessage( - e, - )}`, - ); - } - } - try { - let mergeRequest = await api.MergeRequests.create( - repoID, - branchName, - String(targetBranch), - title, - { - description, - removeSourceBranch: removeSourceBranch ? removeSourceBranch : false, - assigneeId, - reviewerIds, - }, - ); - - if (ctx.input.assignReviewersFromApprovalRules) { - try { - const reviewersFromApprovalRules = - await getReviewersFromApprovalRules( - api, - mergeRequest.iid, + await ctx.checkpoint({ + key: `commit.to.${repoID}.${branchName}`, + fn: async () => { + if (actions.length) { + try { + const commit = await api.Commits.create( repoID, - ctx, + branchName, + title, + actions, ); - if (reviewersFromApprovalRules.length > 0) { - const eligibleUserIds = new Set([ - ...reviewersFromApprovalRules, - ...(reviewerIds ?? []), - ]); - - mergeRequest = await api.MergeRequests.edit( - repoID, - mergeRequest.iid, - { - reviewerIds: Array.from(eligibleUserIds), - }, + return commit.id; + } catch (e) { + throw new InputError( + `Committing the changes to ${branchName} failed. Please check that none of the files created by the template already exists. ${getErrorMessage( + e, + )}`, ); } + } + return null; + }, + }); + + const { mrId, mrWebUrl } = await ctx.checkpoint({ + key: `create.mr.${repoID}.${branchName}`, + fn: async () => { + try { + const mergeRequest = await api.MergeRequests.create( + repoID, + branchName, + String(targetBranch), + title, + { + description, + removeSourceBranch: removeSourceBranch + ? removeSourceBranch + : false, + assigneeId, + reviewerIds, + }, + ); + return { + mrId: mergeRequest.iid, + mrWebUrl: mergeRequest.web_url ?? mergeRequest.webUrl, + }; } catch (e) { - ctx.logger.warn( - `Failed to assign reviewers from approval rules: ${getErrorMessage( - e, - )}.`, + throw new InputError( + `Merge request creation failed. ${getErrorMessage(e)}`, ); } - } - ctx.output('projectid', repoID); - ctx.output('targetBranchName', targetBranch); - ctx.output('projectPath', repoID); - ctx.output( - 'mergeRequestUrl', - mergeRequest.web_url ?? mergeRequest.webUrl, - ); - } catch (e) { - throw new InputError( - `Merge request creation failed. ${getErrorMessage(e)}`, - ); - } + }, + }); + + await ctx.checkpoint({ + key: `create.mr.assign.reviewers.${repoID}.${branchName}`, + fn: async () => { + if (ctx.input.assignReviewersFromApprovalRules) { + try { + const reviewersFromApprovalRules = + await getReviewersFromApprovalRules(api, mrId, repoID, ctx); + if (reviewersFromApprovalRules.length > 0) { + const eligibleUserIds = new Set([ + ...reviewersFromApprovalRules, + ...(reviewerIds ?? []), + ]); + + const mergeRequest = await api.MergeRequests.edit( + repoID, + mrId, + { + reviewerIds: Array.from(eligibleUserIds), + }, + ); + return { + mrWebUrl: mergeRequest.web_url ?? mergeRequest.webUrl, + }; + } + } catch (e) { + ctx.logger.warn( + `Failed to assign reviewers from approval rules: ${getErrorMessage( + e, + )}.`, + ); + } + } + return { mrWebUrl }; + }, + }); + + ctx.output('projectid', repoID); + ctx.output('targetBranchName', targetBranch); + ctx.output('projectPath', repoID); + ctx.output('mergeRequestUrl', mrWebUrl); }, }); };