Added pagination 📑 for listReviews 👥 endpoint (#104)

Implemented the pagination method when fetching reviews, as the default
endpoint has a hard limit of 30 results. By adding pagination we can go
over the limit of 30.

Fixes #102

action.yml

@@ -32,4 +32,4 @@ outputs:
 
 runs:
   using: 'docker'
-  image: 'docker://ghcr.io/paritytech/review-bot/action:2.2.0'
+  image: 'Dockerfile'

jest.config.js

@@ -2,6 +2,5 @@
 module.exports = {
   preset: "ts-jest",
   testEnvironment: "node",
-  testTimeout: 8_000,
   testMatch: [__dirname + "/src/**/test/**/*.test.ts"],
 };

src/github/pullRequest.ts

@@ -58,8 +58,10 @@ export class PullRequestApi {
   /** List all the approved reviews in a PR */
   async listApprovedReviewsAuthors(countAuthor: boolean): Promise<string[]> {
     if (!this.usersThatApprovedThePr) {
-      const request = await this.api.rest.pulls.listReviews({ ...this.repoInfo, pull_number: this.number });
-      const reviews = request.data as PullRequestReview[];
+      const reviews = await this.api.paginate(this.api.rest.pulls.listReviews, {
+        ...this.repoInfo,
+        pull_number: this.number,
+      });
       this.logger.debug(`List of reviews: ${JSON.stringify(reviews)}`);
 
       const latestReviewsMap = new Map<number, PullRequestReview>();
@@ -82,7 +84,7 @@ export class PullRequestApi {
           prevReview.id < review.id
         ) {
           // if the review is more modern (and not a comment) we replace the one in our map
-          latestReviewsMap.set(review.user.id, review);
+          latestReviewsMap.set(review.user.id, review as PullRequestReview);
         }
       }
 

src/test/fellows.test.ts

@@ -4,6 +4,8 @@ import { mock, mockClear, MockProxy } from "jest-mock-extended";
 import { ActionLogger, TeamApi } from "../github/types";
 import { PolkadotFellows } from "../polkadot/fellows";
 
+const timeout = 15_000;
+
 describe("CAPI test", () => {
   let fellows: TeamApi;
   let logger: MockProxy<ActionLogger>;
@@ -13,20 +15,28 @@ describe("CAPI test", () => {
     fellows = new PolkadotFellows(logger);
   });
 
-  test("Should fetch fellows", async () => {
-    const members = await fellows.getTeamMembers("2");
-    expect(members.length).toBeGreaterThan(0);
-  });
+  test(
+    "Should fetch fellows",
+    async () => {
+      const members = await fellows.getTeamMembers("2");
+      expect(members.length).toBeGreaterThan(0);
+    },
+    timeout,
+  );
 
-  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");
-  });
+  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");
+    },
+    timeout,
+  );
 
   describe("Fetch by rank", () => {
     beforeEach(() => {
@@ -39,27 +49,35 @@ describe("CAPI test", () => {
       // 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"]);
+    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 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 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 rank4 = await fellows.getTeamMembers("4");
+        expect(rank4).toEqual(["user-4", "user-5"]);
 
-      const rank5 = await fellows.getTeamMembers("5");
-      expect(rank5).toEqual(["user-5"]);
-    });
+        const rank5 = await fellows.getTeamMembers("5");
+        expect(rank5).toEqual(["user-5"]);
+      },
+      timeout,
+    );
 
-    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",
-      );
-    });
+    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",
+        );
+      },
+      timeout,
+    );
   });
 });

src/test/github.test.ts

@@ -1,5 +1,5 @@
 import { PullRequest, PullRequestReview } from "@octokit/webhooks-types";
-import { DeepMockProxy, mock, mockDeep, MockProxy } from "jest-mock-extended";
+import { any, DeepMockProxy, mock, mockDeep, MockProxy } from "jest-mock-extended";
 
 import { PullRequestApi } from "../github/pullRequest";
 import { ActionLogger, GitHubClient } from "../github/types";
@@ -25,9 +25,7 @@ describe("Pull Request API Tests", () => {
     let reviews: PullRequestReview[];
     beforeEach(() => {
       reviews = [];
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore because the official type and the library type do not match
-      client.rest.pulls.listReviews.mockResolvedValue({ data: reviews as unknown });
+      client.paginate.calledWith(client.rest.pulls.listReviews, any()).mockResolvedValue(reviews as unknown);
     });
 
     test("Should return approval", async () => {
@@ -51,7 +49,7 @@ describe("Pull Request API Tests", () => {
         expect(approvals).toEqual(["yes-user"]);
       }
 
-      expect(client.rest.pulls.listReviews).toHaveBeenCalledTimes(1);
+      expect(client.paginate).toHaveBeenCalledTimes(1);
     });
 
     test("Should return approvals and ignore other reviews", async () => {

src/test/index.test.ts

@@ -1,7 +1,7 @@
 /* eslint-disable @typescript-eslint/ban-ts-comment */
 import { PullRequest, PullRequestReview } from "@octokit/webhooks-types";
 import { existsSync, openSync, readFileSync, unlinkSync } from "fs";
-import { DeepMockProxy, mock, mockDeep, MockProxy } from "jest-mock-extended";
+import { any, DeepMockProxy, mock, mockDeep, MockProxy } from "jest-mock-extended";
 import { join } from "path";
 
 import { GitHubChecksApi } from "../github/check";
@@ -67,8 +67,7 @@ describe("Integration testing", () => {
       return { state, id, user: { login, id: getHash(login) } };
     });
 
-    // @ts-ignore because the official type and the library type do not match
-    client.rest.pulls.listReviews.mockResolvedValue({ data });
+    client.paginate.calledWith(client.rest.pulls.listReviews, any()).mockResolvedValue(data);
   };
 
   const summaryTestFile = "./summary-test.html";