Repository: srvaroa/labeler (go)

Issue:
[feat] improve `mergeable` param or add a paramater for Has/Needs code review
The `mergeable` parameter is pretty good, however there is one limitation with it. It will pass (when `True`) if all of the checks pass, however there may be other things blocking merging (specifically it may require review). See below:


<img width="279" alt="image" src="https://github.com/user-attachments/assets/943fa38d-48b2-4a55-81cd-ee90b2639015" />

It would be good to either:

1. Update `mergeable` to account for this.
1. Add a new param for has/needs code review.


I think if we wanted to do number 1, we could maybe read [`MergeableState`](https://github.com/google/go-github/blob/3c06fc105fea5cd7216af67c1455620c8689ff1e/github/pulls.go#L46) instead so that we actually check that the PR is indeed in a clean state. I also found [this Enum](https://docs.github.com/en/graphql/reference/enums#mergestatestatus) which I don't see in the API library so not sure if we have access to that.



Pull request description:
Check that MergeableState is clean for mergeable
Resolves #162

Extends the `mergeable` check to also ensure that the the [mergeable state](https://docs.github.com/en/graphql/reference/enums#mergestatestatus) is `'clean'`. This ensures that the pull request is truly mergeable. This resolves cases where all status checks have passed (`GetMergeable` returns true) but review is required on the PR before merging.

Patch:
diff --git a/pkg/condition_ismergeable.go b/pkg/condition_ismergeable.go
index 85d6eb6..a011be4 100644
--- a/pkg/condition_ismergeable.go
+++ b/pkg/condition_ismergeable.go
@@ -18,10 +18,14 @@ func IsMergeableCondition() Condition {
 			if err != nil {
 				return false, fmt.Errorf("mergeable is not set in config")
 			}
+
+			//  Check both the mergeable state and the mergeable flag
+			isMergeable := target.ghPR.GetMergeable() && target.ghPR.GetMergeableState() == "clean"
+
 			if b {
-				return target.ghPR.GetMergeable(), nil
+				return isMergeable, nil
 			}
-			return !target.ghPR.GetMergeable(), nil
+			return !isMergeable, nil
 		},
 	}
 }


Test patch:
diff --git a/pkg/labeler_test.go b/pkg/labeler_test.go
index 9c9ad96..5503ae8 100644
--- a/pkg/labeler_test.go
+++ b/pkg/labeler_test.go
@@ -1049,6 +1049,22 @@ func TestHandleEvent(t *testing.T) {
 			initialLabels:  []string{"Meh"},
 			expectedLabels: []string{"Meh", "ShouldAppear"},
 		},
+		{
+			event:    "pull_request",
+			payloads: []string{"create_pr_mergeable_not_clean"},
+			name:     "Don't add label when PR is mergeable but state is not clean",
+			config: LabelerConfigV1{
+				Version: 1,
+				Labels: []LabelMatcher{
+					{
+						Label:     "CanMerge",
+						Mergeable: "True",
+					},
+				},
+			},
+			initialLabels:  []string{},
+			expectedLabels: []string{},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1059,6 +1075,7 @@ func TestHandleEvent(t *testing.T) {
 					fmt.Printf("Test `%s`: failed to load %s\n", tc.name, file)
 					t.Fatal(err)
 				}
+				fmt.Printf("Loaded payload: %s\n", string(payload))
 
 				fmt.Printf("--> TEST: %s \n", tc.name)
 				l := NewTestLabeler(t, tc)
diff --git a/test_data/create_pr_mergeable_not_clean_payload b/test_data/create_pr_mergeable_not_clean_payload
new file mode 100644
index 0000000..cdc7c11
--- /dev/null
+++ b/test_data/create_pr_mergeable_not_clean_payload
@@ -0,0 +1,41 @@
+{
+  "action": "opened",
+  "number": 1,
+  "pull_request": {
+    "number": 1,
+    "state": "open",
+    "locked": false,
+    "title": "Update test",
+    "user": {
+      "login": "srvaroa",
+      "type": "User"
+    },
+    "body": "Test PR",
+    "base": {
+      "repo": {
+        "name": "labeler",
+        "owner": {
+          "login": "srvaroa"
+        }
+      }
+    },
+    "created_at": "2019-05-24T22:03:59Z",
+    "updated_at": "2019-05-24T22:03:59Z",
+    "closed_at": null,
+    "merged_at": null,
+    "merge_commit_sha": null,
+    "assignee": null,
+    "assignees": [],
+    "requested_reviewers": [],
+    "requested_teams": [],
+    "labels": [],
+    "milestone": null,
+    "commits": 1,
+    "additions": 1,
+    "deletions": 1,
+    "changed_files": 1,
+    "mergeable": true,
+    "mergeable_state": "blocked",
+    "author_association": "OWNER"
+  }
+}