Skip to content

Commit

Permalink
feat: ignore unavailable users (#9406)
Browse files Browse the repository at this point in the history
  • Loading branch information
fgreinacher committed Apr 22, 2021
1 parent 8c0f7c1 commit 2cc751a
Show file tree
Hide file tree
Showing 14 changed files with 232 additions and 2 deletions.
6 changes: 6 additions & 0 deletions docs/usage/configuration-options.md
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions lib/config/definitions.ts
Expand Up @@ -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.',
Expand Down
1 change: 1 addition & 0 deletions lib/config/types.ts
Expand Up @@ -207,6 +207,7 @@ export interface AssigneesAndReviewersConfig {
reviewers?: string[];
reviewersSampleSize?: number;
additionalReviewers?: string[];
filterUnavailableUsers?: boolean;
}

export type UpdateType =
Expand Down
77 changes: 77 additions & 0 deletions lib/platform/gitlab/__snapshots__/index.spec.ts.snap
Expand Up @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions 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();

Expand All @@ -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;
}
}
40 changes: 40 additions & 0 deletions lib/platform/gitlab/index.spec.ts
Expand Up @@ -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();
});
});
});
14 changes: 13 additions & 1 deletion lib/platform/gitlab/index.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
8 changes: 8 additions & 0 deletions lib/platform/gitlab/types.ts
Expand Up @@ -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';
}
1 change: 1 addition & 0 deletions lib/platform/types.ts
Expand Up @@ -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[]>;
}
22 changes: 22 additions & 0 deletions lib/workers/pr/__snapshots__/index.spec.ts.snap
Expand Up @@ -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).",
Expand Down
10 changes: 10 additions & 0 deletions lib/workers/pr/index.spec.ts
Expand Up @@ -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']);
Expand Down
11 changes: 11 additions & 0 deletions lib/workers/pr/index.ts
Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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))];
Expand Down
15 changes: 14 additions & 1 deletion 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';
Expand Down Expand Up @@ -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'."
);
});
});
});
10 changes: 10 additions & 0 deletions 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';
Expand All @@ -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> {
Expand All @@ -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);
Expand Down

0 comments on commit 2cc751a

Please sign in to comment.