From ca1bc9f9191b3c8550e05ef06808b9f2f13fb732 Mon Sep 17 00:00:00 2001
From: Keyrxng <106303466+Keyrxng@users.noreply.github.com>
Date: Thu, 12 Sep 2024 13:22:50 +0100
Subject: [PATCH] fix: use task assignment event fallback for lastCheck

---
 src/handlers/collect-linked-pulls.ts |   2 +-
 src/helpers/task-metadata.ts         | 116 ++++++++++++++++++---------
 src/types/github-types.ts            |   3 +-
 tests/__mocks__/handlers.ts          |   4 +-
 tests/__mocks__/helpers.ts           |  49 ++++++++++-
 tests/__mocks__/strings.ts           |   4 +
 tests/main.test.ts                   |  59 +++++---------
 7 files changed, 154 insertions(+), 83 deletions(-)

diff --git a/src/handlers/collect-linked-pulls.ts b/src/handlers/collect-linked-pulls.ts
index 15d3180..6875c01 100644
--- a/src/handlers/collect-linked-pulls.ts
+++ b/src/handlers/collect-linked-pulls.ts
@@ -63,4 +63,4 @@ export async function collectLinkedPullRequests(context: Context, issue: {
   });
 
   return result.repository.issue.closedByPullRequestsReferences.edges.map((edge) => edge.node);
-}
+}
\ No newline at end of file
diff --git a/src/helpers/task-metadata.ts b/src/helpers/task-metadata.ts
index b5048eb..bd475bc 100644
--- a/src/helpers/task-metadata.ts
+++ b/src/helpers/task-metadata.ts
@@ -7,7 +7,7 @@ export async function getTaskMetadata(
     context: Context,
     repo: ListForOrg["data"][0],
     issue: ListIssueForRepo
-) {
+): Promise<{ metadata: { taskDeadline: string; taskAssignees: number[] }, lastCheck: DateTime } | false> {
     const { logger, octokit } = context;
 
     const comments = (await octokit.paginate(octokit.rest.issues.listComments, {
@@ -18,8 +18,7 @@ export async function getTaskMetadata(
     })) as ListCommentsForIssue[];
 
     const botComments = comments.filter((o) => o.user?.type === "Bot");
-    const taskDeadlineJsonRegex = /"taskDeadline": "([^"]*)"/g;
-    const taskAssigneesJsonRegex = /"taskAssignees": \[([^\]]*)\]/g;
+    // Has the bot assigned them, typically via the `/start` command
     const assignmentRegex = /Ubiquity - Assignment - start -/gi;
     const botAssignmentComments = botComments.filter(
         (o) => assignmentRegex.test(o?.body || "")
@@ -27,31 +26,42 @@ export async function getTaskMetadata(
         DateTime.fromISO(a.created_at).toMillis() - DateTime.fromISO(b.created_at).toMillis()
     )
 
+    // Has the bot previously reminded them?
     const botFollowup = /<!-- Ubiquity - Followup - remindAssignees/gi;
-    const botFollowupComments = botComments.filter((o) => botFollowup.test(o?.body || ""));
-
-    if (!botAssignmentComments.length && !botFollowupComments.length) {
-        logger.info(`No assignment or followup comments found for ${issue.html_url}`);
-        return false;
-    }
+    const botFollowupComments = botComments.filter((o) => botFollowup.test(o?.body || ""))
+        .sort((a, b) =>
+            DateTime.fromISO(a.created_at).toMillis() - DateTime.fromISO(b.created_at).toMillis()
+        );
 
+    // `lastCheck` represents the last time the bot intervened in the issue, separate from the activity tracking of a user.
     const lastCheckComment = botFollowupComments[0]?.created_at ? botFollowupComments[0] : botAssignmentComments[0];
-    const lastCheck = DateTime.fromISO(lastCheckComment.created_at);
+    let lastCheck = lastCheckComment?.created_at ? DateTime.fromISO(lastCheckComment.created_at) : null;
+
+    // if we don't have a lastCheck yet, use the assignment event
+    if (!lastCheck) {
+        const assignmentEvents = await octokit.paginate(octokit.rest.issues.listEvents, {
+            owner: repo.owner.login,
+            repo: repo.name,
+            issue_number: issue.number
+        });
 
-    const taskDeadlineMatch = taskDeadlineJsonRegex.exec(botAssignmentComments[0]?.body || "");
-    const taskAssigneesMatch = taskAssigneesJsonRegex.exec(botAssignmentComments[0]?.body || "");
+        const assignmentEvent = assignmentEvents.find((o) => o.event === "assigned");
+        if (assignmentEvent) {
+            lastCheck = DateTime.fromISO(assignmentEvent.created_at);
+        } else {
+            logger.error(`Failed to find last check for ${issue.html_url}`);
+            return false;
+        }
+    }
+
+    if (!lastCheck) {
+        logger.error(`Failed to find last check for ${issue.html_url}`);
+        return false;
+    }
 
     const metadata = {
-        taskDeadline: taskDeadlineMatch?.[1] || "",
-        taskAssignees: taskAssigneesMatch?.[1]
-            .split(",")
-            .map((o) => o.trim())
-            .map(Number),
-    };
-
-    // supporting legacy format
-    if (!metadata.taskAssignees?.length) {
-        metadata.taskAssignees = issue.assignees ? issue.assignees.map((o) => o.id) : issue.assignee ? [issue.assignee.id] : [];
+        taskDeadline: "",
+        taskAssignees: issue.assignees ? issue.assignees.map((o) => o.id) : issue.assignee ? [issue.assignee.id] : []
     }
 
     if (!metadata.taskAssignees?.length) {
@@ -59,24 +69,54 @@ export async function getTaskMetadata(
         return false;
     }
 
-    if (!metadata.taskDeadline) {
-        const taskDeadlineJsonRegex = /"duration": ([^,]*),/g;
-        const taskDeadlineMatch = taskDeadlineJsonRegex.exec(botAssignmentComments[0]?.body || "");
-        if (!taskDeadlineMatch) {
-            logger.error(`Missing deadline from ${issue.html_url}`);
-            return false;
+    const durationInMs = parseTimeLabel(issue.labels);
+
+    if (durationInMs === 0) {
+        // it could mean there was no time label set on the issue
+        // but it could still be workable and priced
+    } else if (durationInMs < 0 || !durationInMs) {
+        logger.error(`Invalid deadline found on ${issue.html_url}`);
+        return false;
+    }
+
+    metadata.taskDeadline = DateTime.fromMillis(lastCheck.toMillis() + durationInMs).toISO() || "";
+
+    return { metadata, lastCheck };
+}
+
+function parseTimeLabel(labels: (string | {
+    id?: number;
+    node_id?: string;
+    url?: string;
+    name?: string;
+    description?: string | null;
+    color?: string | null;
+    default?: boolean;
+})[]): number {
+    let taskTimeEstimate = 0;
+
+    for (const label of labels) {
+        let timeLabel = "";
+        if (typeof label === "string") {
+            timeLabel = label;
+        } else {
+            timeLabel = label.name || "";
         }
-        const duration = taskDeadlineMatch[1] || "";
-        const durationInMs = ms(duration);
-        if (durationInMs === 0) {
-            // it could mean there was no time label set on the issue
-            // but it could still be workable and priced
-        } else if (durationInMs < 0 || !durationInMs) {
-            logger.error(`Invalid deadline found on ${issue.html_url}`);
-            return false;
+
+        if (timeLabel.startsWith("Time:")) {
+            const matched = timeLabel.match(/Time: <(\d+) (\w+)/i);
+            if (!matched) {
+                return 0;
+            }
+
+            const [_, duration, unit] = matched;
+            taskTimeEstimate = ms(`${duration} ${unit}`);
+        }
+
+        if (taskTimeEstimate) {
+            break;
         }
-        metadata.taskDeadline = DateTime.fromMillis(lastCheck.toMillis() + durationInMs).toISO() || "";
     }
 
-    return { metadata, lastCheck };
+    return taskTimeEstimate;
 }
\ No newline at end of file
diff --git a/src/types/github-types.ts b/src/types/github-types.ts
index c6d5995..23f60ce 100644
--- a/src/types/github-types.ts
+++ b/src/types/github-types.ts
@@ -3,4 +3,5 @@ import { RestEndpointMethodTypes } from "@octokit/rest";
 export type ListForOrg = RestEndpointMethodTypes["repos"]["listForOrg"]["response"];
 export type ListIssueForRepo = RestEndpointMethodTypes["issues"]["listForRepo"]["response"]["data"][0];
 export type ListCommentsForIssue = RestEndpointMethodTypes["issues"]["listComments"]["response"]["data"][0];
-export type GitHubListEvents = RestEndpointMethodTypes["issues"]["listEvents"]["response"]["data"][0];
\ No newline at end of file
+export type GitHubListEvents = RestEndpointMethodTypes["issues"]["listEvents"]["response"]["data"][0];
+export type Label = RestEndpointMethodTypes["issues"]["getLabel"]["response"]["data"];
\ No newline at end of file
diff --git a/tests/__mocks__/handlers.ts b/tests/__mocks__/handlers.ts
index c5469c7..c2e555f 100644
--- a/tests/__mocks__/handlers.ts
+++ b/tests/__mocks__/handlers.ts
@@ -1,6 +1,5 @@
 import { http, HttpResponse } from "msw";
 import { db } from "./db";
-import issueEventsGet from "./routes/get-events.json";
 import issuesLabelsGet from "./routes/get-labels.json";
 import issueTimeline from "./routes/get-timeline.json";
 
@@ -9,7 +8,7 @@ import issueTimeline from "./routes/get-timeline.json";
  */
 export const handlers = [
   http.get("https://api.github.com/repos/:owner/:repo/issues/:id/events", () => {
-    return HttpResponse.json(issueEventsGet);
+    return HttpResponse.json(db.event.getAll());
   }),
   http.get("https://api.github.com/repos/:owner/:repo/issues/:id/labels", () => {
     return HttpResponse.json(issuesLabelsGet);
@@ -37,7 +36,6 @@ export const handlers = [
     if (!comment) {
       return HttpResponse.json({ message: "No body" });
     }
-
     db.issueComments.create({ issueId: Number(id), body: comment, created_at: new Date().toISOString(), id: db.issueComments.count() + 1, owner: { login: owner as string }, repo: { name: repo as string } });
     return HttpResponse.json({ message: "Comment created" });
   }),
diff --git a/tests/__mocks__/helpers.ts b/tests/__mocks__/helpers.ts
index 24fa7f3..0b29e03 100644
--- a/tests/__mocks__/helpers.ts
+++ b/tests/__mocks__/helpers.ts
@@ -6,7 +6,11 @@ import { STRINGS, getIssueUrl, getIssueHtmlUrl } from "./strings";
 export const ONE_DAY = 1000 * 60 * 60 * 24;
 
 export function createRepo(name?: string, id?: number, owner?: string) {
-    db.repo.create({ ...repoTemplate, id: id || 1, name: name || repoTemplate.name, owner: { login: owner || STRINGS.UBIQUITY } });
+    db.repo.create({
+        ...repoTemplate, id: id || 1, name: name || repoTemplate.name, owner: { login: owner || STRINGS.UBIQUITY },
+        url: `https://api.github.com/repos/${owner || STRINGS.UBIQUITY}/${name || repoTemplate.name}`,
+        html_url: `https://github.com/${owner || STRINGS.UBIQUITY}/${name || repoTemplate.name}`
+    });
 }
 
 export function createComment(
@@ -39,11 +43,50 @@ export function createIssue(
         id,
         assignees: assignees.length ? assignees : [{ login: STRINGS.UBIQUITY, id: 1 }],
         created_at: created_at || new Date(Date.now() - ONE_DAY).toISOString(),
-        url: getIssueUrl(id),
-        html_url: getIssueHtmlUrl(id),
+        url: `https://github.com/${owner || STRINGS.UBIQUITY}/${repo || "test-repo"}/issues/${id}`,
+        html_url: `https://github.com/${owner || STRINGS.UBIQUITY}/${repo || "test-repo"}/issues/${id}`,
         owner: { login: owner || STRINGS.UBIQUITY },
         number: id,
         repo: repo || "test-repo",
         body: body || "test",
     });
 }
+
+export function createEvent() {
+    db.event.create({
+        id: 1,
+        actor: {
+            id: 1,
+            type: "User",
+            login: "ubiquity",
+            name: null,
+        },
+        owner: "ubiquity",
+        repo: "test-repo",
+        issue_number: 1,
+        event: "assigned",
+        commit_id: null,
+        commit_url: "https://github.com/ubiquity/test-repo/commit/1",
+        created_at: new Date(Date.now() - ONE_DAY).toISOString(),
+        assignee: {
+            login: "ubiquity",
+        },
+        source: {
+            issue: {
+                number: 1,
+                html_url: getIssueHtmlUrl(1),
+                body: "test",
+                pull_request: {
+                    merged_at: new Date(Date.now() - ONE_DAY).toISOString(),
+                },
+                repository: {
+                    full_name: "ubiquity/test-repo",
+                },
+                state: "open",
+                user: {
+                    login: "ubiquity",
+                },
+            }
+        }
+    });
+}
\ No newline at end of file
diff --git a/tests/__mocks__/strings.ts b/tests/__mocks__/strings.ts
index 8d12f78..aea51a1 100644
--- a/tests/__mocks__/strings.ts
+++ b/tests/__mocks__/strings.ts
@@ -55,6 +55,10 @@ export function botAssignmentComment(assigneeId: number, deadlineStr: string) {
 -->`;
 }
 
+export function lastCheckWasOn(deadlineStr: string) {
+    return `Last check was on ${deadlineStr}`;
+}
+
 export function updatingRemindersFor(repo: string) {
     return `Updating reminders for ${repo}`;
 }
diff --git a/tests/main.test.ts b/tests/main.test.ts
index 83a6e2d..7efe18c 100644
--- a/tests/main.test.ts
+++ b/tests/main.test.ts
@@ -10,10 +10,9 @@ import dotenv from "dotenv";
 import { Logs } from "@ubiquity-dao/ubiquibot-logger";
 import { Context } from "../src/types/context";
 import mockUsers from "./__mocks__/mock-users";
-import { botAssignmentComment, getIssueHtmlUrl, getIssueUrl, noAssignmentCommentFor, STRINGS, updatingRemindersFor } from "./__mocks__/strings";
-import { createComment, createIssue, createRepo, ONE_DAY } from "./__mocks__/helpers";
+import { botAssignmentComment, getIssueHtmlUrl, STRINGS } from "./__mocks__/strings";
+import { createComment, createEvent, createIssue, createRepo, ONE_DAY } from "./__mocks__/helpers";
 import { collectLinkedPullRequests } from "../src/handlers/collect-linked-pulls";
-import { createStructuredMetadata } from "../src/helpers/structured-metadata";
 
 dotenv.config();
 const octokit = jest.requireActual("@octokit/rest");
@@ -57,22 +56,23 @@ describe("User start/stop", () => {
     const infoSpy = jest.spyOn(context.logger, "info");
     await runPlugin(context);
 
-    expect(infoSpy).toHaveBeenNthCalledWith(1, noAssignmentCommentFor(getIssueHtmlUrl(1)));
-    expect(infoSpy).toHaveBeenNthCalledWith(2, noAssignmentCommentFor(getIssueHtmlUrl(2)));
-    expect(infoSpy).toHaveBeenNthCalledWith(3, noAssignmentCommentFor(getIssueHtmlUrl(3)));
-    expect(infoSpy).toHaveBeenNthCalledWith(4, noAssignmentCommentFor(getIssueHtmlUrl(4)));
+    expect(infoSpy).toHaveBeenNthCalledWith(1, `Nothing to do for ${getIssueHtmlUrl(1)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(3, `Nothing to do for ${getIssueHtmlUrl(2)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(5, `Nothing to do for ${getIssueHtmlUrl(3)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(7, `Nothing to do for ${getIssueHtmlUrl(4)}, still within due-time.`);
+    expect(infoSpy).not.toHaveBeenNthCalledWith(9, `Nothing to do for https://github.com/ubiquity/private-repo/issues/5, still within due-time.`);
   });
 
   it("Should include the previously excluded repo", async () => {
-    const context = createContext(1, 1);
+    const context = createContext(1, 1, []);
     const infoSpy = jest.spyOn(context.logger, "info");
-    context.config.watch.optOut = [];
     await runPlugin(context);
 
-    expect(infoSpy).toHaveBeenNthCalledWith(1, noAssignmentCommentFor(getIssueHtmlUrl(1)));
-    expect(infoSpy).toHaveBeenNthCalledWith(2, noAssignmentCommentFor(getIssueHtmlUrl(2)));
-    expect(infoSpy).toHaveBeenNthCalledWith(3, noAssignmentCommentFor(getIssueHtmlUrl(3)));
-    expect(infoSpy).toHaveBeenNthCalledWith(4, noAssignmentCommentFor(getIssueHtmlUrl(4)));
+    expect(infoSpy).toHaveBeenNthCalledWith(1, `Nothing to do for ${getIssueHtmlUrl(1)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(3, `Nothing to do for ${getIssueHtmlUrl(2)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(5, `Nothing to do for ${getIssueHtmlUrl(3)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(7, `Nothing to do for ${getIssueHtmlUrl(4)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(9, `Nothing to do for https://github.com/ubiquity/private-repo/issues/5, still within due-time.`);
   });
 
   it("Should eject the user after the disqualification period", async () => {
@@ -87,8 +87,7 @@ describe("User start/stop", () => {
 
     await runPlugin(context);
 
-    expect(infoSpy).toHaveBeenNthCalledWith(1, `Assignees mismatch found for ${getIssueHtmlUrl(1)}`, { caller: STRINGS.LOGS_ANON_CALLER, assigneeIds: [1], metadata: { taskDeadline: timestamp, taskAssignees: [2] } });
-    expect(infoSpy).toHaveBeenNthCalledWith(2, `Passed the deadline on ${getIssueHtmlUrl(1)} and no activity is detected, removing assignees.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(1, `Nothing to do for ${getIssueHtmlUrl(1)}, still within due-time.`);
     expect(infoSpy).toHaveBeenNthCalledWith(3, `Passed the deadline on ${getIssueHtmlUrl(2)} and no activity is detected, removing assignees.`);
     expect(infoSpy).toHaveBeenNthCalledWith(4, `Passed the deadline on ${getIssueHtmlUrl(3)} and no activity is detected, removing assignees.`);
 
@@ -98,8 +97,6 @@ describe("User start/stop", () => {
 
   it("Should warn the user after the warning period", async () => {
     const context = createContext(4, 2);
-    const infoSpy = jest.spyOn(context.logger, "info");
-
     const timestamp = daysPriorToNow(5);
 
     createComment(3, 3, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);
@@ -109,13 +106,6 @@ describe("User start/stop", () => {
 
     await runPlugin(context);
 
-    expect(infoSpy).toHaveBeenNthCalledWith(1, `Assignees mismatch found for ${getIssueHtmlUrl(1)}`, {
-      caller: STRINGS.LOGS_ANON_CALLER, assigneeIds: [1], metadata: {
-        taskDeadline: timestamp,
-        taskAssignees: [2],
-      }
-    });
-
     const updatedIssue = db.issue.findFirst({ where: { id: { equals: 4 } } });
     expect(updatedIssue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);
 
@@ -137,17 +127,9 @@ describe("User start/stop", () => {
 
     await runPlugin(context);
 
-    expect(infoSpy).toHaveBeenNthCalledWith(1, `Assignees mismatch found for ${getIssueHtmlUrl(1)}`, {
-      caller: STRINGS.LOGS_ANON_CALLER,
-      assigneeIds: [1],
-      metadata: {
-        taskDeadline: timestamp,
-        taskAssignees: [2],
-      }
-    });
-    expect(infoSpy).toHaveBeenNthCalledWith(2, `Nothing to do for ${getIssueHtmlUrl(1)}, still within due-time.`);
-    expect(infoSpy).toHaveBeenNthCalledWith(4, `Nothing to do for ${getIssueHtmlUrl(2)}, still within due-time.`);
-    expect(infoSpy).toHaveBeenNthCalledWith(6, `Nothing to do for ${getIssueHtmlUrl(3)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(1, `Nothing to do for ${getIssueHtmlUrl(1)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(3, `Nothing to do for ${getIssueHtmlUrl(2)}, still within due-time.`);
+    expect(infoSpy).toHaveBeenNthCalledWith(5, `Nothing to do for ${getIssueHtmlUrl(3)}, still within due-time.`);
 
     const updatedIssue = db.issue.findFirst({ where: { id: { equals: 4 } } });
     expect(updatedIssue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);
@@ -197,16 +179,19 @@ async function setupTests() {
   createIssue(3, [{ login: STRINGS.USER, id: 2 }], STRINGS.UBIQUITY, daysPriorToNow(4), "fixes #2");
   // disqualification
   createIssue(4, [{ login: STRINGS.USER, id: 2 }], STRINGS.UBIQUITY, daysPriorToNow(12), "closes #3");
+  createIssue(5, [{ login: STRINGS.USER, id: 2 }], STRINGS.UBIQUITY, daysPriorToNow(12), "closes #1", STRINGS.PRIVATE_REPO_NAME);
 
   createComment(1, 1, STRINGS.UBIQUITY);
   createComment(2, 2, STRINGS.UBIQUITY);
+
+  createEvent()
 }
 
 function daysPriorToNow(days: number) {
   return new Date(Date.now() - ONE_DAY * days).toISOString();
 }
 
-function createContext(issueId: number, senderId: number): Context<"issue_comment.created"> {
+function createContext(issueId: number, senderId: number, optOut = [STRINGS.PRIVATE_REPO_NAME]): Context<"issue_comment.created"> {
   return {
     payload: {
       issue: db.issue.findFirst({ where: { id: { equals: issueId } } }) as unknown as Context<"issue_comment.created">["payload"]["issue"],
@@ -221,7 +206,7 @@ function createContext(issueId: number, senderId: number): Context<"issue_commen
     config: {
       disqualification: ONE_DAY * 7,
       warning: ONE_DAY * 3.5,
-      watch: { optOut: [STRINGS.PRIVATE_REPO_NAME] },
+      watch: { optOut },
     },
     octokit: new octokit.Octokit(),
     eventName: "issue_comment.created",