Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort by open bot PRs #41

Merged
merged 6 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2es/seedTestData.js
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
11 changes: 7 additions & 4 deletions e2es/testData/repos.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -27,7 +28,8 @@
"topics": ["govpress", "wordpress-theme"],
"openIssues": 1,
"dependencies": [],
"openPrsCount": 0
"openPrCount": 20,
"openBotPrCount": 10
},
{
"name": "php-missing",
Expand All @@ -41,7 +43,8 @@
"topics": ["composer", "govpress", "packagist", "php"],
"openIssues": 7,
"dependencies": [],
"openPrsCount": 2
"openPrCount": 200,
"openBotPrCount": 3
}
]
}
56 changes: 54 additions & 2 deletions e2es/towtruck.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
8 changes: 5 additions & 3 deletions index.njk
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
Expand Down
10 changes: 6 additions & 4 deletions utils/githubApi/fetchAllRepos.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down
13 changes: 12 additions & 1 deletion utils/githubApi/fetchOpenPrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
};
62 changes: 58 additions & 4 deletions utils/githubApi/fetchOpenPrs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
2 changes: 0 additions & 2 deletions utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,4 @@ export const mapRepoFromApiForStorage = (repo) => ({
language: repo.language,
topics: repo.topics,
openIssues: repo.open_issues,
dependencies: repo.dependencies,
openPrsCount: repo.openPrsCount,
});
21 changes: 0 additions & 21 deletions utils/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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=",
Expand Down Expand Up @@ -258,8 +241,6 @@ describe("mapRepoFromStorageToUi", () => {
triage: false,
pull: false,
},
dependencies: repoDependencies,
openPrsCount: 0,
};

const repoToSave = {
Expand All @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions utils/sorting.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
12 changes: 6 additions & 6 deletions utils/sorting.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down Expand Up @@ -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")
);
});

Expand Down
Loading