Added required score grouping to fellows reviews

package.json

@@ -39,7 +39,7 @@
     "@actions/github": "^6.0.0",
     "@eng-automation/js": "^1.0.2",
     "@polkadot/api": "^10.11.2",
-    "joi": "^17.12.0",
+    "joi": "^17.6.4",
     "yaml": "^2.3.4"
   }
 }

src/failures/fellowsRules.ts

@@ -42,3 +42,64 @@ export class FellowMissingRankFailure extends ReviewFailure {
     return { users: [], teams: [] };
   }
 }
+
+export class FellowMissingScoreFailure extends ReviewFailure {
+  public readonly currentScore: number;
+  constructor(
+    ruleInfo: Omit<RuleFailedSummary, "missingUsers" | "countingReviews" | "missingReviews">,
+    public readonly requiredScore: number,
+    approvalsWithScores: [string, number][],
+    missingFellowsWithScore: [string, number][],
+  ) {
+    const unifyFellowWithScore = ([handle, score]: [string, number]) => `${handle} => **${score}**`;
+    super({
+      ...ruleInfo,
+      countingReviews: approvalsWithScores.map(unifyFellowWithScore),
+      missingUsers: missingFellowsWithScore.map(unifyFellowWithScore),
+      missingReviews: 1,
+    });
+
+    this.currentScore = approvalsWithScores.reduce((n, [_, score]) => n + score, 0);
+  }
+
+  generateSummary(): typeof summary {
+    let text = summary
+      .emptyBuffer()
+      .addHeading(this.name, 2)
+      .addHeading("Missing minimum required score from Fellows", 4)
+      .addDetails(
+        "Rule explanation",
+        "Rule 'Fellows' gives every fellow a score based on their rank, and required that the sum of all the scores is greater than the required score." +
+          "\n\n" +
+          "For more info found out how the rules work in [Review-bot types](/~https://github.com/paritytech/review-bot#types)",
+      );
+
+    text = text
+      .addHeading(`Missing a score of ${this.requiredScore}`, 3)
+      .addEOL()
+      .addRaw(`Missing ${this.requiredScore - this.currentScore} in the required score.`)
+      .addEOL()
+      .addRaw(`Current score is ${this.currentScore}/${this.requiredScore}`);
+    if (this.missingUsers.length > 0)
+      text = text.addDetails(
+        "GitHub users whose approval counts",
+        `This is a list of all the Fellows that have not reviewed with their current scores:\n\n - ${this.missingUsers
+          .map(toHandle)
+          .join("\n - ")}`,
+      );
+
+    if (this.countingReviews.length > 0) {
+      text = text
+        .addHeading("Users approvals that counted towards this rule with their scores", 3)
+        .addEOL()
+        .addList(this.countingReviews.map(toHandle))
+        .addEOL();
+    }
+
+    return text;
+  }
+
+  getRequestLogins(): { users: string[]; teams: string[] } {
+    return { users: [], teams: [] };
+  }
+}

src/polkadot/fellows.ts

@@ -10,7 +10,7 @@ export class PolkadotFellows implements TeamApi {
 
   constructor(private readonly logger: ActionLogger) {}
 
-  async fetchAllFellows(): Promise<Map<string, number>> {
+  private async fetchAllFellows(): Promise<Map<string, number>> {
     let api: ApiPromise;
     this.logger.debug("Connecting to collective parachain");
     // we connect to the collective rpc node
@@ -91,6 +91,18 @@ export class PolkadotFellows implements TeamApi {
     }
   }
 
+  /** Returns all the fellows with their rankings */
+  async listFellows(): Promise<[string, number][]> {
+    this.logger.info("Fetching all fellows with their ranks");
+
+    if (this.fellowsCache.size < 1) {
+      this.logger.debug("Cache not found. Fetching fellows.");
+      this.fellowsCache = await this.fetchAllFellows();
+    }
+
+    return Array.from(this.fellowsCache.entries());
+  }
+
   async getTeamMembers(ranking: string): Promise<string[]> {
     const requiredRank = Number(ranking);
     this.logger.info(`Fetching members of rank '${requiredRank}' or higher`);

src/rules/types.ts

@@ -38,6 +38,19 @@ export interface FellowsRule extends Rule {
   type: RuleTypes.Fellows;
   minRank: number;
   minApprovals: number;
+  minScore?: number;
+}
+
+export interface FellowsScore {
+  dan1: number;
+  dan2: number;
+  dan3: number;
+  dan4: number;
+  dan5: number;
+  dan6: number;
+  dan7: number;
+  dan8: number;
+  dan9: number;
 }
 
 export interface ConfigurationFile {
@@ -49,4 +62,5 @@ export interface ConfigurationFile {
     teams?: string[];
     users?: string[];
   };
+  score?: FellowsScore;
 }

src/rules/validator.ts

@@ -2,7 +2,7 @@ import { validate } from "@eng-automation/js";
 import Joi from "joi";
 
 import { ActionLogger } from "../github/types";
-import { AndRule, BasicRule, ConfigurationFile, FellowsRule, Reviewers, Rule, RuleTypes } from "./types";
+import { AndRule, BasicRule, ConfigurationFile, FellowsRule, FellowsScore, Reviewers, Rule, RuleTypes } from "./types";
 
 /** For the users or team schema. Will be recycled A LOT
  * Remember to add `.or("users", "teams")` to force at least one of the two to be defined
@@ -30,13 +30,27 @@ const ruleSchema = Joi.object<Rule & { type: string }>().keys({
     .required(),
 });
 
+/** Schema for ensuring that all the dan ranks are set properly */
+export const fellowScoreSchema = Joi.object<FellowsScore>().keys({
+  dan1: Joi.number().default(0),
+  dan2: Joi.number().default(0),
+  dan3: Joi.number().default(0),
+  dan4: Joi.number().default(0),
+  dan5: Joi.number().default(0),
+  dan6: Joi.number().default(0),
+  dan7: Joi.number().default(0),
+  dan8: Joi.number().default(0),
+  dan9: Joi.number().default(0),
+});
+
 /** General Configuration schema.
  * Evaluates all the upper level field plus the generic rules fields.
  * Remember to evaluate the rules with their custom rules
  */
 export const generalSchema = Joi.object<ConfigurationFile>().keys({
   rules: Joi.array<ConfigurationFile["rules"]>().items(ruleSchema).unique("name").required(),
   preventReviewRequests: Joi.object().keys(reviewersObj).optional().or("users", "teams"),
+  score: fellowScoreSchema,
 });
 
 /** Basic rule schema
@@ -59,6 +73,7 @@ export const fellowsRuleSchema = Joi.object<FellowsRule>().keys({
   countAuthor: Joi.boolean().default(false),
   minRank: Joi.number().required().min(1).empty(null),
   minApprovals: Joi.number().min(1).default(1),
+  minScore: Joi.number().min(1).optional(),
 });
 
 /**

src/runner.ts

@@ -5,6 +5,7 @@ import { Inputs } from ".";
 import {
   CommonRuleFailure,
   FellowMissingRankFailure,
+  FellowMissingScoreFailure,
   RequiredReviewersData,
   ReviewFailure,
   RuleFailedReport,
@@ -13,9 +14,18 @@ import {
 import { GitHubChecksApi } from "./github/check";
 import { PullRequestApi } from "./github/pullRequest";
 import { ActionLogger, CheckData, TeamApi } from "./github/types";
-import { AndDistinctRule, ConfigurationFile, FellowsRule, Reviewers, Rule, RuleTypes } from "./rules/types";
+import { PolkadotFellows } from "./polkadot/fellows";
+import {
+  AndDistinctRule,
+  ConfigurationFile,
+  FellowsRule,
+  FellowsScore,
+  Reviewers,
+  Rule,
+  RuleTypes,
+} from "./rules/types";
 import { validateConfig, validateRegularExpressions } from "./rules/validator";
-import { concatArraysUniquely } from "./util";
+import { concatArraysUniquely, rankToScore } from "./util";
 
 type BaseRuleReport = RuleFailedReport & RequiredReviewersData;
 
@@ -31,7 +41,7 @@ export class ActionRunner {
   constructor(
     private readonly prApi: PullRequestApi,
     private readonly teamApi: TeamApi,
-    private readonly polkadotApi: TeamApi,
+    private readonly polkadotApi: PolkadotFellows,
     private readonly checks: GitHubChecksApi,
     private readonly logger: ActionLogger,
   ) {}
@@ -61,7 +71,7 @@ export class ActionRunner {
    * The action evaluates if the rules requirements are meet for a PR
    * @returns an array of error reports for each failed rule. An empty array means no errors
    */
-  async validatePullRequest({ rules }: ConfigurationFile): Promise<PullRequestReport> {
+  async validatePullRequest({ rules, score }: ConfigurationFile): Promise<PullRequestReport> {
     const modifiedFiles = await this.prApi.listModifiedFiles();
 
     const errorReports: ReviewFailure[] = [];
@@ -150,7 +160,7 @@ export class ActionRunner {
             break;
           }
           case RuleTypes.Fellows: {
-            const fellowReviewError = await this.fellowsEvaluation(rule);
+            const fellowReviewError = await this.fellowsEvaluation(rule, score);
             if (fellowReviewError) {
               this.logger.error(`Missing the reviews from ${JSON.stringify(fellowReviewError.missingReviews)}`);
               // errorReports.push({ ...missingData, name: rule.name, type: rule.type });
@@ -440,7 +450,7 @@ export class ActionRunner {
     }
   }
 
-  async fellowsEvaluation(rule: FellowsRule): Promise<ReviewFailure | null> {
+  async fellowsEvaluation(rule: FellowsRule, scores?: FellowsScore): Promise<ReviewFailure | null> {
     // This is a list of all the users that need to approve a PR
     const requiredUsers: string[] = await this.polkadotApi.getTeamMembers(rule.minRank.toString());
 
@@ -472,9 +482,10 @@ export class ActionRunner {
       }
     }
 
+    const author = this.prApi.getAuthor();
+
     // Now we verify if we have any remaining missing review.
     if (missingReviews > 0) {
-      const author = this.prApi.getAuthor();
       this.logger.warn(`${missingReviews} reviews are missing.`);
       // If we have at least one missing review, we return an object with the list of missing reviewers, and
       // which users/teams we should request to review
@@ -487,11 +498,45 @@ export class ActionRunner {
         },
         rule.minRank,
       );
-    } else {
-      this.logger.info("Rule requirements fulfilled");
-      // If we don't have any missing reviews, we return no error
-      return null;
+      // Then we verify if we need to have a minimum score
+    } else if (rule.minScore && scores) {
+      // We get all the fellows with their ranks and convert them to their score
+      const fellows: [string, number][] = (await this.polkadotApi.listFellows()).map(([handle, rank]) => [
+        handle,
+        rankToScore(rank, scores),
+      ]);
+      let score = 0;
+
+      const countingFellows: [string, number][] = [];
+
+      // We iterate over all the approvals and convert their rank to their score
+      for (const [handle, fellowScore] of fellows) {
+        // We filter fellows whose score is 0
+        if (approvals.indexOf(handle) > -1 && fellowScore > 0) {
+          score += fellowScore;
+          countingFellows.push([handle, fellowScore]);
+        }
+      }
+
+      this.logger.debug(`Current score is ${score} and the minimum required score is ${rule.minScore}`);
+
+      if (rule.minScore > score) {
+        const missingUsers = fellows
+          // Remove all the fellows who score is worth 0
+          .filter(([_, fellowScore]) => fellowScore > 0)
+          // Remove the author
+          .filter(([handle]) => handle != author)
+          // Remove the approvals
+          .filter(([handle]) => approvals.indexOf(handle) < 0);
+
+        this.logger.warn(`Missing score of ${rule.minScore} by ${score - rule.minScore}`);
+
+        return new FellowMissingScoreFailure(rule, rule.minScore, countingFellows, missingUsers);
+      }
     }
+    this.logger.info("Rule requirements fulfilled");
+    // If we don't have any missing reviews, we return no error
+    return null;
   }
 
   /** Using the include and exclude condition, it returns a list of all the files in a PR that matches the criteria */

src/test/index.test.ts

@@ -9,6 +9,7 @@ import { GitHubChecksApi } from "../github/check";
 import { PullRequestApi } from "../github/pullRequest";
 import { GitHubTeamsApi } from "../github/teams";
 import { ActionLogger, GitHubClient, TeamApi } from "../github/types";
+import { PolkadotFellows } from "../polkadot/fellows";
 import { ActionRunner } from "../runner";
 
 type ReportName =
@@ -83,7 +84,7 @@ describe("Integration testing", () => {
     api = new PullRequestApi(client, pr, logger);
     teams = new GitHubTeamsApi(client, "org", logger);
     checks = new GitHubChecksApi(client, pr, logger, "example");
-    runner = new ActionRunner(api, teams, mock<TeamApi>(), checks, logger);
+    runner = new ActionRunner(api, teams, mock<PolkadotFellows>(), checks, logger);
 
     // @ts-ignore problem with the type being mocked
     client.rest.repos.getContent.mockResolvedValue({ data: { content: Buffer.from(config, "utf-8") } });

src/test/rules/andDistinct.test.ts

@@ -5,17 +5,18 @@ import { mock, MockProxy } from "jest-mock-extended";
 
 import { GitHubChecksApi } from "../../github/check";
 import { PullRequestApi } from "../../github/pullRequest";
-import { ActionLogger, TeamApi } from "../../github/types";
+import { ActionLogger } from "../../github/types";
+import { PolkadotFellows } from "../../polkadot/fellows";
 import { AndDistinctRule } from "../../rules/types";
 import { ActionRunner } from "../../runner";
 
 describe("'AndDistinctRule' rule parsing", () => {
   let api: MockProxy<PullRequestApi>;
   let runner: ActionRunner;
-  let teamsApi: MockProxy<TeamApi>;
+  let teamsApi: MockProxy<PolkadotFellows>;
   beforeEach(() => {
     api = mock<PullRequestApi>();
-    runner = new ActionRunner(api, teamsApi, mock<TeamApi>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
+    runner = new ActionRunner(api, teamsApi, mock<PolkadotFellows>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
   });
   test("should get minimal config", async () => {
     api.getConfigFile.mockResolvedValue(`

src/test/rules/andRule.test.ts

@@ -5,17 +5,18 @@ import { mock, MockProxy } from "jest-mock-extended";
 
 import { GitHubChecksApi } from "../../github/check";
 import { PullRequestApi } from "../../github/pullRequest";
-import { ActionLogger, TeamApi } from "../../github/types";
+import { ActionLogger } from "../../github/types";
+import { PolkadotFellows } from "../../polkadot/fellows";
 import { AndRule } from "../../rules/types";
 import { ActionRunner } from "../../runner";
 
 describe("'And' rule parsing", () => {
   let api: MockProxy<PullRequestApi>;
   let runner: ActionRunner;
-  let teamsApi: MockProxy<TeamApi>;
+  let teamsApi: MockProxy<PolkadotFellows>;
   beforeEach(() => {
     api = mock<PullRequestApi>();
-    runner = new ActionRunner(api, teamsApi, mock<TeamApi>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
+    runner = new ActionRunner(api, teamsApi, mock<PolkadotFellows>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
   });
   test("should get minimal config", async () => {
     api.getConfigFile.mockResolvedValue(`

src/test/rules/basicRule.test.ts

@@ -5,17 +5,18 @@ import { mock, MockProxy } from "jest-mock-extended";
 
 import { GitHubChecksApi } from "../../github/check";
 import { PullRequestApi } from "../../github/pullRequest";
-import { ActionLogger, TeamApi } from "../../github/types";
+import { ActionLogger } from "../../github/types";
+import { PolkadotFellows } from "../../polkadot/fellows";
 import { BasicRule } from "../../rules/types";
 import { ActionRunner } from "../../runner";
 
 describe("Basic rule parsing", () => {
   let api: MockProxy<PullRequestApi>;
   let runner: ActionRunner;
-  let teamsApi: MockProxy<TeamApi>;
+  let teamsApi: MockProxy<PolkadotFellows>;
   beforeEach(() => {
     api = mock<PullRequestApi>();
-    runner = new ActionRunner(api, teamsApi, mock<TeamApi>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
+    runner = new ActionRunner(api, teamsApi, mock<PolkadotFellows>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
   });
   test("should get minimal config", async () => {
     api.getConfigFile.mockResolvedValue(`

src/test/rules/config.test.ts

@@ -6,6 +6,7 @@ import { mock, MockProxy } from "jest-mock-extended";
 import { GitHubChecksApi } from "../../github/check";
 import { PullRequestApi } from "../../github/pullRequest";
 import { ActionLogger, TeamApi } from "../../github/types";
+import { PolkadotFellows } from "../../polkadot/fellows";
 import { RuleTypes } from "../../rules/types";
 import { ActionRunner } from "../../runner";
 
@@ -17,7 +18,7 @@ describe("Config Parsing", () => {
   beforeEach(() => {
     logger = mock<ActionLogger>();
     api = mock<PullRequestApi>();
-    runner = new ActionRunner(api, teamsApi, mock<TeamApi>(), mock<GitHubChecksApi>(), logger);
+    runner = new ActionRunner(api, teamsApi, mock<PolkadotFellows>(), mock<GitHubChecksApi>(), logger);
   });
   test("should get minimal config", async () => {
     api.getConfigFile.mockResolvedValue(`