Skip to content

Commit 88d2f92

Browse files
authored
fix(aci): correctly query open periods within date range (#103924)
we were only returning open periods that started during/after `query_start`, which does not include open periods that started before `query_start` but were still ongoing during the query range
1 parent c97350f commit 88d2f92

File tree

2 files changed

+153
-7
lines changed

2 files changed

+153
-7
lines changed

src/sentry/models/groupopenperiod.py

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,21 +144,41 @@ def get_open_periods_for_group(
144144
query_end: datetime | None = None,
145145
limit: int | None = None,
146146
) -> BaseQuerySet[GroupOpenPeriod]:
147+
"""
148+
Get open periods for a group that overlap with the query time range.
149+
150+
To overlap with [query_start, query_end], an open period must:
151+
1. Start before the query ends
152+
2. End after the query starts (or still be open)
153+
154+
This covers all overlap cases:
155+
- Period starts before query and ends within query range
156+
- Period starts before query and ends after query (open period spans entire query range)
157+
- Period starts within query and ends within query (open period completely inside query range)
158+
- Period starts within query and ends after query
159+
- Period starts before query and is still open
160+
- Period starts within query and is still open
161+
"""
147162
if not should_create_open_periods(group.type):
148163
return GroupOpenPeriod.objects.none()
149164

150165
if not query_start:
151166
# use whichever date is more recent to reduce the query range. first_seen could be > 90 days ago
152167
query_start = max(group.first_seen, timezone.now() - timedelta(days=90))
168+
if not query_end:
169+
query_end = timezone.now()
170+
171+
started_before_query_ends = Q(date_started__lte=query_end)
172+
ended_after_query_starts = Q(date_ended__gte=query_start)
173+
still_open = Q(date_ended__isnull=True)
153174

154-
group_open_periods = GroupOpenPeriod.objects.filter(
155-
group=group,
156-
date_started__gte=query_start,
157-
).order_by("-date_started")
158-
if query_end:
159-
group_open_periods = group_open_periods.filter(
160-
Q(date_ended__lte=query_end) | Q(date_ended__isnull=True)
175+
group_open_periods = (
176+
GroupOpenPeriod.objects.filter(
177+
group=group,
161178
)
179+
.filter(started_before_query_ends & (ended_after_query_starts | still_open))
180+
.order_by("-date_started")
181+
)
162182

163183
return group_open_periods[:limit]
164184

tests/sentry/workflow_engine/endpoints/test_organization_open_periods.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from datetime import timedelta
2+
13
from django.utils import timezone
24

35
from sentry.incidents.grouptype import MetricIssue
@@ -280,3 +282,127 @@ def test_open_periods_limit(self) -> None:
280282
assert resp["start"] == unresolved_time
281283
assert resp["end"] == second_resolved_time
282284
assert resp["isOpen"] is False
285+
286+
def test_get_open_periods_time_range_starts_after_query_start(self) -> None:
287+
"""Test that open periods starting after query_start and ending after query_end are included."""
288+
base_time = timezone.now() - timedelta(days=10)
289+
GroupOpenPeriod.objects.filter(group=self.group).delete()
290+
291+
# Open period: Day 2 to Day 7
292+
open_period = GroupOpenPeriod.objects.create(
293+
group=self.group,
294+
project=self.group.project,
295+
date_started=base_time + timedelta(days=2),
296+
date_ended=base_time + timedelta(days=7),
297+
)
298+
299+
# Query range: Day 0 to Day 5
300+
query_start = base_time.isoformat()
301+
query_end = (base_time + timedelta(days=5)).isoformat()
302+
303+
response = self.get_success_response(
304+
*self.get_url_args(),
305+
qs_params={
306+
"groupId": self.group.id,
307+
"start": query_start,
308+
"end": query_end,
309+
},
310+
)
311+
312+
assert len(response.data) == 1
313+
resp = response.data[0]
314+
assert resp["id"] == str(open_period.id)
315+
316+
def test_get_open_periods_time_range_starts_before_ends_within(self) -> None:
317+
"""Test that open periods starting before query_start and ending before query_end are included."""
318+
319+
base_time = timezone.now() - timedelta(days=10)
320+
321+
GroupOpenPeriod.objects.filter(group=self.group).delete()
322+
323+
# Open period: Day 0 to Day 3 (ends within range)
324+
open_period = GroupOpenPeriod.objects.create(
325+
group=self.group,
326+
project=self.group.project,
327+
date_started=base_time,
328+
date_ended=base_time + timedelta(days=3),
329+
)
330+
331+
# Query range: Day 2 to Day 7
332+
query_start = (base_time + timedelta(days=2)).isoformat()
333+
query_end = (base_time + timedelta(days=7)).isoformat()
334+
335+
response = self.get_success_response(
336+
*self.get_url_args(),
337+
qs_params={
338+
"groupId": self.group.id,
339+
"start": query_start,
340+
"end": query_end,
341+
},
342+
)
343+
344+
assert len(response.data) == 1
345+
resp = response.data[0]
346+
assert resp["id"] == str(open_period.id)
347+
348+
def test_get_open_periods_time_range_starts_before_still_ongoing(self) -> None:
349+
"""Test that open periods starting before query_start and still ongoing (date_ended=None) are included."""
350+
351+
base_time = timezone.now() - timedelta(days=10)
352+
353+
GroupOpenPeriod.objects.filter(group=self.group).delete()
354+
355+
# Open period: Day 1 to ongoing
356+
open_period = GroupOpenPeriod.objects.create(
357+
group=self.group,
358+
project=self.group.project,
359+
date_started=base_time + timedelta(days=1),
360+
date_ended=None,
361+
)
362+
363+
# Query range: Day 0 to Day 7
364+
query_start = base_time.isoformat()
365+
query_end = (base_time + timedelta(days=7)).isoformat()
366+
367+
response = self.get_success_response(
368+
*self.get_url_args(),
369+
qs_params={
370+
"groupId": self.group.id,
371+
"start": query_start,
372+
"end": query_end,
373+
},
374+
)
375+
376+
assert len(response.data) == 1
377+
resp = response.data[0]
378+
assert resp["id"] == str(open_period.id)
379+
380+
def test_get_open_periods_none_in_range(self) -> None:
381+
"""Test that open periods outside the query range are not included."""
382+
383+
base_time = timezone.now() - timedelta(days=10)
384+
385+
GroupOpenPeriod.objects.filter(group=self.group).delete()
386+
387+
# Open period: Day 0 to Day 1 (starts + ends before query range)
388+
GroupOpenPeriod.objects.create(
389+
group=self.group,
390+
project=self.group.project,
391+
date_started=base_time,
392+
date_ended=base_time + timedelta(days=1),
393+
)
394+
395+
# Query range: Day 2 to Day 7
396+
query_start = (base_time + timedelta(days=2)).isoformat()
397+
query_end = (base_time + timedelta(days=7)).isoformat()
398+
399+
response = self.get_success_response(
400+
*self.get_url_args(),
401+
qs_params={
402+
"groupId": self.group.id,
403+
"start": query_start,
404+
"end": query_end,
405+
},
406+
)
407+
408+
assert len(response.data) == 0

0 commit comments

Comments
 (0)