populate legacy_rule_id and/or workflow_id in action dispatch

cathteng · cathteng · commit d649b5b62d12 · 2026-01-09T17:22:09.000-08:00
diff --git a/src/sentry/notifications/notification_action/types.py b/src/sentry/notifications/notification_action/types.py
@@ -6,7 +6,6 @@
 
 from django.core.exceptions import ValidationError
 
-from sentry import features
 from sentry.constants import ObjectStatus
 from sentry.exceptions import InvalidIdentity
 from sentry.incidents.grouptype import MetricIssueEvidenceData
@@ -193,49 +192,39 @@ def create_rule_instance_from_action(
         workflow_id = getattr(action, "workflow_id", None)
 
         label = detector.name
-        # We need to pass the legacy rule id when the workflow-engine-ui-links feature flag is disabled
-        # This is so we can build the old link to the rule
-        if not features.has(
-            "organizations:workflow-engine-ui-links", detector.project.organization
-        ):
-            if workflow_id is None:
-                raise ValueError("Workflow ID is required when triggering an action")
-
-            # If test event, just set the legacy rule id to -1
-            if workflow_id == TEST_NOTIFICATION_ID:
-                data["actions"][0]["legacy_rule_id"] = TEST_NOTIFICATION_ID
-
-            else:
-                try:
-                    alert_rule_workflow = AlertRuleWorkflow.objects.get(
-                        workflow_id=workflow_id,
-                    )
-                except AlertRuleWorkflow.DoesNotExist:
-                    raise ValueError(
-                        "AlertRuleWorkflow not found when querying for AlertRuleWorkflow"
-                    )
-
-                if alert_rule_workflow.rule_id is None:
-                    raise ValueError("Rule not found when querying for AlertRuleWorkflow")
-
-                data["actions"][0]["legacy_rule_id"] = alert_rule_workflow.rule_id
-
-                # Get the legacy rule label
-                try:
-                    rule = Rule.objects.get(id=alert_rule_workflow.rule_id)
-                    label = rule.label
-                except Rule.DoesNotExist:
-                    logger.exception(
-                        "Rule not found when querying for AlertRuleWorkflow",
-                        extra={"rule_id": alert_rule_workflow.rule_id},
-                    )
-                    # We shouldn't fail badly here since we can still send the notification, so just set it to the rule id
-                    label = f"Rule {alert_rule_workflow.rule_id}"
-
-        # In the new UI, we need this for to build the link to the new rule in the notification action
+        # Build link to the rule if it exists, otherwise build link to the workflow.
+        # FE will handle redirection if necessary from rule -> workflow
+
+        # If test event, just set the legacy rule id to -1
+        if workflow_id == TEST_NOTIFICATION_ID:
+            data["actions"][0]["legacy_rule_id"] = TEST_NOTIFICATION_ID
         else:
             data["actions"][0]["workflow_id"] = workflow_id
 
+            # attempt to find legacy_rule_id from the alert rule workflow
+            rule_id = None
+            try:
+                alert_rule_workflow = AlertRuleWorkflow.objects.get(
+                    workflow_id=workflow_id,
+                )
+
+                rule = Rule.objects.get(id=alert_rule_workflow.rule_id)
+                label = rule.label
+                rule_id = rule.id
+            except AlertRuleWorkflow.DoesNotExist:
+                pass
+            except Rule.DoesNotExist:
+                logger.exception(
+                    "Rule not found when querying for AlertRuleWorkflow",
+                    extra={"rule_id": alert_rule_workflow.rule_id},
+                )
+
+            if rule_id:
+                data["actions"][0]["legacy_rule_id"] = rule_id
+
+            if workflow_id is None and rule_id is None:
+                raise ValueError("Workflow ID or rule ID is required to fire notification")
+
         if workflow_id == TEST_NOTIFICATION_ID and action.type == Action.Type.EMAIL:
             # mail action needs to have skipDigests set to True
             data["actions"][0]["skipDigests"] = True
diff --git a/tests/sentry/integrations/opsgenie/test_client.py b/tests/sentry/integrations/opsgenie/test_client.py
@@ -15,7 +15,6 @@
     assert_slo_metric,
 )
 from sentry.testutils.cases import APITestCase
-from sentry.testutils.helpers import with_feature
 from sentry.testutils.helpers.options import override_options
 from sentry.testutils.skips import requires_snuba
 
@@ -187,7 +186,6 @@ def test_send_notification_with_workflow_engine_trigger_actions(
 
     @responses.activate
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
-    @with_feature("organizations:workflow-engine-ui-links")
     def test_send_notification_with_workflow_engine_ui_links(self, mock_record: MagicMock) -> None:
         resp_data = {
             "result": "Request will be processed",
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
@@ -14,7 +14,6 @@
 from sentry.silo.base import SiloMode
 from sentry.testutils.asserts import assert_failure_metric
 from sentry.testutils.cases import RuleTestCase
-from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.options import override_options
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.types.rules import RuleFuture
@@ -372,7 +371,6 @@ def test_after_noa_test_action(
         assert thread_ts_success.args[0] == EventLifecycleOutcome.SUCCESS
 
     @override_options({"workflow_engine.issue_alert.group.type_id.ga": [1]})
-    @with_feature("organizations:workflow-engine-ui-links")
     @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     @patch("sentry.integrations.slack.sdk_client.SlackSdkClient.chat_postMessage")
     @patch("slack_sdk.web.client.WebClient._perform_urllib_http_request")
diff --git a/tests/sentry/integrations/slack/notifications/test_issue_alert.py b/tests/sentry/integrations/slack/notifications/test_issue_alert.py
@@ -30,7 +30,6 @@
 from sentry.silo.base import SiloMode
 from sentry.tasks.digests import deliver_digest
 from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest
-from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE, TEST_PERF_ISSUE_OCCURRENCE
 from sentry.testutils.helpers.options import override_options
 from sentry.testutils.silo import assume_test_silo_mode
@@ -276,7 +275,6 @@ def test_generic_issue_alert_user_block_workflow_engine_dual_write(
         return_value=TEST_ISSUE_OCCURRENCE,
         new_callable=mock.PropertyMock,
     )
-    @with_feature("organizations:workflow-engine-ui-links")
     def test_generic_issue_alert_user_block_workflow_engine_ui_links(
         self, occurrence: MagicMock
     ) -> None:
diff --git a/tests/sentry/mail/test_actions.py b/tests/sentry/mail/test_actions.py
@@ -170,7 +170,6 @@ def setUp(self):
         )
 
     @with_feature("organizations:workflow-engine-single-process-workflows")
-    @with_feature("organizations:workflow-engine-ui-links")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_full_integration(self) -> None:
         action_config = {
@@ -197,7 +196,6 @@ def test_full_integration(self) -> None:
         assert sent.to == [self.user.email]
         assert "uh oh" in sent.subject
 
-    @with_feature("organizations:workflow-engine-ui-links")
     @with_feature("organizations:workflow-engine-single-process-workflows")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_full_integration_all_members_fallthrough(self) -> None:
@@ -223,7 +221,6 @@ def test_full_integration_all_members_fallthrough(self) -> None:
         assert sent.to == [self.user.email]
         assert "uh oh" in sent.subject
 
-    @with_feature("organizations:workflow-engine-ui-links")
     @with_feature("organizations:workflow-engine-single-process-workflows")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_full_integration_noone_fallthrough(self) -> None:
@@ -246,7 +243,6 @@ def test_full_integration_noone_fallthrough(self) -> None:
 
         assert len(mail.outbox) == 0
 
-    @with_feature("organizations:workflow-engine-ui-links")
     @with_feature("organizations:workflow-engine-single-process-workflows")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_full_integration_fallthrough_not_provided(self) -> None:
@@ -270,7 +266,6 @@ def test_full_integration_fallthrough_not_provided(self) -> None:
         assert sent.to == [self.user.email]
         assert "uh oh" in sent.subject
 
-    @with_feature("organizations:workflow-engine-ui-links")
     @with_feature("organizations:workflow-engine-single-process-workflows")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_hack_mail_workflow(self) -> None:
diff --git a/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py b/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py
@@ -34,7 +34,6 @@
     JIRA_SERVER_ACTION_DATA_BLOBS,
     WEBHOOK_ACTION_DATA_BLOBS,
 )
-from sentry.testutils.helpers.features import with_feature
 from sentry.workflow_engine.models import Action
 from sentry.workflow_engine.types import ActionInvocation, WorkflowEventData
 from sentry.workflow_engine.typings.notification_action import (
@@ -113,26 +112,12 @@ def get_additional_fields(cls, action: Action, mapping: ActionFieldMapping):
         with pytest.raises(ValueError):
             handler.create_rule_instance_from_action(self.action, self.detector, self.event_data)
 
-    def test_create_rule_instance_from_action_missing_workflow_id_raises_value_error(self) -> None:
-        job = WorkflowEventData(
-            event=self.group_event, workflow_env=self.environment, group=self.group
-        )
-        action = self.create_action(
-            type=Action.Type.DISCORD,
-            integration_id="1234567890",
-            config={"target_identifier": "channel456", "target_type": ActionTarget.SPECIFIC},
-            data={"tags": "environment,user,my_tag"},
-        )
-
-        with pytest.raises(ValueError):
-            self.handler.create_rule_instance_from_action(action, self.detector, job)
-
-    def test_create_rule_instance_from_action_missing_rule_raises_value_error(self) -> None:
+    def test_create_rule_instance_from_action_missing_rule_workflow_id_raises_value_error(
+        self,
+    ) -> None:
         job = WorkflowEventData(
             event=self.group_event, workflow_env=self.environment, group=self.group
         )
-        alert_rule = self.create_alert_rule(projects=[self.project], organization=self.organization)
-        self.create_alert_rule_workflow(workflow=self.workflow, alert_rule_id=alert_rule.id)
         action = self.create_action(
             type=Action.Type.DISCORD,
             integration_id="1234567890",
@@ -164,15 +149,16 @@ def test_create_rule_instance_from_action(self) -> None:
                     "channel_id": "channel456",
                     "tags": "environment,user,my_tag",
                     "legacy_rule_id": self.rule.id,
+                    "workflow_id": self.workflow.id,
                 }
             ],
         }
         assert rule.status == ObjectStatus.ACTIVE
         assert rule.source == RuleSource.ISSUE
 
-    @with_feature("organizations:workflow-engine-ui-links")
-    def test_create_rule_instance_from_action_with_workflow_engine_ui_feature_flag(self) -> None:
+    def test_create_rule_instance_from_action_with_workflow_only(self) -> None:
         """Test that create_rule_instance_from_action creates a Rule with correct attributes"""
+        self.rule.delete()
         rule = self.handler.create_rule_instance_from_action(
             self.action, self.detector, self.event_data
         )
@@ -217,36 +203,9 @@ def test_create_rule_instance_from_action_no_environment(self) -> None:
                     "channel_id": "channel456",
                     "tags": "environment,user,my_tag",
                     "legacy_rule_id": self.rule.id,
-                }
-            ],
-        }
-        assert rule.status == ObjectStatus.ACTIVE
-        assert rule.source == RuleSource.ISSUE
-
-    @with_feature("organizations:workflow-engine-ui-links")
-    def test_create_rule_instance_from_action_no_environment_with_workflow_engine_ui_feature_flag(
-        self,
-    ):
-        """Test that create_rule_instance_from_action creates a Rule with correct attributes"""
-        self.create_workflow()
-        job = WorkflowEventData(event=self.group_event, workflow_env=None, group=self.group)
-        rule = self.handler.create_rule_instance_from_action(self.action, self.detector, job)
-
-        assert isinstance(rule, Rule)
-        assert rule.id == self.action.id
-        assert rule.project == self.detector.project
-        assert rule.environment_id is None
-        assert rule.label == self.detector.name
-        assert rule.data == {
-            "actions": [
-                {
-                    "id": "sentry.integrations.discord.notify_action.DiscordNotifyServiceAction",
-                    "server": "1234567890",
-                    "channel_id": "channel456",
-                    "tags": "environment,user,my_tag",
                     "workflow_id": self.workflow.id,
                 }
-            ]
+            ],
         }
         assert rule.status == ObjectStatus.ACTIVE
         assert rule.source == RuleSource.ISSUE
diff --git a/tests/sentry/rules/actions/test_notify_event_service.py b/tests/sentry/rules/actions/test_notify_event_service.py
@@ -105,7 +105,6 @@ def test_applies_correctly_for_legacy_webhooks(self) -> None:
 
     @responses.activate
     @with_feature("organizations:workflow-engine-single-process-workflows")
-    @with_feature("organizations:workflow-engine-ui-links")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_applies_correctly_for_legacy_webhooks_aci(self):
         responses.add(responses.POST, "http://my-fake-webhook.io")
@@ -163,7 +162,6 @@ def test_applies_correctly_for_legacy_webhooks_aci(self):
 
     @responses.activate
     @with_feature("organizations:workflow-engine-single-process-workflows")
-    @with_feature("organizations:workflow-engine-ui-links")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     def test_legacy_webhooks_uneven_dual_write_aci(self):
         """
@@ -203,7 +201,6 @@ def test_legacy_webhooks_uneven_dual_write_aci(self):
 
     @responses.activate
     @with_feature("organizations:workflow-engine-single-process-workflows")
-    @with_feature("organizations:workflow-engine-ui-links")
     @override_options({"workflow_engine.issue_alert.group.type_id.rollout": [1]})
     @patch("sentry.plugins.sentry_webhooks.plugin.WebHooksPlugin.notify_users")
     def test_error_for_legacy_webhooks_dual_write_aci(self, mock_notify_users):
