Merge branch 'master' into feature/709-flag-invalid-prs

Author: prathmeshkulkarni-coder

.github/actions/bot-autoassign/base.py

@@ -22,7 +22,6 @@ def __init__(self):
         self.bot_login = (
             bot_username if bot_username.endswith("[bot]") else f"{bot_username}[bot]"
         )
-
         if self.github_token and self.repository_name:
             try:
                 self.github = Github(self.github_token)
@@ -68,11 +67,8 @@ def get_issue_projects(self, owner, repo_name, issue_number):
         """
         variables = {"owner": owner, "repo": repo_name, "issueNumber": issue_number}
         headers, result = self.github.requester.graphql_query(query, variables)
-        print(f"DEBUG: Raw GraphQL response: {result}")
-
         if "errors" in result:
             raise ValueError(f"GraphQL API Permission Error: {result['errors']}")
-
         repo_data = result.get("data", {}).get("repository")
         if not repo_data:
             raise ValueError(
@@ -108,13 +104,11 @@ def validate_pr_issues(self, pr):
         if not self.github or not self.repository_name:
             print("GitHub client or repository name not initialized")
             return False
-
         pr_author = (
             pr.user.login
             if pr.user and isinstance(getattr(pr.user, "login", None), str)
             else ""
         )
-
         exclude_authors_env = os.environ.get(
             "EXCLUDE_PR_AUTHORS", DEFAULT_EXCLUDE_PR_AUTHORS
         )
@@ -124,36 +118,31 @@ def validate_pr_issues(self, pr):
         if pr_author in excluded_authors:
             print(f"Author {pr_author} is in the exclude list. Proceeding.")
             return True
-
         author_association = str(getattr(pr, "author_association", "") or "")
         if author_association in MAINTAINER_ROLES:
             print(
                 f"Author {pr_author} is exempt due to association: "
                 f"{author_association}. Proceeding."
             )
             return True
-
         from utils import extract_all_linked_issues
 
         pr_body = pr.body if isinstance(pr.body, str) else ""
         linked_issues = extract_all_linked_issues(pr_body, self.repository_name)
         if not linked_issues:
             print("No linked issues found in PR body for external contributor.")
             return False
-
         current_org = self.repository_name.split("/")[0].lower()
         required_projects = {
             self._project_title_key(p) for p in REQUIRED_CONTRIBUTOR_PROJECTS
         }
-
         for owner, repo_name, issue_number in linked_issues:
             if owner.lower() != current_org:
                 print(
                     f"Issue {owner}/{repo_name}#{issue_number} does not belong "
                     f"to organization {current_org}, skipping validation."
                 )
                 continue
-
             try:
                 from github import GithubException
 
@@ -167,19 +156,16 @@ def validate_pr_issues(self, pr):
                     continue
                 print(f"Error fetching issue {owner}/{repo_name}#{issue_number}: {e}")
                 raise
-
             if issue.pull_request:
                 print(
                     f"Reference {owner}/{repo_name}#{issue_number} is a pull request, skipping validation."
                 )
                 continue
-
             if issue.state != "open":
                 print(
                     f"Issue {owner}/{repo_name}#{issue_number} is not open, skipping validation."
                 )
                 continue
-
             issue_labels = [label.name.lower() for label in issue.labels]
             valid_labels = [
                 lbl for lbl in issue_labels if lbl not in INVALID_ISSUE_LABELS
@@ -195,22 +181,14 @@ def validate_pr_issues(self, pr):
                     "invalid/wontfix label, skipping validation."
                 )
                 continue
-
             try:
                 projects = self.get_issue_projects(owner, repo_name, issue_number)
             except Exception as e:
                 print(
                     f"Error fetching projects for issue {owner}/{repo_name}#{issue_number}: {e}"
                 )
                 raise
-
-            print(
-                f"DEBUG: Raw projects for {owner}/{repo_name}#{issue_number}: {projects}"
-            )
             project_keys = [self._project_title_key(p) for p in projects]
-            print(f"DEBUG: Normalized project keys: {project_keys}")
-            print(f"DEBUG: Required project keys: {required_projects}")
-
             has_valid_project = any(key in required_projects for key in project_keys)
             if has_valid_project:
                 print(
@@ -223,12 +201,10 @@ def validate_pr_issues(self, pr):
                     f"to any required project (found: {projects or 'none'}), "
                     "skipping validation."
                 )
-
         return False
 
     def get_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
         """Get the comment of this bot with the given marker if it exists.
-
         If ``after_date`` is provided, only considers comments posted after that date.
         """
         try:
@@ -251,7 +227,6 @@ def get_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None
 
     def has_bot_comment(self, pr, comment_type, after_date=None, issue_comments=None):
         """Check if PR already has a specific type of bot comment.
-
         Uses HTML markers. If ``after_date`` is provided,
         only considers comments posted after that date.
         """

.github/actions/bot-autoassign/issue_assignment_bot.py

@@ -40,7 +40,6 @@ def get_contributing_guidelines_url(self):
 
     def detect_issue_type(self, issue):
         """Analyzes labels, title and body.
-
         Returns 'bug', 'feature', or None.
         """
         bug_keywords = [
@@ -365,11 +364,9 @@ def auto_assign_issues_from_pr(self, pr_number, pr_author, pr_body, max_issues=1
 
     def unassign_issues_from_pr(self, pr_body, pr_author):
         """Unassign linked issues from PR author"""
-
         if not self.repo:
             print("GitHub client not initialized")
             return []
-
         try:
             return unassign_linked_issues_helper(
                 self.repo, self.repository_name, pr_body, pr_author
@@ -442,17 +439,16 @@ def handle_pull_request(self):
                 else:
                     self.unassign_issues_from_pr(pr_body, pr_author)
                 return True
-
             if action in ["opened", "reopened", "edited", "ready_for_review"]:
-                self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
                 pr_obj = self.repo.get_pull(pr_number)
                 is_valid = self.validate_pr_issues(pr_obj)
+                if is_valid:
+                    self.auto_assign_issues_from_pr(pr_number, pr_author, pr_body)
                 labels_lower = set()
                 try:
                     labels_lower = {label.name.lower() for label in pr_obj.labels}
                 except (TypeError, AttributeError):
                     pass
-
                 if is_valid:
                     if "invalid" in labels_lower:
                         pr_obj.remove_from_labels("invalid")
@@ -461,7 +457,6 @@ def handle_pull_request(self):
                     if "invalid" not in labels_lower:
                         pr_obj.add_to_labels("invalid")
                         print(f"Added 'invalid' label to PR #{pr_number}")
-
                     if not self.has_bot_comment(pr_obj, "invalid_unvalidated_issue"):
                         comment_body = self.get_invalid_unvalidated_issue_comment(
                             pr_author

.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py

@@ -1,6 +1,6 @@
 import os
 import sys
-from unittest.mock import Mock, patch
+from unittest.mock import MagicMock, Mock, patch
 
 # Add the parent directory to path for importing bot modules
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
@@ -516,6 +516,7 @@ def test_opened(self, bot_env):
         )
         mock_issue = _make_issue_with_assignment("testuser")
         bot_env["repo"].get_issue.return_value = mock_issue
+        bot.validate_pr_issues = MagicMock(return_value=True)
         assert bot.handle_pull_request()
         mock_issue.add_to_assignees.assert_called_once_with("testuser")
 
@@ -533,8 +534,27 @@ def test_reopened(self, bot_env):
         )
         mock_issue = _make_issue_with_assignment("testuser")
         bot_env["repo"].get_issue.return_value = mock_issue
+        bot.validate_pr_issues = MagicMock(return_value=True)
         assert bot.handle_pull_request()
 
+    def test_skips_auto_assign_if_invalid(self, bot_env):
+        bot = IssueAssignmentBot()
+        bot.load_event_payload(
+            {
+                "action": "opened",
+                "pull_request": {
+                    "number": 100,
+                    "user": {"login": "testuser"},
+                    "body": "Fixes #123",
+                },
+            }
+        )
+        mock_issue = _make_issue_with_assignment("testuser")
+        bot_env["repo"].get_issue.return_value = mock_issue
+        bot.validate_pr_issues = MagicMock(return_value=False)
+        assert bot.handle_pull_request()
+        mock_issue.add_to_assignees.assert_not_called()
+
     def test_closed(self, bot_env):
         bot = IssueAssignmentBot()
         bot.load_event_payload(
@@ -1037,11 +1057,9 @@ def test_validate_pr_issues_is_pr(self, bot_env):
         mock_pr.user.login = "external-contributor"
         mock_pr.author_association = "NONE"
         mock_pr.body = "Fixes #12"
-
         mock_issue = Mock()
         mock_issue.pull_request = {"url": "..."}
         bot_env["repo"].get_issue.return_value = mock_issue
-
         assert not bot.validate_pr_issues(mock_pr)
 
     def test_validate_pr_issues_closed(self, bot_env):
@@ -1050,12 +1068,10 @@ def test_validate_pr_issues_closed(self, bot_env):
         mock_pr.user.login = "external-contributor"
         mock_pr.author_association = "NONE"
         mock_pr.body = "Fixes #12"
-
         mock_issue = Mock()
         mock_issue.pull_request = None
         mock_issue.state = "closed"
         bot_env["repo"].get_issue.return_value = mock_issue
-
         assert not bot.validate_pr_issues(mock_pr)
 
     def test_validate_pr_issues_invalid_labels(self, bot_env):
@@ -1064,21 +1080,18 @@ def test_validate_pr_issues_invalid_labels(self, bot_env):
         mock_pr.user.login = "external-contributor"
         mock_pr.author_association = "NONE"
         mock_pr.body = "Fixes #12"
-
         # Test case 1: No labels
         mock_issue = Mock()
         mock_issue.pull_request = None
         mock_issue.state = "open"
         mock_issue.labels = []
         bot_env["repo"].get_issue.return_value = mock_issue
         assert not bot.validate_pr_issues(mock_pr)
-
         # Test case 2: Has wontfix/invalid labels
         label_wontfix = Mock()
         label_wontfix.name = "wontfix"
         mock_issue.labels = [label_wontfix]
         assert not bot.validate_pr_issues(mock_pr)
-
         # Test case 3: Has a mix of valid and invalid/wontfix labels
         label_bug = Mock()
         label_bug.name = "bug"
@@ -1091,15 +1104,13 @@ def test_validate_pr_issues_not_in_project(self, bot_env):
         mock_pr.user.login = "external-contributor"
         mock_pr.author_association = "NONE"
         mock_pr.body = "Fixes #12"
-
         label_bug = Mock()
         label_bug.name = "bug"
         mock_issue = Mock()
         mock_issue.pull_request = None
         mock_issue.state = "open"
         mock_issue.labels = [label_bug]
         bot_env["repo"].get_issue.return_value = mock_issue
-
         with patch.object(
             bot, "get_issue_projects", return_value=["Some Other Project"]
         ):
@@ -1111,15 +1122,13 @@ def test_validate_pr_issues_fully_valid(self, bot_env):
         mock_pr.user.login = "external-contributor"
         mock_pr.author_association = "NONE"
         mock_pr.body = "Fixes #12"
-
         label_bug = Mock()
         label_bug.name = "bug"
         mock_issue = Mock()
         mock_issue.pull_request = None
         mock_issue.state = "open"
         mock_issue.labels = [label_bug]
         bot_env["repo"].get_issue.return_value = mock_issue
-
         with patch.object(
             bot, "get_issue_projects", return_value=["OpenWISP Contributor's Board"]
         ):
@@ -1138,11 +1147,9 @@ def test_handle_pull_request_invalid_label_and_comment(self, bot_env):
                 },
             }
         )
-
         mock_pr_obj = Mock()
         mock_pr_obj.labels = []
         bot_env["repo"].get_pull.return_value = mock_pr_obj
-
         with patch.object(bot, "validate_pr_issues", return_value=False), patch.object(
             bot, "has_bot_comment", return_value=False
         ):
@@ -1167,13 +1174,11 @@ def test_handle_pull_request_valid_removes_label(self, bot_env):
                 },
             }
         )
-
         mock_label = Mock()
         mock_label.name = "invalid"
         mock_pr_obj = Mock()
         mock_pr_obj.labels = [mock_label]
         bot_env["repo"].get_pull.return_value = mock_pr_obj
-
         with patch.object(bot, "validate_pr_issues", return_value=True):
             assert bot.handle_pull_request()
             mock_pr_obj.remove_from_labels.assert_called_once_with("invalid")