Merge pull request #30 from ubiquity-os-marketplace/ubq-testing-fix-i…

…ssues

fix: #21

.cspell.json

@@ -4,7 +4,22 @@
   "ignorePaths": ["**/*.json", "**/*.css", "node_modules", "**/*.log", "**/*.http", "**/*.toml", "src/types/database.ts", "supabase/migrations/**", "tests/**"],
   "useGitignore": true,
   "language": "en",
-  "words": ["dataurl", "devpool", "outdir", "servedir", "typebox", "supabase", "ubiquibot", "mswjs", "luxon", "millis", "handl"],
+  "words": [
+    "dataurl",
+    "devpool",
+    "outdir",
+    "servedir",
+    "typebox",
+    "supabase",
+    "ubiquibot",
+    "mswjs",
+    "luxon",
+    "millis",
+    "handl",
+    "sonarjs",
+    "mischeck",
+    "unassigns"
+  ],
   "dictionaries": ["typescript", "node", "software-terms"],
   "import": ["@cspell/dict-typescript/cspell-ext.json", "@cspell/dict-node/cspell-ext.json", "@cspell/dict-software-terms"],
   "ignoreRegExpList": ["[0-9a-fA-F]{6}"]

README.md

@@ -43,4 +43,9 @@ yarn test
       optOut:
         - "repoName"
         - "repoName2"
+    eventWhitelist: # these are the tail of the webhook event i.e pull_request.review_requested
+      - "review_requested"
+      - "ready_for_review"
+      - "commented"
+      - "committed"
 ```

manifest.json

@@ -7,6 +7,7 @@
     "push"
   ],
   "configuration": {
+    "default": {},
     "type": "object",
     "properties": {
       "warning": {
@@ -30,12 +31,26 @@
       "disqualification": {
         "default": "7 days",
         "type": "string"
+      },
+      "eventWhitelist": {
+        "default": [
+          "pull_request.review_requested",
+          "pull_request.ready_for_review",
+          "pull_request_review_comment.created",
+          "issue_comment.created",
+          "push"
+        ],
+        "type": "array",
+        "items": {
+          "type": "string"
+        }
       }
     },
     "required": [
       "warning",
       "watch",
-      "disqualification"
+      "disqualification",
+      "eventWhitelist"
     ]
   }
 }

src/helpers/get-assignee-activity.ts

@@ -2,14 +2,14 @@ import { DateTime } from "luxon";
 import { collectLinkedPullRequests } from "../handlers/collect-linked-pulls";
 import { Context } from "../types/context";
 import { parseIssueUrl } from "./github-url";
-import { GitHubListEvents, ListIssueForRepo } from "../types/github-types";
+import { GitHubTimelineEvents, ListIssueForRepo } from "../types/github-types";
 
 /**
  * Retrieves all the activity for users that are assigned to the issue. Also takes into account linked pull requests.
  */
 export async function getAssigneesActivityForIssue(context: Context, issue: ListIssueForRepo, assigneeIds: number[]) {
   const gitHubUrl = parseIssueUrl(issue.html_url);
-  const issueEvents: GitHubListEvents[] = await context.octokit.paginate(context.octokit.rest.issues.listEvents, {
+  const issueEvents: GitHubTimelineEvents[] = await context.octokit.paginate(context.octokit.rest.issues.listEventsForTimeline, {
     owner: gitHubUrl.owner,
     repo: gitHubUrl.repo,
     issue_number: gitHubUrl.issue_number,
@@ -18,7 +18,7 @@ export async function getAssigneesActivityForIssue(context: Context, issue: List
   const linkedPullRequests = await collectLinkedPullRequests(context, gitHubUrl);
   for (const linkedPullRequest of linkedPullRequests) {
     const { owner, repo, issue_number } = parseIssueUrl(linkedPullRequest.url || "");
-    const events = await context.octokit.paginate(context.octokit.rest.issues.listEvents, {
+    const events: GitHubTimelineEvents[] = await context.octokit.paginate(context.octokit.rest.issues.listEventsForTimeline, {
       owner,
       repo,
       issue_number,
@@ -27,7 +27,53 @@ export async function getAssigneesActivityForIssue(context: Context, issue: List
     issueEvents.push(...events);
   }
 
-  return issueEvents
-    .filter((o) => o.actor && o.actor.id && assigneeIds.includes(o.actor.id))
-    .sort((a, b) => DateTime.fromISO(b.created_at).toMillis() - DateTime.fromISO(a.created_at).toMillis());
+  return filterEvents(issueEvents, assigneeIds);
+}
+
+function filterEvents(issueEvents: GitHubTimelineEvents[], assigneeIds: number[]) {
+  const userIdMap = new Map<string, number>();
+
+  let assigneeEvents = [];
+
+  for (const event of issueEvents) {
+    let actorId = null;
+    let actorLogin = null;
+    let createdAt = null;
+    let eventName = event.event;
+
+    if ("actor" in event && event.actor) {
+      actorLogin = event.actor.login.toLowerCase();
+      if (!userIdMap.has(actorLogin)) {
+        userIdMap.set(actorLogin, event.actor.id);
+      }
+      actorId = userIdMap.get(actorLogin);
+      createdAt = event.created_at;
+    } else if (event.event === "committed") {
+      const commitAuthor = "author" in event ? event.author : null;
+      const commitCommitter = "committer" in event ? event.committer : null;
+
+      if (commitAuthor || commitCommitter) {
+        assigneeEvents.push({
+          event: eventName,
+          created_at: createdAt,
+        });
+
+        continue;
+      }
+    }
+
+    if (actorId && assigneeIds.includes(actorId)) {
+      assigneeEvents.push({
+        event: eventName,
+        created_at: createdAt,
+      });
+    }
+  }
+
+  return assigneeEvents.sort((a, b) => {
+    if (!a.created_at || !b.created_at) {
+      return 0;
+    }
+    return DateTime.fromISO(b.created_at).toMillis() - DateTime.fromISO(a.created_at).toMillis();
+  });
 }

src/helpers/task-deadline.ts

@@ -2,17 +2,29 @@ import { DateTime } from "luxon";
 import { Context } from "../types/context";
 import { ListIssueForRepo } from "../types/github-types";
 import { getAssigneesActivityForIssue } from "./get-assignee-activity";
+import { TimelineEvents } from "../types/plugin-inputs";
 
+/**
+ * Retrieves the deadline with the threshold for the issue.
+ *
+ * Uses `startPlusLabelDuration` to set a base deadline and then checks for any activity that has happened after that.
+ *
+ * If activity if detected after the deadline, it will adjust the `deadlineWithThreshold` to the most recent activity.
+ *
+ * Recent activity is determined by the `eventWhitelist`.
+ */
 export async function getDeadlineWithThreshold(
   context: Context,
   metadata: {
-    taskDeadline: string;
+    startPlusLabelDuration: string | null;
     taskAssignees: number[] | undefined;
   },
-  issue: ListIssueForRepo,
-  lastCheck: DateTime
+  issue: ListIssueForRepo
 ) {
-  const { logger, config } = context;
+  const {
+    logger,
+    config: { disqualification, warning, eventWhitelist },
+  } = context;
 
   const assigneeIds = issue.assignees?.map((o) => o.id) || [];
 
@@ -23,26 +35,43 @@ export async function getDeadlineWithThreshold(
     });
   }
 
-  const deadline = DateTime.fromISO(metadata.taskDeadline);
-  const now = DateTime.now();
-
-  if (!deadline.isValid && !lastCheck.isValid) {
-    logger.error(`Invalid date found on ${issue.html_url}`);
+  const deadline = DateTime.fromISO(metadata.startPlusLabelDuration || issue.created_at);
+  if (!deadline.isValid) {
+    logger.error(`Invalid deadline date found on ${issue.html_url}`);
     return false;
   }
 
+  // activity which has happened after either: A) issue start + time label duration or B) just issue creation date
   const activity = (await getAssigneesActivityForIssue(context, issue, assigneeIds)).filter((o) => {
-    return DateTime.fromISO(o.created_at) > lastCheck;
+    if (!o.created_at) {
+      return false;
+    }
+    return DateTime.fromISO(o.created_at) >= deadline;
+  });
+
+  const filteredActivity = activity.filter((o) => {
+    if (!o.event) {
+      return false;
+    }
+    return eventWhitelist.includes(o.event as TimelineEvents);
   });
 
-  let deadlineWithThreshold = deadline.plus({ milliseconds: config.disqualification });
-  let reminderWithThreshold = deadline.plus({ milliseconds: config.warning });
+  // adding the buffer onto the already established issueStart + timeLabelDuration
+  let deadlineWithThreshold = deadline.plus({ milliseconds: disqualification });
+  let reminderWithThreshold = deadline.plus({ milliseconds: warning });
 
-  if (activity?.length) {
-    const lastActivity = DateTime.fromISO(activity[0].created_at);
-    deadlineWithThreshold = lastActivity.plus({ milliseconds: config.disqualification });
-    reminderWithThreshold = lastActivity.plus({ milliseconds: config.warning });
+  // if there is any activity that has happened after the deadline, we need to adjust the deadlineWithThreshold
+  if (filteredActivity?.length) {
+    // use the most recent activity or the initial deadline
+    const lastActivity = filteredActivity[0].created_at ? DateTime.fromISO(filteredActivity[0].created_at) : deadline;
+    if (!lastActivity.isValid) {
+      logger.error(`Invalid date found on last activity for ${issue.html_url}`);
+      return false;
+    }
+    // take the last activity and add the buffer onto it
+    deadlineWithThreshold = lastActivity.plus({ milliseconds: disqualification });
+    reminderWithThreshold = lastActivity.plus({ milliseconds: warning });
   }
 
-  return { deadlineWithThreshold, reminderWithThreshold, now };
+  return { deadlineWithThreshold, reminderWithThreshold };
 }

src/helpers/task-metadata.ts

@@ -1,64 +1,45 @@
 import { DateTime } from "luxon";
 import { Context } from "../types/context";
-import { ListCommentsForIssue, ListForOrg, ListIssueForRepo } from "../types/github-types";
+import { ListForOrg, ListIssueForRepo } from "../types/github-types";
 import ms from "ms";
 
-export async function getTaskMetadata(
+/**
+ * Retrieves assignment events from the timeline of an issue and calculates the deadline based on the time label.
+ *
+ * It does not care about previous updates, comments or other events that might have happened on the issue.
+ *
+ * It returns who is assigned and the initial calculated deadline (start + time label duration).
+ */
+export async function getTaskAssignmentDetails(
   context: Context,
   repo: ListForOrg["data"][0],
   issue: ListIssueForRepo
-): Promise<{ metadata: { taskDeadline: string; taskAssignees: number[] }; lastCheck: DateTime } | false> {
+): Promise<{ startPlusLabelDuration: string; taskAssignees: number[] } | false> {
   const { logger, octokit } = context;
 
-  const comments = (await octokit.paginate(octokit.rest.issues.listComments, {
+  const assignmentEvents = await octokit.paginate(octokit.rest.issues.listEvents, {
     owner: repo.owner.login,
     repo: repo.name,
     issue_number: issue.number,
-    per_page: 100,
-  })) as ListCommentsForIssue[];
-
-  const botComments = comments.filter((o) => o.user?.type === "Bot");
-  // 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 || ""))
-    .sort((a, b) => 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 || ""))
-    .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];
-  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) {
-    logger.info("No last check found, using assignment event");
-    const assignmentEvents = await octokit.paginate(octokit.rest.issues.listEvents, {
-      owner: repo.owner.login,
-      repo: repo.name,
-      issue_number: issue.number,
-    });
-
-    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 assignedEvents = assignmentEvents
+    .filter((o) => o.event === "assigned")
+    .sort((a, b) => DateTime.fromISO(b.created_at).toMillis() - DateTime.fromISO(a.created_at).toMillis());
+
+  const latestUserAssignment = assignedEvents.find((o) => o.actor?.type === "User");
+  const latestBotAssignment = assignedEvents.find((o) => o.actor?.type === "Bot");
+
+  let mostRecentAssignmentEvent = latestUserAssignment || latestBotAssignment;
+
+  if (latestUserAssignment && latestBotAssignment && DateTime.fromISO(latestUserAssignment.created_at) > DateTime.fromISO(latestBotAssignment.created_at)) {
+    mostRecentAssignmentEvent = latestUserAssignment;
+  } else {
+    mostRecentAssignmentEvent = latestBotAssignment;
   }
 
   const metadata = {
-    taskDeadline: "",
+    startPlusLabelDuration: DateTime.fromISO(issue.created_at).toISO() || "",
     taskAssignees: issue.assignees ? issue.assignees.map((o) => o.id) : issue.assignee ? [issue.assignee.id] : [],
   };
 
@@ -77,9 +58,13 @@ export async function getTaskMetadata(
     return false;
   }
 
-  metadata.taskDeadline = DateTime.fromMillis(lastCheck.toMillis() + durationInMs).toISO() || "";
+  // if there are no assignment events, we can assume the deadline is the issue creation date
+  metadata.startPlusLabelDuration =
+    DateTime.fromISO(mostRecentAssignmentEvent?.created_at || issue.created_at)
+      .plus({ milliseconds: durationInMs })
+      .toISO() || "";
 
-  return { metadata, lastCheck };
+  return metadata;
 }
 
 function parseTimeLabel(