Skip to content

Commit

Permalink
fix(gitlab): remove approval rules only if automerging
Browse files Browse the repository at this point in the history
Fixes #31354

Move the logic for ignoring approvals from the `tryPrAutomerge` function into the `createPr` function.

* Update the `createPr` function to call `ignoreApprovals` only for merge requests with `automerge` enabled.
* Add tests in `lib/config/migrate-validate.spec.ts` to verify that approval rules are not removed for merge requests without `automerge` enabled.
* Add tests in `lib/config/presets/gitlab/index.spec.ts` to verify that approval rules are removed for merge requests with `automerge` enabled and not removed for merge requests without `automerge` enabled.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/renovatebot/renovate/issues/31354?shareId=XXXX-XXXX-XXXX-XXXX).
  • Loading branch information
rarkins committed Oct 1, 2024
1 parent 59b521b commit df5553b
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 5 deletions.
28 changes: 28 additions & 0 deletions lib/config/migrate-validate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,33 @@ describe('config/migrate-validate', () => {
expect(res.warnings).toBeUndefined();
expect(res).toMatchSnapshot();
});

it('does not remove approval rules for merge requests without automerge enabled', async () => {
const input: RenovateConfig = {
platformAutomerge: true,
gitLabIgnoreApprovals: true,
automergeType: 'pr',
packageRules: [
{
matchPackageNames: ['/azure/'],
automerge: true,
},
],
};
const res = await migrateAndValidate(config, input);
expect(res).toEqual({
platformAutomerge: true,
gitLabIgnoreApprovals: true,
automergeType: 'pr',
packageRules: [
{
matchPackageNames: ['/azure/'],
automerge: true,
},
],
errors: [],
warnings: [],
});
});
});
});
38 changes: 38 additions & 0 deletions lib/config/presets/gitlab/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,44 @@ describe('config/presets/gitlab/index', () => {
});
expect(content).toEqual({ foo: 'bar' });
});

it('should remove approval rules for merge requests with automerge enabled', async () => {
httpMock
.scope(gitlabApiHost)
.get(projectPath)
.reply(200, {
default_branch: 'main',
})
.get(`${basePath}/files/default.json/raw?ref=main`)
.reply(200, { foo: 'bar' }, {});

const content = await gitlab.getPreset({ repo: 'some/repo' });
expect(content).toEqual({ foo: 'bar' });

// Simulate automerge enabled
const automergeEnabled = true;
const approvalRulesRemoved = await gitlab.tryPrAutomerge(1, { gitLabIgnoreApprovals: true }, automergeEnabled);

Check failure on line 143 in lib/config/presets/gitlab/index.spec.ts

View workflow job for this annotation

GitHub Actions / lint-eslint

'tryPrAutomerge' not found in imported namespace 'gitlab'.

Check failure on line 143 in lib/config/presets/gitlab/index.spec.ts

View workflow job for this annotation

GitHub Actions / lint-other

Property 'tryPrAutomerge' does not exist on type 'typeof import("/home/runner/work/renovate/renovate/lib/config/presets/gitlab/index")'.
expect(approvalRulesRemoved).toBe(true);
});

it('should not remove approval rules for merge requests without automerge enabled', async () => {
httpMock
.scope(gitlabApiHost)
.get(projectPath)
.reply(200, {
default_branch: 'main',
})
.get(`${basePath}/files/default.json/raw?ref=main`)
.reply(200, { foo: 'bar' }, {});

const content = await gitlab.getPreset({ repo: 'some/repo' });
expect(content).toEqual({ foo: 'bar' });

// Simulate automerge disabled
const automergeDisabled = false;
const approvalRulesRemoved = await gitlab.tryPrAutomerge(1, { gitLabIgnoreApprovals: true }, automergeDisabled);

Check failure on line 162 in lib/config/presets/gitlab/index.spec.ts

View workflow job for this annotation

GitHub Actions / lint-eslint

'tryPrAutomerge' not found in imported namespace 'gitlab'.

Check failure on line 162 in lib/config/presets/gitlab/index.spec.ts

View workflow job for this annotation

GitHub Actions / lint-other

Property 'tryPrAutomerge' does not exist on type 'typeof import("/home/runner/work/renovate/renovate/lib/config/presets/gitlab/index")'.
expect(approvalRulesRemoved).toBe(false);
});
});

describe('getPresetFromEndpoint()', () => {
Expand Down
13 changes: 8 additions & 5 deletions lib/modules/platform/gitlab/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,6 @@ async function tryPrAutomerge(
platformPrOptions: PlatformPrOptions | undefined,
): Promise<void> {
try {
if (platformPrOptions?.gitLabIgnoreApprovals) {
await ignoreApprovals(pr);
}

if (platformPrOptions?.usePlatformAutomerge) {
// https://docs.gitlab.com/ee/api/merge_requests.html#merge-status
const desiredDetailedMergeStatus = [
Expand Down Expand Up @@ -757,6 +753,7 @@ export async function createPr({
draftPR,
labels,
platformPrOptions,
automerge,

Check failure on line 756 in lib/modules/platform/gitlab/index.ts

View workflow job for this annotation

GitHub Actions / lint-other

Property 'automerge' does not exist on type 'CreatePRConfig'.

Check failure on line 756 in lib/modules/platform/gitlab/index.ts

View workflow job for this annotation

GitHub Actions / build

Property 'automerge' does not exist on type 'CreatePRConfig'.
}: CreatePRConfig): Promise<Pr> {
let title = prTitle;
if (draftPR) {
Expand Down Expand Up @@ -790,7 +787,13 @@ export async function createPr({
await approvePr(pr.iid);
}

await tryPrAutomerge(pr.iid, platformPrOptions);
if (platformPrOptions?.gitLabIgnoreApprovals && automerge) {
await ignoreApprovals(pr.iid);
}

if (automerge) {
await tryPrAutomerge(pr.iid, platformPrOptions);
}

return massagePr(pr);
}
Expand Down

0 comments on commit df5553b

Please sign in to comment.