Skip to content

Commit

Permalink
chore: added required validation check
Browse files Browse the repository at this point in the history
  • Loading branch information
gentlementlegen committed Jul 7, 2024
1 parent c9c88d3 commit ccf831a
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 13 deletions.
22 changes: 18 additions & 4 deletions src/helpers/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,36 @@ export type IssueParams = ReturnType<typeof parseGitHubUrl>;
* Gets the merge timeout depending on the status of the assignee. If there are multiple assignees with different
* statuses, the longest timeout is chosen.
*/
export async function getMergeTimeout(context: Context, { repo, owner }: IssueParams) {
export async function getMergeTimeoutAndApprovalRequiredCount(context: Context, { repo, owner }: IssueParams) {
const assignees = context.payload.pull_request.assignees || [];
let timeout = context.config.contributorMergeTimeout;
let timeout = { mergeTimeout: context.config.contributorMergeTimeout, requiredApprovalCount: context.config.contributorMinimumApprovalsRequired };
for (const assignee of assignees) {
try {
await context.octokit.repos.checkCollaborator({
repo,
owner,
username: assignee.login,
});
timeout = context.config.collaboratorMergeTimeout;
timeout = { mergeTimeout: context.config.collaboratorMergeTimeout, requiredApprovalCount: context.config.collaboratorMinimumApprovalsRequired };
} catch (e) {
context.logger.debug(`${assignee.login} is not a collaborator of ${owner}/${repo}: ${e}`);
timeout = context.config.contributorMergeTimeout;
timeout = { mergeTimeout: context.config.contributorMergeTimeout, requiredApprovalCount: context.config.contributorMinimumApprovalsRequired };
break;
}
}
return timeout;
}

export async function getApprovalCount({ octokit, logger }: Context, { owner, repo, issue_number: pullNumber }: IssueParams) {
try {
const { data: reviews } = await octokit.pulls.listReviews({
owner,
repo,
pull_number: pullNumber,
});
return reviews.filter((review) => review.state === "APPROVED").length;
} catch (error) {
logger.error(`Error fetching reviews: ${error}`);
return 0;
}
}
14 changes: 9 additions & 5 deletions src/helpers/update-pull-requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import ms from "ms";
import { PullRequest } from "../adapters/sqlite/entities/pull-request";
import { getAllTimelineEvents } from "../handlers/github-events";
import { Context } from "../types";
import { getMergeTimeout, IssueParams, parseGitHubUrl } from "./github";
import { getApprovalCount, getMergeTimeoutAndApprovalRequiredCount, IssueParams, parseGitHubUrl } from "./github";

type IssueEvent = {
created_at?: string;
Expand Down Expand Up @@ -53,10 +53,14 @@ export async function updatePullRequests(context: Context) {

const lastActivityDate = new Date(Math.max(...eventDates.map((date) => date.getTime())));

const mergeTimeout = await getMergeTimeout(context, gitHubUrl);
if (isNaN(lastActivityDate.getTime()) || isPastOffset(lastActivityDate, mergeTimeout)) {
context.logger.info(`Pull-request ${pullRequest.url} is past its due date (${mergeTimeout} after ${lastActivityDate}), will merge.`);
await mergePullRequest(context, pullRequest, gitHubUrl);
const requirements = await getMergeTimeoutAndApprovalRequiredCount(context, gitHubUrl);
if (isNaN(lastActivityDate.getTime()) || isPastOffset(lastActivityDate, requirements.mergeTimeout)) {
if ((await getApprovalCount(context, gitHubUrl)) > 0) {
context.logger.info(`Pull-request ${pullRequest.url} is past its due date (${requirements} after ${lastActivityDate}), will merge.`);
await mergePullRequest(context, pullRequest, gitHubUrl);
} else {
context.logger.info(`Pull-request ${pullRequest.url} does not have sufficient reviewer approvals to be merged.`);
}
} else {
await context.adapters.sqlite.pullRequest.update(pullRequest.url, lastActivityDate);
context.logger.info(`Updated PR ${pullRequest.url} to a new timestamp (${lastActivityDate})`);
Expand Down
18 changes: 14 additions & 4 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { http, HttpResponse } from "msw";
import * as fs from "node:fs";
import { initializeDataSource } from "../src/adapters/sqlite/data-source";
import { PullRequest } from "../src/adapters/sqlite/entities/pull-request";
import { getMergeTimeout } from "../src/helpers/github";
import { getMergeTimeoutAndApprovalRequiredCount } from "../src/helpers/github";
import { server } from "./__mocks__/node";
import { expect, describe, beforeAll, beforeEach, afterAll, afterEach, it, jest } from "@jest/globals";
import { Context } from "../src/types";
Expand Down Expand Up @@ -187,9 +187,11 @@ describe("Action tests", () => {
expect(pullRequests).toHaveLength(0);
});

it("Should pick the timeout according to the assignee's status", async () => {
it("Should pick the timeout according to the assignees status", async () => {
const contributorMergeTimeout = "7 days";
const collaboratorMergeTimeout = "3.5 days";
const collaboratorMinimumApprovalsRequired = 2;
const contributorMinimumApprovalsRequired = 1;
const context = {
logger: {
debug: console.log,
Expand All @@ -202,10 +204,15 @@ describe("Action tests", () => {
config: {
contributorMergeTimeout,
collaboratorMergeTimeout,
collaboratorMinimumApprovalsRequired,
contributorMinimumApprovalsRequired,
},
octokit: new Octokit(),
} as unknown as Context;
await expect(getMergeTimeout(context, { owner: "ubiquibot", repo: "automated-merging", issue_number: 1 })).resolves.toEqual(collaboratorMergeTimeout);
await expect(getMergeTimeoutAndApprovalRequiredCount(context, { owner: "ubiquibot", repo: "automated-merging", issue_number: 1 })).resolves.toEqual({
mergeTimeout: collaboratorMergeTimeout,
requiredApprovalCount: collaboratorMinimumApprovalsRequired,
});
server.use(
http.get(
"https://api.github.com/repos/:org/:repo/collaborators/:login",
Expand All @@ -215,6 +222,9 @@ describe("Action tests", () => {
{ once: true }
)
);
await expect(getMergeTimeout(context, { owner: "ubiquibot", repo: "automated-merging", issue_number: 1 })).resolves.toEqual(contributorMergeTimeout);
await expect(getMergeTimeoutAndApprovalRequiredCount(context, { owner: "ubiquibot", repo: "automated-merging", issue_number: 1 })).resolves.toEqual({
mergeTimeout: contributorMergeTimeout,
requiredApprovalCount: contributorMinimumApprovalsRequired,
});
});
});

0 comments on commit ccf831a

Please sign in to comment.