From e9b07352d6a83b9dc318d28ecdc02d176d2a2b1f Mon Sep 17 00:00:00 2001 From: Mak Date: Mon, 24 Jul 2023 22:09:43 +0100 Subject: [PATCH 1/2] fix checking membership after some commands found --- src/bot/events/onIssueCommentCreated.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bot/events/onIssueCommentCreated.ts b/src/bot/events/onIssueCommentCreated.ts index 248a313..0ed1172 100644 --- a/src/bot/events/onIssueCommentCreated.ts +++ b/src/bot/events/onIssueCommentCreated.ts @@ -54,10 +54,6 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as try { const commands: (ParsedCommand | SkipEvent)[] = []; - if (!(await isRequesterAllowed(ctx, octokit, requester))) { - return getError("Requester could not be detected as a member of an allowed organization."); - } - for (const line of getLines(comment.body)) { const parsedCommand = await parsePullRequestBotCommandLine(line, ctx, pr.repo); @@ -72,6 +68,10 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as return new SkipEvent("No commands found within a comment"); } + if (!(await isRequesterAllowed(ctx, octokit, requester))) { + return getError("Requester could not be detected as a member of an allowed organization."); + } + for (const parsedCommand of commands) { logger.debug({ parsedCommand }, "Processing parsed command"); From c6e89797f8ca3dedd777d334689da43119d1c715 Mon Sep 17 00:00:00 2001 From: Mak Date: Mon, 24 Jul 2023 22:32:13 +0100 Subject: [PATCH 2/2] move parsing skip event above --- src/bot/events/onIssueCommentCreated.ts | 20 ++++++++++---------- src/bot/setupEvent.ts | 11 ++++++++++- src/bot/types.ts | 2 +- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/bot/events/onIssueCommentCreated.ts b/src/bot/events/onIssueCommentCreated.ts index 0ed1172..a32d9f0 100644 --- a/src/bot/events/onIssueCommentCreated.ts +++ b/src/bot/events/onIssueCommentCreated.ts @@ -61,7 +61,16 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as return getError(parsedCommand.message); } - commands.push(parsedCommand); + if (parsedCommand instanceof SkipEvent) { + const skip = getMetricsPrData("skip", eventName, pr, parsedCommand.reason); + counters.commandsRun.inc({ ...skip }); + logger.debug( + { command: comment.body, payload, ...skip }, + `Skip command with reason: "${parsedCommand.reason}"`, + ); + } else { + commands.push(parsedCommand); + } } if (commands.length === 0) { @@ -75,15 +84,6 @@ export const onIssueCommentCreated: WebhookHandler<"issue_comment.created"> = as for (const parsedCommand of commands) { logger.debug({ parsedCommand }, "Processing parsed command"); - if (parsedCommand instanceof SkipEvent) { - const skip = getMetricsPrData("skip", eventName, pr, parsedCommand.reason); - counters.commandsRun.inc({ ...skip }); - logger.debug( - { command: comment.body, payload, ...skip }, - `Skip command with reason: "${parsedCommand.reason}"`, - ); - } - if (parsedCommand instanceof HelpCommand) { await createComment(ctx, octokit, { ...commentParams, diff --git a/src/bot/setupEvent.ts b/src/bot/setupEvent.ts index c21cfaf..8da5fd1 100644 --- a/src/bot/setupEvent.ts +++ b/src/bot/setupEvent.ts @@ -1,7 +1,14 @@ import { Probot } from "probot"; import { extractPullRequestData } from "src/bot/parse/extractPullRequestData"; -import { FinishedEvent, PullRequestError, WebhookEventPayload, WebhookEvents, WebhookHandler } from "src/bot/types"; +import { + FinishedEvent, + PullRequestError, + SkipEvent, + WebhookEventPayload, + WebhookEvents, + WebhookHandler, +} from "src/bot/types"; import { createComment, getOctokit, updateComment } from "src/github"; import { logger as parentLogger } from "src/logger"; import { counters, getMetricsPrData, summaries } from "src/metrics"; @@ -55,6 +62,8 @@ export const setupEvent = ( const ok = getMetricsPrData("ok", eventName, prData, result.comment.body); counters.commandsRun.inc({ ...ok, repo: result.pr.repo, pr: result.pr.number }); eventLogger.info({ result }, "Finished command"); + } else if (result instanceof SkipEvent) { + eventLogger.debug({ result }, "Skipping command"); } else { const message = "Unknown result type"; const error = getMetricsPrData("error", eventName, prData, message); diff --git a/src/bot/types.ts b/src/bot/types.ts index e235952..fe84c50 100644 --- a/src/bot/types.ts +++ b/src/bot/types.ts @@ -38,7 +38,7 @@ export type WebhookHandler = ( ctx: Context, octokit: ExtendedOctokit, event: WebhookEventPayload, -) => Promise; +) => Promise; export class SkipEvent { constructor(public reason: string = "") {}