Merge pull request #41 from dxw/sort-by-open-bot-prs

Sort by open bot PRs

e2es/seedTestData.js

@@ -2,7 +2,7 @@ import { copyFile, mkdir } from "fs/promises";
 
 export const createDestinationDirectory = async () => {
   console.info("Creating destination directory...");
-  await mkdir("./data");
+  await mkdir("./data", { recursive: true });
 };
 
 export const seedTestData = async () => {

e2es/testData/repos.json

@@ -11,9 +11,10 @@
       "updatedAt": "2024-07-08T12:39:14Z",
       "language": "PHP",
       "topics": ["composer", "govpress", "packagist", "php"],
-      "openIssues": 0,
+      "openIssues": 10,
       "dependencies": [],
-      "openPrsCount": 0
+      "openPrCount": 0,
+      "openBotPrCount": 0
     },
     {
       "name": "govuk-blogs",
@@ -27,7 +28,8 @@
       "topics": ["govpress", "wordpress-theme"],
       "openIssues": 1,
       "dependencies": [],
-      "openPrsCount": 0
+      "openPrCount": 20,
+      "openBotPrCount": 10
     },
     {
       "name": "php-missing",
@@ -41,7 +43,8 @@
       "topics": ["composer", "govpress", "packagist", "php"],
       "openIssues": 7,
       "dependencies": [],
-      "openPrsCount": 2
+      "openPrCount": 200,
+      "openBotPrCount": 3
     }
   ]
 }

e2es/towtruck.spec.js

@@ -13,10 +13,62 @@ test("has dependency info", async ({ page, baseURL }) => {
     "Language",
     "Last Updated",
     "Open issues count",
-    "Open PRs count",
+    "Open PR count",
     "Dependencies",
   ];
   tableHeadings.forEach((heading) => {
-    page.getByRole("columnheader", { name: heading });
+    expect(page.getByRole("columnheader", { name: heading })).toBeTruthy();
   });
+
+  await testSortingForColumn(
+    {
+      name: "Open issues count",
+      topAscending: "govuk-blogs",
+      topDescending: "optionparser",
+    },
+    page
+  );
+
+  await testSortingForColumn(
+    {
+      name: "Open bot PR count",
+      topAscending: "optionparse",
+      topDescending: "govuk-blogs",
+    },
+    page
+  );
+
+  await testSortingForColumn(
+    {
+      name: "Open PR count",
+      topAscending: "optionparser",
+      topDescending: "php-missing",
+    },
+    page
+  );
+
+  await testSortingForColumn(
+    {
+      name: "Updated at",
+      topAscending: "optionparser",
+      topDescending: "govuk-blogs ",
+    },
+    page
+  );
 });
+
+const testSortingForColumn = async (
+  { name, topAscending, topDescending },
+  page
+) => {
+  await page.getByRole("link", { name, exact: true }).click();
+  await assertFirstDependencyRow(topAscending, page);
+
+  await page.getByRole("link", { name, exact: true }).click();
+  await assertFirstDependencyRow(topDescending, page);
+};
+
+const assertFirstDependencyRow = async (expectedFirstDependency, page) => {
+  const firstDependencyRow = page.getByRole("row").nth(1);
+  await expect(firstDependencyRow).toContainText(expectedFirstDependency);
+};

index.njk

@@ -38,20 +38,22 @@
                 <th class="py-2 pl-2" scope="col">Language</th>
                 <th class="py-2 pl-2" scope="col">Topics</th>
                 {{macros.sortableTableHeader("Open issues count", "openIssues", sortDirection, sortBy)}}
-                {{macros.sortableTableHeader("Open PRs count", "openPrsCount", sortDirection, sortBy)}}
+                {{macros.sortableTableHeader("Open bot PR count", "openBotPrCount", sortDirection, sortBy)}}
+                {{macros.sortableTableHeader("Open PR count", "openPrCount", sortDirection, sortBy)}}
                 {{macros.sortableTableHeader("Updated at", "updatedAt", sortDirection, sortBy)}}
                 <th class="py-2 pl-2 last:pr-2" scope="col">Dependencies</th>
               </tr>
             </thead>
             <tbody>
               {% for repo in repos %}
               <tr class="even:bg-white">
-                 <td class="py-2 pl-2"><a target="_blank" class="text-blue-600 dark:text-blue-500 hover:underline" href="{{repo.htmlUrl}}">{{ repo.name }}</a></td>
+                <td class="py-2 pl-2" scope="row"><a target="_blank" class="text-blue-600 dark:text-blue-500 hover:underline" href="{{repo.htmlUrl}}">{{ repo.name }}</a></td>
                 <td class="py-2 pl-2">{{ repo.description }}</td>
                 <td class="py-2 pl-2">{{ repo.language }}</td>
                 <td class="py-2 pl-2">{{ repo.topics | join(", ")}}</td>
                 <td class="py-2 pl-2">{{ repo.openIssues }}</td>
-                <td class="py-2 pl-2">{{ repo.openPrsCount }}</td>
+                <td class="py-2 pl-2">{{ repo.openBotPrCount }}</td>
+                <td class="py-2 pl-2">{{ repo.openPrCount }}</td>
                 <td class="py-2 pl-2 last:pr-2">{{ repo.updatedAt }}</td>
 
                 {% if repo.dependencies.length %}

utils/githubApi/fetchAllRepos.js

@@ -16,6 +16,8 @@ const fetchAllRepos = async () => {
   await OctokitApp.app.eachRepository(async ({ repository, octokit }) => {
     if (repository.archived) return;
 
+    let repo = mapRepoFromApiForStorage(repository);
+
     await Promise.all([
       await getDependenciesForRepo({
         repository,
@@ -25,12 +27,12 @@ const fetchAllRepos = async () => {
         repository,
         octokit,
       }),
-    ]).then(([dependencies, openPrsCount]) => {
-      repository.dependencies = dependencies;
-      repository.openPrsCount = openPrsCount;
+    ]).then(([dependencies, prInfo]) => {
+      repo.dependencies = dependencies;
+      repo = { ...repo, ...prInfo };
     });
 
-    repos.push(mapRepoFromApiForStorage(repository));
+    repos.push(repo);
   });
 
   return repos;

utils/githubApi/fetchOpenPrs.js

@@ -8,9 +8,20 @@ export const getOpenPRsForRepo = async ({ octokit, repository }) => {
   return octokit.request(repository.pulls_url).then(handlePrsApiResponse);
 };
 
+const depdencyUpdateBots = ["renovate[bot]", "dependabot[bot]"];
+
 /**
  * Returns the length of the PRs array or 0 if there are no PRs
  * @param {any} {data}
  * @returns {number}
  */
-export const handlePrsApiResponse = ({ data }) => data?.length || 0;
+export const handlePrsApiResponse = ({ data }) => {
+  const openBotPrCount = data.reduce((acc, pr) => {
+    if (depdencyUpdateBots.includes(pr?.user?.login)) {
+      return acc + 1;
+    }
+    return acc;
+  }, 0);
+
+  return { openPrCount: data?.length || 0, openBotPrCount };
+};

utils/githubApi/fetchOpenPrs.test.js

@@ -3,11 +3,65 @@ import expect from "node:assert";
 import { handlePrsApiResponse } from "./fetchOpenPrs.js";
 
 describe("handlePrsApiResponse", () => {
-  it("returns the length of the array containing PRs", () => {
-    expect.equal(handlePrsApiResponse({ data: [1, 2, 3] }), 3);
+  describe("openPrCount", () => {
+    it("returns the length of the array containing PRs", () => {
+      const actual = handlePrsApiResponse({ data: [1, 2, 3] });
+      const expected = { openPrCount: 3, openBotPrCount: 0 };
+
+      expect.deepEqual(actual, expected);
+    });
+
+    it("returns 0 if there are no open PRs", () => {
+      const actual = handlePrsApiResponse({ data: [] });
+      const expected = { openPrCount: 0, openBotPrCount: 0 };
+
+      expect.deepEqual(actual, expected);
+    });
   });
 
-  it("returns 0 if there are no open PRs", () => {
-    expect.equal(handlePrsApiResponse({ data: undefined }), 0);
+  describe("openBotPrCount", () => {
+    it("returns 1 if there is a PR authored by dependabot", () => {
+      const dependabotPr = { user: { login: "dependabot[bot]" } };
+      const actual = handlePrsApiResponse({ data: [dependabotPr] });
+
+      expect.equal(actual.openBotPrCount, 1);
+    });
+
+    it("returns 1 if there is a PR authored by renovate", () => {
+      const renovatePr = { user: { login: "renovate[bot]" } };
+      const actual = handlePrsApiResponse({ data: [renovatePr] });
+
+      expect.equal(actual.openBotPrCount, 1);
+    });
+
+    it("returns 0 if there is a PR authored by other authors", () => {
+      const humanPr = { user: { login: "rich" } };
+      const actual = handlePrsApiResponse({ data: [humanPr] });
+
+      expect.equal(actual.openBotPrCount, 0);
+    });
+
+    it("handles a variety of PR authors correctly", () => {
+      const dependabotPr = {
+        user: {
+          login: "dependabot[bot]",
+        },
+      };
+      const renovatePr = {
+        user: {
+          login: "renovate[bot]",
+        },
+      };
+      const humanPr = {
+        user: {
+          login: "rich",
+        },
+      };
+      const actual = handlePrsApiResponse({
+        data: [dependabotPr, renovatePr, humanPr],
+      });
+
+      expect.equal(actual.openBotPrCount, 2);
+    });
   });
 });

utils/index.js

@@ -171,6 +171,4 @@ export const mapRepoFromApiForStorage = (repo) => ({
   language: repo.language,
   topics: repo.topics,
   openIssues: repo.open_issues,
-  dependencies: repo.dependencies,
-  openPrsCount: repo.openPrsCount,
 });

utils/index.test.js

@@ -95,23 +95,6 @@ describe("mapRepoFromStorageToUi", () => {
 
   describe("mapRepo", () => {
     it("maps the repo from the data returned from the api", async () => {
-      const repoDependencies = [
-        {
-          user: {
-            login: "some-user",
-          },
-          pull_request: {},
-          body: "Here's a pull request to manually update dependency `foo-utils 1.2.3` to `foo-utils 1.2.4`.",
-        },
-        {
-          user: {
-            login: "renovate[bot]",
-          },
-          pull_request: {},
-          body: "Configure Renovate",
-        },
-      ];
-
       const apiRepo = {
         id: 248204237,
         node_id: "MDEwOlJlcG9zaXRvcnkyNDgyMDQyMzc=",
@@ -258,8 +241,6 @@ describe("mapRepoFromStorageToUi", () => {
           triage: false,
           pull: false,
         },
-        dependencies: repoDependencies,
-        openPrsCount: 0,
       };
 
       const repoToSave = {
@@ -276,8 +257,6 @@ describe("mapRepoFromStorageToUi", () => {
         language: "Ruby",
         topics: ["delivery-plus", "internal", "tech-ops"],
         openIssues: 2,
-        dependencies: repoDependencies,
-        openPrsCount: 0,
       };
 
       expect.deepEqual(mapRepoFromApiForStorage(apiRepo), repoToSave);

utils/sorting.js

@@ -41,15 +41,18 @@ export const sortByUpdatedAt = (repos, sortDirection) => {
 
 /**
  * Sorts repositories based on the specified type and direction.
- * @param {"openPrsCount"|"openIssues"} sortBy - The type of sorting to apply, either by open PRs count or open issues.
+ * @param {"openPrCount"|"openBotPrCount"|"openIssues"|} sortBy - The type of sorting to apply, either by open PRs count or open issues.
  * @param {SortDirection} sortDirection - The direction to sort, either "asc" for ascending or "desc" for descending.
  * @param {UiRepo[]} repos - An array of repository objects to sort.
  * @returns {UiRepo[]} The sorted array of repository objects.
  */
 export const sortByType = (repos, sortDirection, sortBy) => {
   switch (sortBy) {
-    case "openPrsCount":
-      return sortByNumericValue(repos, sortDirection, "openPrsCount");
+    case "openPrCount":
+      return sortByNumericValue(repos, sortDirection, "openPrCount");
+
+    case "openBotPrCount":
+      return sortByNumericValue(repos, sortDirection, "openBotPrCount");
 
     case "openIssues":
       return sortByNumericValue(repos, sortDirection, "openIssues");

utils/sorting.test.js

@@ -48,7 +48,7 @@ describe("sortByNumericValue", () => {
   });
 });
 
-describe.only("sortByUpdatedAt", () => {
+describe("sortByUpdatedAt", () => {
   it("sorts the repos by the date they were last updated in ascending order", () => {
     const reposToSort = [
       { name: "Repo 1", updatedAtISO8601: "2022-01-01T00:00:00Z" },
@@ -81,14 +81,14 @@ describe.only("sortByUpdatedAt", () => {
 describe("sortByType", () => {
   it("sorts the repos by number of open PRs", () => {
     const reposToSort = [
-      { name: "Repo 1", openPrsCount: 5 },
-      { name: "Repo 2", openPrsCount: 3 },
-      { name: "Repo 3", openPrsCount: 7 },
+      { name: "Repo 1", openPrCount: 5 },
+      { name: "Repo 2", openPrCount: 3 },
+      { name: "Repo 3", openPrCount: 7 },
     ];
 
     expect.deepEqual(
-      sortByType(reposToSort, "asc", "openPrsCount"),
-      sortByNumericValue(reposToSort, "asc", "openPrsCount")
+      sortByType(reposToSort, "asc", "openPrCount"),
+      sortByNumericValue(reposToSort, "asc", "openPrCount")
     );
   });