feat: Add direct PR approval check for LGTM

This commit introduces a new feature that enables the
PRHandler to also consider direct PR approvals when
determining if the LGTM threshold is met. The code now
fetches PR reviews from the GitHub API and counts 'approved'
reviews towards the LGTM count, in addition to comment-based
/lgtm votes. Self-approvals are skipped.

Fixes #42

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

Author: chmouel

.pylintrc

@@ -1,3 +1,3 @@
 [MESSAGES CONTROL]
-disable=redefined-outer-name,line-too-long,missing-function-docstring,missing-module-docstring,too-many-locals,invalid-name,protected-access,consider-using-with,fixme
+disable=redefined-outer-name,line-too-long,missing-function-docstring,missing-module-docstring,too-many-locals,invalid-name,protected-access,consider-using-with,fixme,too-many-branches
 

Makefile

@@ -5,9 +5,9 @@ UVCMD = uv run
 PIPELINE_PROW = pipeline-boussole.yaml
 GH_REPO_OWNER = openshift-pipelines
 GH_REPO_NAME = pac-boussole
-GH_PR_NUM = 16
-GH_PR_SENDER = chmouel
-GH_COMMENT_SENDER = anotheruser
+GH_PR_NUM = 44
+GH_PR_SENDER = anotheruser
+GH_COMMENT_SENDER = chmouel
 PASS_TOKEN = github/chmouel-token
 PRURL = https://github.com/$(GH_REPO_OWNER)/$(GH_REPO_NAME)/pull/$(GH_PR_NUM)
 CONTAINER_IMAGE = ghcr.io/openshift-pipelines/pac-boussole:nightly
@@ -29,7 +29,7 @@ directtest: ## Run a specific command directly (e.g., make directtest CMD=/lgtm)
 	@env GH_PR_NUM=$(GH_PR_NUM) GH_REPO_NAME=$(GH_REPO_NAME) GH_REPO_OWNER=$(GH_REPO_OWNER) \
 		GH_PR_SENDER=$(GH_PR_SENDER) GH_COMMENT_SENDER=$(GH_COMMENT_SENDER) \
 		PAC_TRIGGER_COMMENT="$(CMD)" GITHUB_TOKEN=`pass show $(PASS_TOKEN)` \
-		python3 ./boussole/boussole.py
+		./pac-boussole
 
 open_pr: ## Open the PR in the browser
 	@if type -p xdg-open > /dev/null; then xdg-open $(PRURL); \

boussole/boussole.py

@@ -76,13 +76,32 @@ def _post_comment(self, message: str) -> RequestResponse:
         endpoint = f"issues/{self.pr_num}/comments"
         return self.api.post(endpoint, {"body": message})
 
-    def _fetch_and_validate_lgtm_votes(self):
+    def _fetch_and_validate_lgtm_votes(self) -> Tuple[int, Dict[str, Optional[str]]]:
         """
         Fetches LGTM votes and validates them.
 
         Returns the number of valid votes and a dictionary of users with their
         permissions.
         """
+        reviews_endpoint = f"pulls/{self.pr_num}/reviews"
+        reviews_response = self.api.get(reviews_endpoint)
+        if reviews_response.status_code != 200:
+            error_message = COMMENTS_FETCH_ERROR.format(
+                status_code=reviews_response.status_code,
+                response_text=reviews_response.text,
+                pr_num=self.pr_num,
+            )
+            print(error_message, file=sys.stderr)
+            sys.exit(1)
+
+        lgtm_users: Dict[str, Optional[str]] = {}
+        reviews = reviews_response.json()
+        for review in reviews:
+            if review["state"].lower() == "approved":
+                user = review["user"]["login"]
+                if user != self.pr_sender:  # Skip self-approvals
+                    lgtm_users[user] = None
+
         endpoint = f"issues/{self.pr_num}/comments"
         response = self.api.get(endpoint)
         if response.status_code != 200:
@@ -93,9 +112,7 @@ def _fetch_and_validate_lgtm_votes(self):
             )
             print(error_message, file=sys.stderr)
             sys.exit(1)
-
         comments = response.json()
-        lgtm_users: Dict[str, Optional[str]] = {}
         for comment in comments:
             body = comment.get("body", "")
             if re.search(r"^/lgtm\b", body, re.IGNORECASE):
@@ -331,40 +348,11 @@ def rebase(self) -> RequestResponse:
     def lgtm(self, send_comment: bool = True) -> int:
         """
         Processes LGTM votes and approves the PR if the threshold is met.
-        """
-        endpoint = f"issues/{self.pr_num}/comments"
-        response = self.api.get(endpoint)
-        if response.status_code != 200:
-            error_message = COMMENTS_FETCH_ERROR.format(
-                status_code=response.status_code,
-                response_text=response.text,
-                pr_num=self.pr_num,
-            )
-            print(error_message, file=sys.stderr)
-            sys.exit(1)
-
-        comments = response.json()
-        lgtm_users: Dict[str, Optional[str]] = {}
-        for comment in comments:
-            body = comment.get("body", "")
-            if re.search(r"^/lgtm\b", body, re.IGNORECASE):
-                user = comment["user"]["login"]
-                if user == self.pr_sender:
-                    msg = SELF_APPROVAL_ERROR.format(
-                        user=user, comment_url=comment["html_url"]
-                    )
-                    self._post_comment(msg)
-                    print(msg, file=sys.stderr)
-                    sys.exit(1)
-                lgtm_users[user] = None
-
-        valid_votes = 0
-        for user in lgtm_users:
-            permission, is_valid = self._check_membership(user)
-            lgtm_users[user] = permission
-            if is_valid:
-                valid_votes += 1
 
+        Includes both comment-based LGTM and direct PR approvals.
+        """
+        # First check direct PR approvals
+        valid_votes, lgtm_users = self._fetch_and_validate_lgtm_votes()
         if valid_votes >= self.lgtm_threshold:
             users_table = ""
             for user, permission in lgtm_users.items():
@@ -431,7 +419,6 @@ def merge_pr(self) -> bool:
 
         # Fetch LGTM votes and check if the threshold is met
         valid_votes, lgtm_users = self._fetch_and_validate_lgtm_votes()
-
         if valid_votes >= self.lgtm_threshold:
             endpoint = f"pulls/{self.pr_num}/merge"
             data = {

boussole/test_boussole.py

@@ -109,6 +109,7 @@ def mock_json_permissions():
 
     # Assign the side_effect to return different results on multiple calls
     mock_api.get.return_value.json.side_effect = [
+        [],
         mock_json_comments(),  # First call to json() -> PR comments
         mock_json_permissions(),  # Second call to json() -> reviewer1 permission
         mock_json_permissions(),  # Third call to json() -> reviewer2 permission
@@ -118,12 +119,36 @@ def mock_json_permissions():
     mock_api.get.return_value.status_code = 200
 
 
+def test_lgtm_by_review_request(pr_handler, mock_api, capsys):
+    mock_api.get.return_value.status_code = 200
+
+    # Mock response for PR comments with self-approval
+    mock_api.get.return_value.json.side_effect = [
+        [
+            {"state": "APPROVED", "user": {"login": "reviewer1"}},
+            {"state": "APPROVED", "user": {"login": "reviewer2"}},
+        ],
+        [],
+        {"permission": "write"},
+        {"permission": "write"},
+    ]
+    pr_handler.lgtm()
+    assert "PR approved with LGTM votes" in capsys.readouterr().out
+
+
 def test_lgtm_self_approval(pr_handler, mock_api):
     mock_api.get.return_value.status_code = 200
 
     # Mock response for PR comments with self-approval
-    mock_api.get.return_value.json.return_value = [
-        {"body": "/lgtm", "user": {"login": "test_user"}, "html_url": "http://test.url"}
+    mock_api.get.return_value.json.side_effect = [
+        [],
+        [
+            {
+                "body": "/lgtm",
+                "user": {"login": "test_user"},
+                "html_url": "http://test.url",
+            },
+        ],
     ]
 
     with pytest.raises(SystemExit) as exc_info:
@@ -209,6 +234,7 @@ def test_merge_pr_success(pr_handler, mock_api):
         MyFakeResponse(200, {"permission": "admin"}),
         MyFakeResponse(200, {"head": {"sha": "abc123"}}),
         all_checks,
+        MyFakeResponse(200, {}),
         all_comments,
         MyFakeResponse(200, {"permission": "write"}),  # reviewer1
         MyFakeResponse(200, {"permission": "write"}),  # reviewer2