From a94fb60e0e876e9dbeadb3a3fd3ba56597f78aaf Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:47:05 +0000 Subject: [PATCH 1/3] ref(aci): Prefer workflow id when building alert links Co-Authored-By: Kyle Consalus --- .../discord/message_builder/issues.py | 2 +- .../integrations/messaging/message_builder.py | 2 +- .../slack/message_builder/issues.py | 23 ++++++++------ .../slack/message_builder/util.py | 2 +- .../notifications/notifications/rules.py | 2 +- src/sentry/notifications/utils/rules.py | 28 ++++++++++------- .../slack/notifications/test_issue_alert.py | 30 +++++++++---------- .../slack/test_message_builder.py | 8 +++-- .../sentry/notifications/utils/test_rules.py | 28 +++++++++++++++++ 9 files changed, 85 insertions(+), 40 deletions(-) create mode 100644 tests/sentry/notifications/utils/test_rules.py diff --git a/src/sentry/integrations/discord/message_builder/issues.py b/src/sentry/integrations/discord/message_builder/issues.py index 2066450115f6..452f963907ce 100644 --- a/src/sentry/integrations/discord/message_builder/issues.py +++ b/src/sentry/integrations/discord/message_builder/issues.py @@ -62,7 +62,7 @@ def build(self, notification_uuid: str | None = None) -> DiscordMessage: key: RuleIdType = "legacy_rule_id" if self.rules: rule_environment_id = self.rules[0].environment_id - key, rule_id = get_rule_or_workflow_id(self.rules[0]) + key, rule_id = get_rule_or_workflow_id(self.rules[0], prefer="workflow_id") url = None match key: diff --git a/src/sentry/integrations/messaging/message_builder.py b/src/sentry/integrations/messaging/message_builder.py index 292efde433d5..2d2fad7e921d 100644 --- a/src/sentry/integrations/messaging/message_builder.py +++ b/src/sentry/integrations/messaging/message_builder.py @@ -267,7 +267,7 @@ def build_footer( ) -> str: footer = f"{group.qualified_short_id}" if rules: - key, value = get_rule_or_workflow_id(rules[0]) + key, value = get_rule_or_workflow_id(rules[0], prefer="workflow_id") match key: case "workflow_id": rule_url = absolute_uri(create_link_to_workflow(group.organization.slug, value)) diff --git a/src/sentry/integrations/slack/message_builder/issues.py b/src/sentry/integrations/slack/message_builder/issues.py index 2f1ed2f40e68..140a19411dd6 100644 --- a/src/sentry/integrations/slack/message_builder/issues.py +++ b/src/sentry/integrations/slack/message_builder/issues.py @@ -57,7 +57,7 @@ dedupe_suggested_assignees, get_suspect_commit_users, ) -from sentry.notifications.utils.rules import get_rule_or_workflow_id +from sentry.notifications.utils.rules import RuleIdType, get_rule_or_workflow_id from sentry.seer.entrypoints.operator import SeerAutofixOperator from sentry.seer.entrypoints.types import SeerEntrypointKey from sentry.services.eventstore.models import Event, GroupEvent @@ -606,20 +606,25 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock: rule_id = None workflow_id = self.workflow_id rule_environment_id = None - key = "legacy_rule_id" + link_key: RuleIdType = "legacy_rule_id" + link_id = None if self.rules: - key, value = get_rule_or_workflow_id(self.rules[0]) + # The block id's "rule" is resolved back to a Rule by the Slack action + # handler, so it keeps preferring the legacy rule id. + _, value = get_rule_or_workflow_id(self.rules[0]) rule_id = int(value) action = self.rules[0].data.get("actions", [{}])[0] if action.get("workflow_id") is not None: workflow_id = int(action["workflow_id"]) - match key: + link_key, link_value = get_rule_or_workflow_id(self.rules[0], prefer="workflow_id") + link_id = int(link_value) + match link_key: case "workflow_id": - workflow = Workflow.objects.filter(id=rule_id).first() + workflow = Workflow.objects.filter(id=link_id).first() rule_environment_id = workflow.environment_id if workflow else None case "legacy_rule_id": - rule = Rule.objects.filter(id=rule_id).first() + rule = Rule.objects.filter(id=link_id).first() rule_environment_id = rule.environment_id if rule else None # build up actions text @@ -629,7 +634,7 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock: has_action = True title_link = None - match key: + match link_key: case "workflow_id": title_link = get_title_link_workflow_engine_ui( self.group, @@ -638,7 +643,7 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock: self.issue_details, self.notification, ExternalProviders.SLACK, - rule_id, + link_id, rule_environment_id, notification_uuid=notification_uuid, ) @@ -650,7 +655,7 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock: self.issue_details, self.notification, ExternalProviders.SLACK, - rule_id, + link_id, rule_environment_id, notification_uuid=notification_uuid, ) diff --git a/src/sentry/integrations/slack/message_builder/util.py b/src/sentry/integrations/slack/message_builder/util.py index adb5876e9420..2c302ecb4914 100644 --- a/src/sentry/integrations/slack/message_builder/util.py +++ b/src/sentry/integrations/slack/message_builder/util.py @@ -18,7 +18,7 @@ def build_slack_footer( footer = f"{group.qualified_short_id}" if rules: - key, value = get_rule_or_workflow_id(rules[0]) + key, value = get_rule_or_workflow_id(rules[0], prefer="workflow_id") match key: case "workflow_id": rule_url = absolute_uri(create_link_to_workflow(group.organization.slug, value)) diff --git a/src/sentry/notifications/notifications/rules.py b/src/sentry/notifications/notifications/rules.py index 188e1c09af12..5606c67321a1 100644 --- a/src/sentry/notifications/notifications/rules.py +++ b/src/sentry/notifications/notifications/rules.py @@ -295,7 +295,7 @@ def get_notification_title( title_str = "Alert triggered" if self.rules: - key, value = get_rule_or_workflow_id(self.rules[0]) + key, value = get_rule_or_workflow_id(self.rules[0], prefer="workflow_id") match key: case "workflow_id": diff --git a/src/sentry/notifications/utils/rules.py b/src/sentry/notifications/utils/rules.py index 05b3dfa12784..39de8a922488 100644 --- a/src/sentry/notifications/utils/rules.py +++ b/src/sentry/notifications/utils/rules.py @@ -32,13 +32,21 @@ def split_rules_by_rule_workflow_id(rules: Sequence[Rule]) -> RulesAndWorkflows: return RulesAndWorkflows(rules=parsed_rules, workflow_rules=workflow_rules) -def get_rule_or_workflow_id(rule: Rule) -> tuple[RuleIdType, str]: - try: - return ("legacy_rule_id", get_key_from_rule_data(rule, "legacy_rule_id")) - except AssertionError: - pass - - try: - return ("workflow_id", get_key_from_rule_data(rule, "workflow_id")) - except AssertionError: - return ("legacy_rule_id", str(rule.id)) +def get_rule_or_workflow_id( + rule: Rule, *, prefer: RuleIdType = "legacy_rule_id" +) -> tuple[RuleIdType, str]: + """ + Returns which id the rule data carries, and its value. When both a legacy + rule id and a workflow id are present, `prefer` decides which one wins. + """ + keys: tuple[RuleIdType, RuleIdType] = ( + ("workflow_id", "legacy_rule_id") + if prefer == "workflow_id" + else ("legacy_rule_id", "workflow_id") + ) + for key in keys: + try: + return (key, get_key_from_rule_data(rule, key)) + except AssertionError: + pass + return ("legacy_rule_id", str(rule.id)) diff --git a/tests/sentry/integrations/slack/notifications/test_issue_alert.py b/tests/sentry/integrations/slack/notifications/test_issue_alert.py index e269cd068b4d..135b48e6d1ea 100644 --- a/tests/sentry/integrations/slack/notifications/test_issue_alert.py +++ b/tests/sentry/integrations/slack/notifications/test_issue_alert.py @@ -81,13 +81,13 @@ def test_issue_alert_user_block(self) -> None: fallback_text = self.mock_post.call_args.kwargs["text"] assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text assert event.group assert ( blocks[1]["text"]["text"] - == f":red_circle: " + == f":red_circle: " ) assert ( blocks[4]["elements"][0]["text"] @@ -119,7 +119,7 @@ def test_performance_issue_alert_user_block(self, occurrence) -> None: fallback_text = self.mock_post.call_args.kwargs["text"] assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text self.assert_performance_issue_blocks_with_culprit_blocks( @@ -129,7 +129,7 @@ def test_performance_issue_alert_user_block(self, occurrence) -> None: event.group, "issue_alert-slack", alert_type=FineTuningAPIKey.ALERTS, - issue_link_extra_params=f"&alert_rule_id={self.rule.id}&alert_type=issue", + issue_link_extra_params=f"&workflow_id={self.rule.data['actions'][0]['workflow_id']}&alert_type=issue", ) @mock.patch("sentry.integrations.slack.message_builder.issues.get_tags", new=fake_get_tags) @@ -172,7 +172,7 @@ def test_crons_issue_alert_user_block(self) -> None: fallback_text = self.mock_post.call_args.kwargs["text"] assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert len(blocks) == 4 @@ -201,7 +201,7 @@ def test_generic_issue_alert_user_block(self, occurrence: MagicMock) -> None: fallback_text = self.mock_post.call_args.kwargs["text"] assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text @@ -212,7 +212,7 @@ def test_generic_issue_alert_user_block(self, occurrence: MagicMock) -> None: event.group, "issue_alert-slack", alert_type="alerts", - issue_link_extra_params=f"&alert_rule_id={self.rule.id}&alert_type=issue", + issue_link_extra_params=f"&workflow_id={self.rule.data['actions'][0]['workflow_id']}&alert_type=issue", ) @patch( @@ -315,13 +315,13 @@ def test_issue_alert_issue_owners_block(self) -> None: notification_uuid = notification.notification_uuid assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text assert event.group assert ( blocks[1]["text"]["text"] - == f":red_circle: " + == f":red_circle: " ) assert ( blocks[4]["elements"][0]["text"] @@ -349,13 +349,13 @@ def _assert_issue_owners_env_block(self, rule: Rule, environment: Environment) - notification_uuid = notification.notification_uuid assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text assert event.group assert ( blocks[1]["text"]["text"] - == f":red_circle: " + == f":red_circle: " ) assert ( blocks[4]["elements"][0]["text"] @@ -485,13 +485,13 @@ def test_issue_alert_team_issue_owners_block(self) -> None: assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text assert event.group assert ( blocks[1]["text"]["text"] - == f":red_circle: " + == f":red_circle: " ) # Verify suggested assignees are in the context block and footer is last context_blocks = [b for b in blocks if b.get("type") == "context"] @@ -721,13 +721,13 @@ def test_issue_alert_team_block(self) -> None: assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text assert event.group assert ( blocks[1]["text"]["text"] - == f":red_circle: " + == f":red_circle: " ) assert ( blocks[5]["elements"][0]["text"] diff --git a/tests/sentry/integrations/slack/test_message_builder.py b/tests/sentry/integrations/slack/test_message_builder.py index 964736cdec88..00c80ca60577 100644 --- a/tests/sentry/integrations/slack/test_message_builder.py +++ b/tests/sentry/integrations/slack/test_message_builder.py @@ -83,7 +83,9 @@ def build_test_message_blocks( title_link += f"/events/{event.event_id}" title_link += "/?referrer=slack" if rule: - if legacy_rule_id: + if workflow_id: + title_link += f"&workflow_id={workflow_id}&alert_type=issue" + elif legacy_rule_id: title_link += f"&alert_rule_id={legacy_rule_id}&alert_type=issue" else: title_link += f"&alert_rule_id={rule.id}&alert_type=issue" @@ -216,7 +218,9 @@ def build_test_message_blocks( blocks.append(notes_section) if rule: - if legacy_rule_id: + if workflow_id: + context_text = f"Project: Alert: Short ID: {group.qualified_short_id}" + elif legacy_rule_id: context_text = f"Project: Alert: Short ID: {group.qualified_short_id}" else: context_text = f"Project: Alert: Short ID: {group.qualified_short_id}" diff --git a/tests/sentry/notifications/utils/test_rules.py b/tests/sentry/notifications/utils/test_rules.py new file mode 100644 index 000000000000..f85f26d03136 --- /dev/null +++ b/tests/sentry/notifications/utils/test_rules.py @@ -0,0 +1,28 @@ +from sentry.models.rule import Rule +from sentry.notifications.utils.rules import get_rule_or_workflow_id + + +def _rule(action: dict[str, str]) -> Rule: + return Rule(id=99, data={"actions": [action]}) + + +def test_get_rule_or_workflow_id_prefers_legacy_rule_id_by_default() -> None: + rule = _rule({"legacy_rule_id": "1", "workflow_id": "2"}) + assert get_rule_or_workflow_id(rule) == ("legacy_rule_id", "1") + + +def test_get_rule_or_workflow_id_prefer_workflow() -> None: + rule = _rule({"legacy_rule_id": "1", "workflow_id": "2"}) + assert get_rule_or_workflow_id(rule, prefer="workflow_id") == ("workflow_id", "2") + + +def test_get_rule_or_workflow_id_falls_back_to_available_id() -> None: + assert get_rule_or_workflow_id(_rule({"legacy_rule_id": "1"}), prefer="workflow_id") == ( + "legacy_rule_id", + "1", + ) + assert get_rule_or_workflow_id(_rule({"workflow_id": "2"})) == ("workflow_id", "2") + + +def test_get_rule_or_workflow_id_falls_back_to_rule_id() -> None: + assert get_rule_or_workflow_id(_rule({}), prefer="workflow_id") == ("legacy_rule_id", "99") From 7ceadb9b8f0476550414c57516e9e4c8c732999f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:59:59 +0000 Subject: [PATCH 2/3] test: Update MS Teams alert title expectations for workflow links Co-Authored-By: Kyle Consalus --- .../integrations/msteams/notifications/test_issue_alert.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/sentry/integrations/msteams/notifications/test_issue_alert.py b/tests/sentry/integrations/msteams/notifications/test_issue_alert.py index 9405c52aa879..ddd8bf40d194 100644 --- a/tests/sentry/integrations/msteams/notifications/test_issue_alert.py +++ b/tests/sentry/integrations/msteams/notifications/test_issue_alert.py @@ -54,7 +54,7 @@ def test_issue_alert_user(self, mock_send_card: MagicMock) -> None: assert 4 == len(body) assert ( - f"Alert triggered [{rule.label}](http://testserver/organizations/baz/issues/alerts/rules/bar/{rule.id}/details/)" + f"Alert triggered [{rule.label}](http://testserver/organizations/baz/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/)" == body[0]["text"] ) assert ( @@ -104,7 +104,7 @@ def test_issue_alert_owners(self, mock_send_card: MagicMock) -> None: assert 4 == len(body) assert ( - f"Alert triggered [{rule.label}](http://testserver/organizations/baz/issues/alerts/rules/bar/{rule.id}/details/)" + f"Alert triggered [{rule.label}](http://testserver/organizations/baz/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/)" == body[0]["text"] ) assert ( From a7e771d69841e1fa21b16b23624ce7317e545a11 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:10:09 +0000 Subject: [PATCH 3/3] test: Update Slack notify action expectations for workflow links --- .../notification/test_slack_notify_service_action.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py b/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py index adbb5c35710b..fed5f6d9e6d9 100644 --- a/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py +++ b/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py @@ -216,7 +216,7 @@ def test_after_noa_test_action( assert ( blocks[0]["text"]["text"] - == f":large_yellow_circle: " + == f":large_yellow_circle: " ) # Test action should not create a notification message @@ -298,7 +298,7 @@ def test_after_with_threads_noa( assert ( blocks[0]["text"]["text"] - == f":large_yellow_circle: " + == f":large_yellow_circle: " ) assert NotificationMessage.objects.all().count() == 1 @@ -355,7 +355,7 @@ def test_after_reply_in_thread_noa( assert ( blocks[0]["text"]["text"] - == f":large_yellow_circle: " + == f":large_yellow_circle: " ) assert NotificationMessage.objects.all().count() == 2