diff --git a/lib/platform/gitlab/__snapshots__/index.spec.ts.snap b/lib/platform/gitlab/__snapshots__/index.spec.ts.snap
index 4ddc39db5ccda508e08a7d6766575430d948c3da..732b9c57b0043c773c2be2bd602873b5a714c273 100644
--- a/lib/platform/gitlab/__snapshots__/index.spec.ts.snap
+++ b/lib/platform/gitlab/__snapshots__/index.spec.ts.snap
@@ -141,8 +141,6 @@ Array [
]
`;
-exports[`platform/gitlab addReviewers(issueNo, reviewers) should add the given reviewers to the PR 1`] = `Array []`;
-
exports[`platform/gitlab createPr(branchName, title, body) auto-accepts the MR when requested 1`] = `
Array [
Object {
@@ -968,23 +966,16 @@ Array [
exports[`platform/gitlab getBranchPr(branchName) should return the PR object 1`] = `
Object {
- "additions": 1,
- "base": Object {
- "sha": "1234",
- },
"body": undefined,
- "commits": 1,
- "deletions": 1,
"displayNumber": "Merge Request #91",
"hasAssignees": false,
"hasReviewers": false,
- "iid": 91,
+ "labels": undefined,
"number": 91,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "open",
"targetBranch": "master",
- "target_branch": "master",
"title": "some change",
}
`;
@@ -1040,24 +1031,17 @@ Array [
exports[`platform/gitlab getBranchPr(branchName) should strip deprecated draft prefix from title 1`] = `
Object {
- "additions": 1,
- "base": Object {
- "sha": "1234",
- },
"body": undefined,
- "commits": 1,
- "deletions": 1,
"displayNumber": "Merge Request #91",
"hasAssignees": false,
"hasReviewers": false,
- "iid": 91,
"isDraft": true,
+ "labels": undefined,
"number": 91,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "open",
"targetBranch": "master",
- "target_branch": "master",
"title": "some change",
}
`;
@@ -1113,24 +1097,17 @@ Array [
exports[`platform/gitlab getBranchPr(branchName) should strip draft prefix from title 1`] = `
Object {
- "additions": 1,
- "base": Object {
- "sha": "1234",
- },
"body": undefined,
- "commits": 1,
- "deletions": 1,
"displayNumber": "Merge Request #91",
"hasAssignees": false,
"hasReviewers": false,
- "iid": 91,
"isDraft": true,
+ "labels": undefined,
"number": 91,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "open",
"targetBranch": "master",
- "target_branch": "master",
"title": "some change",
}
`;
@@ -1609,22 +1586,17 @@ exports[`platform/gitlab getPr(prNo) removes deprecated draft prefix from return
Object {
"body": "a merge request",
"canMerge": false,
- "description": "a merge request",
"displayNumber": "Merge Request #12345",
- "diverged_commits_count": 5,
"hasAssignees": false,
"hasReviewers": false,
- "id": 1,
- "iid": 12345,
"isConflicted": true,
"isDraft": true,
- "merge_status": "cannot_be_merged",
+ "labels": undefined,
"number": 12345,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "merged",
"targetBranch": "master",
- "target_branch": "master",
"title": "do something",
}
`;
@@ -1649,22 +1621,17 @@ exports[`platform/gitlab getPr(prNo) removes draft prefix from returned title 1`
Object {
"body": "a merge request",
"canMerge": false,
- "description": "a merge request",
"displayNumber": "Merge Request #12345",
- "diverged_commits_count": 5,
"hasAssignees": false,
"hasReviewers": false,
- "id": 1,
- "iid": 12345,
"isConflicted": true,
"isDraft": true,
- "merge_status": "cannot_be_merged",
+ "labels": undefined,
"number": 12345,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "merged",
"targetBranch": "master",
- "target_branch": "master",
"title": "do something",
}
`;
@@ -1689,21 +1656,16 @@ exports[`platform/gitlab getPr(prNo) returns the PR 1`] = `
Object {
"body": "a merge request",
"canMerge": false,
- "description": "a merge request",
"displayNumber": "Merge Request #12345",
- "diverged_commits_count": 5,
"hasAssignees": false,
"hasReviewers": false,
- "id": 1,
- "iid": 12345,
"isConflicted": true,
- "merge_status": "cannot_be_merged",
+ "labels": undefined,
"number": 12345,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "merged",
"targetBranch": "master",
- "target_branch": "master",
"title": "do something",
}
`;
@@ -1728,21 +1690,16 @@ exports[`platform/gitlab getPr(prNo) returns the PR with nonexisting branch 1`]
Object {
"body": "a merge request",
"canMerge": false,
- "description": "a merge request",
"displayNumber": "Merge Request #12345",
- "diverged_commits_count": 2,
"hasAssignees": true,
"hasReviewers": false,
- "id": 1,
- "iid": 12345,
"isConflicted": true,
- "merge_status": "cannot_be_merged",
+ "labels": undefined,
"number": 12345,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "open",
"targetBranch": "master",
- "target_branch": "master",
"title": "do something",
}
`;
@@ -1767,19 +1724,15 @@ exports[`platform/gitlab getPr(prNo) returns the mergeable PR 1`] = `
Object {
"body": "a merge request",
"canMerge": true,
- "description": "a merge request",
"displayNumber": "Merge Request #12345",
- "diverged_commits_count": 5,
"hasAssignees": true,
"hasReviewers": false,
- "id": 1,
- "iid": 12345,
+ "labels": undefined,
"number": 12345,
+ "sha": undefined,
"sourceBranch": "some-branch",
- "source_branch": "some-branch",
"state": "open",
"targetBranch": "master",
- "target_branch": "master",
"title": "do something",
}
`;
diff --git a/lib/platform/gitlab/http.ts b/lib/platform/gitlab/http.ts
new file mode 100644
index 0000000000000000000000000000000000000000..51a836de629ab27f2f9b5526a2f9de4bd97fb50b
--- /dev/null
+++ b/lib/platform/gitlab/http.ts
@@ -0,0 +1,9 @@
+import { GitlabHttp } from '../../util/http/gitlab';
+
+export const gitlabApi = new GitlabHttp();
+
+export async function getUserID(username: string): Promise<number> {
+ return (
+ await gitlabApi.getJson<{ id: number }[]>(`users?username=${username}`)
+ ).body[0].id;
+}
diff --git a/lib/platform/gitlab/index.spec.ts b/lib/platform/gitlab/index.spec.ts
index 7bf98bc4aa5a90870b99c090ba7ce067bd01ce34..9831c459db8dd3b1df8271772d19eeffd55540e0 100644
--- a/lib/platform/gitlab/index.spec.ts
+++ b/lib/platform/gitlab/index.spec.ts
@@ -9,6 +9,7 @@ import {
REPOSITORY_EMPTY,
REPOSITORY_MIRRORED,
} from '../../constants/error-messages';
+import { logger as _logger } from '../../logger';
import { BranchStatus, PrState } from '../../types';
import * as _git from '../../util/git';
import * as _hostRules from '../../util/host-rules';
@@ -19,11 +20,15 @@ describe('platform/gitlab', () => {
let gitlab: Platform;
let hostRules: jest.Mocked<typeof _hostRules>;
let git: jest.Mocked<typeof _git>;
+ let logger: jest.Mocked<typeof _logger>;
+
beforeEach(async () => {
// reset module
jest.resetModules();
jest.resetAllMocks();
gitlab = await import('.');
+ jest.mock('../../logger');
+ logger = (await import('../../logger')).logger as never;
jest.mock('../../util/host-rules');
jest.mock('delay');
hostRules = require('../../util/host-rules');
@@ -44,6 +49,25 @@ describe('platform/gitlab', () => {
httpMock.reset();
});
+ async function initFakePlatform(version: string) {
+ httpMock
+ .scope(gitlabApiHost)
+ .get('/api/v4/user')
+ .reply(200, {
+ email: 'a@b.com',
+ name: 'Renovate Bot',
+ })
+ .get('/api/v4/version')
+ .reply(200, {
+ version: `${version}-ee`,
+ });
+
+ await gitlab.initPlatform({
+ token: 'some-token',
+ endpoint: undefined,
+ });
+ }
+
describe('initPlatform()', () => {
it(`should throw if no token`, async () => {
await expect(gitlab.initPlatform({} as any)).rejects.toThrow();
@@ -830,14 +854,115 @@ describe('platform/gitlab', () => {
expect(httpMock.getTrace()).toMatchSnapshot();
});
});
- describe('addReviewers(issueNo, reviewers)', () => {
- it('should add the given reviewers to the PR', async () => {
- // no-op
- httpMock.scope(gitlabApiHost);
- await gitlab.addReviewers(42, ['someuser', 'someotheruser']);
- expect(httpMock.getTrace()).toMatchSnapshot();
+
+ describe('addReviewers(iid, reviewers)', () => {
+ describe('13.8.0', () => {
+ it('should not be supported in too low version', async () => {
+ await initFakePlatform('13.8.0');
+ await gitlab.addReviewers(42, ['someuser', 'foo', 'someotheruser']);
+ expect(logger.warn).toHaveBeenCalledWith(
+ { version: '13.8.0' },
+ 'Adding reviewers is only available in GitLab 13.9 and onwards'
+ );
+ });
+ });
+
+ describe('13.9.0', () => {
+ beforeEach(async () => {
+ await initFakePlatform('13.9.0');
+ });
+
+ const existingReviewers = [
+ { id: 1, username: 'foo' },
+ { id: 2, username: 'bar' },
+ ];
+
+ it('should fail to get existing reviewers', async () => {
+ const scope = httpMock
+ .scope(gitlabApiHost)
+ .get(
+ '/api/v4/projects/undefined/merge_requests/42?include_diverged_commits_count=1'
+ )
+ .reply(404);
+
+ await gitlab.addReviewers(42, ['someuser', 'foo', 'someotheruser']);
+ expect(scope.isDone()).toBeTrue();
+ });
+
+ it('should fail to get user IDs', async () => {
+ const scope = httpMock
+ .scope(gitlabApiHost)
+ .get(
+ '/api/v4/projects/undefined/merge_requests/42?include_diverged_commits_count=1'
+ )
+ .reply(200, { reviewers: existingReviewers })
+ .get('/api/v4/users?username=someuser')
+ .reply(200, [{ id: 10 }])
+ .get('/api/v4/users?username=someotheruser')
+ .reply(404);
+
+ await gitlab.addReviewers(42, ['someuser', 'foo', 'someotheruser']);
+ expect(scope.isDone()).toBeTrue();
+ });
+
+ it('should fail to add reviewers to the MR', async () => {
+ const scope = httpMock
+ .scope(gitlabApiHost)
+ .get(
+ '/api/v4/projects/undefined/merge_requests/42?include_diverged_commits_count=1'
+ )
+ .reply(200, { reviewers: existingReviewers })
+ .get('/api/v4/users?username=someuser')
+ .reply(200, [{ id: 10 }])
+ .get('/api/v4/users?username=someotheruser')
+ .reply(200, [{ id: 15 }])
+ .put('/api/v4/projects/undefined/merge_requests/42', {
+ reviewer_ids: [1, 2, 10, 15],
+ })
+ .reply(404);
+
+ await gitlab.addReviewers(42, ['someuser', 'foo', 'someotheruser']);
+ expect(scope.isDone()).toBeTrue();
+ });
+
+ it('should add the given reviewers to the MR', async () => {
+ const scope = httpMock
+ .scope(gitlabApiHost)
+ .get(
+ '/api/v4/projects/undefined/merge_requests/42?include_diverged_commits_count=1'
+ )
+ .reply(200, { reviewers: existingReviewers })
+ .get('/api/v4/users?username=someuser')
+ .reply(200, [{ id: 10 }])
+ .get('/api/v4/users?username=someotheruser')
+ .reply(200, [{ id: 15 }])
+ .put('/api/v4/projects/undefined/merge_requests/42', {
+ reviewer_ids: [1, 2, 10, 15],
+ })
+ .reply(200);
+
+ await gitlab.addReviewers(42, ['someuser', 'foo', 'someotheruser']);
+ expect(scope.isDone()).toBeTrue();
+ });
+
+ it('should only add reviewers if necessary', async () => {
+ const scope = httpMock
+ .scope(gitlabApiHost)
+ .get(
+ '/api/v4/projects/undefined/merge_requests/42?include_diverged_commits_count=1'
+ )
+ .reply(200, { reviewers: existingReviewers })
+ .get('/api/v4/users?username=someuser')
+ .reply(200, [{ id: 1 }])
+ .get('/api/v4/users?username=someotheruser')
+ .reply(200, [{ id: 2 }]);
+
+ await gitlab.addReviewers(42, ['someuser', 'foo', 'someotheruser']);
+ expect(scope.isDone()).toBeTrue();
+ });
});
});
+
describe('ensureComment', () => {
it('add comment if not found', async () => {
const scope = await initRepo();
diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts
index 09f806d9993d0efed6b652ebb500f9cb788774ea..02976cf76a110f9bbf5db523af24cf36cf729f9f 100755
--- a/lib/platform/gitlab/index.ts
+++ b/lib/platform/gitlab/index.ts
@@ -1,7 +1,8 @@
import URL from 'url';
import is from '@sindresorhus/is';
import delay from 'delay';
-import semver from 'semver';
+import pAll from 'p-all';
+import { lt } from 'semver';
import {
PLATFORM_AUTHENTICATION_ERROR,
REPOSITORY_ACCESS_FORBIDDEN,
@@ -19,7 +20,7 @@ import { BranchStatus, PrState, VulnerabilityAlert } from '../../types';
import * as git from '../../util/git';
import * as hostRules from '../../util/host-rules';
import { HttpResponse } from '../../util/http';
-import { GitlabHttp, setBaseUrl } from '../../util/http/gitlab';
+import { setBaseUrl } from '../../util/http/gitlab';
import { sanitize } from '../../util/sanitize';
import { ensureTrailingSlash, getQueryString } from '../../util/url';
import type {
@@ -39,15 +40,16 @@ import type {
UpdatePrConfig,
} from '../types';
import { smartTruncate } from '../utils/pr-body';
+import { getUserID, gitlabApi } from './http';
+import { getMR, updateMR } from './merge-request';
import type {
+ GitLabMergeRequest,
GitlabComment,
GitlabIssue,
MergeMethod,
RepoResponse,
} from './types';
-const gitlabApi = new GitlabHttp();
-
let config: {
repository: string;
localDir: string;
@@ -63,6 +65,7 @@ let config: {
const defaults = {
hostType: PLATFORM_TYPE_GITLAB,
endpoint: 'https://gitlab.com/api/v4/',
+ version: '0.0.0',
};
const DRAFT_PREFIX = 'Draft: ';
@@ -100,6 +103,7 @@ export async function initPlatform({
await gitlabApi.getJson<{ version: string }>('version', { token })
).body.version.split('-')[0];
logger.debug('GitLab version is: ' + gitlabVersion);
+ defaults.version = gitlabVersion;
} catch (err) {
logger.debug(
{ err },
@@ -107,7 +111,7 @@ export async function initPlatform({
);
throw new Error('Init: Authentication failure');
}
- draftPrefix = semver.lt(gitlabVersion, '13.2.0')
+ draftPrefix = lt(gitlabVersion, '13.2.0')
? DRAFT_PREFIX_DEPRECATED
: DRAFT_PREFIX;
const platformConfig: PlatformResult = {
@@ -518,33 +522,24 @@ export async function createPr({
export async function getPr(iid: number): Promise<Pr> {
logger.debug(`getPr(${iid})`);
- const url = `projects/${config.repository}/merge_requests/${iid}?include_diverged_commits_count=1`;
- const pr = (
- await gitlabApi.getJson<
- Pr & {
- iid: number;
- source_branch: string;
- target_branch: string;
- description: string;
- diverged_commits_count: number;
- merge_status: string;
- assignee?: { id?: number };
- assignees?: { id?: number }[];
- }
- >(url)
- ).body;
+ const mr = await getMR(config.repository, iid);
+
// Harmonize fields with GitHub
- pr.sourceBranch = pr.source_branch;
- pr.targetBranch = pr.target_branch;
- pr.number = pr.iid;
- pr.displayNumber = `Merge Request #${pr.iid}`;
- pr.body = pr.description;
- pr.state = pr.state === 'opened' ? PrState.Open : pr.state;
- pr.hasAssignees = !!(pr.assignee?.id || pr.assignees?.[0]?.id);
- delete pr.assignee;
- delete pr.assignees;
- pr.hasReviewers = false;
- if (pr.merge_status === 'cannot_be_merged') {
+ const pr: Pr = {
+ sourceBranch: mr.source_branch,
+ targetBranch: mr.target_branch,
+ number: mr.iid,
+ displayNumber: `Merge Request #${mr.iid}`,
+ body: mr.description,
+ state: mr.state === 'opened' ? PrState.Open : mr.state,
+ hasAssignees: !!(mr.assignee?.id || mr.assignees?.[0]?.id),
+ hasReviewers: !!mr.reviewers?.length,
+ title: mr.title,
+ labels: mr.labels,
+ sha: mr.sha,
+ };
+
+ if (mr.merge_status === 'cannot_be_merged') {
logger.debug('pr cannot be merged');
pr.canMerge = false;
pr.isConflicted = true;
@@ -847,22 +842,14 @@ export async function addAssignees(
): Promise<void> {
logger.debug(`Adding assignees '${assignees.join(', ')}' to #${iid}`);
try {
- let assigneeId = (
- await gitlabApi.getJson<{ id: number }[]>(
- `users?username=${assignees[0]}`
- )
- ).body[0].id;
+ let assigneeId = await getUserID(assignees[0]);
let url = `projects/${config.repository}/merge_requests/${iid}?assignee_id=${assigneeId}`;
await gitlabApi.putJson(url);
try {
if (assignees.length > 1) {
url = `projects/${config.repository}/merge_requests/${iid}?assignee_ids[]=${assigneeId}`;
for (let i = 1; i < assignees.length; i += 1) {
- assigneeId = (
- await gitlabApi.getJson<{ id: number }[]>(
- `users?username=${assignees[i]}`
- )
- ).body[0].id;
+ assigneeId = await getUserID(assignees[i]);
url += `&assignee_ids[]=${assigneeId}`;
}
await gitlabApi.putJson(url);
@@ -876,10 +863,54 @@ export async function addAssignees(
}
}
-export function addReviewers(iid: number, reviewers: string[]): Promise<void> {
- logger.debug(`addReviewers('${iid}, [${reviewers.join(', ')}])`);
- logger.warn('Unimplemented in GitLab: approvals');
- return Promise.resolve();
+export async function addReviewers(
+ iid: number,
+ reviewers: string[]
+): Promise<void> {
+ logger.debug(`Adding reviewers '${reviewers.join(', ')}' to #${iid}`);
+
+ if (lt(defaults.version, '13.9.0')) {
+ logger.warn(
+ { version: defaults.version },
+ 'Adding reviewers is only available in GitLab 13.9 and onwards'
+ );
+ return;
+ }
+
+ let mr: GitLabMergeRequest;
+ try {
+ mr = await getMR(config.repository, iid);
+ } catch (err) {
+ logger.warn({ err }, 'Failed to get existing reviewers');
+ return;
+ }
+
+ mr.reviewers = mr.reviewers ?? [];
+ const existingReviewers = mr.reviewers.map((r) => r.username);
+ const existingReviewerIDs = mr.reviewers.map((r) => r.id);
+
+ // Figure out which reviewers (of the ones we want to add) are not already on the MR as a reviewer
+ const newReviewers = reviewers.filter((r) => !existingReviewers.includes(r));
+
+ // Gather the IDs for all the reviewers we want to add
+ let newReviewerIDs: number[];
+ try {
+ newReviewerIDs = await pAll(
+ newReviewers.map((r) => () => getUserID(r)),
+ { concurrency: 5 }
+ );
+ } catch (err) {
+ logger.warn({ err }, 'Failed to get IDs of the new reviewers');
+ return;
+ }
+
+ try {
+ await updateMR(config.repository, iid, {
+ reviewer_ids: [...existingReviewerIDs, ...newReviewerIDs],
+ });
+ } catch (err) {
+ logger.warn({ err }, 'Failed to add reviewers');
+ }
}
export async function deleteLabel(
diff --git a/lib/platform/gitlab/merge-request.ts b/lib/platform/gitlab/merge-request.ts
new file mode 100644
index 0000000000000000000000000000000000000000..497294f275059d221aef02e2faebc150d36301e2
--- /dev/null
+++ b/lib/platform/gitlab/merge-request.ts
@@ -0,0 +1,26 @@
+import { logger } from '../../logger';
+import { gitlabApi } from './http';
+import type { GitLabMergeRequest, UpdateMergeRequest } from './types';
+
+export async function getMR(
+ repository: string,
+ iid: number
+): Promise<GitLabMergeRequest> {
+ logger.debug(`getMR(${iid})`);
+
+ const url = `projects/${repository}/merge_requests/${iid}?include_diverged_commits_count=1`;
+ return (await gitlabApi.getJson<GitLabMergeRequest>(url)).body;
+}
+
+export async function updateMR(
+ repository: string,
+ iid: number,
+ data: UpdateMergeRequest
+): Promise<void> {
+ logger.debug(`updateMR(${iid})`);
+
+ const url = `projects/${repository}/merge_requests/${iid}`;
+ await gitlabApi.putJson(url, {
+ body: data,
+ });
+}
diff --git a/lib/platform/gitlab/types.ts b/lib/platform/gitlab/types.ts
index fda51a9ffcf971f962233184b70dad01c6c7992e..607b3e973ea0c1d93e48ae6571f666c6a6b73186 100644
--- a/lib/platform/gitlab/types.ts
+++ b/lib/platform/gitlab/types.ts
@@ -8,9 +8,38 @@ export interface GitlabComment {
id: number;
}
+export interface GitLabUser {
+ id: number;
+ username: string;
+}
+
+export interface GitLabMergeRequest {
+ iid: number;
+ title: string;
+ state: string;
+ source_branch: string;
+ target_branch: string;
+ description: string;
+ diverged_commits_count: number;
+ merge_status: string;
+ assignee?: GitLabUser;
+ assignees?: GitLabUser[];
+ reviewers?: GitLabUser[];
+ labels: string[];
+ sha: string;
+}
+
+export interface UpdateMergeRequest {
+ target_branch?: string;
+ title?: string;
+ assignee_id?: number;
+ assignee_ids?: number[];
+ reviewer_ids?: number[];
+}
+
export type MergeMethod = 'merge' | 'rebase_merge' | 'ff';
-export type RepoResponse = {
+export interface RepoResponse {
archived: boolean;
mirror: boolean;
default_branch: string;
@@ -21,4 +50,4 @@ export type RepoResponse = {
merge_requests_access_level: 'disabled' | 'private' | 'enabled';
merge_method: MergeMethod;
path_with_namespace: string;
-};
+}