Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gitlab): ensure getPrList() runtime integrity #32291

Merged
merged 13 commits into from
Nov 25, 2024
Merged

fix(gitlab): ensure getPrList() runtime integrity #32291

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d9ec900
update runtime prList when updating pr
RahulGautamSingh Nov 2, 2024
8dc82c0
add tests
RahulGautamSingh Nov 7, 2024
4524188
refactor
RahulGautamSingh Nov 7, 2024
5cbfbc8
refactor
RahulGautamSingh Nov 7, 2024
ea00473
add warning
RahulGautamSingh Nov 7, 2024
ddeb08d
apply suggestions
RahulGautamSingh Nov 9, 2024
306a8a9
Merge branch 'main' into refactor(gitlab)/update-pr-list
RahulGautamSingh Nov 22, 2024
8cfd9c6
fix tests
RahulGautamSingh Nov 24, 2024
67b3d18
test: refactor
RahulGautamSingh Nov 24, 2024
f266184
Merge branch 'main' into refactor(gitlab)/update-pr-list
RahulGautamSingh Nov 24, 2024
a1c47e3
types: refactor
RahulGautamSingh Nov 24, 2024
cf1a679
Merge branch 'refactor(gitlab)/update-pr-list' of https://github.com/…
RahulGautamSingh Nov 24, 2024
1c4a192
Update lib/modules/platform/gitlab/index.ts
RahulGautamSingh Nov 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 83 additions & 17 deletions lib/modules/platform/gitlab/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2947,11 +2947,16 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'branch a pr',
state: 'open',
state: 'opened',
},
])
.put('/api/v4/projects/undefined/merge_requests/1')
.reply(200);
.reply(200, {
iid: 1,
source_branch: 'branch-a',
title: 'title',
state: 'opened',
});
await expect(
gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body' }),
).toResolve();
Expand All @@ -2969,11 +2974,16 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'Draft: foo',
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',
state: 'opened',
});
await expect(
gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body' }),
).toResolve();
Expand All @@ -2991,11 +3001,16 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'WIP: foo',
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',
state: 'opened',
});
await expect(
gitlab.updatePr({ number: 1, prTitle: 'title', prBody: 'body' }),
).toResolve();
Expand All @@ -3013,19 +3028,24 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'branch a pr',
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',
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();
});
Expand All @@ -3042,11 +3062,16 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'branch a pr',
state: 'open',
state: 'opened',
},
])
.put('/api/v4/projects/undefined/merge_requests/1')
.reply(200)
.reply(200, {
iid: 1,
source_branch: 'branch-a',
title: 'title',
state: 'opened',
})
.post('/api/v4/projects/undefined/merge_requests/1/approve')
.reply(200);
await expect(
Expand All @@ -3073,11 +3098,16 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'branch a pr',
state: 'open',
state: 'opened',
},
])
.put('/api/v4/projects/undefined/merge_requests/1')
.reply(200);
.reply(200, {
iid: 1,
source_branch: 'branch-a',
title: 'title',
state: 'closed',
});
await expect(
gitlab.updatePr({
number: 1,
Expand All @@ -3100,22 +3130,58 @@ describe('modules/platform/gitlab/index', () => {
iid: 1,
source_branch: 'branch-a',
title: 'branch a pr',
state: 'open',
state: 'opened',
},
])
.put('/api/v4/projects/undefined/merge_requests/1')
.reply(200);
.reply(200, {
iid: 1,
source_branch: 'branch-a',
title: 'title',
state: 'opened',
});
await expect(
gitlab.updatePr({
number: 1,
prTitle: 'title',
prBody: 'body',
state: 'closed',
addLabels: ['new_label'],
removeLabels: ['old_label'],
}),
).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',
state: 'opened',
},
])
.put('/api/v4/projects/undefined/merge_requests/1')
.reply(200, {
iid: 1,
source_branch: 'branch-a',
title: 'new_title',
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');
});
});

describe('reattemptPlatformAutomerge(number, platformPrOptions)', () => {
Expand Down
37 changes: 33 additions & 4 deletions lib/modules/platform/gitlab/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,10 +855,39 @@ export async function updatePr({
body.remove_labels = removeLabels;
}

await gitlabApi.putJson(
`projects/${config.repository}/merge_requests/${iid}`,
{ body },
);
const updatedPrInfo = (
await gitlabApi.putJson<{
iid: number;
source_branch: string;
title: string;
state: string;
created_at: string;
}>(`projects/${config.repository}/merge_requests/${iid}`, { body })
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
).body;

const updatedPr: Pr = massagePr({
number: updatedPrInfo.iid,
state: updatedPrInfo.state === 'opened' ? 'open' : updatedPrInfo.state,
title: updatedPrInfo.title,
createdAt: updatedPrInfo.created_at,
sourceBranch: updatedPrInfo.source_branch,
});
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved

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);
Expand Down