Skip to content

Commit

Permalink
chore: use only the timeline events
Browse files Browse the repository at this point in the history
  • Loading branch information
@Keyrxng
Keyrxng committed Sep 24, 2024
1 parent 1dc9c33 commit 68a3b58
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 93 deletions.
35 changes: 25 additions & 10 deletions src/helpers/task-deadline.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,22 @@ import { Context } from "../types/context";
import { ListIssueForRepo } from "../types/github-types";
import { getAssigneesActivityForIssue } from "./get-assignee-activity";

/**
* 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,
Expand All @@ -26,19 +34,18 @@ 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) => {
if (!o.created_at) {
return false;
}
return DateTime.fromISO(o.created_at) > lastCheck;
return DateTime.fromISO(o.created_at) >= deadline;
});

const filteredActivity = activity.filter((o) => {
Expand All @@ -48,14 +55,22 @@ export async function getDeadlineWithThreshold(
return eventWhitelist.includes(o.event);
});

// adding the buffer onto the already established issueStart + timeLabelDuration
let deadlineWithThreshold = deadline.plus({ milliseconds: disqualification });
let reminderWithThreshold = deadline.plus({ milliseconds: 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 intial 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 };
}
58 changes: 17 additions & 41 deletions src/helpers/task-metadata.ts
View file
Original file line number Diff line number Diff line change
@@ -1,40 +1,22 @@
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, {
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(b.created_at).toMillis() - DateTime.fromISO(a.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(b.created_at).toMillis() - DateTime.fromISO(a.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;

// incase their was self-assigning after the lastCheck
const assignmentEvents = await octokit.paginate(octokit.rest.issues.listEvents, {
owner: repo.owner.login,
repo: repo.name,
Expand All @@ -56,18 +38,8 @@ export async function getTaskMetadata(
mostRecentAssignmentEvent = latestBotAssignment;
}

if (mostRecentAssignmentEvent && (!lastCheck || DateTime.fromISO(mostRecentAssignmentEvent.created_at) > lastCheck)) {
lastCheck = DateTime.fromISO(mostRecentAssignmentEvent.created_at);
logger.debug(`Using assignment event`, { mostRecentAssignmentEvent });
}

if (!lastCheck) {
logger.error(`No last check found for ${issue.html_url}`);
return false;
}

const metadata = {
taskDeadline: "",
startPlusLabelDuration: DateTime.fromISO(issue.created_at).toISO() || "",
taskAssignees: issue.assignees ? issue.assignees.map((o) => o.id) : issue.assignee ? [issue.assignee.id] : [],
};

Expand All @@ -86,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(
Expand Down
30 changes: 15 additions & 15 deletions src/helpers/task-update.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,41 +3,41 @@ import { Context } from "../types/context";
import { ListForOrg, ListIssueForRepo } from "../types/github-types";
import { remindAssigneesForIssue, unassignUserFromIssue } from "./remind-and-remove";
import { getDeadlineWithThreshold } from "./task-deadline";
import { getTaskMetadata } from "./task-metadata";
import { getTaskAssignmentDetails } from "./task-metadata";

export async function updateTaskReminder(context: Context, repo: ListForOrg["data"][0], issue: ListIssueForRepo) {
const { logger } = context;

let metadata, lastCheck, deadlineWithThreshold, reminderWithThreshold, now;
let deadlineWithThreshold, reminderWithThreshold;
const now = DateTime.now();

const handledMetadata = await getTaskMetadata(context, repo, issue);
const handledMetadata = await getTaskAssignmentDetails(context, repo, issue);

if (handledMetadata) {
metadata = handledMetadata.metadata;
lastCheck = handledMetadata.lastCheck;

const handledDeadline = await getDeadlineWithThreshold(context, metadata, issue, lastCheck);
const handledDeadline = await getDeadlineWithThreshold(context, handledMetadata, issue);
if (handledDeadline) {
deadlineWithThreshold = handledDeadline.deadlineWithThreshold;
reminderWithThreshold = handledDeadline.reminderWithThreshold;
now = handledDeadline.now;

logger.info(`Handling metadata and deadline for ${issue.html_url}`, {
initialDeadline: DateTime.fromISO(handledMetadata.startPlusLabelDuration).toLocaleString(DateTime.DATETIME_MED),
now: now.toLocaleString(DateTime.DATETIME_MED),
reminderWithThreshold: reminderWithThreshold.toLocaleString(DateTime.DATETIME_MED),
deadlineWithThreshold: deadlineWithThreshold.toLocaleString(DateTime.DATETIME_MED),
});
}
}

if (!metadata || !lastCheck || !deadlineWithThreshold || !reminderWithThreshold || !now) {
if (!deadlineWithThreshold || !reminderWithThreshold) {
logger.error(`Failed to handle metadata or deadline for ${issue.html_url}`);
return false;
}

logger.info(`Last check was on ${lastCheck.toLocaleString(DateTime.DATETIME_MED)}`, {
now: now.toLocaleString(DateTime.DATETIME_MED),
reminder: reminderWithThreshold.toLocaleString(DateTime.DATETIME_MED),
deadline: deadlineWithThreshold.toLocaleString(DateTime.DATETIME_MED),
});

if (now >= deadlineWithThreshold) {
// if the issue is past due, we should unassign the user
await unassignUserFromIssue(context, issue);
} else if (now >= reminderWithThreshold) {
// if the issue is within the reminder threshold, we should remind the assignees
await remindAssigneesForIssue(context, issue);
} else {
logger.info(`Nothing to do for ${issue.html_url}, still within due-time.`);
Expand Down
64 changes: 37 additions & 27 deletions tests/main.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,69 +81,81 @@ describe("User start/stop", () => {
expect(result).toBe(true);
});

it("Should process update for all repos except optOut", async () => {
it("Should process updates for all repos except optOut", async () => {
const context = createContext(1, 1);
const infoSpy = jest.spyOn(context.logger, "info");
createComment(5, 3, STRINGS.BOT, "Bot", botAssignmentComment(2, daysPriorToNow(1)), daysPriorToNow(1));
createComment(5, 1, STRINGS.BOT, "Bot", botAssignmentComment(2, daysPriorToNow(1)), daysPriorToNow(1));
createEvent(2, daysPriorToNow(1));

await expect(runPlugin(context)).resolves.toBe(true);

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(8, `Nothing to do for ${getIssueHtmlUrl(4)}, still within due-time.`);
expect(infoSpy).not.toHaveBeenNthCalledWith(10, `Nothing to do for https://github.com/ubiquity/private-repo/issues/5, still within due-time.`);
expect(infoSpy).toHaveBeenNthCalledWith(6, `Passed the reminder threshold on ${getIssueHtmlUrl(3)}, sending a reminder.`);
expect(infoSpy).toHaveBeenNthCalledWith(7, `@user2, this task has been idle for a while. Please provide an update.\n\n`, {
taskAssignees: [2],
caller: STRINGS.LOGS_ANON_CALLER,
});
expect(infoSpy).toHaveBeenNthCalledWith(9, `Passed the deadline on ${getIssueHtmlUrl(4)} and no activity is detected, removing assignees.`);
expect(infoSpy).not.toHaveBeenCalledWith(expect.stringContaining(STRINGS.PRIVATE_REPO_NAME));
});

it("Should include the previously excluded repo", async () => {
const context = createContext(1, 1, []);
const infoSpy = jest.spyOn(context.logger, "info");
createComment(5, 3, STRINGS.BOT, "Bot", botAssignmentComment(2, daysPriorToNow(1)), daysPriorToNow(1));
createComment(5, 1, STRINGS.BOT, "Bot", botAssignmentComment(2, daysPriorToNow(1)), daysPriorToNow(1));
createEvent(2, daysPriorToNow(1));

await expect(runPlugin(context)).resolves.toBe(true);

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(8, `Nothing to do for ${getIssueHtmlUrl(4)}, still within due-time.`);
expect(infoSpy).toHaveBeenNthCalledWith(10, `Nothing to do for https://github.com/ubiquity/private-repo/issues/5, still within due-time.`);
expect(infoSpy).toHaveBeenNthCalledWith(6, `Passed the reminder threshold on ${getIssueHtmlUrl(3)}, sending a reminder.`);
expect(infoSpy).toHaveBeenNthCalledWith(7, `@user2, this task has been idle for a while. Please provide an update.\n\n`, {
taskAssignees: [2],
caller: STRINGS.LOGS_ANON_CALLER,
});
expect(infoSpy).toHaveBeenNthCalledWith(9, `Passed the deadline on ${getIssueHtmlUrl(4)} and no activity is detected, removing assignees.`);
expect(infoSpy).toHaveBeenCalledWith(expect.stringContaining(STRINGS.PRIVATE_REPO_NAME));
});

it("Should eject the user after the disqualification period", async () => {
const context = createContext(4, 2);
const infoSpy = jest.spyOn(context.logger, "info");

const timestamp = daysPriorToNow(9);
createComment(3, 3, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);
createComment(3, 4, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);
createEvent(2, timestamp);

const issue = db.issue.findFirst({ where: { id: { equals: 4 } } });
expect(issue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);

await runPlugin(context);

expect(infoSpy).toHaveBeenNthCalledWith(2, `Passed the deadline on ${getIssueHtmlUrl(1)} and no activity is detected, removing assignees.`);
expect(infoSpy).toHaveBeenNthCalledWith(4, `Passed the deadline on ${getIssueHtmlUrl(2)} and no activity is detected, removing assignees.`);
expect(infoSpy).toHaveBeenNthCalledWith(6, `Passed the deadline on ${getIssueHtmlUrl(3)} and no activity is detected, removing assignees.`);

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, `Passed the reminder threshold on ${getIssueHtmlUrl(3)}, sending a reminder.`);
expect(infoSpy).toHaveBeenNthCalledWith(7, `@user2, this task has been idle for a while. Please provide an update.\n\n`, {
taskAssignees: [2],
caller: STRINGS.LOGS_ANON_CALLER,
});
expect(infoSpy).toHaveBeenNthCalledWith(9, `Passed the deadline on ${getIssueHtmlUrl(4)} and no activity is detected, removing assignees.`);
const updatedIssue = db.issue.findFirst({ where: { id: { equals: 4 } } });
expect(updatedIssue?.assignees).toEqual([]);
});

it("Should warn the user after the warning period", async () => {
const context = createContext(4, 2);
const context = createContext(3, 2);
const timestamp = daysPriorToNow(5);

createComment(3, 3, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);
createComment(3, 2, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);

const issue = db.issue.findFirst({ where: { id: { equals: 4 } } });
const issue = db.issue.findFirst({ where: { id: { equals: 3 } } });
expect(issue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);

await runPlugin(context);

const updatedIssue = db.issue.findFirst({ where: { id: { equals: 4 } } });
const updatedIssue = db.issue.findFirst({ where: { id: { equals: 3 } } });
expect(updatedIssue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);

const comments = db.issueComments.getAll();
Expand All @@ -153,23 +165,21 @@ describe("User start/stop", () => {
});

it("Should have nothing do within the warning period", async () => {
const context = createContext(4, 2);
const context = createContext(1, 2);
const infoSpy = jest.spyOn(context.logger, "info");

const timestamp = daysPriorToNow(2);
createComment(3, 3, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);
createComment(3, 1, STRINGS.BOT, "Bot", botAssignmentComment(2, timestamp), timestamp);

const issue = db.issue.findFirst({ where: { id: { equals: 4 } } });
expect(issue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);
const issue = db.issue.findFirst({ where: { id: { equals: 1 } } });
expect(issue?.assignees).toEqual([{ login: STRINGS.UBIQUITY, id: 1 }]);

await runPlugin(context);

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).toHaveBeenCalledWith(`Nothing to do for ${getIssueHtmlUrl(1)}, still within due-time.`);

const updatedIssue = db.issue.findFirst({ where: { id: { equals: 4 } } });
expect(updatedIssue?.assignees).toEqual([{ login: STRINGS.USER, id: 2 }]);
const updatedIssue = db.issue.findFirst({ where: { id: { equals: 1 } } });
expect(updatedIssue?.assignees).toEqual([{ login: STRINGS.UBIQUITY, id: 1 }]);
});

it("Should handle collecting linked PRs", async () => {
Expand Down

0 comments on commit 68a3b58

Please sign in to comment.