Skip to content

Commit 165be91

Browse files
authored
fix(aci): Don't dedup actions by workflow (#109251)
We previously deduped by workflow id to preserve consistency with legacy behavior. However, conceptually, we don't want to send the same notification to the same destination for the same issue just because it came from different sources. Fixes ISWF-1946.
1 parent 1730e13 commit 165be91

File tree

4 files changed

+8
-13
lines changed

4 files changed

+8
-13
lines changed

src/sentry/workflow_engine/models/action.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,8 @@ def trigger(self, event_data: WorkflowEventData, notification_uuid: str) -> None
152152
},
153153
)
154154

155-
def get_dedup_key(self, workflow_id: int | None) -> str:
155+
def get_dedup_key(self) -> str:
156156
key_parts = [self.type]
157-
if workflow_id is not None:
158-
key_parts.append(str(workflow_id))
159-
160157
if self.integration_id:
161158
key_parts.append(str(self.integration_id))
162159

src/sentry/workflow_engine/processors/action.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,7 @@ def get_unique_active_actions(
231231
if action.status != ObjectStatus.ACTIVE:
232232
continue
233233

234-
# workflow_id is annotated in the queryset
235-
workflow_id = getattr(action, "workflow_id")
236-
dedup_key = action.get_dedup_key(workflow_id)
234+
dedup_key = action.get_dedup_key()
237235
previous_action_id = dedup_key_to_action_id.get(dedup_key)
238236
if previous_action_id is not None:
239237
dropped[dedup_key].add(previous_action_id)

tests/sentry/workflow_engine/processors/test_action_deduplication.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ def test_deduplicate_actions_single_action(self) -> None:
588588
assert result_ids[0] == single_action.id
589589

590590
def test_deduplicate_actions_same_actions_different_workflows(self) -> None:
591-
"""Test that identical actions from different workflows are NOT deduplicated."""
591+
"""Test that identical actions from different workflows are deduplicated."""
592592
# Create two identical Slack actions
593593
slack_action_1 = self.create_action(
594594
type=Action.Type.SLACK,
@@ -611,6 +611,8 @@ def test_deduplicate_actions_same_actions_different_workflows(self) -> None:
611611
)
612612

613613
# Annotate with different workflow IDs
614+
# We don't use this, but it is expected to be present,
615+
# so this mostly confirms we don't use it for deduplication.
614616
actions_queryset = Action.objects.filter(
615617
id__in=[slack_action_1.id, slack_action_2.id]
616618
).annotate(
@@ -623,8 +625,6 @@ def test_deduplicate_actions_same_actions_different_workflows(self) -> None:
623625

624626
result = get_unique_active_actions(actions_queryset, self.group)
625627

626-
# Both actions should remain since they're from different workflows
628+
# Only one action should remain; we don't take workflow into account for deduplication
627629
result_ids = list(result.values_list("id", flat=True))
628-
assert len(result_ids) == 2
629-
assert slack_action_1.id in result_ids
630-
assert slack_action_2.id in result_ids
630+
assert len(result_ids) == 1

tests/sentry/workflow_engine/processors/test_workflow.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ def test_workflow_fire_history_with_action_deduping(
427427
process_workflows(self.batch_client, self.event_data, FROZEN_TIME)
428428

429429
assert WorkflowFireHistory.objects.count() == 3
430-
assert mock_trigger_action.call_count == 3
430+
assert mock_trigger_action.call_count == 1
431431

432432
def test_uses_issue_stream_workflows(self) -> None:
433433
issue_occurrence = self.build_occurrence()

0 commit comments

Comments
 (0)