Skip to content

Fix rejection automation to update when min2 and unanimous rejection #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix rejection automation to update when min2 and unanimous rejection #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fae1d7
Vibe-code (GPT 4.1) to update table logic in pipeline_proposals.yml t…
jfy133 Sep 5, 2025
9dfa3e9
Human-code to tweak the minimum conditions to ensure rejection is una…
jfy133 Sep 5, 2025
75b2c53
Vibe code (GPT-4.1) updating test that rejection only set when no app…
jfy133 Sep 5, 2025
41905e0
[automated] Fix code linting
nf-core-bot Sep 5, 2025
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
86 changes: 86 additions & 0 deletions .github/workflows/lib/approval.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,4 +603,90 @@ describe("ApprovalManager", () => {
expect(approvalManager.maintainerApprovals.has("user1")).toBe(false);
});
});

describe("rejection automation", () => {
beforeEach(() => {
mockGithub.rest.issues.get.mockResolvedValue({
data: { labels: [] },
});
});

it("should set status to rejected if 2 core rejections", async () => {
approvalManager.coreRejections = new Set(["core1", "core2"]);
approvalManager.maintainerRejections = new Set();
// Simulate status logic from workflow
let status;
if (
approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("❌ Rejected");
await approvalManager.updateIssueStatus(status);
expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
owner: mockOrg,
repo: mockRepo,
issue_number: mockIssueNumber,
labels: ["rejected"].map((l) => (l === "rejected" ? "turned-down" : l)), // label mapping
});
});

it("should set status to rejected if 1 core + 1 maintainer rejection", async () => {
approvalManager.coreRejections = new Set(["core1"]);
approvalManager.maintainerRejections = new Set(["maintainer1"]);
let status;
if (
approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("❌ Rejected");
await approvalManager.updateIssueStatus(status);
expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
owner: mockOrg,
repo: mockRepo,
issue_number: mockIssueNumber,
labels: ["rejected"].map((l) => (l === "rejected" ? "turned-down" : l)),
});
});

it("should not set status to rejected if only 1 core rejection", async () => {
approvalManager.coreRejections = new Set(["core1"]);
approvalManager.maintainerRejections = new Set();
let status;
if (
approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("🕐 Pending");
});

it("should stay pending if 2 rejections but also 1 acceptance", async () => {
approvalManager.coreRejections = new Set(["core1", "core2"]);
approvalManager.maintainerRejections = new Set();
approvalManager.coreApprovals = new Set(["core3"]);
let status;
if (
(approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)) &&
approvalManager.coreApprovals.size === 0 &&
approvalManager.maintainerApprovals.size === 0
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("🕐 Pending");
});
});
});
80 changes: 80 additions & 0 deletions .github/workflows/lib/workflow-integration.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,4 +375,84 @@ describe("Workflow Integration Tests", () => {
}
});
});

describe("Pipeline Proposal Workflow - Rejection Automation", () => {
let approvalManager;
const mockOrg = "nf-core";
const mockRepo = "proposals";
const mockIssueNumber = 99;

beforeEach(async () => {
mockGithub.request
.mockResolvedValueOnce({ data: [{ login: "core1" }, { login: "core2" }] })
.mockResolvedValueOnce({ data: [{ login: "maintainer1" }] });
mockGithub.paginate.mockResolvedValue([]);
mockGithub.rest.issues.get.mockResolvedValue({ data: { labels: [] } });
approvalManager = await new ApprovalManager(mockGithub, mockOrg, mockRepo, mockIssueNumber).initialize();
});

it("should set status to rejected if 2 core rejections", async () => {
approvalManager.coreRejections = new Set(["core1", "core2"]);
approvalManager.maintainerRejections = new Set();
let status;
if (
approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("❌ Rejected");
});

it("should set status to rejected if 1 core + 1 maintainer rejection", async () => {
approvalManager.coreRejections = new Set(["core1"]);
approvalManager.maintainerRejections = new Set(["maintainer1"]);
let status;
if (
approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("❌ Rejected");
});

it("should not set status to rejected if only 1 core rejection", async () => {
approvalManager.coreRejections = new Set(["core1"]);
approvalManager.maintainerRejections = new Set();
let status;
if (
approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("🕐 Pending");
});

it("should stay pending if 2 rejections but also 1 approval", async () => {
approvalManager.coreRejections = new Set(["core1", "core2"]);
approvalManager.maintainerRejections = new Set();
approvalManager.coreApprovals = new Set(["core3"]);
approvalManager.maintainerApprovals = new Set();
let status;
if (
(approvalManager.coreRejections.size >= 2 ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1)) &&
approvalManager.coreApprovals.size === 0 &&
approvalManager.maintainerApprovals.size === 0
) {
status = "❌ Rejected";
} else {
status = "🕐 Pending";
}
expect(status).toBe("🕐 Pending");
});
});
});
16 changes: 12 additions & 4 deletions .github/workflows/pipeline_proposals.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
if (maintainerApprovers.length > 0) {
body += `| ✅ Approved (Maintainer) | ${approvalManager.formatUserList(maintainerApprovers)} |\n`;
}
if (rejecters.length > 0) {
if (rejecters.length > 1 && coreApprovers.length === 0 && maintainerApprovers.length === 0) {
body += `| ❌ Rejected | ${approvalManager.formatUserList(rejecters)} |\n`;
}
if (awaitingCore.length > 0) {
Expand Down Expand Up @@ -92,13 +92,21 @@ jobs:



// Determine status
let status = '🕐 Pending';

if (context.eventName === 'issues' && context.payload.action === 'closed' && context.payload.issue.state_reason === 'not_planned' && (approvalManager.coreRejections.size > 0 || approvalManager.maintainerRejections.size > 0)) {
// Determine status
let status;

// If rejection threshold is reached, set status to Rejected
if (
(approvalManager.coreRejections.size >= 2) ||
(approvalManager.coreRejections.size >= 1 && approvalManager.maintainerRejections.size >= 1) ||
(context.eventName === 'issues' && context.payload.action === 'closed' && context.payload.issue.state_reason === 'not_planned' && (approvalManager.coreRejections.size > 0 || approvalManager.maintainerRejections.size > 0))
) {
status = '❌ Rejected';
} else if ((approvalManager.coreApprovals.size >= 2) || (approvalManager.coreApprovals.size >= 1 && approvalManager.maintainerApprovals.size >= 1)) {
status = '✅ Approved';
} else {
status = '🕐 Pending';
}

const statusBody = generateStatusBody(status);
Expand Down