Skip to content

Commit

Permalink
Fellow members fetching (#79)
Browse files Browse the repository at this point in the history
Created the method to fetch the fellows from the parachain using
Polkadot-JS. This will be updated to the new library once it becomes
available.

The system fetches all the available fellows with a GitHub handle and
caches them. When requesting a fellow, it automatically filters the one
it needs and returns an array of handles.

This resolves #61

Co-authored-by: Yuri Volkov <mutantcornholio@users.noreply.github.com>
Co-authored-by: Maksym Hlukhovtsov <mordamax@users.noreply.github.com>
Co-authored-by: Przemek Rzad <roopert7@gmail.com>
  • Loading branch information
4 people authored Sep 15, 2023
1 parent 34a473c commit 29b16b5
Show file tree
Hide file tree
Showing 24 changed files with 813 additions and 65 deletions.
19 changes: 14 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ rules:
users:
- user-1
- user-2
minFellowsRank: 2
countAuthor: true
allowedToSkipRule:
teams:
Expand All @@ -256,9 +257,13 @@ It has the same parameters as a normal rule:
- **Optional**: Defaults to 1.
- Must be greater than the number of users available (you cannot request 5 approvals from a team of 4 users)
- **teams**: An array of team *slugs* that need to review this file.
- *Optional if **users** is defined*.
- *Optional if **minFellowsRank** or **users** are defined*.
- **users**: An array of the GitHub usernames of the users who need to review this file.
- *Optional if **teams** is defined*.
- *Optional if **teams** or **minFellowsRank** are defined*.
- **minFellowsRank**: A number defining the minimum [minFellowsRank a fellow](https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama.api.onfinality.io%2Fpublic-ws#/fellowship) needs to have to approve this PR.
- *Optional if **teams** or **users** are defined*.
- If it is set to minFellowsRank 3, it will need the approval of a fellow of minFellowsRank 3 or higher to pass.
- The username is fetched [from the additional field](/~https://github.com/polkadot-fellows/runtimes/issues/7) in the identity of the fellows. If it is null, the fellow’s review won’t count towards the required approval.
- **countAuthor**: If the pull request author should be considered as an approval.
- If the author belongs to the list of approved users (either by team or by users) his approval will be counted (requiring one less approvals in total).
- ** Optional**: Defaults to `false`
Expand Down Expand Up @@ -303,9 +308,13 @@ rules:
- **Optional**: Defaults to 1.
- Must be greater than the number of users available (you cannot request 5 approvals from a team of 4 users)
- **teams**: An array of team *slugs* that need to review this file.
- *Optional if **users** is defined*.
- *Optional if **minFellowsRank** or **users** are defined*.
- **users**: An array of the GitHub usernames of the users who need to review this file.
- *Optional if **teams** is defined*.
- *Optional if **teams** or **minFellowsRank** are defined*.
- **minFellowsRank**: A number defining the minimum [minFellowsRank a fellow](https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama.api.onfinality.io%2Fpublic-ws#/fellowship) needs to have to approve this PR.
- *Optional if **teams** or **users** are defined*.
- If it is set to minFellowsRank 3, it will need the approval of a fellow of minFellowsRank 3 or higher to pass.
- The username is fetched [from the additional field](/~https://github.com/polkadot-fellows/runtimes/issues/7) in the identity of the fellows. If it is null, the fellow’s review won’t count towards the required approval.
##### Or rule logic
This is a rule that has at least two available options of reviewers and needs **at least one group to approve**.

Expand Down Expand Up @@ -381,4 +390,4 @@ To deploy a new version you need to update two files:

When a commit is pushed to the main branch and the versions have changed, the system will automatically tag the commit and release a new package with such version.

You can find all the available versions in the [release section](./releases).
You can find all the available versions in the [release section](../releases).
7 changes: 6 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,7 @@
/** @type {import('ts-jest').JestConfigWithTsJest} */
module.exports = { preset: "ts-jest", testEnvironment: "node", testMatch: [__dirname + "/src/**/test/**/*.test.ts"] };
module.exports = {
preset: "ts-jest",
testEnvironment: "node",
testTimeout: 8_000,
testMatch: [__dirname + "/src/**/test/**/*.test.ts"],
};
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"@actions/core": "^1.10.0",
"@actions/github": "^5.1.1",
"@eng-automation/js": "^1.0.2",
"@polkadot/api": "^10.9.1",
"joi": "^17.10.0",
"yaml": "^2.3.2"
}
Expand Down
11 changes: 1 addition & 10 deletions src/github/teams.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import { ActionLogger, GitHubClient } from "./types";

/**
* Interface for the acquisition of members of a team.
* As we may be using blockchain instead of GitHub teams, let's wrap the functionality inside a interface
*/
export interface TeamApi {
/** Returns all the GitHub account's logins which belong to a given team. */
getTeamMembers(teamName: string): Promise<string[]>;
}
import { ActionLogger, GitHubClient, TeamApi } from "./types";

/**
* Implementation of the TeamApi interface using GitHub teams
Expand Down
9 changes: 9 additions & 0 deletions src/github/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import type { GitHub } from "@actions/github/lib/utils";

/**
* Interface for the acquisition of members of a team.
* As we may be using blockchain instead of GitHub teams, let's wrap the functionality inside a interface
*/
export interface TeamApi {
/** Returns all the GitHub account's logins which belong to a given team. */
getTeamMembers(teamName: string): Promise<string[]>;
}

export interface ActionLogger {
debug(message: string): void;
info(message: string): void;
Expand Down
5 changes: 4 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { PullRequest } from "@octokit/webhooks-types";
import { GitHubChecksApi } from "./github/check";
import { PullRequestApi } from "./github/pullRequest";
import { GitHubTeamsApi } from "./github/teams";
import { PolkadotFellows } from "./polkadot/fellows";
import { ActionRunner } from "./runner";
import { generateCoreLogger } from "./util";

Expand Down Expand Up @@ -63,7 +64,9 @@ const teamApi = new GitHubTeamsApi(getOctokit(inputs.teamApiToken), repo.owner,

const checks = new GitHubChecksApi(getOctokit(inputs.teamApiToken), pr, logger, actionId);

const runner = new ActionRunner(api, teamApi, checks, logger);
const fellows = new PolkadotFellows(logger);

const runner = new ActionRunner(api, teamApi, fellows, checks, logger);

runner
.runAction(inputs)
Expand Down
108 changes: 108 additions & 0 deletions src/polkadot/fellows.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import { ApiPromise, WsProvider } from "@polkadot/api";

import { ActionLogger, TeamApi } from "../github/types";

type FellowData = { address: string; rank: number };

export class PolkadotFellows implements TeamApi {
private fellowsCache: Map<string, number> = new Map<string, number>();

constructor(private readonly logger: ActionLogger) {}

async fetchAllFellows(): Promise<Map<string, number>> {
let api: ApiPromise;
this.logger.debug("Connecting to collective parachain");
// we connect to the collective rpc node
const wsProvider = new WsProvider("wss://polkadot-collectives-rpc.polkadot.io");
api = await ApiPromise.create({ provider: wsProvider });
try {
// We fetch all the members
const membersObj = await api.query.fellowshipCollective.members.entries();

// We iterate over the fellow data and convert them into usable values
const fellows: FellowData[] = [];
for (const [key, rank] of membersObj) {
// @ts-ignore
const [address]: [string] = key.toHuman();
fellows.push({ address, ...(rank.toHuman() as object) } as FellowData);
}
this.logger.debug(JSON.stringify(fellows));

// Once we obtained this information, we disconnect this api.
await api.disconnect();

this.logger.debug("Connecting to relay parachain.");
// We connect to the relay chain
api = await ApiPromise.create({ provider: new WsProvider("wss://rpc.polkadot.io") });

// We iterate over the different members and extract their data
const users: Map<string, number> = new Map<string, number>();
for (const fellow of fellows) {
this.logger.debug(`Fetching identity of '${fellow.address}', rank: ${fellow.rank}`);
const fellowData = (await api.query.identity.identityOf(fellow.address)).toHuman() as
| Record<string, unknown>
| undefined;

// If the identity is null, we ignore it.
if (!fellowData) {
this.logger.debug("Identity is null. Skipping");
continue;
}

// @ts-ignore
const additional = fellowData.info.additional as [{ Raw: string }, { Raw: string }][] | undefined;

// If it does not have additional data (GitHub handle goes here) we ignore it
if (!additional || additional.length < 1) {
this.logger.debug("Additional data is null. Skipping");
continue;
}

for (const additionalData of additional) {
const [key, value] = additionalData;
// We verify that they have an additional data of the key "github"
// If it has a handle defined, we push it into the array
if (key?.Raw && key?.Raw === "github" && value?.Raw && value?.Raw.length > 0) {
this.logger.debug(`Found handles: '${value.Raw}'`);
// We add it to the array and remove the @ if they add it to the handle
users.set(value.Raw.replace("@", ""), fellow.rank);
}
}
}

this.logger.info(`Found users: ${JSON.stringify(Array.from(users.entries()))}`);

return users;
} catch (error) {
this.logger.error(error as Error);
throw error;
} finally {
await api.disconnect();
}
}

async getTeamMembers(ranking: string): Promise<string[]> {
const requiredRank = Number(ranking);
this.logger.info(`Fetching members of rank '${requiredRank}' or higher`);

if (this.fellowsCache.size < 1) {
this.logger.debug("Cache not found. Fetching fellows.");
this.fellowsCache = await this.fetchAllFellows();
}
const users: string[] = [];
for (const [user, rank] of this.fellowsCache) {
if (rank >= requiredRank) {
users.push(user);
}
}

if (users.length === 0) {
throw new Error(`Found no members of rank ${requiredRank} or higher. Please see debug logs`);
}

this.logger.info(`GitHub members of rank '${requiredRank}' or higher are: ${users.join(",")}`);

return users;
}
}
2 changes: 1 addition & 1 deletion src/rules/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export enum RuleTypes {
AndDistinct = "and-distinct",
}

export type Reviewers = { users?: string[]; teams?: string[]; min_approvals: number };
export type Reviewers = { users?: string[]; teams?: string[]; minFellowsRank?: number; min_approvals: number };

export interface Rule {
name: string;
Expand Down
7 changes: 4 additions & 3 deletions src/rules/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { AndRule, BasicRule, ConfigurationFile, Reviewers, Rule, RuleTypes } fro
const reviewersObj = {
users: Joi.array().items(Joi.string()).optional().empty(null),
teams: Joi.array().items(Joi.string()).optional().empty(null),
minFellowsRank: Joi.number().optional().min(1).empty(null),
};

const reviewerConditionObj = { ...reviewersObj, min_approvals: Joi.number().min(1).default(1) };
Expand All @@ -34,20 +35,20 @@ const ruleSchema = Joi.object<Rule & { type: string }>().keys({
*/
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"),
preventReviewRequests: Joi.object().keys(reviewersObj).optional().or("users", "teams", "minFellowsRank"),
});

/** Basic rule schema
* This rule is quite simple as it only has the min_approvals field and the required reviewers
*/
export const basicRuleSchema = Joi.object<BasicRule>()
.keys({ ...reviewerConditionObj, countAuthor: Joi.boolean().default(false) })
.or("users", "teams");
.or("users", "teams", "minFellowsRank");

/** As, with the exception of basic, every other schema has the same structure, we can recycle this */
export const otherRulesSchema = Joi.object<AndRule>().keys({
reviewers: Joi.array<AndRule["reviewers"]>()
.items(Joi.object<Reviewers>().keys(reviewerConditionObj).or("users", "teams"))
.items(Joi.object<Reviewers>().keys(reviewerConditionObj).or("users", "teams", "minFellowsRank"))
.min(2)
.required(),
countAuthor: Joi.boolean().default(false),
Expand Down
10 changes: 8 additions & 2 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import { parse } from "yaml";
import { Inputs } from ".";
import { GitHubChecksApi } from "./github/check";
import { PullRequestApi } from "./github/pullRequest";
import { TeamApi } from "./github/teams";
import { ActionLogger, CheckData } from "./github/types";
import { ActionLogger, CheckData, TeamApi } from "./github/types";
import { AndDistinctRule, ConfigurationFile, Reviewers, Rule } from "./rules/types";
import { validateConfig, validateRegularExpressions } from "./rules/validator";
import { caseInsensitiveEqual, concatArraysUniquely } from "./util";
Expand Down Expand Up @@ -37,6 +36,7 @@ export class ActionRunner {
constructor(
private readonly prApi: PullRequestApi,
private readonly teamApi: TeamApi,
private readonly polkadotApi: TeamApi,
private readonly checks: GitHubChecksApi,
private readonly logger: ActionLogger,
) {}
Expand Down Expand Up @@ -472,6 +472,12 @@ export class ActionRunner {
users.add(user);
}
}
if (reviewers.minFellowsRank) {
const members = await this.polkadotApi.getTeamMembers(reviewers.minFellowsRank.toString());
for (const member of members) {
users.add(member);
}
}

return Array.from(users);
}
Expand Down
65 changes: 65 additions & 0 deletions src/test/fellows.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/* eslint-disable @typescript-eslint/unbound-method */
import { mock, mockClear, MockProxy } from "jest-mock-extended";

import { ActionLogger, TeamApi } from "../github/types";
import { PolkadotFellows } from "../polkadot/fellows";

describe("CAPI test", () => {
let fellows: TeamApi;
let logger: MockProxy<ActionLogger>;

beforeEach(() => {
logger = mock<ActionLogger>();
fellows = new PolkadotFellows(logger);
});

test("Should fetch fellows", async () => {
const members = await fellows.getTeamMembers("2");
expect(members.length).toBeGreaterThan(0);
});

test("Should cache fellows", async () => {
const members = await fellows.getTeamMembers("2");
expect(members.length).toBeGreaterThan(0);
expect(logger.debug).toHaveBeenCalledWith("Connecting to collective parachain");
mockClear(logger);
const members2 = await fellows.getTeamMembers("2");
expect(members2.length).toBeGreaterThan(0);
expect(logger.debug).not.toHaveBeenCalledWith("Connecting to collective parachain");
});

describe("Fetch by rank", () => {
beforeEach(() => {
const fellowsMap = new Map<string, number>();
fellowsMap.set("user-1", 1);
fellowsMap.set("user-2", 2);
fellowsMap.set("user-3", 3);
fellowsMap.set("user-4", 4);
fellowsMap.set("user-5", 5);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(fellows as any).fellowsCache = fellowsMap;
});
test("should return fellows of a give rank", async () => {
const rank1 = await fellows.getTeamMembers("1");
expect(rank1).toEqual(["user-1", "user-2", "user-3", "user-4", "user-5"]);

const rank2 = await fellows.getTeamMembers("2");
expect(rank2).toEqual(["user-2", "user-3", "user-4", "user-5"]);

const rank3 = await fellows.getTeamMembers("3");
expect(rank3).toEqual(["user-3", "user-4", "user-5"]);

const rank4 = await fellows.getTeamMembers("4");
expect(rank4).toEqual(["user-4", "user-5"]);

const rank5 = await fellows.getTeamMembers("5");
expect(rank5).toEqual(["user-5"]);
});

test("should throw if there are no fellows available", async () => {
await expect(fellows.getTeamMembers("6")).rejects.toThrowError(
"Found no members of rank 6 or higher. Please see debug logs",
);
});
});
});
6 changes: 3 additions & 3 deletions src/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { join } from "path";

import { GitHubChecksApi } from "../github/check";
import { PullRequestApi } from "../github/pullRequest";
import { GitHubTeamsApi, TeamApi } from "../github/teams";
import { ActionLogger, GitHubClient } from "../github/types";
import { GitHubTeamsApi } from "../github/teams";
import { ActionLogger, GitHubClient, TeamApi } from "../github/types";
import { ActionRunner, RuleReport } from "../runner";

type ReportName =
Expand Down Expand Up @@ -83,7 +83,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, checks, logger);
runner = new ActionRunner(api, teams, mock<TeamApi>(), checks, logger);

// @ts-ignore problem with the type being mocked
client.rest.repos.getContent.mockResolvedValue({ data: { content: Buffer.from(config, "utf-8") } });
Expand Down
5 changes: 2 additions & 3 deletions src/test/rules/andDistinct.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import { mock, MockProxy } from "jest-mock-extended";

import { GitHubChecksApi } from "../../github/check";
import { PullRequestApi } from "../../github/pullRequest";
import { TeamApi } from "../../github/teams";
import { ActionLogger } from "../../github/types";
import { ActionLogger, TeamApi } from "../../github/types";
import { AndDistinctRule } from "../../rules/types";
import { ActionRunner } from "../../runner";

Expand All @@ -16,7 +15,7 @@ describe("'AndDistinctRule' rule parsing", () => {
let teamsApi: MockProxy<TeamApi>;
beforeEach(() => {
api = mock<PullRequestApi>();
runner = new ActionRunner(api, teamsApi, mock<GitHubChecksApi>(), mock<ActionLogger>());
runner = new ActionRunner(api, teamsApi, mock<TeamApi>(), mock<GitHubChecksApi>(), mock<ActionLogger>());
});
test("should get minimal config", async () => {
api.getConfigFile.mockResolvedValue(`
Expand Down
Loading

0 comments on commit 29b16b5

Please sign in to comment.