🐛 fix(aci): dedupe actions based on workflow_id as well (#97784)

Author: iamrajjoshi

src/sentry/workflow_engine/models/action.py

@@ -110,8 +110,10 @@ def trigger(self, event_data: WorkflowEventData, detector: Detector) -> None:
             },
         )
 
-    def get_dedup_key(self) -> str:
+    def get_dedup_key(self, workflow_id: int | None) -> str:
         key_parts = [self.type]
+        if workflow_id is not None:
+            key_parts.append(str(workflow_id))
 
         if self.integration_id:
             key_parts.append(str(self.integration_id))

src/sentry/workflow_engine/processors/action.py

@@ -133,7 +133,9 @@ def deduplicate_actions(
     dedup_key_to_action_id: dict[str, int] = {}
 
     for action in actions_queryset:
-        dedup_key = action.get_dedup_key()
+        # workflow_id is annotated in the queryset
+        workflow_id = getattr(action, "workflow_id")
+        dedup_key = action.get_dedup_key(workflow_id)
         dedup_key_to_action_id[dedup_key] = action.id
 
     return actions_queryset.filter(id__in=dedup_key_to_action_id.values())

tests/sentry/workflow_engine/processors/test_action_deduplication.py

@@ -1,3 +1,6 @@
+from django.db import models
+from django.db.models import Value
+
 from sentry.notifications.models.notificationaction import ActionTarget
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import region_silo_test
@@ -61,7 +64,9 @@ def test_deduplicate_actions_different_types(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[self.slack_action.id, email_action.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[self.slack_action.id, email_action.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -84,7 +89,9 @@ def test_deduplicate_actions_same_slack_channels(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[slack_action_1.id, slack_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action_1.id, slack_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -111,7 +118,9 @@ def test_deduplicate_actions_different_slack_channels(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[slack_action_1.id, slack_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action_1.id, slack_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -135,7 +144,9 @@ def test_deduplicate_multiple_slack_actions_same_channel_different_name(self) ->
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[slack_action_1.id, slack_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action_1.id, slack_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -167,7 +178,9 @@ def test_deduplicate_actions_same_slack_different_data(self) -> None:
             data={"notes": "second action"},
         )
 
-        actions_queryset = Action.objects.filter(id__in=[slack_action_1.id, slack_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action_1.id, slack_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -202,7 +215,9 @@ def test_deduplicate_actions_different_slack_integrations(self) -> None:
             data={"notes": "second action"},
         )
 
-        actions_queryset = Action.objects.filter(id__in=[slack_action_1.id, slack_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action_1.id, slack_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -231,7 +246,9 @@ def test_deduplicate_actions_email_same_target(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[email_action_1.id, email_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[email_action_1.id, email_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -257,7 +274,9 @@ def test_deduplicate_actions_email_different_target_identifier(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[email_action_1.id, email_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[email_action_1.id, email_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -285,7 +304,9 @@ def test_deduplicate_actions_email_different_target_type(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[email_action_1.id, email_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[email_action_1.id, email_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -319,7 +340,9 @@ def test_deduplicate_actions_email_different_fallthrough_type(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[email_action_1.id, email_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[email_action_1.id, email_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -352,7 +375,9 @@ def test_deduplicate_actions_email_everything_is_same(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[email_action_1.id, email_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[email_action_1.id, email_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -383,7 +408,7 @@ def test_deduplicate_actions_sentry_app_same_identifier(self) -> None:
 
         actions_queryset = Action.objects.filter(
             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)
 
@@ -408,7 +433,9 @@ def test_deduplicate_actions_webhook_same_target_identifier(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[webhook_action_1.id, webhook_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[webhook_action_1.id, webhook_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -421,7 +448,9 @@ def test_deduplicate_actions_plugin_actions(self) -> None:
 
         plugin_action_2 = self.create_action(type=Action.Type.PLUGIN)
 
-        actions_queryset = Action.objects.filter(id__in=[plugin_action_1.id, plugin_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[plugin_action_1.id, plugin_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -451,7 +480,9 @@ def test_deduplicate_actions_mixed_types_integration_bucket(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[slack_action.id, pagerduty_action.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action.id, pagerduty_action.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -486,7 +517,9 @@ def test_deduplicate_actions_ticketing_actions(self) -> None:
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[jira_action_1.id, jira_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[jira_action_1.id, jira_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -520,7 +553,9 @@ def test_deduplicate_actions_ticketing_actions_same_integration_and_data(self) -
             },
         )
 
-        actions_queryset = Action.objects.filter(id__in=[jira_action_1.id, jira_action_2.id])
+        actions_queryset = Action.objects.filter(
+            id__in=[jira_action_1.id, jira_action_2.id]
+        ).annotate(workflow_id=Value(1, output_field=models.IntegerField()))
 
         result = deduplicate_actions(actions_queryset)
 
@@ -541,11 +576,55 @@ def test_deduplicate_actions_single_action(self) -> None:
         """Test deduplication with single action."""
         single_action = self.slack_action
 
-        actions_queryset = Action.objects.filter(id=single_action.id)
+        actions_queryset = Action.objects.filter(id=single_action.id).annotate(
+            workflow_id=Value(1, output_field=models.IntegerField())
+        )
 
         result = deduplicate_actions(actions_queryset)
 
         # Should return the single action
         result_ids = list(result.values_list("id", flat=True))
         assert len(result_ids) == 1
         assert result_ids[0] == single_action.id
+
+    def test_deduplicate_actions_same_actions_different_workflows(self) -> None:
+        """Test that identical actions from different workflows are NOT deduplicated."""
+        # Create two identical Slack actions
+        slack_action_1 = self.create_action(
+            type=Action.Type.SLACK,
+            integration_id=self.slack_integration.id,
+            config={
+                "target_type": ActionTarget.SPECIFIC,
+                "target_identifier": "channel-123",
+                "target_display": "Test Channel",
+            },
+        )
+
+        slack_action_2 = self.create_action(
+            type=Action.Type.SLACK,
+            integration_id=self.slack_integration.id,
+            config={
+                "target_type": ActionTarget.SPECIFIC,
+                "target_identifier": "channel-123",
+                "target_display": "Test Channel",
+            },
+        )
+
+        # Annotate with different workflow IDs
+        actions_queryset = Action.objects.filter(
+            id__in=[slack_action_1.id, slack_action_2.id]
+        ).annotate(
+            workflow_id=models.Case(
+                models.When(id=slack_action_1.id, then=models.Value(1)),
+                models.When(id=slack_action_2.id, then=models.Value(2)),
+                output_field=models.IntegerField(),
+            )
+        )
+
+        result = deduplicate_actions(actions_queryset)
+
+        # Both actions should remain since they're from different workflows
+        result_ids = list(result.values_list("id", flat=True))
+        assert len(result_ids) == 2
+        assert slack_action_1.id in result_ids
+        assert slack_action_2.id in result_ids

tests/sentry/workflow_engine/processors/test_workflow.py

@@ -302,9 +302,7 @@ def test_workflow_fire_history_with_action_deduping(
         process_workflows(self.event_data)
 
         assert WorkflowFireHistory.objects.count() == 3
-        assert (
-            mock_trigger_action.call_count == 1
-        )  # TODO: determine if this is ideal, the actions are duped but not in the same workflows
+        assert mock_trigger_action.call_count == 3
 
 
 @mock_redis_buffer()