Merge pull request #62 from gentlementlegen/fix/reminders

fix: avoid comments on closed / merged pull-requests

package.json

@@ -37,7 +37,6 @@
     "@ubiquity-os/plugin-sdk": "^1.1.1",
     "@ubiquity-os/ubiquity-os-logger": "^1.3.2",
     "dotenv": "16.4.5",
-    "hono": "^4.6.12",
     "luxon": "3.4.4",
     "ms": "2.1.3"
   },

src/helpers/remind-and-remove.ts

@@ -58,9 +58,11 @@ async function remindAssignees(context: ContextPlugin, issue: ListIssueForRepo)
       body: [logMessage.logMessage.raw, metadata].join("\n"),
     });
   } else {
-    const pullRequests = await collectLinkedPullRequests(context, { repo, owner, issue_number });
-    let shouldPostToMainIssue = false;
-    for (const pullRequest of pullRequests) {
+    const openedLinkedPullRequests = (await collectLinkedPullRequests(context, { repo, owner, issue_number }))
+      // We filter out closed and merged PRs to avoid commenting on these
+      .filter((o) => o.state === "OPEN");
+    let shouldPostToMainIssue = openedLinkedPullRequests.length === 0;
+    for (const pullRequest of openedLinkedPullRequests) {
       const { owner: prOwner, repo: prRepo, issue_number: prNumber } = parseIssueUrl(pullRequest.url);
       try {
         await octokit.rest.issues.createComment({

src/helpers/task-update.ts

@@ -51,12 +51,15 @@ export async function updateTaskReminder(context: ContextPlugin, repo: ListForOr
   const activityDate = activityEvent?.created_at ? DateTime.fromISO(activityEvent.created_at) : undefined;
   let mostRecentActivityDate = getMostRecentActivityDate(assignedDate, activityDate);
 
-  const linkedPrUrls: string[] = (await collectLinkedPullRequests(context, { issue_number: issue.number, repo: repo.name, owner: repo.owner.login })).map(
-    (o) => o.url
-  );
-  linkedPrUrls.push(issue.html_url);
+  const openedLinkedPullRequestUrls: string[] = (
+    await collectLinkedPullRequests(context, { issue_number: issue.number, repo: repo.name, owner: repo.owner.login })
+  )
+    // We filter out closed and merged PRs to avoid commenting on these
+    .filter((o) => o.state === "OPEN")
+    .map((o) => o.url);
+  openedLinkedPullRequestUrls.push(issue.html_url);
   const lastReminders: RestEndpointMethodTypes["issues"]["listComments"]["response"]["data"][] = await Promise.all(
-    linkedPrUrls.map(async (url) => {
+    openedLinkedPullRequestUrls.map(async (url) => {
       const { issue_number, owner, repo } = parseIssueUrl(url);
       const comments = await getCommentsFromMetadata(context, issue_number, owner, repo, FOLLOWUP_HEADER);
       return comments.filter((o) => DateTime.fromISO(o.created_at) > mostRecentActivityDate);

tests/reminders.test.ts

@@ -0,0 +1,87 @@
+import { beforeEach, describe, jest } from "@jest/globals";
+import { Logs } from "@ubiquity-os/ubiquity-os-logger";
+import { FOLLOWUP_HEADER } from "../src/types/constants";
+import { ListForOrg, ListIssueForRepo } from "../src/types/github-types";
+import { ContextPlugin } from "../src/types/plugin-input";
+
+describe("Reminder tests", () => {
+  beforeEach(() => {
+    jest.resetModules();
+    jest.resetAllMocks();
+  });
+
+  it("Should post reminders only on opened linked pull-requests", async () => {
+    jest.unstable_mockModule("../src/helpers/task-metadata", () => {
+      return {
+        getTaskAssignmentDetails: jest.fn(() => ({ startPlusLabelDuration: "1", taskAssignees: [1] })),
+        parsePriorityLabel: jest.fn(),
+      };
+    });
+    jest.unstable_mockModule("../src/helpers/get-assignee-activity", () => {
+      return {
+        getAssigneesActivityForIssue: jest.fn(() => []),
+      };
+    });
+    jest.unstable_mockModule("../src/helpers/collect-linked-pulls", () => {
+      return {
+        collectLinkedPullRequests: jest.fn(() => [
+          {
+            id: 2,
+            state: "MERGED",
+            url: "https://github.com/ubiquity-os/daemon-disqualifier/pull/2",
+          },
+          {
+            id: 3,
+            state: "CLOSE",
+            url: "https://github.com/ubiquity-os/daemon-disqualifier/pull/3",
+          },
+          {
+            id: 4,
+            state: "OPEN",
+            url: "https://github.com/ubiquity-os/daemon-disqualifier/pull/4",
+          },
+        ]),
+      };
+    });
+    const f = jest.fn(() => []);
+    jest.unstable_mockModule("../src/helpers/structured-metadata", () => {
+      return {
+        getCommentsFromMetadata: f,
+        createStructuredMetadata: jest.fn(() => ""),
+      };
+    });
+    const { updateTaskReminder } = await import("../src/helpers/task-update");
+    await updateTaskReminder(
+      {
+        logger: new Logs("debug"),
+        octokit: {
+          rest: {
+            issues: {
+              listEvents: jest.fn(() => [
+                {
+                  event: "assigned",
+                  actor: {
+                    id: 1,
+                  },
+                },
+              ]),
+            },
+          },
+          paginate: jest.fn((func: Function, args: unknown) => func(args)),
+        },
+        config: {},
+      } as unknown as ContextPlugin,
+      {
+        owner: {
+          login: "ubiquity-os",
+        },
+        name: "daemon-disqualifier",
+      } as unknown as ListForOrg["data"][0],
+      { number: 1, html_url: "https://github.com/ubiquity-os/daemon-disqualifier/issue/1" } as unknown as ListIssueForRepo
+    );
+    // We expect it to be called 2 times because one pull-request is merged and one is closed
+    expect(f).toHaveBeenCalledTimes(2);
+    expect(f).toHaveBeenCalledWith(expect.anything(), 1, "ubiquity-os", "daemon-disqualifier", FOLLOWUP_HEADER);
+    expect(f).toHaveBeenCalledWith(expect.anything(), 4, "ubiquity-os", "daemon-disqualifier", FOLLOWUP_HEADER);
+  });
+});