diff --git a/README.md b/README.md index fb1a2b0..c4218ef 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ on: permissions: contents: read checks: write + pull-requests: write jobs: review-approvals: @@ -103,6 +104,7 @@ jobs: repo-token: ${{ github.token }} team-token: ${{ secrets.TEAM_TOKEN }} checks-token: ${{ secrets.CHECKS_TOKEN }} + request-reviewers: false pr-number: ${{ steps.number.outputs.content }} ``` Create a new PR and see if it is working. @@ -149,6 +151,10 @@ You can find all the inputs in [the action file](./action.yml), but let's walk t - You can use the same GitHub app for `checks-token` and `team-token`. - `config-file`: The location of the config file. - **default**: `.github/review-bot.yml` +- `request-reviewers`: If the system should automatically request the required reviewers. + - **default**: false. + - If enabled, when there are missing reviews, the system will request the appropriate users and/or team to review. (Note: It won't assign fellowship members as reviewers) + - If enabled, and using teams, this requires a GitHub action with `write` permission for `pull request`. #### Using a GitHub app instead of a PAT In some cases, specially in big organizations, it is more organized to use a GitHub app to authenticate, as it allows us to give it permissions per repository, and we can fine-grain them even better. If you wish to do that, you need to create a GitHub app with the following permissions: diff --git a/action.yml b/action.yml index 0f12d46..d183e1c 100644 --- a/action.yml +++ b/action.yml @@ -21,6 +21,9 @@ inputs: pr-number: description: 'The number of the pull request to review. Required if event is `workflow_run`' required: false + request-reviewers: + description: If the system should automatically request the required reviewers. + required: false outputs: repo: description: 'The name of the repo in owner/repo pattern' diff --git a/src/github/pullRequest.ts b/src/github/pullRequest.ts index 633ee03..ec26004 100644 --- a/src/github/pullRequest.ts +++ b/src/github/pullRequest.ts @@ -1,9 +1,13 @@ import { PullRequest, PullRequestReview } from "@octokit/webhooks-types"; +import { Reviewers } from "../rules/types"; import { caseInsensitiveEqual } from "../util"; import { ActionLogger, GitHubClient } from "./types"; -/** API class that uses the default token to access the data from the pull request and the repository */ +/** API class that uses the default token to access the data from the pull request and the repository + * If we are using the assign reviewers features with teams, it requires a GitHub app + * (Action token doesn't have permission to assign teams) + */ export class PullRequestApi { private readonly number: number; private readonly repoInfo: { repo: string; owner: string }; @@ -107,6 +111,27 @@ export class PullRequestApi { return approvals; } + async requestReview({ users, teams }: Pick): Promise { + if (users || teams) { + const validArray = (array: string[] | undefined): boolean => !!array && array.length > 0; + const reviewersLog = [ + validArray(users) ? `Users: ${JSON.stringify(users)}` : undefined, + validArray(teams) ? `Teams: ${JSON.stringify(teams)}` : undefined, + ] + .filter((e) => !!e) + .join(" - "); + + this.logger.info(`Requesting reviews from ${reviewersLog}`); + + await this.api.rest.pulls.requestReviewers({ + ...this.repoInfo, + pull_number: this.number, + reviewers: users, + team_reviewers: teams, + }); + } + } + /** Returns the login of the PR's author */ getAuthor(): string { return this.pr.user.login; diff --git a/src/index.ts b/src/index.ts index d7eecef..d34370e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,6 +15,8 @@ export interface Inputs { configLocation: string; /** GitHub's action default secret */ repoToken: string; + /** Should automatically request missing reviewers */ + requestReviewers?: boolean; /** A custom access token with the read:org access */ teamApiToken: string; /** Number of the PR to analyze. Optional when it is triggered by `pull_request` event */ @@ -38,10 +40,11 @@ const getRepo = (ctx: Context) => { const getInputs = (): Inputs => { const configLocation = getInput("config-file"); const repoToken = getInput("repo-token", { required: true }); + const requestReviewers = !!getInput("request-reviewers", { required: false }); const teamApiToken = getInput("team-token", { required: true }); const prNumber = getInput("pr-number"); - return { configLocation, repoToken, teamApiToken, prNumber: prNumber ? parseInt(prNumber) : null }; + return { configLocation, requestReviewers, repoToken, teamApiToken, prNumber: prNumber ? parseInt(prNumber) : null }; }; const repo = getRepo(context); diff --git a/src/runner.ts b/src/runner.ts index 151e0be..06c1b90 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -178,7 +178,10 @@ export class ActionRunner { } /** WIP - Class that will assign the requests for review */ - requestReviewers(reports: RuleReport[], preventReviewRequests: ConfigurationFile["preventReviewRequests"]): void { + async requestReviewers( + reports: RuleReport[], + preventReviewRequests: ConfigurationFile["preventReviewRequests"], + ): Promise { if (reports.length === 0) { return; } @@ -191,6 +194,8 @@ export class ActionRunner { finalReport.usersToRequest = concatArraysUniquely(finalReport.usersToRequest, report.usersToRequest); } + this.logger.debug(`Request data: ${JSON.stringify(finalReport)}`); + let { teamsToRequest, usersToRequest } = finalReport; /** @@ -214,13 +219,7 @@ export class ActionRunner { } } - const validArray = (array: string[] | undefined): boolean => !!array && array.length > 0; - const reviewersLog = [ - validArray(teamsToRequest) ? `Teams: ${JSON.stringify(teamsToRequest)}` : "", - validArray(usersToRequest) ? `Users: ${JSON.stringify(usersToRequest)}` : "", - ].join(" - "); - - this.logger.info(`Need to request reviews from ${reviewersLog}`); + await this.prApi.requestReview({ users: usersToRequest, teams: teamsToRequest }); } /** Aggregates all the reports and generate a status report @@ -556,7 +555,9 @@ export class ActionRunner { * 3. It generates a status check in the Pull Request * 4. WIP - It assigns the required reviewers to review the PR */ - async runAction(inputs: Pick): Promise & PullRequestReport> { + async runAction( + inputs: Pick, + ): Promise & PullRequestReport> { const config = await this.getConfigFile(inputs.configLocation); const prValidation = await this.validatePullRequest(config); @@ -567,7 +568,11 @@ export class ActionRunner { const checkRunData = this.generateCheckRunData(reports); await this.checks.generateCheckRun(checkRunData); - this.requestReviewers(reports, config.preventReviewRequests); + if (inputs.requestReviewers) { + await this.requestReviewers(reports, config.preventReviewRequests); + } else { + this.logger.info("'request-reviewers' is disabled. Skipping the request."); + } setOutput("report", JSON.stringify(prValidation)); diff --git a/src/test/github.test.ts b/src/test/github.test.ts index cd8d3a9..1983d6c 100644 --- a/src/test/github.test.ts +++ b/src/test/github.test.ts @@ -147,4 +147,15 @@ describe("Pull Request API Tests", () => { expect(client.rest.pulls.listFiles).toHaveBeenCalledTimes(1); }); }); + + test("Request reviewers", async () => { + await api.requestReview({ users: ["abc"], teams: ["team-abc"] }); + expect(client.rest.pulls.requestReviewers).toHaveBeenCalledWith({ + pull_number: 99, + owner: "org", + repo: pr.base.repo.name, + reviewers: ["abc"], + team_reviewers: ["team-abc"], + }); + }); }); diff --git a/src/test/runner/runner.test.ts b/src/test/runner/runner.test.ts index 0893956..66dc029 100644 --- a/src/test/runner/runner.test.ts +++ b/src/test/runner/runner.test.ts @@ -102,30 +102,27 @@ describe("Shared validations", () => { usersToRequest: ["user-1"], }; - test("should request reviewers if object is not defined", () => { - runner.requestReviewers([exampleReport], undefined); - expect(logger.info).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["team-1"]))); - expect(logger.info).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["user-1"]))); + test("should request reviewers if object is not defined", async () => { + await runner.requestReviewers([exampleReport], undefined); + expect(api.requestReview).toHaveBeenCalledWith({ users: ["user-1"], teams: ["team-1"] }); }); - test("should not request user if he is defined", () => { - runner.requestReviewers([exampleReport], { users: ["user-1"] }); + test("should not request user if he is defined", async () => { + await runner.requestReviewers([exampleReport], { users: ["user-1"] }); + expect(logger.info).toHaveBeenCalledWith("Filtering users to request a review from."); - expect(logger.info).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["team-1"]))); - expect(logger.info).not.toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["user-1"]))); + expect(api.requestReview).toHaveBeenCalledWith({ teams: ["team-1"], users: [] }); }); - test("should not request team if it is defined", () => { - runner.requestReviewers([exampleReport], { teams: ["team-1"] }); + test("should not request team if it is defined", async () => { + await runner.requestReviewers([exampleReport], { teams: ["team-1"] }); expect(logger.info).toHaveBeenCalledWith("Filtering teams to request a review from."); - expect(logger.info).not.toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["team-1"]))); - expect(logger.info).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["user-1"]))); + expect(api.requestReview).toHaveBeenCalledWith({ teams: [], users: ["user-1"] }); }); - test("should request reviewers if the team and user are not the same", () => { - runner.requestReviewers([exampleReport], { users: ["user-pi"], teams: ["team-alpha"] }); - expect(logger.info).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["team-1"]))); - expect(logger.info).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify(["user-1"]))); + test("should request reviewers if the team and user are not the same", async () => { + await runner.requestReviewers([exampleReport], { users: ["user-pi"], teams: ["team-alpha"] }); + expect(api.requestReview).toHaveBeenCalledWith({ users: ["user-1"], teams: ["team-1"] }); }); }); });