fix(aci): Prefer workflow id when building alert links

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:

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))

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,
                 )

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))

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":

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))

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 (

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: <http://testserver/organizations/{self.organization.slug}/issues/{self.event.group.id}/?referrer=slack&alert_rule_id={rule.data['actions'][0]['legacy_rule_id']}&alert_type=issue|*Hello world*>"
+            == f":large_yellow_circle: <http://testserver/organizations/{self.organization.slug}/issues/{self.event.group.id}/?referrer=slack&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
 
         # 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: <http://testserver/organizations/{self.organization.slug}/issues/{self.event.group.id}/?referrer=slack&alert_rule_id={rule.data['actions'][0]['legacy_rule_id']}&alert_type=issue|*Hello world*>"
+            == f":large_yellow_circle: <http://testserver/organizations/{self.organization.slug}/issues/{self.event.group.id}/?referrer=slack&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
 
         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: <http://testserver/organizations/{self.organization.slug}/issues/{self.event.group.id}/?referrer=slack&alert_rule_id={rule.data['actions'][0]['legacy_rule_id']}&alert_type=issue|*Hello world*>"
+            == f":large_yellow_circle: <http://testserver/organizations/{self.organization.slug}/issues/{self.event.group.id}/?referrer=slack&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
 
         assert NotificationMessage.objects.all().count() == 2

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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{self.rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{self.rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         assert blocks[0]["text"]["text"] == fallback_text
         assert event.group
         assert (
             blocks[1]["text"]["text"]
-            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&alert_rule_id={self.rule.id}&alert_type=issue|*Hello world*>"
+            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&workflow_id={self.rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
         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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{self.rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{self.rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{self.rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{self.rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{self.rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{self.rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         assert blocks[0]["text"]["text"] == fallback_text
         assert event.group
         assert (
             blocks[1]["text"]["text"]
-            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&alert_rule_id={rule.id}&alert_type=issue|*Hello world*>"
+            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
         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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         assert blocks[0]["text"]["text"] == fallback_text
         assert event.group
         assert (
             blocks[1]["text"]["text"]
-            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&environment=production&alert_rule_id={rule.id}&alert_type=issue|*Hello world*>"
+            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&environment=production&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
         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 <http://testserver/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://testserver/organizations/{event.organization.slug}/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         assert blocks[0]["text"]["text"] == fallback_text
         assert event.group
         assert (
             blocks[1]["text"]["text"]
-            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&alert_rule_id={rule.id}&alert_type=issue|*Hello world*>"
+            == f":red_circle: <http://testserver/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
         # 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 <http://example.com/organizations/{event.organization.slug}/issues/alerts/rules/{event.project.slug}/{rule.id}/details/|ja rule>"
+            == f"Alert triggered <http://example.com/organizations/{event.organization.slug}/monitors/alerts/{rule.data['actions'][0]['workflow_id']}/|ja rule>"
         )
         assert blocks[0]["text"]["text"] == fallback_text
         assert event.group
         assert (
             blocks[1]["text"]["text"]
-            == f":red_circle: <http://example.com/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&alert_rule_id={rule.id}&alert_type=issue|*Hello world*>"
+            == f":red_circle: <http://example.com/organizations/{event.organization.slug}/issues/{event.group.id}/?referrer=issue_alert-slack&notification_uuid={notification_uuid}&workflow_id={rule.data['actions'][0]['workflow_id']}&alert_type=issue|*Hello world*>"
         )
         assert (
             blocks[5]["elements"][0]["text"]

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: <http://testserver/organizations/{project.organization.slug}/issues/?project={project.id}|{project.slug}>    Alert: <http://testserver/organizations/{project.organization.slug}/monitors/alerts/{workflow_id}/|{rule.label}>    Short ID: {group.qualified_short_id}"
+        elif legacy_rule_id:
             context_text = f"Project: <http://testserver/organizations/{project.organization.slug}/issues/?project={project.id}|{project.slug}>    Alert: <http://testserver/organizations/{project.organization.slug}/issues/alerts/rules/bar/{legacy_rule_id}/details/|{rule.label}>    Short ID: {group.qualified_short_id}"
         else:
             context_text = f"Project: <http://testserver/organizations/{project.organization.slug}/issues/?project={project.id}|{project.slug}>    Alert: <http://testserver/organizations/{project.organization.slug}/issues/alerts/rules/bar/{rule.id}/details/|{rule.label}>    Short ID: {group.qualified_short_id}"