feat: add paginatantion support for compareCommits and compareCommitsWithBasehead (#678)

Author: roryabraham

README.md

@@ -183,6 +183,7 @@ Most of GitHub's paginating REST API endpoints return an array, but there are a
 - [List check suites for a specific ref](https://developer.github.com/v3/checks/suites/#response-1) (key: `check_suites`)
 - [List repositories](https://developer.github.com/v3/apps/installations/#list-repositories) for an installation (key: `repositories`)
 - [List installations for a user](https://developer.github.com/v3/apps/installations/#response-1) (key `installations`)
+- [Compare commits](https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#compare-two-commits) (key `commits`)
 
 `octokit.paginate()` is working around these inconsistencies so you don't have to worry about it.
 

scripts/update-endpoints/typescript.js

@@ -28,11 +28,6 @@ const ENDPOINTS_WITH_PER_PAGE_ATTRIBUTE_THAT_BEHAVE_DIFFERENTLY = [
   // Only the `files` key inside the commit is paginated. The rest is duplicated across
   // all pages. Handling this case properly requires custom code.
   { scope: "repos", id: "get-commit" },
-  // The [docs](https://docs.github.com/en/rest/commits/commits#compare-two-commits) make
-  // these ones sound like a special case too - they must be because they support pagination
-  // but doesn't return an array.
-  { scope: "repos", id: "compare-commits" },
-  { scope: "repos", id: "compare-commits-with-basehead" },
 ];
 
 const hasMatchingEndpoint = (list, id, scope) =>

src/generated/paginating-endpoints.ts

@@ -1195,6 +1195,26 @@ export interface PaginatingEndpoints {
     response: Endpoints["GET /repos/{owner}/{repo}/commits/{ref}/statuses"]["response"];
   };
 
+  /**
+   * @see https://docs.github.com/rest/commits/commits#compare-two-commits
+   */
+  "GET /repos/{owner}/{repo}/compare/{basehead}": {
+    parameters: Endpoints["GET /repos/{owner}/{repo}/compare/{basehead}"]["parameters"];
+    response: Endpoints["GET /repos/{owner}/{repo}/compare/{basehead}"]["response"] & {
+      data: Endpoints["GET /repos/{owner}/{repo}/compare/{basehead}"]["response"]["data"]["commits"];
+    };
+  };
+
+  /**
+   * @see https://docs.github.com/rest/reference/repos#compare-two-commits
+   */
+  "GET /repos/{owner}/{repo}/compare/{base}...{head}": {
+    parameters: Endpoints["GET /repos/{owner}/{repo}/compare/{base}...{head}"]["parameters"];
+    response: Endpoints["GET /repos/{owner}/{repo}/compare/{base}...{head}"]["response"] & {
+      data: Endpoints["GET /repos/{owner}/{repo}/compare/{base}...{head}"]["response"]["data"]["commits"];
+    };
+  };
+
   /**
    * @see https://docs.github.com/rest/repos/repos#list-repository-contributors
    */
@@ -2325,6 +2345,8 @@ export const paginatingEndpoints: (keyof PaginatingEndpoints)[] = [
   "GET /repos/{owner}/{repo}/commits/{ref}/check-suites",
   "GET /repos/{owner}/{repo}/commits/{ref}/status",
   "GET /repos/{owner}/{repo}/commits/{ref}/statuses",
+  "GET /repos/{owner}/{repo}/compare/{basehead}",
+  "GET /repos/{owner}/{repo}/compare/{base}...{head}",
   "GET /repos/{owner}/{repo}/contributors",
   "GET /repos/{owner}/{repo}/dependabot/alerts",
   "GET /repos/{owner}/{repo}/dependabot/secrets",

src/iterator.ts

@@ -40,6 +40,18 @@ export function iterator(
             /<([^<>]+)>;\s*rel="next"/,
           ) || [])[1];
 
+          if (!url && "total_commits" in normalizedResponse.data) {
+            const parsedUrl = new URL(normalizedResponse.url);
+            const params = parsedUrl.searchParams;
+            const page = parseInt(params.get("page") || "1", 10);
+            /* v8 ignore next */
+            const per_page = parseInt(params.get("per_page") || "250", 10);
+            if (page * per_page < normalizedResponse.data.total_commits) {
+              params.set("page", String(page + 1));
+              url = parsedUrl.toString();
+            }
+          }
+
           return { value: normalizedResponse };
         } catch (error: any) {
           // `GET /repos/{owner}/{repo}/commits` throws a `409 Conflict` error for empty repositories

src/normalize-paginated-list-response.ts

@@ -28,17 +28,20 @@ export function normalizePaginatedListResponse(
     };
   }
   const responseNeedsNormalization =
-    "total_count" in response.data && !("url" in response.data);
+    ("total_count" in response.data && !("url" in response.data)) ||
+    "total_commits" in response.data;
   if (!responseNeedsNormalization) return response;
 
   // keep the additional properties intact as there is currently no other way
   // to retrieve the same information.
   const incompleteResults = response.data.incomplete_results;
   const repositorySelection = response.data.repository_selection;
   const totalCount = response.data.total_count;
+  const totalCommits = response.data.total_commits;
   delete response.data.incomplete_results;
   delete response.data.repository_selection;
   delete response.data.total_count;
+  delete response.data.total_commits;
 
   const namespaceKey = Object.keys(response.data)[0];
   const data = response.data[namespaceKey];
@@ -53,5 +56,6 @@ export function normalizePaginatedListResponse(
   }
 
   response.data.total_count = totalCount;
+  response.data.total_commits = totalCommits;
   return response;
 }

src/types.ts

@@ -31,6 +31,7 @@ import type { PaginatingEndpoints } from "./generated/paginating-endpoints.js";
 type PaginationMetadataKeys =
   | "repository_selection"
   | "total_count"
+  | "total_commits"
   | "incomplete_results";
 
 // https://stackoverflow.com/a/58980331/206879

test/issues.test.ts

@@ -57,3 +57,74 @@ describe("https://github.com/octokit/plugin-paginate-rest.js/issues/158", () =>
     expect(result.length).toEqual(0);
   });
 });
+
+describe("https://github.com/octokit/plugin-paginate-rest.js/issues/647", () => {
+  test("paginate compareCommits when link header is missing", async () => {
+    const mock = fetchMock
+      .createInstance()
+      .get(
+        "https://api.github.com/repos/owner/repo/compare/main...feature?per_page=1",
+        {
+          body: {
+            total_commits: 3,
+            commits: [
+              {
+                sha: "abc123",
+              },
+            ],
+          },
+          headers: {}, // missing link header
+        },
+      )
+      .get(
+        "https://api.github.com/repos/owner/repo/compare/main...feature?per_page=1&page=2",
+        {
+          body: {
+            total_commits: 3,
+            commits: [
+              {
+                sha: "def456",
+              },
+            ],
+          },
+          headers: {},
+        },
+      )
+      .get(
+        "https://api.github.com/repos/owner/repo/compare/main...feature?per_page=1&page=3",
+        {
+          body: {
+            total_commits: 3,
+            commits: [
+              {
+                sha: "ghi789",
+              },
+            ],
+          },
+          headers: {},
+        },
+      );
+
+    const TestOctokit = Octokit.plugin(paginateRest);
+    const octokit = new TestOctokit({
+      request: {
+        fetch: mock.fetchHandler,
+      },
+    });
+
+    const result = await octokit.paginate(
+      "GET /repos/{owner}/{repo}/compare/{basehead}",
+      {
+        owner: "owner",
+        repo: "repo",
+        basehead: "main...feature",
+        per_page: 1,
+      },
+    );
+
+    expect(result.length).toEqual(3);
+    expect(result[0].sha).toEqual("abc123");
+    expect(result[1].sha).toEqual("def456");
+    expect(result[2].sha).toEqual("ghi789");
+  });
+});

test/paginate.test.ts

@@ -998,6 +998,60 @@ describe("pagination", () => {
         ]);
       });
   });
+
+  it(".paginate() with results namespace (GET /repos/{owner}/{repo}/compare/{basehead})", () => {
+    const result1 = {
+      total_commits: 2,
+      commits: [
+        {
+          sha: "f3b573e4d60a079d154018d2e2d04aff4d26fc41",
+        },
+      ],
+    };
+    const result2 = {
+      total_commits: 2,
+      commits: [
+        {
+          sha: "a740e83052aea45a4cbcdf2954a3a9e47b5d530d",
+        },
+      ],
+    };
+
+    const mock = fetchMock
+      .createInstance()
+      .get(
+        "https://api.github.com/repos/octocat/hello-world/compare/1.0.0...1.0.1?per_page=1",
+        {
+          body: result1,
+        },
+      )
+      .get(
+        "https://api.github.com/repos/octocat/hello-world/compare/1.0.0...1.0.1?per_page=1&page=2",
+        {
+          body: result2,
+        },
+      );
+
+    const octokit = new TestOctokit({
+      request: {
+        fetch: mock.fetchHandler,
+      },
+    });
+
+    return octokit
+      .paginate({
+        method: "GET",
+        url: "/repos/{owner}/{repo}/compare/{basehead}",
+        owner: "octocat",
+        repo: "hello-world",
+        basehead: "1.0.0...1.0.1",
+        per_page: 1,
+      })
+      .then((results) => {
+        expect(results).toEqual([...result1.commits, ...result2.commits]);
+      });
+  });
+
   it(".paginate() with results namespace (GET /repos/{owner}/{repo}/actions/runs)", () => {
     const result1 = {
       total_count: 2,