diff --git a/README.md b/README.md
index 6a8163b..8054a3f 100644
--- a/README.md
+++ b/README.md
@@ -88,6 +88,13 @@ inputs:
rules:
- name: A single approval
min_approvals: 1
+
+# OPTIONAL: define teams or users whose reviews are not requested by the action
+prevent_review_requests:
+ users:
+ - user_name
+ teams:
+ - team_name
```
### Rules syntax
diff --git a/src/core.ts b/src/core.ts
index 663b37a..c4f5594 100644
--- a/src/core.ts
+++ b/src/core.ts
@@ -137,6 +137,7 @@ export const runChecks = async function (
"action-review-team": actionReviewTeam,
},
rules,
+ prevent_review_requests: preventReviewRequests,
} = configValidationResult.value
// Set up a teams cache so that teams used multiple times don't have to be
@@ -626,23 +627,29 @@ export const runChecks = async function (
const users: Set = new Set()
for (const [user, userInfo] of usersToAskForReview) {
if (userInfo.teams === null) {
- users.add(user)
+ if (!preventReviewRequests?.users?.includes(user)) {
+ users.add(user)
+ }
} else {
for (const team of userInfo.teams) {
- teams.add(team)
+ if (!preventReviewRequests?.teams?.includes(team)) {
+ teams.add(team)
+ }
}
}
}
- await octokit.request(
- "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
- {
- owner: pr.base.repo.owner.login,
- repo: pr.base.repo.name,
- pull_number: pr.number,
- reviewers: Array.from(users),
- team_reviewers: Array.from(teams),
- },
- )
+ if (users.size || teams.size) {
+ await octokit.request(
+ "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers",
+ {
+ owner: pr.base.repo.owner.login,
+ repo: pr.base.repo.name,
+ pull_number: pr.number,
+ reviewers: Array.from(users),
+ team_reviewers: Array.from(teams),
+ },
+ )
+ }
}
if (highestMinApprovalsRule !== null) {
diff --git a/src/types.ts b/src/types.ts
index b5399d6..dbb1244 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -84,7 +84,18 @@ export type RulesConfigurations = {
export type Configuration = {
rules: Rule[]
- inputs: Inputs
+ inputs: {
+ "locks-review-team": string
+ "team-leads-team": string
+ "action-review-team": string
+ }
+ prevent_review_requests:
+ | {
+ users: string[]
+ teams: string[]
+ }
+ | undefined
+ | null
}
export type RuleUserInfo = {
@@ -118,9 +129,3 @@ export class RuleFailure {
public usersToAskForReview: Map,
) {}
}
-
-export type Inputs = {
- "locks-review-team": string
- "team-leads-team": string
- "action-review-team": string
-}
diff --git a/src/validation.ts b/src/validation.ts
index 2150e39..eb41699 100644
--- a/src/validation.ts
+++ b/src/validation.ts
@@ -5,7 +5,6 @@ import {
AndRule,
BasicRule,
Configuration,
- Inputs,
OrRule,
RuleCriteria,
} from "./types"
@@ -78,10 +77,19 @@ const ruleSchema = Joi.alternatives([
])
export const configurationSchema = Joi.object().keys({
- inputs: Joi.object().keys({
+ inputs: Joi.object().keys({
"locks-review-team": Joi.string().required(),
"team-leads-team": Joi.string().required(),
"action-review-team": Joi.string().required(),
}),
rules: Joi.array().items(ruleSchema).required(),
+ prevent_review_requests: Joi.object<
+ Configuration["prevent_review_requests"]
+ >()
+ .keys({
+ users: Joi.array().items(Joi.string()).optional().allow(null),
+ teams: Joi.array().items(Joi.string()).optional().allow(null),
+ })
+ .optional()
+ .allow(null),
})
diff --git a/test/batch/rules.ts b/test/batch/rules.ts
index d748d19..8798933 100644
--- a/test/batch/rules.ts
+++ b/test/batch/rules.ts
@@ -26,7 +26,7 @@ import {
maxGithubApiTeamMembersPerPage,
} from "src/constants"
import { runChecks } from "src/core"
-import { Rule } from "src/types"
+import { Configuration, Rule } from "src/types"
describe("Rules", function () {
let logger: Logger
@@ -42,89 +42,94 @@ describe("Rules", function () {
teamMembers = new Map()
})
- for (const scenario of [
- "Approved",
- "Is missing approval",
- "Has no approval",
- ] as const) {
- const setup = function ({
+ const setup = function (options: {
+ users?: string[]
+ diff?: string
+ teams?: { name: string; members: string[] }[]
+ changedFiles?: string[]
+ scenario: "Approved" | "Is missing approval" | "Has no approval"
+ preventReviewRequests?: Configuration["prevent_review_requests"]
+ rules?: Configuration["rules"]
+ }) {
+ let {
users,
diff,
teams,
rules,
changedFiles,
- }: {
- users?: string[]
- diff?: string
- teams?: { name: string; members: string[] }[]
- rules?: Rule[]
- changedFiles?: string[]
- } = {}) {
- users ??= coworkers
- diff ??= condition
- teams ??= [{ name: team, members: users }]
- changedFiles ??= [condition]
- rules ??= []
+ scenario,
+ preventReviewRequests,
+ } = options
+
+ users ??= coworkers
+ diff ??= condition
+ teams ??= [{ name: team, members: users }]
+ changedFiles ??= [condition]
+ rules ??= []
+
+ nock(githubApi)
+ .get(reviewsApiPath)
+ .reply(
+ 200,
+ scenario === "Approved"
+ ? users.map(function (login, id) {
+ return { id, user: { id, login }, state: "APPROVED" }
+ })
+ : scenario === "Is missing approval"
+ ? [{ id: 1, user: { id: 1, login: coworkers[0] }, state: "APPROVED" }]
+ : [],
+ )
+ for (const { name, members } of teams) {
+ teamMembers.set(name, members)
nock(githubApi)
- .get(reviewsApiPath)
- .reply(
- 200,
- scenario === "Approved"
- ? users.map(function (login, id) {
- return { id, user: { id, login }, state: "APPROVED" }
- })
- : scenario === "Is missing approval"
- ? [
- {
- id: 1,
- user: { id: 1, login: coworkers[0] },
- state: "APPROVED",
- },
- ]
- : [],
+ .get(
+ `/orgs/${org}/teams/${name}/members?per_page=${maxGithubApiTeamMembersPerPage}`,
)
-
- for (const { name, members } of teams) {
- teamMembers.set(name, members)
- nock(githubApi)
- .get(
- `/orgs/${org}/teams/${name}/members?per_page=${maxGithubApiTeamMembersPerPage}`,
- )
- .reply(
- 200,
- members.map(function (login, id) {
- return { id, login }
- }),
- )
- }
-
- nock(githubApi)
- .get(changedFilesApiPath)
.reply(
200,
- changedFiles.map(function (filename) {
- return { filename }
+ members.map(function (login, id) {
+ return { id, login }
}),
)
+ }
- nock(githubApi)
- .get(prApiPath)
- .matchHeader("accept", "application/vnd.github.v3.diff")
- .reply(200, diff)
+ nock(githubApi)
+ .get(changedFilesApiPath)
+ .reply(
+ 200,
+ changedFiles.map(function (filename) {
+ return { filename }
+ }),
+ )
- nock(githubApi)
- .get(configFileContentsApiPath)
- .reply(200, {
- content: Buffer.from(YAML.stringify({ inputs, rules })).toString(
- "base64",
- ),
- })
- }
+ nock(githubApi)
+ .get(prApiPath)
+ .matchHeader("accept", "application/vnd.github.v3.diff")
+ .reply(200, diff)
+
+ nock(githubApi)
+ .get(configFileContentsApiPath)
+ .reply(200, {
+ content: Buffer.from(
+ YAML.stringify({
+ inputs,
+ rules,
+ prevent_review_requests: preventReviewRequests,
+ }),
+ ).toString("base64"),
+ })
+ }
+ for (const scenario of [
+ "Approved",
+ "Is missing approval",
+ "Has no approval",
+ ] as const) {
for (const checkType of ["diff", "changed_files"] as const) {
it(`${scenario} on rule including only users for ${checkType}`, async function () {
setup({
+ scenario,
rules: [
{
name: condition,
@@ -172,6 +177,7 @@ describe("Rules", function () {
it(`${scenario} on rule including only teams for ${checkType}`, async function () {
setup({
+ scenario,
rules: [
{
name: condition,
@@ -206,6 +212,7 @@ describe("Rules", function () {
const userAskedIndividually = coworkers[1]
setup({
+ scenario,
rules: [
{
name: condition,
@@ -250,6 +257,7 @@ describe("Rules", function () {
it(`${scenario} on rule not specifying users or teams`, async function () {
setup({
+ scenario,
rules: [
{
name: condition,
@@ -289,6 +297,7 @@ describe("Rules", function () {
] as const) {
it(`Rule kind ${ruleKind}: ${scenario} specifying only users for ${checkType}`, async function () {
setup({
+ scenario,
rules: [
{
name: condition,
@@ -354,6 +363,7 @@ describe("Rules", function () {
const team2 = "team2"
setup({
+ scenario,
teams: [
{
name: team1,
@@ -436,6 +446,7 @@ describe("Rules", function () {
const team2 = "team2"
setup({
+ scenario,
users: coworkers.concat(userCoworker3),
teams: [
{
@@ -535,6 +546,7 @@ describe("Rules", function () {
] as const) {
it(`${scenario} with ${description} for ${checkType}`, async function () {
setup({
+ scenario,
rules: [{ ...rule, min_approvals: 2, check_type: checkType }],
})
@@ -567,6 +579,7 @@ describe("Rules", function () {
for (const diffSign of ["+", "-"]) {
it(`${scenario} when lock line is modified (${diffSign})`, async function () {
setup({
+ scenario,
diff: `${diffSign}🔒 deleting the lock line`,
teams: [
{ name: team, members: [coworkers[0]] },
@@ -600,6 +613,7 @@ describe("Rules", function () {
it(`${scenario} when line after lock is modified (${diffSign})`, async function () {
setup({
+ scenario,
diff: `🔒 lock line\n${diffSign} modified`,
teams: [
{ name: team, members: [coworkers[0]] },
@@ -635,6 +649,7 @@ describe("Rules", function () {
for (const actionReviewFile of actionReviewTeamFiles) {
it(`${scenario} when ${actionReviewFile} is changed`, async function () {
setup({
+ scenario,
changedFiles: [actionReviewFile],
teams: [{ name: team3, members: [coworkers[1]] }],
})
@@ -665,6 +680,7 @@ describe("Rules", function () {
it(`${scenario} for AndDistinctRule when user belongs to multiple teams`, async function () {
setup({
+ scenario,
rules: [
{
name: condition,
@@ -707,6 +723,24 @@ describe("Rules", function () {
})
}
+ for (const variant of ["user", "team"]) {
+ it(`Reviews are not requested if prevent_review_requests is set for ${variant}`, async function () {
+ setup({
+ scenario: "Has no approval",
+ changedFiles: actionReviewTeamFiles,
+ teams: [{ name: inputs["action-review-team"], members: coworkers }],
+ preventReviewRequests: {
+ users: coworkers,
+ teams: Object.values(inputs),
+ },
+ })
+
+ expect(await runChecks(basePR, octokit, logger)).toBe("failure")
+
+ expect(logHistory).toMatchSnapshot()
+ })
+ }
+
afterEach(function () {
nock.cleanAll()
nock.enableNetConnect()
diff --git a/test/batch/rules.ts.snap b/test/batch/rules.ts.snap
index 5a58505..3ce8c40 100644
--- a/test/batch/rules.ts.snap
+++ b/test/batch/rules.ts.snap
@@ -660,6 +660,34 @@ Array [
]
`;
+exports[`Rules Reviews are not requested if prevent_review_requests is set for team 1`] = `
+Array [
+ "Changed files Set(1) { '.github/pr-custom-review.yml' }",
+ "latestReviews [Map Iterator] { }",
+ "usersToAskForReview Map(2) {
+ 'userCoworker' => { teams: Set(1) { 'team3' } },
+ 'userCoworker2' => { teams: Set(1) { 'team3' } }
+}",
+ "ERROR: The following problems were found:",
+ "Rule \\"Action files changed\\" needs at least 1 approvals, but 0 were matched. The following users have not approved yet: userCoworker (team: team3), userCoworker2 (team: team3).",
+ "",
+]
+`;
+
+exports[`Rules Reviews are not requested if prevent_review_requests is set for user 1`] = `
+Array [
+ "Changed files Set(1) { '.github/pr-custom-review.yml' }",
+ "latestReviews [Map Iterator] { }",
+ "usersToAskForReview Map(2) {
+ 'userCoworker' => { teams: Set(1) { 'team3' } },
+ 'userCoworker2' => { teams: Set(1) { 'team3' } }
+}",
+ "ERROR: The following problems were found:",
+ "Rule \\"Action files changed\\" needs at least 1 approvals, but 0 were matched. The following users have not approved yet: userCoworker (team: team3), userCoworker2 (team: team3).",
+ "",
+]
+`;
+
exports[`Rules Rule kind AndDistinctRule: Approved specifying both teams and users for changed_files 1`] = `
Array [
"Changed files Set(1) { 'condition' }",