♻️ ref(aci): only fire active actions

src/sentry/workflow_engine/models/action.py

@@ -4,7 +4,7 @@
 import logging
 from dataclasses import asdict
 from enum import StrEnum
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, ClassVar
 
 from django.db import models
 from django.db.models.signals import pre_save
@@ -16,6 +16,8 @@
 from sentry.db.models import DefaultFieldsModel, region_silo_model, sane_repr
 from sentry.db.models.fields.bounded import BoundedPositiveIntegerField
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
+from sentry.db.models.manager.base import BaseManager
+from sentry.db.models.manager.base_query_set import BaseQuerySet
 from sentry.utils import metrics
 from sentry.workflow_engine.models.json_config import JSONConfigBase
 from sentry.workflow_engine.registry import action_handler_registry
@@ -28,6 +30,15 @@
 logger = logging.getLogger(__name__)
 
 
+class ActionManager(BaseManager["Action"]):
+    def get_queryset(self) -> BaseQuerySet[Action]:
+        return (
+            super()
+            .get_queryset()
+            .exclude(status__in=(ObjectStatus.PENDING_DELETION, ObjectStatus.DELETION_IN_PROGRESS))
+        )
+
+
 @region_silo_model
 class Action(DefaultFieldsModel, JSONConfigBase):
     """
@@ -41,6 +52,9 @@ class Action(DefaultFieldsModel, JSONConfigBase):
     __relocation_scope__ = RelocationScope.Excluded
     __repr__ = sane_repr("id", "type")
 
+    objects: ClassVar[ActionManager] = ActionManager()
+    objects_for_deletion: ClassVar[BaseManager] = BaseManager()
+
     class Type(StrEnum):
         SLACK = "slack"
         MSTEAMS = "msteams"

src/sentry/workflow_engine/processors/action.py

@@ -123,16 +123,19 @@ def update_workflow_action_group_statuses(
     )
 
 
-def deduplicate_actions(
+def get_unique_active_actions(
     actions_queryset: BaseQuerySet[Action],  # decorated with the workflow_ids
 ) -> BaseQuerySet[Action]:
     """
-    Deduplicates actions based on their handler's dedup_key method.
-    Returns a de-duplicated queryset of actions.
+    Returns a queryset of unique active actions based on their handler's dedup_key method.
     """
     dedup_key_to_action_id: dict[str, int] = {}
 
     for action in actions_queryset:
+        # We only want to fire active actions
+        if action.status != ObjectStatus.ACTIVE:
+            continue
+
         # workflow_id is annotated in the queryset
         workflow_id = getattr(action, "workflow_id")
         dedup_key = action.get_dedup_key(workflow_id)
@@ -144,7 +147,8 @@ def deduplicate_actions(
 def fire_actions(
     actions: BaseQuerySet[Action], detector: Detector, event_data: WorkflowEventData
 ) -> None:
-    deduped_actions = deduplicate_actions(actions)
+    deduped_actions = get_unique_active_actions(actions)
+
     for action in deduped_actions:
         task_params = build_trigger_action_task_params(action, detector, event_data)
         trigger_action.apply_async(kwargs=task_params, headers={"sentry-propagate-traces": False})

src/sentry/workflow_engine/processors/workflow.py

@@ -465,6 +465,7 @@ def process_workflows(
     create_workflow_fire_histories(
         detector, actions, event_data, should_trigger_actions, is_delayed=False
     )
+
     fire_actions(actions, detector, event_data)
 
     return triggered_workflows

tests/sentry/workflow_engine/processors/test_action_deduplication.py

@@ -1,11 +1,12 @@
 from django.db import models
 from django.db.models import Value
 
+from sentry.constants import ObjectStatus
 from sentry.notifications.models.notificationaction import ActionTarget
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import region_silo_test
 from sentry.workflow_engine.models import Action
-from sentry.workflow_engine.processors.action import deduplicate_actions
+from sentry.workflow_engine.processors.action import get_unique_active_actions
 from sentry.workflow_engine.typings.notification_action import SentryAppIdentifier
 
 
@@ -68,14 +69,37 @@ def test_deduplicate_actions_different_types(self) -> None:
             id__in=[self.slack_action.id, email_action.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they're different types
         result_ids = list(result.values_list("id", flat=True))
         assert len(result_ids) == 2
         assert self.slack_action.id in result_ids
         assert email_action.id in result_ids
 
+    def test_deduplicate_actions_inactive_actions(self) -> None:
+        """Test that inactive actions are not deduplicated."""
+        email_action = self.create_action(
+            type=Action.Type.EMAIL,
+            config={
+                "target_type": ActionTarget.SPECIFIC,
+                "target_identifier": "test@example.com",
+            },
+            status=ObjectStatus.DISABLED,
+        )
+
+        actions_queryset = Action.objects.filter(
+            id__in=[self.slack_action.id, email_action.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
+
+        result = get_unique_active_actions(actions_queryset)
+
+        # Only one action should remain
+        # The inactive action should be filtered out
+        result_ids = list(result.values_list("id", flat=True))
+        assert len(result_ids) == 1
+        assert self.slack_action.id in result_ids
+
     def test_deduplicate_actions_same_slack_channels(self) -> None:
         """Test that Slack actions to the same channel are deduplicated."""
         slack_action_1 = self.slack_action
@@ -93,7 +117,7 @@ def test_deduplicate_actions_same_slack_channels(self) -> None:
             id__in=[slack_action_1.id, slack_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only one action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -122,7 +146,7 @@ def test_deduplicate_actions_different_slack_channels(self) -> None:
             id__in=[slack_action_1.id, slack_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they target different channels
         result_ids = list(result.values_list("id", flat=True))
@@ -148,7 +172,7 @@ def test_deduplicate_multiple_slack_actions_same_channel_different_name(self) ->
             id__in=[slack_action_1.id, slack_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only one action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -182,7 +206,7 @@ def test_deduplicate_actions_same_slack_different_data(self) -> None:
             id__in=[slack_action_1.id, slack_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they have different data
         result_ids = list(result.values_list("id", flat=True))
@@ -219,7 +243,7 @@ def test_deduplicate_actions_different_slack_integrations(self) -> None:
             id__in=[slack_action_1.id, slack_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they have different data
         result_ids = list(result.values_list("id", flat=True))
@@ -250,7 +274,7 @@ def test_deduplicate_actions_email_same_target(self) -> None:
             id__in=[email_action_1.id, email_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only one action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -278,7 +302,7 @@ def test_deduplicate_actions_email_different_target_identifier(self) -> None:
             id__in=[email_action_1.id, email_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they have different targets
         result_ids = list(result.values_list("id", flat=True))
@@ -308,7 +332,7 @@ def test_deduplicate_actions_email_different_target_type(self) -> None:
             id__in=[email_action_1.id, email_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they have different targets
         result_ids = list(result.values_list("id", flat=True))
@@ -344,7 +368,7 @@ def test_deduplicate_actions_email_different_fallthrough_type(self) -> None:
             id__in=[email_action_1.id, email_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they have different targets
         result_ids = list(result.values_list("id", flat=True))
@@ -379,7 +403,7 @@ def test_deduplicate_actions_email_everything_is_same(self) -> None:
             id__in=[email_action_1.id, email_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only one action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -410,7 +434,7 @@ def test_deduplicate_actions_sentry_app_same_identifier(self) -> None:
             id__in=[sentry_app_action_1.id, sentry_app_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only one action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -437,7 +461,7 @@ def test_deduplicate_actions_webhook_same_target_identifier(self) -> None:
             id__in=[webhook_action_1.id, webhook_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only one action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -452,7 +476,7 @@ def test_deduplicate_actions_plugin_actions(self) -> None:
             id__in=[plugin_action_1.id, plugin_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # One action should remain since its a plugin action
         result_ids = list(result.values_list("id", flat=True))
@@ -484,7 +508,7 @@ def test_deduplicate_actions_mixed_types_integration_bucket(self) -> None:
             id__in=[slack_action.id, pagerduty_action.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they're for different integrations
         result_ids = list(result.values_list("id", flat=True))
@@ -521,7 +545,7 @@ def test_deduplicate_actions_ticketing_actions(self) -> None:
             id__in=[jira_action_1.id, jira_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since ticketing actions are deduplicated by integration_id and dynamic form field data
         result_ids = list(result.values_list("id", flat=True))
@@ -557,7 +581,7 @@ def test_deduplicate_actions_ticketing_actions_same_integration_and_data(self) -
             id__in=[jira_action_1.id, jira_action_2.id]
         ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Only 1 action should remain
         result_ids = list(result.values_list("id", flat=True))
@@ -567,7 +591,7 @@ def test_deduplicate_actions_empty_queryset(self) -> None:
         """Test deduplication with empty queryset."""
         actions_queryset = Action.objects.none()
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Should return empty queryset
         assert list(result) == []
@@ -580,7 +604,7 @@ def test_deduplicate_actions_single_action(self) -> None:
             workflow_id=Value(1, output_field=models.IntegerField())
         )
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Should return the single action
         result_ids = list(result.values_list("id", flat=True))
@@ -621,7 +645,7 @@ def test_deduplicate_actions_same_actions_different_workflows(self) -> None:
             )
         )
 
-        result = deduplicate_actions(actions_queryset)
+        result = get_unique_active_actions(actions_queryset)
 
         # Both actions should remain since they're from different workflows
         result_ids = list(result.values_list("id", flat=True))