GitHub: Consider only the events we are interested in (#2066)

GitHub: Consider only the events we are interested in

This extends the interested() method in the GitHub webhook, so that checks on the event type and action are done in the API. This will help avoid creating Celery tasks for these events.
This change makes similar checks in Parser() redundant, but I've decided not to remove those, yet, mainly b/c it's hard to tell how that will influence parsing and telling which event is handled.
TODO:

 Write new tests or update the old ones to cover new functionality.

Reviewed-by: Jiri Popelka

packit_service/service/api/webhooks.py

@@ -106,16 +106,27 @@ def post(self):
     def interested():
         """
         Check whether we want to process this event.
+
         Returns:
-             `False` if we are not interested in this kind of event
+             False if we are not interested in this kind of event
         """
-        event_type = request.headers.get("X-GitHub-Event")
+        event = request.headers.get("X-GitHub-Event")
         uuid = request.headers.get("X-GitHub-Delivery")
         action = request.json.get("action")
+        deleted = request.json.get("deleted")
+
+        interests = {
+            "check_run": action == "rerequested",
+            "pull_request": action in {"opened", "reopened", "synchronize"},
+            "pull_request_review_comment": action in {"created", "edited"},
+            "issue_comment": action == "created",
+            "release": action == "published",
+            "push": not deleted,
+            "installation": action == "created",
+        }
+        _interested = interests.get(event, False)
 
-        # we don't care about created or completed check runs
-        _interested = event_type != "check_run" or action == "rerequested"
-        logger.debug(f"{event_type} {uuid}{'' if _interested else ' (not interested)'}")
+        logger.debug(f"{event} {uuid}{'' if _interested else ' (not interested)'}")
         return _interested
 
     @staticmethod

tests/unit/test_webhooks.py

@@ -130,3 +130,133 @@ def test_validate_token(mock_config, headers, is_good):
                 webhooks.GitlabWebhook.validate_token(temp)
         else:
             webhooks.GitlabWebhook.validate_token(temp)
+
+
+@pytest.mark.parametrize(
+    "headers, payload, interested",
+    [
+        (
+            {"X-GitHub-Event": "check_run", "X-GitHub-Delivery": "uuid"},
+            {"action": "rerequested"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "pull_request", "X-GitHub-Delivery": "uuid"},
+            {"action": "opened"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "pull_request", "X-GitHub-Delivery": "uuid"},
+            {"action": "reopened"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "pull_request", "X-GitHub-Delivery": "uuid"},
+            {"action": "closed"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "pull_request", "X-GitHub-Delivery": "uuid"},
+            {"action": "edited"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "pull_request", "X-GitHub-Delivery": "uuid"},
+            {"action": "synchronize"},
+            True,
+        ),
+        (
+            {
+                "X-GitHub-Event": "pull_request_review_comment",
+                "X-GitHub-Delivery": "uuid",
+            },
+            {"action": "created"},
+            True,
+        ),
+        (
+            {
+                "X-GitHub-Event": "pull_request_review_comment",
+                "X-GitHub-Delivery": "uuid",
+            },
+            {"action": "edited"},
+            True,
+        ),
+        (
+            {
+                "X-GitHub-Event": "pull_request_review_comment",
+                "X-GitHub-Delivery": "uuid",
+            },
+            {"action": "deleted"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "issue_comment", "X-GitHub-Delivery": "uuid"},
+            {"action": "created"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "issue_comment", "X-GitHub-Delivery": "uuid"},
+            {"action": "created"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "issue_comment", "X-GitHub-Delivery": "uuid"},
+            {"action": "edited"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "release", "X-GitHub-Delivery": "uuid"},
+            {"action": "created"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "release", "X-GitHub-Delivery": "uuid"},
+            {"action": "published"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "release", "X-GitHub-Delivery": "uuid"},
+            {"action": "released"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "push", "X-GitHub-Delivery": "uuid"},
+            {"deleted": False},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "push", "X-GitHub-Delivery": "uuid"},
+            {"deleted": True},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "installation", "X-GitHub-Delivery": "uuid"},
+            {"action": "created"},
+            True,
+        ),
+        (
+            {"X-GitHub-Event": "installation", "X-GitHub-Delivery": "uuid"},
+            {"action": "deleted"},
+            False,
+        ),
+        (
+            {"X-GitHub-Event": "label"},
+            {"action": "created"},
+            False,
+        ),
+    ],
+)
+def test_interested(mock_config, headers, payload, interested):
+    # flexmock config before import as it fails on looking for config
+    flexmock(ServiceConfig).should_receive("get_service_config").and_return(
+        flexmock(ServiceConfig)
+    )
+
+    from packit_service.service.api import webhooks
+
+    webhooks.config = mock_config
+
+    with Flask(__name__).test_request_context(
+        json=payload, content_type="application/json", headers=headers
+    ):
+        assert webhooks.GithubWebhook.interested() == interested