Skip to content

Run multiple actions when multiple whiteboard tags #1109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion jbi/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
73 changes: 55 additions & 18 deletions jbi/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()))

Expand Down Expand Up @@ -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(
Expand All @@ -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}"
Expand All @@ -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
)
Expand Down Expand Up @@ -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})"
)
Expand Down Expand Up @@ -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",
Expand All @@ -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
1 change: 1 addition & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def mocked_statsd():
yield _mocked_statsd


register(factories.ActionStepsFactory)
register(factories.ActionContextFactory)
register(factories.ActionFactory)
register(factories.ActionsFactory, "_actions")
Expand Down
6 changes: 6 additions & 0 deletions tests/fixtures/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
149 changes: 145 additions & 4 deletions tests/unit/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -15,7 +16,7 @@
Executor,
execute_action,
execute_or_queue,
lookup_action,
lookup_actions,
)


Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
[
Expand All @@ -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
Loading