From ccf831abf84e412d5afd7f7fef122f71031bf83a Mon Sep 17 00:00:00 2001
From: Mentlegen <9807008+gentlementlegen@users.noreply.github.com>
Date: Sun, 7 Jul 2024 12:29:34 +0900
Subject: [PATCH] chore: added required validation check

---
 src/helpers/github.ts               | 22 ++++++++++++++++++----
 src/helpers/update-pull-requests.ts | 14 +++++++++-----
 tests/main.test.ts                  | 18 ++++++++++++++----
 3 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/src/helpers/github.ts b/src/helpers/github.ts
index 277d4898..aa5011a6 100644
--- a/src/helpers/github.ts
+++ b/src/helpers/github.ts
@@ -18,9 +18,9 @@ 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({
@@ -28,12 +28,26 @@ export async function getMergeTimeout(context: Context, { repo, owner }: IssuePa
         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;
+  }
+}
diff --git a/src/helpers/update-pull-requests.ts b/src/helpers/update-pull-requests.ts
index fb1f127f..88920756 100644
--- a/src/helpers/update-pull-requests.ts
+++ b/src/helpers/update-pull-requests.ts
@@ -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;
@@ -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})`);
diff --git a/tests/main.test.ts b/tests/main.test.ts
index 851c0fab..c4e7b97d 100644
--- a/tests/main.test.ts
+++ b/tests/main.test.ts
@@ -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";
@@ -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,
@@ -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",
@@ -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,
+    });
   });
 });