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.

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

feat: ignore unavailable users #9406

Merged
merged 32 commits into from Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
9a0cb6b
feat: ignore unavailable users
fgreinacher Apr 5, 2021
42f282d
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 5, 2021
c5817da
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 14, 2021
f7a2207
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 15, 2021
d044ceb
filter users via platform
fgreinacher Apr 15, 2021
9c59d35
add config option
fgreinacher Apr 15, 2021
9271fe7
implement filterUnavailableUsers for GitLab
fgreinacher Apr 15, 2021
ac8cac8
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 18, 2021
b0431ba
Update docs/usage/configuration-options.md
fgreinacher Apr 18, 2021
12af97a
Rename filterUnavailableUsers to filterOutUnavailableUsers
fgreinacher Apr 18, 2021
8b57588
Merge branch 'feat/filter-unavailable-users' of https://github.com/fg…
fgreinacher Apr 18, 2021
3fb2649
Update docs/usage/configuration-options.md
fgreinacher Apr 19, 2021
e9b23f2
Move isUserBusy and related types to appropriate files
fgreinacher Apr 19, 2021
9c1a659
Merge branch 'feat/filter-unavailable-users' of https://github.com/fg…
fgreinacher Apr 19, 2021
d1a1c7a
Update lib/platform/gitlab/http.ts
viceice Apr 19, 2021
013691e
Merge branch 'master' into feat/filter-unavailable-users
viceice Apr 19, 2021
e1edc49
Warn when enabling unsupported option
fgreinacher Apr 19, 2021
35bb528
Merge branch 'feat/filter-unavailable-users' of https://github.com/fg…
fgreinacher Apr 19, 2021
100b97c
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 21, 2021
9c19517
Rename filterOutUnavailableUsers to filterUnavailableUsers
fgreinacher Apr 21, 2021
c3c601c
Merge branch 'feat/filter-unavailable-users' of https://github.com/fg…
fgreinacher Apr 21, 2021
cbd5430
Assert warnings
fgreinacher Apr 21, 2021
6881ce4
Update snapshots to accomodate for http layer changes
fgreinacher Apr 21, 2021
3af5ff6
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 21, 2021
e641e7c
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 21, 2021
167dbeb
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 22, 2021
6962a6f
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 22, 2021
544ca07
Merge branch 'master' into feat/filter-unavailable-users
fgreinacher Apr 22, 2021
f25cd46
Merge branch 'master' into feat/filter-unavailable-users
viceice Apr 22, 2021
6d4aa14
Apply suggestions from code review
fgreinacher Apr 22, 2021
f25f0a6
fix assertion
fgreinacher Apr 22, 2021
591c84b
Merge branch 'main' into feat/filter-unavailable-users
fgreinacher Apr 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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/).

## filterOutUnavailableUsers
fgreinacher marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -1505,6 +1505,12 @@ const options: RenovateOptions[] = [
type: 'boolean',
default: false,
},
{
name: 'filterOutUnavailableUsers',
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 @@ -206,6 +206,7 @@ export interface AssigneesAndReviewersConfig {
reviewers?: string[];
reviewersSampleSize?: number;
additionalReviewers?: string[];
filterOutUnavailableUsers?: 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",
"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",
"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",
"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",
"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.filterOutUnavailableUsers = true;
platform.filterUnavailableUsers = jest.fn();
viceice marked this conversation as resolved.
Show resolved Hide resolved
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.filterOutUnavailableUsers && platform.filterUnavailableUsers
fgreinacher marked this conversation as resolved.
Show resolved Hide resolved
? 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
13 changes: 12 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,16 @@ 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({
filterOutUnavailableUsers: true,
});
config.mergeRenovateConfig.mockResolvedValueOnce({});
secrets.applySecretsToConfig.mockReturnValueOnce({} as never);
await initRepo({});
expect(logger.logger.warn).toHaveBeenCalled();
fgreinacher marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
10 changes: 10 additions & 0 deletions lib/workers/repository/init/index.ts
@@ -1,6 +1,7 @@
import { RenovateConfig } from '../../../config';
import { applySecretsToConfig } from '../../../config/secrets';
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.filterOutUnavailableUsers && !platform.filterUnavailableUsers) {
logger.warn(
`Configuration option 'filterOutUnavailableUsers' 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