Fix #475, #382: Fix lookup of actions when tag contains dash (#472)

* Updates to lookup_action logic-- instead of constructing tags from the whiteboard, then trying to see which tag exists as an action. In this commit we attempt to use the action.whiteboard_tag as the element which must exist within the bug.whiteboard element--iterating through the actions, and acting on the first action that applies to the bug. The whiteboard_tag presented in the action must be contained within the bug’s whiteboard field with a prefix of [

* Fix mypy

* Fixing test: prefix of [ required

* Resolve tests

* Updating logs to reflect intent

* minor update to search string

* Defaulted behavior of pre/post fixed brackets as a requirement, is now toggleable

* Changes to readme

* Adjust factory-usage; make factory method available as fixture. Add tests to test_bugzilla

* Fix regular expression and rewrite tests using parameterize

* Remove brackets_required option

* Remove redundant tests

* Remove whiteboard manipulation with reverse lookup

* Rename variables in tests

* Rewrite test to match scope of PR changes

---------

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

Author: bsieber-mozilla

README.md

@@ -48,6 +48,7 @@ graph TD
 - `make start`: run the application locally (http://localhost:8000)
 - `make test`: run the unit tests suites
 - `make lint`: static analysis of the code base
+- `make format`: automatically format code to align to linting standards
 
 You may consider:
 

jbi/models.py

@@ -5,6 +5,7 @@
 import functools
 import importlib
 import logging
+import re
 import warnings
 from inspect import signature
 from types import ModuleType
@@ -233,40 +234,21 @@ def is_assigned(self) -> bool:
 
     def get_whiteboard_as_list(self) -> list[str]:
         """Convert string whiteboard into list, splitting on ']' and removing '['."""
-        if self.whiteboard is not None:
-            split_list = self.whiteboard.replace("[", "").split("]")
-            return [x.strip() for x in split_list if x not in ["", " "]]
-        return []
-
-    def get_whiteboard_with_brackets_as_list(self) -> list[str]:
-        """Convert string whiteboard into list, splitting on ']' and removing '['; then re-adding."""
-        wb_list = self.get_whiteboard_as_list()
-        if wb_list is not None and len(wb_list) > 0:
-            return [f"[{element}]" for element in wb_list]
-        return []
+        split_list = (
+            self.whiteboard.replace("[", "").split("]") if self.whiteboard else []
+        )
+        return [x.strip() for x in split_list if x not in ["", " "]]
 
     def get_jira_labels(self) -> list[str]:
         """
         whiteboard labels are added as a convenience for users to search in jira;
         bugzilla is an expected label in Jira
         since jira-labels can't contain a " ", convert to "."
         """
-        wb_list = [wb.replace(" ", ".") for wb in self.get_whiteboard_as_list()]
-        wb_bracket_list = [
-            wb.replace(" ", ".") for wb in self.get_whiteboard_with_brackets_as_list()
-        ]
-
-        return ["bugzilla"] + wb_list + wb_bracket_list
-
-    def get_potential_whiteboard_config_list(self) -> list[str]:
-        """Get all possible tags from `whiteboard` field"""
-        converted_list: list = []
-        for whiteboard in self.get_whiteboard_as_list():
-            first_tag = whiteboard.strip().lower().split(sep="-", maxsplit=1)[0]
-            if first_tag:
-                converted_list.append(first_tag)
-
-        return converted_list
+        wb_list = self.get_whiteboard_as_list()
+        wb_bracket_list = [f"[{wb}]" for wb in wb_list]
+        without_spaces = [wb.replace(" ", ".") for wb in (wb_list + wb_bracket_list)]
+        return ["bugzilla"] + without_spaces
 
     def issue_type(self) -> str:
         """Get the Jira issue type for this bug"""
@@ -304,11 +286,13 @@ def extract_from_see_also(self):
 
     def lookup_action(self, actions: Actions) -> Action:
         """Find first matching action from bug's whiteboard list"""
-        tags: list[str] = self.get_potential_whiteboard_config_list()
-        for tag in tags:
-            if action := actions.get(tag):
-                return action
-        raise ActionNotFoundError(", ".join(tags))
+        if self.whiteboard:
+            for tag, action in actions.by_tag.items():
+                search_string = r"\[[^\]]*" + tag + r"[^\]]*\]"
+                if re.search(search_string, self.whiteboard, flags=re.IGNORECASE):
+                    return action
+
+        raise ActionNotFoundError(", ".join(actions.by_tag.keys()))
 
 
 class BugzillaWebhookRequest(BaseModel):

jbi/runner.py

@@ -64,7 +64,7 @@ def execute_action(
             action = bug.lookup_action(actions)
         except ActionNotFoundError as err:
             raise IgnoreInvalidRequestError(
-                f"no action matching bug whiteboard tags: {err}"
+                f"no bug whiteboard matching action tags: {err}"
             ) from err
         runner_context = runner_context.update(action=action)
 

tests/conftest.py

@@ -20,15 +20,7 @@
     BugzillaWebhookRequest,
 )
 from jbi.services import bugzilla, jira
-from tests.fixtures.factories import (
-    action_context_factory,
-    action_factory,
-    bug_factory,
-    jira_context_factory,
-    webhook_event_factory,
-    webhook_factory,
-    webhook_user_factory,
-)
+from tests.fixtures import factories
 
 
 @pytest.fixture(autouse=True)
@@ -85,25 +77,29 @@ def mocked_responses():
 
 @pytest.fixture
 def context_create_example() -> ActionContext:
-    return action_context_factory(
+    return factories.action_context_factory(
         operation=Operation.CREATE,
     )
 
 
 @pytest.fixture
 def context_update_example() -> ActionContext:
-    bug = bug_factory(see_also=["https://mozilla.atlassian.net/browse/JBI-234"])
-    context = action_context_factory(
+    bug = factories.bug_factory(
+        see_also=["https://mozilla.atlassian.net/browse/JBI-234"]
+    )
+    context = factories.action_context_factory(
         operation=Operation.UPDATE,
         bug=bug,
-        jira=jira_context_factory(issue=bug.extract_from_see_also()),
+        jira=factories.jira_context_factory(issue=bug.extract_from_see_also()),
     )
     return context
 
 
 @pytest.fixture
 def context_update_status_assignee() -> ActionContext:
-    bug = bug_factory(see_also=["https://mozilla.atlassian.net/browse/JBI-234"])
+    bug = factories.bug_factory(
+        see_also=["https://mozilla.atlassian.net/browse/JBI-234"]
+    )
     changes = [
         {
             "field": "status",
@@ -116,84 +112,88 @@ def context_update_status_assignee() -> ActionContext:
             "added": "[email protected]",
         },
     ]
-    event = webhook_event_factory(routing_key="bug.modify", changes=changes)
-    context = action_context_factory(
+    event = factories.webhook_event_factory(routing_key="bug.modify", changes=changes)
+    context = factories.action_context_factory(
         operation=Operation.UPDATE,
         bug=bug,
         event=event,
-        jira=jira_context_factory(issue=bug.extract_from_see_also()),
+        jira=factories.jira_context_factory(issue=bug.extract_from_see_also()),
     )
     return context
 
 
 @pytest.fixture
 def context_comment_example() -> ActionContext:
-    user = webhook_user_factory(login="[email protected]")
+    user = factories.webhook_user_factory(login="[email protected]")
     comment = BugzillaWebhookComment.parse_obj({"number": 2, "body": "hello"})
-    bug = bug_factory(
+    bug = factories.bug_factory(
         see_also=["https://mozilla.atlassian.net/browse/JBI-234"],
         comment=comment,
     )
-    event = webhook_event_factory(target="comment", user=user)
-    context = action_context_factory(
+    event = factories.webhook_event_factory(target="comment", user=user)
+    context = factories.action_context_factory(
         operation=Operation.COMMENT,
         bug=bug,
         event=event,
-        jira=jira_context_factory(issue=bug.extract_from_see_also()),
+        jira=factories.jira_context_factory(issue=bug.extract_from_see_also()),
     )
     return context
 
 
 @pytest.fixture
 def context_update_resolution_example() -> ActionContext:
-    bug = bug_factory(see_also=["https://mozilla.atlassian.net/browse/JBI-234"])
-    event = webhook_event_factory(action="modify", routing_key="bug.modify:resolution")
-    context = action_context_factory(
+    bug = factories.bug_factory(
+        see_also=["https://mozilla.atlassian.net/browse/JBI-234"]
+    )
+    event = factories.webhook_event_factory(
+        action="modify", routing_key="bug.modify:resolution"
+    )
+    context = factories.action_context_factory(
         operation=Operation.UPDATE,
         bug=bug,
         event=event,
-        jira=jira_context_factory(issue=bug.extract_from_see_also()),
+        jira=factories.jira_context_factory(issue=bug.extract_from_see_also()),
     )
     return context
 
 
 @pytest.fixture
 def webhook_create_example() -> BugzillaWebhookRequest:
-    webhook_payload = webhook_factory()
+    webhook_payload = factories.webhook_factory()
 
     return webhook_payload
 
 
 @pytest.fixture
 def webhook_comment_example() -> BugzillaWebhookRequest:
-    user = webhook_user_factory(login="[email protected]")
+    user = factories.webhook_user_factory(login="[email protected]")
     comment = BugzillaWebhookComment.parse_obj({"number": 2, "body": "hello"})
-    bug = bug_factory(
+    bug = factories.bug_factory(
         see_also=["https://mozilla.atlassian.net/browse/JBI-234"],
         comment=comment,
     )
-    event = webhook_event_factory(target="comment", user=user)
-    webhook_payload = webhook_factory(bug=bug, event=event)
+    event = factories.webhook_event_factory(target="comment", user=user)
+    webhook_payload = factories.webhook_factory(bug=bug, event=event)
 
     return webhook_payload
 
 
 @pytest.fixture
 def webhook_private_comment_example() -> BugzillaWebhookRequest:
-    user = webhook_user_factory(login="[email protected]")
-    event = webhook_event_factory(target="comment", user=user)
-    bug = bug_factory(
+    user = factories.webhook_user_factory(login="[email protected]")
+    event = factories.webhook_event_factory(target="comment", user=user)
+    bug = factories.bug_factory(
         comment={"id": 344, "number": 2, "is_private": True},
         see_also=["https://mozilla.atlassian.net/browse/JBI-234"],
     )
-    webhook_payload = webhook_factory(bug=bug, event=event)
+    webhook_payload = factories.webhook_factory(bug=bug, event=event)
     return webhook_payload
 
 
 @pytest.fixture
 def webhook_create_private_example() -> BugzillaWebhookRequest:
-    return webhook_factory(
-        event=webhook_event_factory(),
+    return factories.webhook_factory(
+        event=factories.webhook_event_factory(),
         bug={"id": 654321, "is_private": True},
     )
 
@@ -212,23 +212,30 @@ def webhook_change_status_assignee():
             "added": "[email protected]",
         },
     ]
-    event = webhook_event_factory(routing_key="bug.modify", changes=changes)
-    webhook_payload = webhook_factory(event=event)
+    event = factories.webhook_event_factory(routing_key="bug.modify", changes=changes)
+    webhook_payload = factories.webhook_factory(event=event)
     return webhook_payload
 
 
 @pytest.fixture
 def webhook_modify_private_example() -> BugzillaWebhookRequest:
-    event = webhook_event_factory(action="modify", routing_key="bug.modify:status")
-    webhook_payload = webhook_factory(
+    event = factories.webhook_event_factory(
+        action="modify", routing_key="bug.modify:status"
+    )
+    webhook_payload = factories.webhook_factory(
         bug={"id": 654321, "is_private": True}, event=event
     )
     return webhook_payload
 
 
+@pytest.fixture
+def action_factory() -> Action:
+    return factories.action_factory
+
+
 @pytest.fixture
 def action_example() -> Action:
-    return action_factory()
+    return factories.action_factory()
 
 
 @pytest.fixture

tests/fixtures/factories.py

@@ -45,7 +45,7 @@ def bug_factory(**overrides):
         "status": "NEW",
         "summary": "JBI Test",
         "type": "defect",
-        "whiteboard": "devtest",
+        "whiteboard": "[devtest]",
         **overrides,
     }
     return BugzillaBug.parse_obj(bug)

tests/unit/test_bugzilla.py

@@ -1,6 +1,7 @@
 import pytest
 
 from jbi.errors import ActionNotFoundError
+from jbi.models import Actions
 from tests.fixtures.factories import bug_factory
 
 
@@ -53,18 +54,34 @@ def test_extract_see_also(see_also, expected):
     assert bug.extract_from_see_also() == expected
 
 
-def test_lookup_action(actions_example):
-    bug = bug_factory(id=1234, whiteboard="[example][DevTest]")
+@pytest.mark.parametrize(
+    "whiteboard",
+    [
+        "[example][DevTest]",
+        "[example][DevTest-test]",
+        "[example][test-DevTest]",
+        "[example][foo-DevTest-bar]",
+    ],
+)
+def test_lookup_action_found(whiteboard, actions_example):
+    bug = bug_factory(id=1234, whiteboard=whiteboard)
     action = bug.lookup_action(actions_example)
     assert action.whiteboard_tag == "devtest"
     assert "test config" in action.description
 
 
-def test_lookup_action_missing(actions_example):
-    bug = bug_factory(id=1234, whiteboard="example DevTest")
+@pytest.mark.parametrize(
+    "whiteboard",
+    [
+        "example DevTest",
+        "[foo] devtest [bar]",
+    ],
+)
+def test_lookup_action_not_found(whiteboard, actions_example):
+    bug = bug_factory(id=1234, whiteboard=whiteboard)
     with pytest.raises(ActionNotFoundError) as exc_info:
         bug.lookup_action(actions_example)
-    assert str(exc_info.value) == "example devtest"
+    assert str(exc_info.value) == "devtest"
 
 
 def test_payload_empty_changes_list(webhook_change_status_assignee):

tests/unit/test_router.py

@@ -125,7 +125,7 @@ def test_webhook_is_200_if_action_raises_IgnoreInvalidRequestError(
         assert response.status_code == 200
         assert (
             response.json()["error"]
-            == "no action matching bug whiteboard tags: unmatched"
+            == "no bug whiteboard matching action tags: devtest"
         )