Merge pull request #73 from nf-core/fix-labelling

Vibe code (Claude Sonnet 4) label updating code to just replace status labels and not replace entirely all labels

Author: kenibrewer

.github/workflows/lib/approval.js

@@ -59,28 +59,44 @@ class ApprovalManager {
 
   // Helper to update issue status and labels
   async updateIssueStatus(status) {
-    const labels = [];
+    // Get current issue to preserve existing labels
+    const issue = await this.github.rest.issues.get({
+      owner: this.org,
+      repo: this.repo,
+      issue_number: this.issueNumber,
+    });
+
+    // Filter out existing status labels
+    const statusLabels = ["accepted", "turned-down", "timed-out", "proposed"];
+    const existingLabels = issue.data.labels
+      .map((label) => (typeof label === "string" ? label : label.name))
+      .filter((label) => !statusLabels.includes(label));
 
+    // Determine new status label
+    let newStatusLabel;
     switch (status) {
       case "✅ Approved":
-        labels.push("accepted");
+        newStatusLabel = "accepted";
         break;
       case "❌ Rejected":
-        labels.push("turned-down");
+        newStatusLabel = "turned-down";
         break;
       case "⏰ Timed Out":
-        labels.push("timed-out");
+        newStatusLabel = "timed-out";
         break;
       default:
-        labels.push("proposed");
+        newStatusLabel = "proposed";
     }
 
+    // Combine existing non-status labels with new status label
+    const updatedLabels = [...existingLabels, newStatusLabel];
+
     // Update labels
     await this.github.rest.issues.update({
       owner: this.org,
       repo: this.repo,
       issue_number: this.issueNumber,
-      labels: labels,
+      labels: updatedLabels,
     });
   }
 

.github/workflows/lib/approval.test.js

@@ -7,6 +7,7 @@ const mockGithub = {
   rest: {
     issues: {
       listComments: jest.fn(),
+      get: jest.fn(),
       update: jest.fn(),
       updateComment: jest.fn(),
       createComment: jest.fn(),
@@ -108,47 +109,106 @@ describe("ApprovalManager", () => {
   });
 
   describe("updateIssueStatus", () => {
+    beforeEach(() => {
+      // Mock the get method to return an issue with existing labels
+      mockGithub.rest.issues.get.mockResolvedValue({
+        data: {
+          labels: [
+            { name: "bug" },
+            { name: "proposed" }, // This should be replaced
+            { name: "enhancement" },
+          ],
+        },
+      });
+    });
+
     it("should add accepted label for approved status", async () => {
       await approvalManager.updateIssueStatus("✅ Approved");
 
+      expect(mockGithub.rest.issues.get).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        issue_number: mockIssueNumber,
+      });
+
       expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
         owner: mockOrg,
         repo: mockRepo,
         issue_number: mockIssueNumber,
-        labels: ["accepted"],
+        labels: ["bug", "enhancement", "accepted"],
       });
     });
 
     it("should add turned-down label for rejected status", async () => {
       await approvalManager.updateIssueStatus("❌ Rejected");
 
+      expect(mockGithub.rest.issues.get).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        issue_number: mockIssueNumber,
+      });
+
       expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
         owner: mockOrg,
         repo: mockRepo,
         issue_number: mockIssueNumber,
-        labels: ["turned-down"],
+        labels: ["bug", "enhancement", "turned-down"],
       });
     });
 
     it("should add timed-out label for timed out status", async () => {
       await approvalManager.updateIssueStatus("⏰ Timed Out");
 
+      expect(mockGithub.rest.issues.get).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        issue_number: mockIssueNumber,
+      });
+
       expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
         owner: mockOrg,
         repo: mockRepo,
         issue_number: mockIssueNumber,
-        labels: ["timed-out"],
+        labels: ["bug", "enhancement", "timed-out"],
       });
     });
 
     it("should add proposed label for pending status", async () => {
       await approvalManager.updateIssueStatus("🕐 Pending");
 
+      expect(mockGithub.rest.issues.get).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        issue_number: mockIssueNumber,
+      });
+
+      expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
+        owner: mockOrg,
+        repo: mockRepo,
+        issue_number: mockIssueNumber,
+        labels: ["bug", "enhancement", "proposed"],
+      });
+    });
+
+    it("should preserve existing labels when updating status", async () => {
+      // Set up a different set of existing labels
+      mockGithub.rest.issues.get.mockResolvedValueOnce({
+        data: {
+          labels: [
+            { name: "documentation" },
+            { name: "accepted" }, // This should be replaced with turned-down
+            { name: "priority-high" },
+          ],
+        },
+      });
+
+      await approvalManager.updateIssueStatus("❌ Rejected");
+
       expect(mockGithub.rest.issues.update).toHaveBeenCalledWith({
         owner: mockOrg,
         repo: mockRepo,
         issue_number: mockIssueNumber,
-        labels: ["proposed"],
+        labels: ["documentation", "priority-high", "turned-down"],
       });
     });
   });

.github/workflows/lib/workflow-integration.test.js

@@ -7,6 +7,7 @@ const mockGithub = {
   rest: {
     issues: {
       listComments: jest.fn(),
+      get: jest.fn(),
       update: jest.fn(),
       updateComment: jest.fn(),
       createComment: jest.fn(),
@@ -38,6 +39,13 @@ describe("Workflow Integration Tests", () => {
       // Mock empty comments initially
       mockGithub.paginate.mockResolvedValue([]);
 
+      // Mock issues.get to return an issue with no existing labels
+      mockGithub.rest.issues.get.mockResolvedValue({
+        data: {
+          labels: [],
+        },
+      });
+
       approvalManager = await new ApprovalManager(mockGithub, mockOrg, mockRepo, mockIssueNumber).initialize();
     });
 
@@ -184,6 +192,13 @@ describe("Workflow Integration Tests", () => {
 
       mockGithub.paginate.mockResolvedValue([]);
 
+      // Mock issues.get to return an issue with no existing labels
+      mockGithub.rest.issues.get.mockResolvedValue({
+        data: {
+          labels: [],
+        },
+      });
+
       approvalManager = await new ApprovalManager(mockGithub, mockOrg, mockRepo, mockIssueNumber).initialize();
     });
 
@@ -328,6 +343,13 @@ describe("Workflow Integration Tests", () => {
 
       mockGithub.paginate.mockResolvedValue([]);
 
+      // Mock issues.get to return an issue with no existing labels
+      mockGithub.rest.issues.get.mockResolvedValue({
+        data: {
+          labels: [],
+        },
+      });
+
       approvalManager = await new ApprovalManager(mockGithub, "org", "repo", 123).initialize();
     });