From 5659fc3fa7c692f1f5bd7e79298202ac553775e9 Mon Sep 17 00:00:00 2001 From: gentlementlegen Date: Tue, 5 Nov 2024 00:09:53 +0900 Subject: [PATCH 1/5] fix: ensure assignee is checked in userUnassigned handler Check `assignee` before closing pull requests to avoid errors. --- src/handlers/user-start-stop.ts | 11 ++++++++--- src/plugin.ts | 2 +- tests/main.test.ts | 3 +++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/handlers/user-start-stop.ts b/src/handlers/user-start-stop.ts index b5017c94..dbddeebd 100644 --- a/src/handlers/user-start-stop.ts +++ b/src/handlers/user-start-stop.ts @@ -92,13 +92,18 @@ export async function userPullRequest(context: Context<"pull_request.opened" | " return { status: HttpStatusCode.NOT_MODIFIED }; } -export async function userUnassigned(context: Context): Promise { +export async function userUnassigned(context: Context<"issues.unassigned">): Promise { if (!("issue" in context.payload)) { context.logger.debug("Payload does not contain an issue, skipping issues.unassigned event."); return { status: HttpStatusCode.NOT_MODIFIED }; } const { payload } = context; - const { issue, sender, repository } = payload; - await closePullRequestForAnIssue(context, issue.number, repository, sender.login); + const { issue, repository, assignee } = payload; + // 'assignee' is the user that actually got un-assigned during this event. Since it can theoretically be null, + // we display an error if none is found in the payload. + if (!assignee) { + throw context.logger.fatal("No assignee found in payload, failed to close pull-requests."); + } + await closePullRequestForAnIssue(context, issue.number, repository, assignee?.login); return { status: HttpStatusCode.OK, content: "Linked pull-requests closed." }; } diff --git a/src/plugin.ts b/src/plugin.ts index f34f8561..5e4ea832 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -35,7 +35,7 @@ export async function startStopTask(inputs: PluginInputs, env: Env) { case "pull_request.edited": return await userPullRequest(context as Context<"pull_request.edited">); case "issues.unassigned": - return await userUnassigned(context); + return await userUnassigned(context as Context<"issues.unassigned">); default: context.logger.error(`Unsupported event: ${context.eventName}`); } diff --git a/tests/main.test.ts b/tests/main.test.ts index 86d11af0..d246160e 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -622,6 +622,9 @@ export function createContext( action: "created", installation: { id: 1 } as unknown as Context["payload"]["installation"], organization: { login: "ubiquity" } as unknown as Context["payload"]["organization"], + assignee: { + ...sender, + }, } as Context["payload"], logger: new Logs("debug"), config: { From 9c40d09bfc0c4ec178bb874afc03c57409ffe7ec Mon Sep 17 00:00:00 2001 From: gentlementlegen Date: Fri, 8 Nov 2024 14:54:40 +0900 Subject: [PATCH 2/5] fix: prevent bot from double posting messages Added a check to skip message posting if the sender is a bot. --- src/handlers/user-start-stop.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/handlers/user-start-stop.ts b/src/handlers/user-start-stop.ts index dbddeebd..ae74c7f9 100644 --- a/src/handlers/user-start-stop.ts +++ b/src/handlers/user-start-stop.ts @@ -33,6 +33,12 @@ export async function userSelfAssign(context: Context<"issues.assigned">): Promi const { issue } = payload; const deadline = getDeadline(issue.labels); + // We avoid posting a message if the bot is the actor to avoid double posting + if (payload.sender.type === "Bot") { + context.logger.debug("Skipping deadline posting message because the sender is a bot."); + return { status: HttpStatusCode.NOT_MODIFIED }; + } + if (!deadline) { context.logger.debug("Skipping deadline posting message because no deadline has been set."); return { status: HttpStatusCode.NOT_MODIFIED }; From d6f7a1e896112bcd8f14a4288e94034f718c157c Mon Sep 17 00:00:00 2001 From: gentlementlegen Date: Fri, 8 Nov 2024 14:57:44 +0900 Subject: [PATCH 3/5] chore: update compatibility settings in wrangler.toml Changed compatibility_date and added compatibility_flags for Node.js. --- wrangler.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wrangler.toml b/wrangler.toml index ef7f8cdb..b81ff797 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -1,7 +1,7 @@ name = "ubiquity-os-command-start-stop" main = "src/worker.ts" -compatibility_date = "2024-05-23" -node_compat = true +compatibility_date = "2024-09-23" +compatibility_flags = [ "nodejs_compat" ] [env.dev] [env.prod] From f36e4845b8439b7c88f0c665a1c186210229cefa Mon Sep 17 00:00:00 2001 From: gentlementlegen Date: Sat, 9 Nov 2024 11:43:08 +0900 Subject: [PATCH 4/5] chore: merge if statement --- src/handlers/user-start-stop.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/handlers/user-start-stop.ts b/src/handlers/user-start-stop.ts index ae74c7f9..12443e3f 100644 --- a/src/handlers/user-start-stop.ts +++ b/src/handlers/user-start-stop.ts @@ -34,13 +34,11 @@ export async function userSelfAssign(context: Context<"issues.assigned">): Promi const deadline = getDeadline(issue.labels); // We avoid posting a message if the bot is the actor to avoid double posting - if (payload.sender.type === "Bot") { - context.logger.debug("Skipping deadline posting message because the sender is a bot."); - return { status: HttpStatusCode.NOT_MODIFIED }; - } - - if (!deadline) { - context.logger.debug("Skipping deadline posting message because no deadline has been set."); + if (!deadline || payload.sender.type === "Bot") { + context.logger.debug("Skipping deadline posting message.", { + senderType: payload.sender.type, + deadline: deadline, + }); return { status: HttpStatusCode.NOT_MODIFIED }; } From ef6198ae51b9d8a39f61fe669144e8826e724924 Mon Sep 17 00:00:00 2001 From: rndquu Date: Mon, 18 Nov 2024 14:16:22 +0300 Subject: [PATCH 5/5] feat: new error msg on missing required label --- src/handlers/shared/start.ts | 13 ++++++++----- tests/main.test.ts | 4 +++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/handlers/shared/start.ts b/src/handlers/shared/start.ts index f8629c06..4159c669 100644 --- a/src/handlers/shared/start.ts +++ b/src/handlers/shared/start.ts @@ -22,11 +22,14 @@ export async function start( if (requiredLabelsToStart.length && !requiredLabelsToStart.some((label) => issueLabels.includes(label))) { // The "Priority" label must reflect a business priority, not a development one. - throw logger.error("This task does not reflect a business priority at the moment and cannot be started. This will be reassessed in the coming weeks.", { - requiredLabelsToStart, - issueLabels, - issue: issue.html_url, - }); + throw logger.error( + `This task does not reflect a business priority at the moment. You may start tasks with one of the following labels: ${requiredLabelsToStart.join(", ")}`, + { + requiredLabelsToStart, + issueLabels, + issue: issue.html_url, + } + ); } if (!sender) { diff --git a/tests/main.test.ts b/tests/main.test.ts index f98a6758..6eb001bf 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -292,7 +292,9 @@ describe("User start/stop", () => { context.adapters = createAdapters(getSupabase(), context); await expect(userStartStop(context)).rejects.toMatchObject({ - logMessage: { raw: "This task does not reflect a business priority at the moment and cannot be started. This will be reassessed in the coming weeks." }, + logMessage: { + raw: "This task does not reflect a business priority at the moment. You may start tasks with one of the following labels: Priority: 3 (High), Priority: 4 (Urgent), Priority: 5 (Emergency)", + }, }); }); });