From 2cc751a0a1ba2e0d3bc35f51eed4cdaf07f8282c Mon Sep 17 00:00:00 2001
From: Florian Greinacher <florian@greinacher.de>
Date: Thu, 22 Apr 2021 21:16:58 +0200
Subject: [PATCH] feat: ignore unavailable users (#9406)
---
docs/usage/configuration-options.md | 6 ++
lib/config/definitions.ts | 6 ++
lib/config/types.ts | 1 +
.../gitlab/__snapshots__/index.spec.ts.snap | 77 +++++++++++++++++++
lib/platform/gitlab/http.ts | 13 ++++
lib/platform/gitlab/index.spec.ts | 40 ++++++++++
lib/platform/gitlab/index.ts | 14 +++-
lib/platform/gitlab/types.ts | 8 ++
lib/platform/types.ts | 1 +
.../pr/__snapshots__/index.spec.ts.snap | 22 ++++++
lib/workers/pr/index.spec.ts | 10 +++
lib/workers/pr/index.ts | 11 +++
lib/workers/repository/init/index.spec.ts | 15 +++-
lib/workers/repository/init/index.ts | 10 +++
14 files changed, 232 insertions(+), 2 deletions(-)
diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md
index b21d513dc1..cf9b4062f3 100644
--- a/docs/usage/configuration-options.md
+++ b/docs/usage/configuration-options.md
@@ -631,6 +631,12 @@ Because `fileMatch` is mergeable, you don't need to duplicate the defaults and c
If you configure `fileMatch` then it must be within a manager object (e.g. `dockerfile` in the above example).
The full list of supported managers can be found [here](https://docs.renovatebot.com/modules/manager/).
+## filterUnavailableUsers
+
+When this option is enabled PRs are not assigned to users that are unavailable.
+This option only works on platforms that support the concept of user availability.
+For now, you can only use this option on the GitLab platform.
+
## followTag
Caution: advanced functionality. Only use it if you're sure you know what you're doing.
diff --git a/lib/config/definitions.ts b/lib/config/definitions.ts
index 76dcf0d28c..3c77b49792 100644
--- a/lib/config/definitions.ts
+++ b/lib/config/definitions.ts
@@ -1497,6 +1497,12 @@ const options: RenovateOptions[] = [
type: 'boolean',
default: false,
},
+ {
+ name: 'filterUnavailableUsers',
+ description: 'Filter reviewers and assignees based on their availability.',
+ type: 'boolean',
+ default: false,
+ },
{
name: 'reviewersSampleSize',
description: 'Take a random sample of given size from reviewers.',
diff --git a/lib/config/types.ts b/lib/config/types.ts
index 2ef022c21c..cbbbd97337 100644
--- a/lib/config/types.ts
+++ b/lib/config/types.ts
@@ -207,6 +207,7 @@ export interface AssigneesAndReviewersConfig {
reviewers?: string[];
reviewersSampleSize?: number;
additionalReviewers?: string[];
+ filterUnavailableUsers?: boolean;
}
export type UpdateType =
diff --git a/lib/platform/gitlab/__snapshots__/index.spec.ts.snap b/lib/platform/gitlab/__snapshots__/index.spec.ts.snap
index e577884354..f63b6d3f9f 100644
--- a/lib/platform/gitlab/__snapshots__/index.spec.ts.snap
+++ b/lib/platform/gitlab/__snapshots__/index.spec.ts.snap
@@ -798,6 +798,83 @@ Array [
]
`;
+exports[`platform/gitlab/index filterUnavailableUsers(users) filters users that are busy 1`] = `
+Array [
+ "john",
+]
+`;
+
+exports[`platform/gitlab/index filterUnavailableUsers(users) filters users that are busy 2`] = `
+Array [
+ Object {
+ "headers": Object {
+ "accept": "application/json",
+ "accept-encoding": "gzip, deflate, br",
+ "authorization": "Bearer abc123",
+ "host": "gitlab.com",
+ "user-agent": "https://github.com/renovatebot/renovate",
+ },
+ "method": "GET",
+ "url": "https://gitlab.com/api/v4/users/maria/status",
+ },
+ Object {
+ "headers": Object {
+ "accept": "application/json",
+ "accept-encoding": "gzip, deflate, br",
+ "authorization": "Bearer abc123",
+ "host": "gitlab.com",
+ "user-agent": "https://github.com/renovatebot/renovate",
+ },
+ "method": "GET",
+ "url": "https://gitlab.com/api/v4/users/john/status",
+ },
+]
+`;
+
+exports[`platform/gitlab/index filterUnavailableUsers(users) keeps users with failing requests 1`] = `
+Array [
+ "maria",
+]
+`;
+
+exports[`platform/gitlab/index filterUnavailableUsers(users) keeps users with failing requests 2`] = `
+Array [
+ Object {
+ "headers": Object {
+ "accept": "application/json",
+ "accept-encoding": "gzip, deflate, br",
+ "authorization": "Bearer abc123",
+ "host": "gitlab.com",
+ "user-agent": "https://github.com/renovatebot/renovate",
+ },
+ "method": "GET",
+ "url": "https://gitlab.com/api/v4/users/maria/status",
+ },
+]
+`;
+
+exports[`platform/gitlab/index filterUnavailableUsers(users) keeps users with missing availability 1`] = `
+Array [
+ "maria",
+]
+`;
+
+exports[`platform/gitlab/index filterUnavailableUsers(users) keeps users with missing availability 2`] = `
+Array [
+ Object {
+ "headers": Object {
+ "accept": "application/json",
+ "accept-encoding": "gzip, deflate, br",
+ "authorization": "Bearer abc123",
+ "host": "gitlab.com",
+ "user-agent": "https://github.com/renovatebot/renovate",
+ },
+ "method": "GET",
+ "url": "https://gitlab.com/api/v4/users/maria/status",
+ },
+]
+`;
+
exports[`platform/gitlab/index findIssue() finds issue 1`] = `
Array [
Object {
diff --git a/lib/platform/gitlab/http.ts b/lib/platform/gitlab/http.ts
index 51a836de62..bab2b0f803 100644
--- a/lib/platform/gitlab/http.ts
+++ b/lib/platform/gitlab/http.ts
@@ -1,4 +1,6 @@
+import { logger } from '../../logger';
import { GitlabHttp } from '../../util/http/gitlab';
+import type { GitlabUserStatus } from './types';
export const gitlabApi = new GitlabHttp();
@@ -7,3 +9,14 @@ export async function getUserID(username: string): Promise<number> {
await gitlabApi.getJson<{ id: number }[]>(`users?username=${username}`)
).body[0].id;
}
+
+export async function isUserBusy(user: string): Promise<boolean> {
+ try {
+ const url = `/users/${user}/status`;
+ const userStatus = (await gitlabApi.getJson<GitlabUserStatus>(url)).body;
+ return userStatus.availability === 'busy';
+ } catch (err) {
+ logger.warn({ err }, 'Failed to get user status');
+ return false;
+ }
+}
diff --git a/lib/platform/gitlab/index.spec.ts b/lib/platform/gitlab/index.spec.ts
index b60e0ac2ee..9f31a17ae2 100644
--- a/lib/platform/gitlab/index.spec.ts
+++ b/lib/platform/gitlab/index.spec.ts
@@ -1614,4 +1614,44 @@ These updates have all been created already. Click a checkbox below to force a r
expect(httpMock.getTrace()).toMatchSnapshot();
});
});
+ describe('filterUnavailableUsers(users)', () => {
+ it('filters users that are busy', async () => {
+ httpMock
+ .scope(gitlabApiHost)
+ .get('/api/v4/users/maria/status')
+ .reply(200, {
+ availability: 'busy',
+ })
+ .get('/api/v4/users/john/status')
+ .reply(200, {
+ availability: 'not_set',
+ });
+ const filteredUsers = await gitlab.filterUnavailableUsers([
+ 'maria',
+ 'john',
+ ]);
+ expect(filteredUsers).toMatchSnapshot();
+ expect(httpMock.getTrace()).toMatchSnapshot();
+ });
+
+ it('keeps users with missing availability', async () => {
+ httpMock
+ .scope(gitlabApiHost)
+ .get('/api/v4/users/maria/status')
+ .reply(200, {});
+ const filteredUsers = await gitlab.filterUnavailableUsers(['maria']);
+ expect(filteredUsers).toMatchSnapshot();
+ expect(httpMock.getTrace()).toMatchSnapshot();
+ });
+
+ it('keeps users with failing requests', async () => {
+ httpMock
+ .scope(gitlabApiHost)
+ .get('/api/v4/users/maria/status')
+ .reply(404);
+ const filteredUsers = await gitlab.filterUnavailableUsers(['maria']);
+ expect(filteredUsers).toMatchSnapshot();
+ expect(httpMock.getTrace()).toMatchSnapshot();
+ });
+ });
});
diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts
index 8ea18a0cd4..6faaf766cc 100755
--- a/lib/platform/gitlab/index.ts
+++ b/lib/platform/gitlab/index.ts
@@ -40,7 +40,7 @@ import type {
UpdatePrConfig,
} from '../types';
import { smartTruncate } from '../utils/pr-body';
-import { getUserID, gitlabApi } from './http';
+import { getUserID, gitlabApi, isUserBusy } from './http';
import { getMR, updateMR } from './merge-request';
import type {
GitLabMergeRequest,
@@ -1067,3 +1067,15 @@ export async function ensureCommentRemoval({
export function getVulnerabilityAlerts(): Promise<VulnerabilityAlert[]> {
return Promise.resolve([]);
}
+
+export async function filterUnavailableUsers(
+ users: string[]
+): Promise<string[]> {
+ const filteredUsers = [];
+ for (const user of users) {
+ if (!(await isUserBusy(user))) {
+ filteredUsers.push(user);
+ }
+ }
+ return filteredUsers;
+}
diff --git a/lib/platform/gitlab/types.ts b/lib/platform/gitlab/types.ts
index 607b3e973e..f35eba0b51 100644
--- a/lib/platform/gitlab/types.ts
+++ b/lib/platform/gitlab/types.ts
@@ -51,3 +51,11 @@ export interface RepoResponse {
merge_method: MergeMethod;
path_with_namespace: string;
}
+
+// See https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/types/user_status_type.rb
+export interface GitlabUserStatus {
+ message?: string;
+ message_html?: string;
+ emoji?: string;
+ availability: 'not_set' | 'busy';
+}
diff --git a/lib/platform/types.ts b/lib/platform/types.ts
index 89345dc451..467dafdd67 100644
--- a/lib/platform/types.ts
+++ b/lib/platform/types.ts
@@ -177,4 +177,5 @@ export interface Platform {
): Promise<BranchStatus>;
getBranchPr(branchName: string): Promise<Pr | null>;
initPlatform(config: PlatformParams): Promise<PlatformResult>;
+ filterUnavailableUsers?(users: string[]): Promise<string[]>;
}
diff --git a/lib/workers/pr/__snapshots__/index.spec.ts.snap b/lib/workers/pr/__snapshots__/index.spec.ts.snap
index 13361625d5..a3941ef731 100644
--- a/lib/workers/pr/__snapshots__/index.spec.ts.snap
+++ b/lib/workers/pr/__snapshots__/index.spec.ts.snap
@@ -184,6 +184,28 @@ Array [
]
`;
+exports[`workers/pr/index ensurePr should filter assignees and reviewers based on their availability 1`] = `
+Array [
+ Array [
+ undefined,
+ Array [
+ "foo",
+ ],
+ ],
+]
+`;
+
+exports[`workers/pr/index ensurePr should filter assignees and reviewers based on their availability 2`] = `
+Array [
+ Array [
+ undefined,
+ Array [
+ "foo",
+ ],
+ ],
+]
+`;
+
exports[`workers/pr/index ensurePr should return modified existing PR 1`] = `
Object {
"body": "This PR contains the following updates:\\n\\n| Package | Type | Update | Change |\\n|---|---|---|---|\\n| [dummy](https://dummy.com) ([source](https://github.com/renovateapp/dummy), [changelog](https://github.com/renovateapp/dummy/changelog.md)) | devDependencies | minor | \`1.0.0\` -> \`1.1.0\` |\\n\\n---\\n\\n### Release Notes\\n\\n<details>\\n<summary>renovateapp/dummy</summary>\\n\\n### [\`v1.1.0\`](https://github.com/renovateapp/dummy/compare/v1.0.0...v1.1.0)\\n\\n[Compare Source](https://github.com/renovateapp/dummy/compare/v1.0.0...v1.1.0)\\n\\n</details>\\n\\n---\\n\\n### Configuration\\n\\n:date: **Schedule**: \\"before 5am\\" (UTC).\\n\\n:vertical_traffic_light: **Automerge**: Enabled.\\n\\n:recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.\\n\\n:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.\\n\\n---\\n\\n - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box.\\n\\n---\\n\\nThis PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).",
diff --git a/lib/workers/pr/index.spec.ts b/lib/workers/pr/index.spec.ts
index 47d8de2712..507d51cda8 100644
--- a/lib/workers/pr/index.spec.ts
+++ b/lib/workers/pr/index.spec.ts
@@ -410,6 +410,16 @@ describe(getName(__filename), () => {
expect(platform.addReviewers).toHaveBeenCalledTimes(1);
expect(platform.addReviewers.mock.calls).toMatchSnapshot();
});
+ it('should filter assignees and reviewers based on their availability', async () => {
+ config.assignees = ['foo', 'bar'];
+ config.reviewers = ['foo', 'bar'];
+ config.filterUnavailableUsers = true;
+ platform.filterUnavailableUsers = jest.fn();
+ platform.filterUnavailableUsers.mockResolvedValue(['foo']);
+ await prWorker.ensurePr(config);
+ expect(platform.addAssignees.mock.calls).toMatchSnapshot();
+ expect(platform.addReviewers.mock.calls).toMatchSnapshot();
+ });
it('should determine assignees from code owners', async () => {
config.assigneesFromCodeOwners = true;
codeOwnersMock.codeOwnersForPr.mockResolvedValueOnce(['@john', '@maria']);
diff --git a/lib/workers/pr/index.ts b/lib/workers/pr/index.ts
index 0ddb2aab61..92efafb868 100644
--- a/lib/workers/pr/index.ts
+++ b/lib/workers/pr/index.ts
@@ -33,6 +33,15 @@ async function addCodeOwners(
return [...new Set(assigneesOrReviewers.concat(await codeOwnersForPr(pr)))];
}
+function filterUnavailableUsers(
+ config: RenovateConfig,
+ users: string[]
+): Promise<string[]> {
+ return config.filterUnavailableUsers && platform.filterUnavailableUsers
+ ? platform.filterUnavailableUsers(users)
+ : Promise.resolve(users);
+}
+
export async function addAssigneesReviewers(
config: RenovateConfig,
pr: Pr
@@ -41,6 +50,7 @@ export async function addAssigneesReviewers(
if (config.assigneesFromCodeOwners) {
assignees = await addCodeOwners(assignees, pr);
}
+ assignees = await filterUnavailableUsers(config, assignees);
if (assignees.length > 0) {
try {
assignees = assignees.map(noLeadingAtSymbol);
@@ -70,6 +80,7 @@ export async function addAssigneesReviewers(
if (config.additionalReviewers.length > 0) {
reviewers = reviewers.concat(config.additionalReviewers);
}
+ reviewers = await filterUnavailableUsers(config, reviewers);
if (reviewers.length > 0) {
try {
reviewers = [...new Set(reviewers.map(noLeadingAtSymbol))];
diff --git a/lib/workers/repository/init/index.spec.ts b/lib/workers/repository/init/index.spec.ts
index 63a04cbcce..71fcba1b87 100644
--- a/lib/workers/repository/init/index.spec.ts
+++ b/lib/workers/repository/init/index.spec.ts
@@ -1,4 +1,4 @@
-import { getName, mocked } from '../../../../test/util';
+import { getName, logger, mocked } from '../../../../test/util';
import * as _secrets from '../../../config/secrets';
import * as _onboarding from '../onboarding/branch';
import * as _apis from './apis';
@@ -29,5 +29,18 @@ describe(getName(__filename), () => {
const renovateConfig = await initRepo({});
expect(renovateConfig).toMatchSnapshot();
});
+ it('warns on unsupported options', async () => {
+ apis.initApis.mockResolvedValue({} as never);
+ onboarding.checkOnboardingBranch.mockResolvedValueOnce({});
+ config.getRepoConfig.mockResolvedValueOnce({
+ filterUnavailableUsers: true,
+ });
+ config.mergeRenovateConfig.mockResolvedValueOnce({});
+ secrets.applySecretsToConfig.mockReturnValueOnce({} as never);
+ await initRepo({});
+ expect(logger.logger.warn).toHaveBeenCalledWith(
+ "Configuration option 'filterUnavailableUsers' is not supported on the current platform 'undefined'."
+ );
+ });
});
});
diff --git a/lib/workers/repository/init/index.ts b/lib/workers/repository/init/index.ts
index 07a47f1f4c..735e1fd44b 100644
--- a/lib/workers/repository/init/index.ts
+++ b/lib/workers/repository/init/index.ts
@@ -1,6 +1,7 @@
import { applySecretsToConfig } from '../../../config/secrets';
import type { RenovateConfig } from '../../../config/types';
import { logger } from '../../../logger';
+import { platform } from '../../../platform';
import { clone } from '../../../util/clone';
import { setUserRepoConfig } from '../../../util/git';
import { checkIfConfigured } from '../configured';
@@ -13,6 +14,14 @@ function initializeConfig(config: RenovateConfig): RenovateConfig {
return { ...clone(config), errors: [], warnings: [], branchList: [] };
}
+function warnOnUnsupportedOptions(config: RenovateConfig): void {
+ if (config.filterUnavailableUsers && !platform.filterUnavailableUsers) {
+ logger.warn(
+ `Configuration option 'filterUnavailableUsers' is not supported on the current platform '${config.platform}'.`
+ );
+ }
+}
+
export async function initRepo(
config_: RenovateConfig
): Promise<RenovateConfig> {
@@ -21,6 +30,7 @@ export async function initRepo(
config = await initApis(config);
config = await getRepoConfig(config);
checkIfConfigured(config);
+ warnOnUnsupportedOptions(config);
config = applySecretsToConfig(config);
await setUserRepoConfig(config);
config = await detectVulnerabilityAlerts(config);
--
GitLab