Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/integrations/project-tracker-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,12 @@ type IssueComment = {
user?: { type?: string; login?: string } | null;
};

const PROJECT_TRACKER_PULL_REQUEST_ACTIONS = new Set(["opened", "edited", "reopened", "synchronize"]);

function shouldSuggestProjectTrackerForWebhook(eventName: string, action: string | undefined): boolean {
return eventName === "pull_request" && action !== undefined && PROJECT_TRACKER_PULL_REQUEST_ACTIONS.has(action);
}

/** Only knows "github" vs. "linear" -- kept as a standalone alias (mirroring {@link ProjectMilestoneMatchModeInput}
* below) rather than importing RepositorySettings, so this integrations module has no dependency on the
* settings type. */
Expand Down Expand Up @@ -395,8 +401,9 @@ export async function maybeSuggestProjectOrMilestoneMatch(

/**
* Webhook-level entry point (#3183): folds the "should this even run" gating (installed app, PR still open,
* feature opted in) AND the best-effort error logging into one call, so the PR-webhook handler in
* processors.ts has a single, unconditional call site with no logic/logging body of its own -- everything
* feature opted in, and a PR lifecycle/title-body webhook) AND the best-effort error logging into one call, so
* the PR-webhook handler in processors.ts has a single, unconditional call site with no logic/logging body of
* its own -- everything
* testable lives here, where it already has dedicated, isolated coverage, rather than in an inline closure
* inside the huge webhook file that only a full pipeline test could exercise.
*/
Expand All @@ -412,7 +419,10 @@ export async function maybeSuggestMilestoneMatchForPr(args: {
mode: ProjectMilestoneMatchModeInput;
backend: ProjectMilestoneMatchBackendInput;
deliveryId: string;
eventName: string;
action: string | undefined;
}): Promise<void> {
if (!shouldSuggestProjectTrackerForWebhook(args.eventName, args.action)) return;
if (!args.installationId) return;
if (args.prState !== "open") return;
if (!args.mode || args.mode === "off") return;
Expand Down
2 changes: 2 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5383,6 +5383,8 @@ async function processGitHubWebhook(
mode: settings.autoProjectMilestoneMatch,
backend: settings.autoProjectMilestoneMatchBackend,
deliveryId,
eventName,
action: payload.action,
});
// Draft-dodge guard (#converted-to-draft): a contributor converting an OPEN PR to draft cannot use
// draft state to keep a gate-rejected PR alive. When a prior gate failure exists for the PR's current
Expand Down
22 changes: 22 additions & 0 deletions test/unit/project-tracker-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,32 @@ describe("maybeSuggestMilestoneMatchForPr (#3183 webhook-level gating)", () => {
mode: "suggest" as const,
backend: "github" as const,
deliveryId: "test-delivery",
eventName: "pull_request",
action: "opened",
...overrides,
};
}

it("does nothing for review webhooks with pull_request payloads", async () => {
let called = false;
vi.stubGlobal("fetch", async () => {
called = true;
return new Response("unexpected", { status: 500 });
});
await maybeSuggestMilestoneMatchForPr(baseArgs({ eventName: "pull_request_review", action: "submitted" }));
expect(called).toBe(false);
});

it("does nothing for pull_request actions that do not change title/body matching inputs", async () => {
let called = false;
vi.stubGlobal("fetch", async () => {
called = true;
return new Response("unexpected", { status: 500 });
});
await maybeSuggestMilestoneMatchForPr(baseArgs({ action: "labeled" }));
expect(called).toBe(false);
});

it("does nothing when installationId is falsy (never touches the network)", async () => {
let called = false;
vi.stubGlobal("fetch", async () => {
Expand Down
Loading