Add filtering for PRs to match issue filtering behavior

sync2jira/upstream_issue.py

@@ -110,25 +110,25 @@
 """
 
 
-def handle_github_message(body, config, is_pr=False):
+def passes_github_filters(item, config, upstream, item_type="issue"):
     """
-    Handle GitHub message from FedMsg.
-
-    :param Dict body: FedMsg Message body
-    :param Dict config: Config File
-    :param Bool is_pr: msg refers to a pull request
-    :returns: Issue object
-    :rtype: sync2jira.intermediary.Issue
+    Apply GitHub filters (labels, milestone, other fields) to an item.
+
+    :param dict item: GitHub issue or PR data
+    :param dict _filter: Filter configuration
+    :param str upstream: Upstream repository name
+    :param str item_type: Type of item for logging ("issue" or "PR")
+    :returns: True if item passes all filters, False otherwise
+    :rtype: bool
     """
-    owner = body["repository"]["owner"]["login"]
-    repo = body["repository"]["name"]
-    upstream = "{owner}/{repo}".format(owner=owner, repo=repo)
-
+    filter_config = (
+        config["sync2jira"].get("filters", {}).get("github", {}).get(upstream, {})
+    )
     mapped_repos = config["sync2jira"]["map"]["github"]
     if upstream not in mapped_repos:
         log.debug("%r not in Github map: %r", upstream, mapped_repos.keys())
         return None
-    key = "pullrequest" if is_pr else "issue"
+    key = "pullrequest" if item_type == "PR" else "issue"
     if key not in mapped_repos[upstream].get("sync", []):
         log.debug(
             "%r not in Github sync map: %r",
@@ -137,36 +137,57 @@ def handle_github_message(body, config, is_pr=False):
         )
         return None
 
-    _filter = config["sync2jira"].get("filters", {}).get("github", {}).get(upstream, {})
-
-    issue = body["issue"]
-    for key, expected in _filter.items():
+    for key, expected in filter_config.items():
         if key == "labels":
-            # special handling for label: we look for it in the list of msg labels
-            actual = {label["name"] for label in issue["labels"]}
+            # special handling for label: we look for it in the list of labels
+            actual = {label["name"] for label in item.get("labels", [])}
             if actual.isdisjoint(expected):
-                log.debug("Labels %s not found on issue: %s", expected, upstream)
-                return None
+                log.debug(
+                    "Labels %s not found on %s: %s", expected, upstream, item_type
+                )
+                return False
         elif key == "milestone":
             # special handling for milestone: use the number
-            milestone = issue.get(key) or {}  # Key might exist with value `None`
+            milestone = item.get(key) or {}  # Key might exist with value `None`
             actual = milestone.get("number")
             if expected != actual:
-                log.debug("Milestone %s not set on issue: %s", expected, upstream)
-                return None
+                log.debug(
+                    "Milestone %s not set on %s: %s", expected, upstream, item_type
+                )
+                return False
         else:
             # direct comparison
-            actual = issue.get(key)
+            actual = item.get(key)
             if actual != expected:
                 log.debug(
-                    "Actual %r %r != expected %r on issue %s",
+                    "Actual %r %r != expected %r on %s %s",
                     key,
                     actual,
                     expected,
                     upstream,
+                    item_type,
                 )
-                return None
+                return False
+    return True
+
+
+def handle_github_message(body, config, is_pr=False):
+    """
+    Handle GitHub message from FedMsg.
+
+    :param Dict body: FedMsg Message body
+    :param Dict config: Config File
+    :param Bool is_pr: msg refers to a pull request
+    :returns: Issue object
+    :rtype: sync2jira.intermediary.Issue
+    """
+    owner = body["repository"]["owner"]["login"]
+    repo = body["repository"]["name"]
+    upstream = "{owner}/{repo}".format(owner=owner, repo=repo)
 
+    issue = body["issue"]
+    if not passes_github_filters(issue, config, upstream, item_type="issue"):
+        return None
     if is_pr and not issue.get("closed_at"):
         log.debug(
             "%r is a pull request.  Ignoring.", issue.get("html_url", "<missing URL>")

sync2jira/upstream_pr.py

@@ -41,16 +41,11 @@ def handle_github_message(body, config, suffix):
     repo = body["repository"]["name"]
     upstream = "{owner}/{repo}".format(owner=owner, repo=repo)
 
-    mapped_repos = config["sync2jira"]["map"]["github"]
-    if upstream not in mapped_repos:
-        log.debug("%r not in Github map: %r", upstream, mapped_repos.keys())
-        return None
-    elif "pullrequest" not in mapped_repos[upstream].get("sync", []):
-        log.debug("%r not in Github PR map: %r", upstream, mapped_repos.keys())
-        return None
-
     pr = body["pull_request"]
-    github_client = Github(config["sync2jira"]["github_token"])
+    if not u_issue.passes_github_filters(pr, config, upstream, item_type="PR"):
+        return None
+    token = config["sync2jira"].get("github_token")
+    github_client = Github(token, retry=5)
     reformat_github_pr(pr, upstream, github_client)
     return i.PR.from_github(upstream, pr, suffix, config)
 

tests/test_upstream_issue.py

@@ -767,3 +767,92 @@ def test_add_project_values_early_exit(self, mock_requests_post):
             self.assertIsNone(result)
             # Reset mock
             mock_requests_post.reset_mock()
+
+    def test_passes_github_filters(self):
+        """
+        Test passes_github_filters for labels, milestone, and other fields.
+        Tests all filtering conditions in one test case.
+        """
+        upstream = "org/repo"
+        self.mock_config["sync2jira"]["filters"]["github"][upstream] = {
+            "filter1": "filter1",
+            "labels": ["custom_tag"],
+            "milestone": 1,
+        }
+
+        # Test 1: Bad label - should return False
+        item = {
+            "labels": [{"name": "bad_label"}],
+            "milestone": {"number": 1},
+            "filter1": "filter1",
+        }
+        self.assertFalse(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )
+
+        # Test 2: Bad milestone - should return False
+        item = {
+            "labels": [{"name": "custom_tag"}],
+            "milestone": {"number": 456},
+            "filter1": "filter1",
+        }
+        self.assertFalse(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )
+
+        # Test 3: Bad other field (filter1) - should return False
+        item = {
+            "labels": [{"name": "custom_tag"}],
+            "milestone": {"number": 1},
+            "filter1": "filter2",
+        }
+        self.assertFalse(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )
+
+        # Test 4: All filters pass - should return True
+        item = {
+            "labels": [{"name": "custom_tag"}],
+            "milestone": {"number": 1},
+            "filter1": "filter1",
+        }
+        self.assertTrue(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )
+
+        # Test 5: Config specifies only labels; item has matching label (wrong milestone/filter1 ignored) → True
+        self.mock_config["sync2jira"]["filters"]["github"][upstream] = {
+            "labels": ["custom_tag"]
+        }
+        item = {
+            "labels": [{"name": "custom_tag"}],
+            "milestone": {"number": 999},
+            "filter1": "wrong",
+        }
+        self.assertTrue(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )
+
+        # Test 6: Config specifies only milestone; item has matching milestone (wrong label/filter1 ignored) → True
+        self.mock_config["sync2jira"]["filters"]["github"][upstream] = {"milestone": 1}
+        item = {
+            "labels": [{"name": "bad_label"}],
+            "milestone": {"number": 1},
+            "filter1": "wrong",
+        }
+        self.assertTrue(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )
+
+        # Test 7: Config specifies only filter1; item has matching filter1 (wrong label/milestone ignored) → True
+        self.mock_config["sync2jira"]["filters"]["github"][upstream] = {
+            "filter1": "filter1"
+        }
+        item = {
+            "labels": [{"name": "bad_label"}],
+            "milestone": {"number": 999},
+            "filter1": "filter1",
+        }
+        self.assertTrue(
+            u.passes_github_filters(item, self.mock_config, upstream, item_type="issue")
+        )

tests/test_upstream_pr.py

@@ -123,7 +123,7 @@ def test_handle_github_message(self, mock_pr_from_github, mock_github):
             "mock_suffix",
             self.mock_config,
         )
-        mock_github.assert_called_with("mock_token")
+        mock_github.assert_called_with("mock_token", retry=5)
         self.assertEqual("Successful Call!", response)
         self.mock_github_client.get_repo.assert_called_with("org/repo")
         self.mock_github_repo.get_pull.assert_called_with(number="mock_number")
@@ -254,3 +254,43 @@ def test_filter_multiple_labels(
             self.mock_config["sync2jira"]["filters"]["github"]["org/repo"]["labels"],
             ["custom_tag", "another_tag", "and_another"],
         )
+
+    @mock.patch("sync2jira.upstream_pr.u_issue.passes_github_filters")
+    @mock.patch("sync2jira.intermediary.PR.from_github")
+    def test_handle_github_message_filter_returns_false(
+        self, mock_pr_from_github, mock_passes
+    ):
+        """When passes_github_filters returns False, handle_github_message returns None."""
+        mock_passes.return_value = False
+
+        response = u.handle_github_message(
+            body=self.mock_github_message_body,
+            config=self.mock_config,
+            suffix="mock_suffix",
+        )
+
+        mock_passes.assert_called_once()
+        mock_pr_from_github.assert_not_called()
+        self.assertIsNone(response)
+
+    @mock.patch(PATH + "Github")
+    @mock.patch("sync2jira.upstream_pr.u_issue.passes_github_filters")
+    @mock.patch("sync2jira.intermediary.PR.from_github")
+    def test_handle_github_message_filter_returns_true(
+        self, mock_pr_from_github, mock_passes, mock_github
+    ):
+        """When passes_github_filters returns True, handle_github_message proceeds to PR.from_github."""
+        mock_passes.return_value = True
+        mock_pr_from_github.return_value = "Successful Call!"
+        mock_github.return_value = self.mock_github_client
+
+        response = u.handle_github_message(
+            body=self.mock_github_message_body,
+            config=self.mock_config,
+            suffix="mock_suffix",
+        )
+
+        mock_passes.assert_called_once()
+        mock_pr_from_github.assert_called_once()
+        mock_github.assert_called_with("mock_token", retry=5)
+        self.assertEqual("Successful Call!", response)