Fix #407, #708: Run multiple actions when multiple whiteboard tags (#1109)

* Mention pandoc version requirement

* Rename and return a list from lookup_actions

* Lookup multiple actions

* Execute multiple actions if configured

This additionally adds a test that verifies
actions are properly detected and executed.

* Address review comments

This adds an additional test to make sure that actions
are dispatched to the appropriate project.

* Add assertion about respective issues calls

---------

Co-authored-by: Mathieu Leplatre <[email protected]>

Author: Dexterp37

README.md

@@ -75,7 +75,7 @@ graph TD
 
 # Development
 
-We use [pandoc](https://pandoc.org) to convert markdown to the Jira syntax. Make sure the binary is found in path or [specify your custom location](https://github.com/JessicaTegner/pypandoc#specifying-the-location-of-pandoc-binaries).
+We use [pandoc](https://pandoc.org) to convert markdown to the Jira syntax. Make sure the binary is found in path or [specify your custom location](https://github.com/JessicaTegner/pypandoc#specifying-the-location-of-pandoc-binaries) and its version is sufficiently recent to support Jira syntax (e.g. 3.6.3).
 
 - `make start`: run the application locally (http://localhost:8000)
 - `make test`: run the unit tests suites

jbi/models.py

@@ -238,7 +238,7 @@ class RunnerContext(Context, extra="forbid"):
 
     operation: Operation
     event: WebhookEvent
-    action: Optional[Action] = None
+    actions: Optional[list[Action]] = None
     bug: BugId | Bug
 
 

jbi/runner.py

@@ -58,20 +58,23 @@ def groups2operation(steps: ActionSteps):
     return by_operation
 
 
-def lookup_action(bug: bugzilla_models.Bug, actions: Actions) -> Action:
+def lookup_actions(bug: bugzilla_models.Bug, actions: Actions) -> list[Action]:
     """
-    Find first matching action from bug's whiteboard field.
+    Find matching actions from bug's whiteboard field.
 
     Tags are strings between brackets and can have prefixes/suffixes
     using dashes (eg. ``[project]``, ``[project-moco]``, ``[project-moco-sprint1]``).
     """
 
     if bug.whiteboard:
+        relevant_actions = []
         for tag, action in actions.by_tag.items():
             # [tag-word], [tag-], [tag], but not [word-tag] or [tagword]
             search_string = r"\[" + tag + r"(-[^\]]*)*\]"
             if re.search(search_string, bug.whiteboard, flags=re.IGNORECASE):
-                return action
+                relevant_actions.append(action)
+        if len(relevant_actions):
+            return relevant_actions
 
     raise ActionNotFoundError(", ".join(actions.by_tag.keys()))
 
@@ -203,12 +206,17 @@ def execute_action(
     request: bugzilla_models.WebhookRequest,
     actions: Actions,
 ):
-    """Execute the configured action for the specified `request`.
+    """Execute the configured actions for the specified `request`.
+
+    If multiple actions are configured for a given request, all of them
+    are executed.
 
     This will raise an `IgnoreInvalidRequestError` error if the request
     does not contain bug data or does not match any action.
 
-    The value returned by the action call is returned.
+    A dictionary containing the values returned by the actions calls
+    is returned. The action tag is used to index the responses in the
+    dictionary.
     """
     bug, event = request.bug, request.event
     runner_context = RunnerContext(
@@ -221,7 +229,7 @@ def execute_action(
             raise IgnoreInvalidRequestError("private bugs are not supported")
 
         try:
-            action = lookup_action(bug, actions)
+            relevant_actions = lookup_actions(bug, actions)
         except ActionNotFoundError as err:
             raise IgnoreInvalidRequestError(
                 f"no bug whiteboard matching action tags: {err}"
@@ -238,10 +246,42 @@ def execute_action(
             # is processed (eg. if it spent some time in the DL queue)
             raise IgnoreInvalidRequestError(str(err)) from err
 
-        runner_context = runner_context.update(bug=bug)
+        runner_context = runner_context.update(bug=bug, actions=relevant_actions)
+
+        return do_execute_actions(runner_context, bug, relevant_actions)
+    except IgnoreInvalidRequestError as exception:
+        logger.info(
+            "Ignore incoming request: %s",
+            exception,
+            extra=runner_context.update(operation=Operation.IGNORE).model_dump(),
+        )
+        statsd.incr("jbi.bugzilla.ignored.count")
+        raise
 
-        runner_context = runner_context.update(action=action)
 
+@statsd.timer("jbi.action.execution.timer")
+def do_execute_actions(
+    runner_context: RunnerContext,
+    bug: bugzilla_models.Bug,
+    actions: Actions,
+):
+    """Execute the provided actions on the bug, within the provided context.
+
+    This will raise an `IgnoreInvalidRequestError` error if the request
+    does not contain bug data or does not match any action.
+
+    A dictionary containing the values returned by the actions calls
+    is returned. The action tag is used to index the responses in the
+    dictionary.
+    """
+    runner_context = runner_context.update(bug=bug)
+
+    runner_context = runner_context.update(actions=actions)
+
+    event = runner_context.event
+
+    details = {}
+    for action in actions:
         linked_issue_key: Optional[str] = bug.extract_from_see_also(
             project_key=action.jira_project_key
         )
@@ -275,6 +315,10 @@ def execute_action(
             if (
                 project_key := jira_issue["fields"]["project"]["key"]
             ) != action_context.jira.project:
+                # TODO: We're now executing multiple actions for a given bug, we
+                # should probably either not fail and instead report which actions
+                # failed to apply, or execute all the changes as a "transaction" and
+                # roll them back if one of them fails.
                 raise IgnoreInvalidRequestError(
                     f"ignore linked project {project_key!r} (!={action_context.jira.project!r})"
                 )
@@ -306,7 +350,8 @@ def execute_action(
             extra=runner_context.update(operation=Operation.EXECUTE).model_dump(),
         )
         executor = Executor(parameters=action.parameters)
-        handled, details = executor(context=action_context)
+        handled, action_details = executor(context=action_context)
+        details[action.whiteboard_tag] = action_details
         statsd.incr(f"jbi.operation.{action_context.operation.lower()}.count")
         logger.info(
             "Action %r executed successfully for Bug %s",
@@ -317,12 +362,4 @@ def execute_action(
             ).model_dump(),
         )
         statsd.incr("jbi.bugzilla.processed.count")
-        return details
-    except IgnoreInvalidRequestError as exception:
-        logger.info(
-            "Ignore incoming request: %s",
-            exception,
-            extra=runner_context.update(operation=Operation.IGNORE).model_dump(),
-        )
-        statsd.incr("jbi.bugzilla.ignored.count")
-        raise
+    return details

tests/conftest.py

@@ -55,6 +55,7 @@ def mocked_statsd():
         yield _mocked_statsd
 
 
+register(factories.ActionStepsFactory)
 register(factories.ActionContextFactory)
 register(factories.ActionFactory)
 register(factories.ActionsFactory, "_actions")

tests/fixtures/factories.py

@@ -23,10 +23,16 @@ def _build(cls, model_class, *args, **kwargs):
         return model_class.model_construct(**kwargs)
 
 
+class ActionStepsFactory(PydanticFactory):
+    class Meta:
+        model = models.ActionSteps
+
+
 class ActionParamsFactory(PydanticFactory):
     class Meta:
         model = models.ActionParams
 
+    steps = factory.SubFactory(ActionStepsFactory)
     jira_project_key = "JBI"
     jira_components = {}
     labels_brackets = "no"

tests/unit/test_runner.py

@@ -5,6 +5,7 @@
 import requests
 import responses
 
+import tests.fixtures.factories as factories
 from jbi import Operation
 from jbi.bugzilla.client import BugNotAccessibleError
 from jbi.environment import get_settings
@@ -15,7 +16,7 @@
     Executor,
     execute_action,
     execute_or_queue,
-    lookup_action,
+    lookup_actions,
 )
 
 
@@ -187,7 +188,7 @@ def test_action_is_logged_as_success_if_returns_true(
         ("Action 'devtest' executed successfully for Bug 654321", Operation.SUCCESS),
     ]
     assert capturelogs.records[-1].bug["id"] == 654321
-    assert capturelogs.records[-1].action["whiteboard_tag"] == "devtest"
+    assert capturelogs.records[-1].actions[0]["whiteboard_tag"] == "devtest"
 
 
 def test_action_is_logged_as_ignore_if_returns_false(
@@ -619,11 +620,42 @@ def test_counter_is_incremented_for_attachment(
 )
 def test_lookup_action_found(whiteboard, actions, bug_factory):
     bug = bug_factory(id=1234, whiteboard=whiteboard)
-    action = lookup_action(bug, actions)
+    action = lookup_actions(bug, actions)[0]
     assert action.whiteboard_tag == "devtest"
     assert "test config" in action.description
 
 
+@pytest.mark.parametrize(
+    "whiteboard,expected_tags",
+    [
+        ("[example][DevTest]", ["devtest"]),
+        ("[DevTest][example]", ["devtest"]),
+        ("[example][DevTest][other]", ["devtest", "other"]),
+    ],
+)
+def test_multiple_lookup_actions_found(whiteboard, expected_tags, bug_factory):
+    actions = factories.ActionsFactory(
+        root=[
+            factories.ActionFactory(
+                whiteboard_tag="devtest",
+                bugzilla_user_id="tbd",
+                description="test config",
+            ),
+            factories.ActionFactory(
+                whiteboard_tag="other",
+                bugzilla_user_id="tbd",
+                description="test config",
+            ),
+        ]
+    )
+    bug = bug_factory(id=1234, whiteboard=whiteboard)
+    acts = lookup_actions(bug, actions)
+    assert len(acts) == len(expected_tags)
+    looked_up_tags = [a.whiteboard_tag for a in acts]
+    assert sorted(looked_up_tags) == sorted(expected_tags)
+    assert all(["test config" == a.description for a in acts])
+
+
 @pytest.mark.parametrize(
     "whiteboard",
     [
@@ -649,5 +681,114 @@ def test_lookup_action_found(whiteboard, actions, bug_factory):
 def test_lookup_action_not_found(whiteboard, actions, bug_factory):
     bug = bug_factory(id=1234, whiteboard=whiteboard)
     with pytest.raises(ActionNotFoundError) as exc_info:
-        lookup_action(bug, actions)
+        lookup_actions(bug, actions)[0]
     assert str(exc_info.value) == "devtest"
+
+
+def test_request_triggers_multiple_actions(
+    webhook_request_factory,
+    mocked_bugzilla,
+):
+    actions = factories.ActionsFactory(
+        root=[
+            factories.ActionFactory(
+                whiteboard_tag="devtest",
+                bugzilla_user_id="tbd",
+                description="test config",
+            ),
+            factories.ActionFactory(
+                whiteboard_tag="other",
+                bugzilla_user_id="tbd",
+                description="test config",
+            ),
+        ]
+    )
+
+    webhook = webhook_request_factory(bug__whiteboard="[devtest][other]")
+    mocked_bugzilla.get_bug.return_value = webhook.bug
+
+    details = execute_action(request=webhook, actions=actions)
+
+    # Details has the following shape:
+    # {'devtest': {'responses': [..]}, 'other': {'responses': [...]}}
+    assert len(actions) == len(details)
+    assert "devtest" in details
+    assert "other" in details
+
+
+def test_request_triggers_multiple_update_actions(
+    webhook_request_factory,
+    mocked_bugzilla,
+    mocked_jira,
+    webhook_event_change_factory,
+):
+    actions = factories.ActionsFactory(
+        root=[
+            factories.ActionFactory(
+                whiteboard_tag="devtest",
+                bugzilla_user_id="tbd",
+                description="test config",
+                parameters__jira_project_key="JBI",
+                parameters__steps__existing=["maybe_update_issue_resolution"],
+                parameters__resolution_map={
+                    "FIXED": "Closed",
+                },
+            ),
+            factories.ActionFactory(
+                whiteboard_tag="other",
+                bugzilla_user_id="tbd",
+                description="test config",
+                parameters__jira_project_key="DE",
+                parameters__steps__existing=["maybe_update_issue_resolution"],
+                parameters__resolution_map={
+                    "FIXED": "Done",
+                },
+            ),
+        ]
+    )
+
+    webhook = webhook_request_factory(
+        bug__whiteboard="[devtest][other]",
+        bug__see_also=[
+            "https://mozilla.atlassian.net/browse/JBI-234",
+            "https://mozilla.atlassian.net/browse/DE-567",
+        ],
+        bug__resolution="FIXED",
+        event__changes=[
+            webhook_event_change_factory(
+                field="resolution", removed="OPEN", added="FIXED"
+            )
+        ],
+    )
+    mocked_bugzilla.get_bug.return_value = webhook.bug
+
+    def side_effect_for_get_issue(issue_key):
+        if issue_key.startswith("JBI-"):
+            return {"fields": {"project": {"key": "JBI"}}}
+        elif issue_key.startswith("DE-"):
+            return {"fields": {"project": {"key": "DE"}}}
+
+        return None
+
+    mocked_jira.get_issue.side_effect = side_effect_for_get_issue
+
+    details = execute_action(request=webhook, actions=actions)
+
+    mocked_jira.update_issue_field.assert_any_call(
+        key="JBI-234",
+        fields={
+            "resolution": {"name": "Closed"},
+        },
+    )
+    mocked_jira.update_issue_field.assert_any_call(
+        key="DE-567",
+        fields={
+            "resolution": {"name": "Done"},
+        },
+    )
+
+    # Details has the following shape:
+    # {'devtest': {'responses': [..]}, 'other': {'responses': [...]}}
+    assert len(actions) == len(details)
+    assert "devtest" in details
+    assert "other" in details