From 78627a915cd6bd05296084cb246fd64c1ceff4c0 Mon Sep 17 00:00:00 2001 From: RahulGautamSingh Date: Mon, 25 Nov 2024 17:12:44 +0530 Subject: [PATCH] fix(gitlab): ensure `getPrList()` runtime integrity (#32291) Co-authored-by: Michael Kriese --- lib/modules/platform/gitlab/index.spec.ts | 110 ++++++++++++++++++---- lib/modules/platform/gitlab/index.ts | 28 +++++- 2 files changed, 118 insertions(+), 20 deletions(-) diff --git a/lib/modules/platform/gitlab/index.spec.ts b/lib/modules/platform/gitlab/index.spec.ts index 9c3aefc983fbca..6eb62b886f7c40 100644 --- a/lib/modules/platform/gitlab/index.spec.ts +++ b/lib/modules/platform/gitlab/index.spec.ts @@ -18,6 +18,7 @@ import type * as _git from '../../../util/git'; import type { LongCommitSha } from '../../../util/git/types'; import type * as _hostRules from '../../../util/host-rules'; import { toBase64 } from '../../../util/string'; +import { getPrBodyStruct } from '../pr-body'; jest.mock('../../../util/host-rules', () => mockDeep()); jest.mock('../../../util/git'); @@ -3004,11 +3005,17 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'branch a pr', description: 'a merge request', - state: 'open', + state: 'opened', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200); + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'title', + description: 'body', + state: 'opened', + }); await expect( gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body' }), ).toResolve(); @@ -3027,11 +3034,17 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'Draft: foo', description: 'a merge request', - state: 'open', + state: 'opened', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200); + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'Draft: title', + description: 'body', + state: 'opened', + }); await expect( gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body' }), ).toResolve(); @@ -3050,11 +3063,17 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'WIP: foo', description: 'a merge request', - state: 'open', + state: 'opened', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200); + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'WIP: title', + description: 'body', + state: 'opened', + }); await expect( gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body' }), ).toResolve(); @@ -3073,19 +3092,25 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'branch a pr', description: 'a merge request', - state: 'open', + state: 'opened', target_branch: 'branch-b', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200); + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'branch a pr', + description: 'body', + state: 'opened', + target_branch: 'branch-new', + }); await expect( gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body', - state: 'closed', - targetBranch: 'branch-b', + targetBranch: 'branch-new', }), ).toResolve(); }); @@ -3103,11 +3128,17 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'branch a pr', description: 'a merge request', - state: 'open', + state: 'opened', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200) + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'title', + description: 'body', + state: 'opened', + }) .post('/api/v4/projects/undefined/merge_requests/1/approve') .reply(200); await expect( @@ -3135,11 +3166,17 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'branch a pr', description: 'a merge request', - state: 'open', + state: 'opened', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200); + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'title', + description: 'a merge requbody', + state: 'closed', + }); await expect( gitlab.updatePr({ number: 1, @@ -3163,11 +3200,17 @@ describe('modules/platform/gitlab/index', () => { source_branch: 'branch-a', title: 'branch a pr', description: 'a merge request', - state: 'open', + state: 'opened', }, ]) .put('/api/v4/projects/undefined/merge_requests/1') - .reply(200); + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'title', + description: 'body', + state: 'opened', + }); await expect( gitlab.updatePr({ number: 1, @@ -3179,6 +3222,41 @@ describe('modules/platform/gitlab/index', () => { }), ).toResolve(); }); + + it('updates runtime pr list when pr is updated', async () => { + await initPlatform('13.3.6-ee'); + httpMock + .scope(gitlabApiHost) + .get( + '/api/v4/projects/undefined/merge_requests?per_page=100&scope=created_by_me', + ) + .reply(200, [ + { + iid: 1, + source_branch: 'branch-a', + title: 'branch a pr', + description: 'a merge request', + state: 'opened', + }, + ]) + .put('/api/v4/projects/undefined/merge_requests/1') + .reply(200, { + iid: 1, + source_branch: 'branch-a', + title: 'new_title', + description: 'body', + state: 'opened', + }); + await gitlab.updatePr({ + number: 1, + prTitle: 'new_title', + prBody: 'body', + }); + const prList = await gitlab.getPrList(); + const updatedPr = prList.find((pr) => pr.number === 1); + expect(updatedPr?.title).toBe('new_title'); + expect(updatedPr?.bodyStruct).toMatchObject(getPrBodyStruct('body')); + }); }); describe('reattemptPlatformAutomerge(number, platformPrOptions)', () => { diff --git a/lib/modules/platform/gitlab/index.ts b/lib/modules/platform/gitlab/index.ts index aa8b86b31d9c26..30965b96308505 100644 --- a/lib/modules/platform/gitlab/index.ts +++ b/lib/modules/platform/gitlab/index.ts @@ -811,10 +811,30 @@ export async function updatePr({ body.remove_labels = removeLabels; } - await gitlabApi.putJson( - `projects/${config.repository}/merge_requests/${iid}`, - { body }, - ); + const updatedPrInfo = ( + await gitlabApi.putJson( + `projects/${config.repository}/merge_requests/${iid}`, + { body }, + ) + ).body; + + const updatedPr = prInfo(updatedPrInfo); + + if (config.prList) { + const existingIndex = config.prList.findIndex( + (pr) => pr.number === updatedPr.number, + ); + // istanbul ignore if: should not happen + if (existingIndex === -1) { + logger.warn( + { pr: updatedPr }, + 'Possible error: Updated PR was not found in the PRs that were returned from getPrList().', + ); + config.prList.push(updatedPr); + } else { + config.prList[existingIndex] = updatedPr; + } + } if (platformPrOptions?.autoApprove) { await approvePr(iid);