Skip to content

Commit f0a4296

Browse files
authored
fix(aci): Limit lastTriggered to single-written history (#96792)
Dual-written WorkflowActionHistory records are non-canonical, and shouldn't be included in results.
1 parent 1876d75 commit f0a4296

File tree

4 files changed

+83
-4
lines changed

4 files changed

+83
-4
lines changed

src/sentry/workflow_engine/endpoints/organization_workflow_index.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,13 @@ def get(self, request, organization):
160160
# the annotated value isn't returned in the results.
161161
long_ago = ensure_aware(datetime(1970, 1, 1))
162162
queryset = queryset.annotate(
163-
last_triggered=Max(Coalesce("workflowfirehistory__date_added", long_ago)),
163+
last_triggered=Coalesce(
164+
Max(
165+
"workflowfirehistory__date_added",
166+
filter=Q(workflowfirehistory__is_single_written=True),
167+
),
168+
long_ago,
169+
),
164170
)
165171

166172
queryset = queryset.order_by(*sort_by.db_order_by)

src/sentry/workflow_engine/endpoints/serializers.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ def get_attrs(
402402
last_triggered_map: dict[int, datetime] = dict(
403403
WorkflowFireHistory.objects.filter(
404404
workflow__in=item_list,
405+
is_single_written=True,
405406
)
406407
.values("workflow_id")
407408
.annotate(last_triggered=Max("date_added"))

tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@
66
from sentry.notifications.models.notificationaction import ActionTarget
77
from sentry.testutils.cases import APITestCase
88
from sentry.testutils.silo import region_silo_test
9-
from sentry.workflow_engine.models import Action, Workflow, WorkflowDataConditionGroup
10-
from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow
11-
from sentry.workflow_engine.models.workflow_fire_history import WorkflowFireHistory
9+
from sentry.workflow_engine.models import (
10+
Action,
11+
DetectorWorkflow,
12+
Workflow,
13+
WorkflowDataConditionGroup,
14+
WorkflowFireHistory,
15+
)
1216

1317

1418
class OrganizationWorkflowAPITestCase(APITestCase):
@@ -40,6 +44,7 @@ def setUp(self) -> None:
4044
workflow=workflow,
4145
group=self.group,
4246
event_id=self.event.event_id,
47+
is_single_written=True,
4348
)
4449

4550
def test_simple(self) -> None:
@@ -190,6 +195,13 @@ def test_sort_by_actions(self) -> None:
190195
][0]
191196

192197
def test_sort_by_last_triggered(self) -> None:
198+
# Fresh fire for self.workflow that would impact sorting if not ignored.
199+
WorkflowFireHistory.objects.create(
200+
workflow=self.workflow,
201+
group=self.group,
202+
event_id=self.event.event_id,
203+
is_single_written=False,
204+
)
193205
response = self.get_success_response(
194206
self.organization.slug, qs_params={"sortBy": "lastTriggered"}
195207
)
@@ -325,6 +337,53 @@ def test_query_filter_by_action(self) -> None:
325337
assert len(response3.data) == 2
326338
assert {self.workflow.name, self.workflow_two.name} == {w["name"] for w in response3.data}
327339

340+
def test_sort_by_last_triggered_with_non_single_written_only(self) -> None:
341+
workflow_never_fired = self.create_workflow(
342+
organization_id=self.organization.id, name="Never Fired"
343+
)
344+
345+
workflow_non_single_written_only = self.create_workflow(
346+
organization_id=self.organization.id, name="Non Single Written Only"
347+
)
348+
WorkflowFireHistory.objects.create(
349+
workflow=workflow_non_single_written_only,
350+
group=self.group,
351+
event_id=self.event.event_id,
352+
is_single_written=False,
353+
)
354+
355+
# Test ascending order (lastTriggered)
356+
response = self.get_success_response(
357+
self.organization.slug, qs_params={"sortBy": "lastTriggered"}
358+
)
359+
360+
# Workflows without single-written history should come first, then those with it
361+
expected_ascending_order = [
362+
self.workflow_three.name,
363+
workflow_never_fired.name,
364+
workflow_non_single_written_only.name,
365+
self.workflow.name,
366+
self.workflow_two.name,
367+
]
368+
369+
assert [w["name"] for w in response.data] == expected_ascending_order
370+
371+
# Test descending order (-lastTriggered)
372+
response_desc = self.get_success_response(
373+
self.organization.slug, qs_params={"sortBy": "-lastTriggered"}
374+
)
375+
376+
# In descending order, workflows with single-written history should come first
377+
expected_descending_order = [
378+
self.workflow_two.name,
379+
self.workflow.name,
380+
workflow_non_single_written_only.name,
381+
workflow_never_fired.name,
382+
self.workflow_three.name,
383+
]
384+
385+
assert [w["name"] for w in response_desc.data] == expected_descending_order
386+
328387
def test_compound_query(self) -> None:
329388
self.create_detector_workflow(
330389
workflow=self.workflow, detector=self.create_detector(project=self.project)

tests/sentry/workflow_engine/endpoints/test_serializers.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,18 +450,31 @@ def test_serialize_full(self) -> None:
450450
detector=detector,
451451
workflow=workflow,
452452
)
453+
453454
history = WorkflowFireHistory.objects.create(
454455
workflow=workflow,
455456
group=self.group,
456457
event_id=self.event.event_id,
458+
is_single_written=True,
457459
)
460+
# Too old, shouldn't be used.
458461
WorkflowFireHistory.objects.create(
459462
workflow=workflow,
460463
group=self.group,
461464
event_id=self.event.event_id,
465+
is_single_written=True,
462466
)
463467
history.date_added = workflow.date_added + timedelta(seconds=1)
464468
history.save()
469+
# Dual written, shouldn't be used.
470+
dual_written_history = WorkflowFireHistory.objects.create(
471+
workflow=workflow,
472+
group=self.group,
473+
event_id=self.event.event_id,
474+
is_single_written=False,
475+
)
476+
dual_written_history.date_added = workflow.date_added + timedelta(seconds=2)
477+
dual_written_history.save()
465478

466479
result = serialize(workflow)
467480

0 commit comments

Comments
 (0)