Skip to content

Commit

Permalink
Added status checks (paritytech#37)
Browse files Browse the repository at this point in the history
Added ability to generate a status check and the ability to overwrite
the latest one.

This resolves paritytech#33 and closes paritytech#26 as it merged it into one ticket.

There are some limitations as we can not select the check suite that the
check will belong (see
paritytech#33 (comment)),
but the name will be constant, making it validable.
  • Loading branch information
Bullrich authored Aug 8, 2023
1 parent 37306a9 commit a36f225
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 27 deletions.
1 change: 1 addition & 0 deletions .github/workflows/review-bot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ on:

permissions:
contents: read
checks: write

jobs:
review-approvals:
Expand Down
53 changes: 52 additions & 1 deletion src/github/pullRequest.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { summary } from "@actions/core";
import { PullRequest, PullRequestReview } from "@octokit/webhooks-types";

import { caseInsensitiveEqual } from "../util";
import { ActionLogger, GitHubClient } from "./types";
import { ActionLogger, CheckData, GitHubClient } from "./types";

/** API class that uses the default token to access the data from the pull request and the repository */
export class PullRequestApi {
Expand All @@ -11,6 +12,7 @@ export class PullRequestApi {
private readonly pr: PullRequest,
private readonly logger: ActionLogger,
private readonly repoInfo: { repo: string; owner: string },
private readonly detailsUrl: string,
) {
this.number = pr.number;
}
Expand Down Expand Up @@ -102,4 +104,53 @@ export class PullRequestApi {
getAuthor(): string {
return this.pr.user.login;
}

/**
* Generates a Check Run or modifies the existing one.
* This way we can aggregate all the results from different causes into a single one
* {@link https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28}
* @param checkResult a CheckData object with the final conclussion of action and the output text
* {@link CheckData}
*/
async generateCheckRun(checkResult: CheckData): Promise<void> {
const checkData = {
...checkResult,
owner: this.repoInfo.owner,
repo: this.repoInfo.repo,
external_id: "review-bot",
head_sha: this.pr.head.sha,
name: "review-bot",
};

const { data } = await this.api.rest.checks.listForRef({
owner: this.repoInfo.owner,
repo: this.repoInfo.repo,
ref: this.pr.head.sha,
});

this.logger.debug(`Searching for a match for id ${checkData.external_id}. Found ${data.total_count} checks`);

for (const check of data.check_runs) {
if (check.external_id === checkData.external_id) {
this.logger.debug(`Found match: ${JSON.stringify(check)}`);
await this.api.rest.checks.update({ ...checkData, check_run_id: check.id });
this.logger.debug("Updated check data");
return;
}
}

this.logger.debug("Did not find any matching status check. Creating a new one");

const check = await this.api.rest.checks.create(checkData);

// We publish it in the action summary
await summary
.addHeading(checkResult.output.title)
// We redirect to the check as it can changed if it is triggered again
.addLink("Find the result here", check.data.html_url ?? "")
.addRaw(checkResult.output.text)
.write();

this.logger.debug(JSON.stringify(check.data));
}
}
9 changes: 9 additions & 0 deletions src/github/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,12 @@ export interface ActionLogger {
}

export type GitHubClient = InstanceType<typeof GitHub>;

export interface CheckData {
conclusion: "action_required" | "failure" | "success";
output: {
title: string;
summary: string;
text: string;
};
}
14 changes: 8 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,14 @@ debug("Got payload:" + JSON.stringify(context.payload.pull_request));

const inputs = getInputs();

const actionId = `${context.serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${context.runId}`;

const api = new PullRequestApi(
getOctokit(inputs.repoToken),
context.payload.pull_request as PullRequest,
generateCoreLogger(),
repo,
actionId,
);

const logger = generateCoreLogger();
Expand All @@ -66,10 +69,9 @@ const runner = new ActionRunner(api, teamApi, logger);
runner
.runAction(inputs)
.then((result) => {
if (result) {
info("Action completed succesfully");
} else {
setFailed("Action failed");
}
info(`Action run without problem. Evaluation result was '${result.conclusion}'`);
})
.catch(setFailed);
.catch((error) => {
console.error(error);
setFailed(error as Error | string);
});
91 changes: 71 additions & 20 deletions src/runner.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { summary } from "@actions/core";
import { parse } from "yaml";

import { Inputs } from ".";
import { ConfigurationFile, Reviewers, Rule } from "./file/types";
import { validateConfig, validateRegularExpressions } from "./file/validator";
import { PullRequestApi } from "./github/pullRequest";
import { TeamApi } from "./github/teams";
import { ActionLogger } from "./github/types";
import { ActionLogger, CheckData } from "./github/types";
import { concatArraysUniquely } from "./util";

type ReviewReport = {
Expand All @@ -18,6 +19,9 @@ type ReviewReport = {
/** If applicable, the users that should be requested to review */
usersToRequest?: string[];
};

type RuleReport = { name: string } & ReviewReport;

type ReviewState = [true] | [false, ReviewReport];

/** Action in charge of running the GitHub action */
Expand Down Expand Up @@ -51,10 +55,10 @@ export class ActionRunner {

/**
* The action evaluates if the rules requirements are meet for a PR
* @returns a true/false statement if the rule failed. This WILL BE CHANGED for an object with information (see issue #26)
* @returns an array of error reports for each failed rule. An empty array means no errors
*/
async validatePullRequest({ rules }: ConfigurationFile): Promise<boolean> {
const errorReports: ReviewReport[] = [];
async validatePullRequest({ rules }: ConfigurationFile): Promise<RuleReport[]> {
const errorReports: RuleReport[] = [];
for (const rule of rules) {
try {
this.logger.info(`Validating rule '${rule.name}'`);
Expand All @@ -70,7 +74,7 @@ export class ActionRunner {
const [result, missingData] = await this.evaluateCondition(rule);
if (!result) {
this.logger.error(`Missing the reviews from ${JSON.stringify(missingData.missingUsers)}`);
errorReports.push(missingData);
errorReports.push({ ...missingData, name: rule.name });
}
}
} catch (error: unknown) {
Expand All @@ -80,18 +84,14 @@ export class ActionRunner {
}
this.logger.info(`Finish validating '${rule.name}'`);
}
if (errorReports.length > 0) {
const finalReport = this.aggregateReports(errorReports);
// Preview, this will be improved in a future commit
this.logger.warn(`Missing reviews: ${JSON.stringify(finalReport)}`);
return false;
}
// TODO: Convert this into a list of users/teams missing and convert the output into a nice summary object -> Issue #26
return true;
return errorReports;
}

/** Aggregates all the reports and generate a final combined one */
aggregateReports(reports: ReviewReport[]): ReviewReport {
/** WIP - Class that will assign the requests for review */
requestReviewers(reports: RuleReport[]): void {
if (reports.length === 0) {
return;
}
const finalReport: ReviewReport = { missingReviews: 0, missingUsers: [], teamsToRequest: [], usersToRequest: [] };

for (const report of reports) {
Expand All @@ -101,7 +101,47 @@ export class ActionRunner {
finalReport.usersToRequest = concatArraysUniquely(finalReport.usersToRequest, report.usersToRequest);
}

return finalReport;
const { teamsToRequest, usersToRequest } = finalReport;
const reviewersLog = [
teamsToRequest ? `Teams: ${JSON.stringify(teamsToRequest)} - ` : "",
usersToRequest ? `Users: ${JSON.stringify(usersToRequest)}` : "",
].join(" - ");

this.logger.info(`Need to request reviews from ${reviewersLog}`);
}

/** Aggregates all the reports and generate a status report */
generateCheckRunData(reports: RuleReport[]): CheckData {
// Count how many reviews are missing
const missingReviews = reports.reduce((a, b) => a + b.missingReviews, 0);
const failed = missingReviews > 0;
const check: CheckData = {
conclusion: failed ? "failure" : "success",
output: {
title: failed ? `Missing ${missingReviews} reviews` : "All required reviews fulfilled",
summary: failed ? "# The following rules have failed:\n" : "All neccesary users have reviewed the PR",
text: failed ? "Details per rule:\n" : "",
},
};

if (!failed) {
return check;
}

for (const report of reports) {
check.output.summary += `- **${report.name}**\n`;
let text = summary.addHeading(report.name, 2).addHeading(`Missing ${report.missingReviews} reviews`, 4);
if (report.usersToRequest && report.usersToRequest.length > 0) {
text = text.addHeading("Missing users", 3).addList(report.usersToRequest);
}
if (report.teamsToRequest && report.teamsToRequest.length > 0) {
text = text.addHeading("Missing reviews from teams", 3).addList(report.teamsToRequest);
}

check.output.text += text.stringify() + "\n";
}

return check;
}

/** Evaluates if the required reviews for a condition have been meet
Expand Down Expand Up @@ -201,13 +241,24 @@ export class ActionRunner {
return matches;
}

async runAction(inputs: Omit<Inputs, "repoToken">): Promise<boolean> {
/** Core runner of the app.
* 1. It fetches the config
* 2. It validates all the pull request requirements based on the config file
* 3. It generates a status check in the Pull Request
* 4. WIP - It assigns the required reviewers to review the PR
*/
async runAction(inputs: Omit<Inputs, "repoToken">): Promise<CheckData> {
const config = await this.getConfigFile(inputs.configLocation);

const success = await this.validatePullRequest(config);
const reports = await this.validatePullRequest(config);

this.logger.info(reports.length > 0 ? "There was an error with the PR reviews." : "The PR has been successful");

const checkRunData = this.generateCheckRunData(reports);
await this.prApi.generateCheckRun(checkRunData);

this.logger.info(success ? "The PR has been successful" : "There was an error with the PR reviews.");
this.requestReviewers(reports);

return success;
return checkRunData;
}
}

0 comments on commit a36f225

Please sign in to comment.