feat(open periods): Expect open periods to exist (#99548)

Now that we are no longer calling `get_open_periods_for_group` in the
`GroupDetailsEndpoint` (#99519)
we can clean up the code that constructs an open period when there isn't
actually a `GroupOpenPeriod` (this code was used before we ran the
backfill) to return the objects and serialize them normally. This PR
also adds the `id` to the response. A near future PR will add a
serializer for `GroupOpenPeriodActivity` and add that to the response as
well.

Author: ceorourke

src/sentry/models/groupopenperiod.py

@@ -1,7 +1,5 @@
 import logging
-from dataclasses import dataclass
 from datetime import datetime, timedelta
-from typing import Any
 
 from django.conf import settings
 from django.contrib.postgres.constraints import ExclusionConstraint
@@ -14,31 +12,13 @@
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_model, sane_repr
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
+from sentry.db.models.manager.base_query_set import BaseQuerySet
 from sentry.models.activity import Activity
 from sentry.models.group import Group, GroupStatus
-from sentry.types.activity import ActivityType
 
 logger = logging.getLogger(__name__)
 
 
-@dataclass
-class OpenPeriod:
-    start: datetime
-    end: datetime | None
-    duration: timedelta | None
-    is_open: bool
-    last_checked: datetime
-
-    def to_dict(self) -> dict[str, Any]:
-        return {
-            "start": self.start,
-            "end": self.end,
-            "duration": self.duration,
-            "isOpen": self.is_open,
-            "lastChecked": self.last_checked,
-        }
-
-
 class TsTzRange(models.Func):
     function = "TSTZRANGE"
     output_field = DateTimeRangeField()
@@ -144,104 +124,23 @@ def get_open_periods_for_group(
     group: Group,
     query_start: datetime | None = None,
     query_end: datetime | None = None,
-    offset: int | None = None,
     limit: int | None = None,
-) -> list[Any]:
-
+) -> BaseQuerySet[GroupOpenPeriod] | list[None]:
     if not features.has("organizations:issue-open-periods", group.organization):
         return []
 
-    # Try to get open periods from the GroupOpenPeriod table first
-    group_open_periods = GroupOpenPeriod.objects.filter(group=group)
-    if group_open_periods.exists() and query_start:
-        group_open_periods = group_open_periods.filter(
-            date_started__gte=query_start, date_ended__lte=query_end, id__gte=offset or 0
-        ).order_by("-date_started")[:limit]
-
-        return [
-            OpenPeriod(
-                start=period.date_started,
-                end=period.date_ended,
-                duration=period.date_ended - period.date_started if period.date_ended else None,
-                is_open=period.date_ended is None,
-                last_checked=get_last_checked_for_open_period(group),
-            )
-            for period in group_open_periods
-        ]
-
-    # If there are no open periods in the table, we need to calculate them
-    # from the activity log.
-    # TODO(snigdha): This is temporary until we have backfilled the GroupOpenPeriod table
-    logger.warning("Open periods not fully backfilled", extra={"group_id": group.id})
-
-    if query_start is None or query_end is None:
-        query_start = timezone.now() - timedelta(days=90)
-        query_end = timezone.now()
-
-    query_limit = limit * 2 if limit else None
-    # Filter to REGRESSION and RESOLVED activties to find the bounds of each open period.
-    # The only UNRESOLVED activity we would care about is the first UNRESOLVED activity for the group creation,
-    # but we don't create an entry for that .
-    activities = Activity.objects.filter(
-        group=group,
-        type__in=[ActivityType.SET_REGRESSION.value, ActivityType.SET_RESOLVED.value],
-        datetime__gte=query_start,
-        datetime__lte=query_end,
-    ).order_by("-datetime")[:query_limit]
-
-    open_periods = []
-    start: datetime | None = None
-    end: datetime | None = None
-    last_checked = get_last_checked_for_open_period(group)
-
-    # Handle currently open period
-    if group.status == GroupStatus.UNRESOLVED and len(activities) > 0:
-        open_periods.append(
-            OpenPeriod(
-                start=activities[0].datetime,
-                end=None,
-                duration=None,
-                is_open=True,
-                last_checked=last_checked,
-            )
-        )
-        activities = activities[1:]
-
-    for activity in activities:
-        if activity.type == ActivityType.SET_RESOLVED.value:
-            end = activity.datetime
-        elif activity.type == ActivityType.SET_REGRESSION.value:
-            start = activity.datetime
-            if end is not None:
-                open_periods.append(
-                    OpenPeriod(
-                        start=start,
-                        end=end,
-                        duration=end - start,
-                        is_open=False,
-                        last_checked=end,
-                    )
-                )
-                end = None
-
-    # Add the very first open period, which has no UNRESOLVED activity for the group creation
-    open_periods.append(
-        OpenPeriod(
-            start=group.first_seen,
-            end=end if end else None,
-            duration=end - group.first_seen if end else None,
-            is_open=False if end else True,
-            last_checked=end if end else last_checked,
-        )
-    )
-
-    if offset and limit:
-        return open_periods[offset : offset + limit]
+    if not query_start:
+        # use whichever date is more recent to reduce the query range. first_seen could be > 90 days ago
+        query_start = max(group.first_seen, timezone.now() - timedelta(days=90))
 
-    if limit:
-        return open_periods[:limit]
+    group_open_periods = GroupOpenPeriod.objects.filter(
+        group=group,
+        date_started__gte=query_start,
+    ).order_by("-date_started")
+    if query_end:
+        group_open_periods = group_open_periods.filter(date_ended__lte=query_end)
 
-    return open_periods
+    return group_open_periods[:limit]
 
 
 def create_open_period(group: Group, start_time: datetime) -> None:

src/sentry/workflow_engine/endpoints/organization_open_periods.py

@@ -1,8 +1,7 @@
 from __future__ import annotations
 
-from typing import Any
-
 from drf_spectacular.utils import OpenApiParameter, extend_schema
+from rest_framework import status
 from rest_framework.exceptions import ParseError, ValidationError
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -11,7 +10,8 @@
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases import OrganizationDetectorPermission, OrganizationEndpoint
-from sentry.api.paginator import GenericOffsetPaginator
+from sentry.api.paginator import OffsetPaginator
+from sentry.api.serializers import serialize
 from sentry.api.utils import get_date_range_from_params
 from sentry.apidocs.constants import (
     RESPONSE_BAD_REQUEST,
@@ -20,11 +20,11 @@
     RESPONSE_UNAUTHORIZED,
 )
 from sentry.apidocs.parameters import CursorQueryParam, GlobalParams, VisibilityParams
-from sentry.apidocs.utils import inline_sentry_response_serializer
 from sentry.exceptions import InvalidParams
 from sentry.models.group import Group
-from sentry.models.groupopenperiod import OpenPeriod, get_open_periods_for_group
+from sentry.models.groupopenperiod import get_open_periods_for_group
 from sentry.models.organization import Organization
+from sentry.workflow_engine.endpoints.serializers import GroupOpenPeriodSerializer
 from sentry.workflow_engine.models import Detector
 from sentry.workflow_engine.models.detector_group import DetectorGroup
 
@@ -92,7 +92,7 @@ def get_group_from_group_id(self, group_id: str, organization: Organization) ->
             ),
         ],
         responses={
-            200: inline_sentry_response_serializer("ListOpenPeriods", list[OpenPeriod]),
+            200: GroupOpenPeriodSerializer,
             400: RESPONSE_BAD_REQUEST,
             401: RESPONSE_UNAUTHORIZED,
             403: RESPONSE_FORBIDDEN,
@@ -125,20 +125,27 @@ def get(self, request: Request, organization: Organization) -> Response:
                 else None
             )
         )
-
-        def data_fn(offset: int, limit: int) -> list[OpenPeriod] | list[Any]:
-            if target_group is None:
-                return []
-            return get_open_periods_for_group(
-                group=target_group,
-                query_start=start,
-                query_end=end,
-                offset=offset,
-                limit=limit,
+        if not target_group:
+            return Response(
+                {"detail": "Group not found. Could not query open periods."},
+                status=status.HTTP_404_NOT_FOUND,
             )
-
+        limit = None
+        per_page = request.GET.get("per_page")
+        if per_page:
+            limit = int(per_page)
+            assert limit > 0
+
+        open_periods = get_open_periods_for_group(
+            group=target_group,
+            query_start=start,
+            query_end=end,
+            limit=limit,
+        )
         return self.paginate(
             request=request,
-            on_results=lambda results: [result.to_dict() for result in results],
-            paginator=GenericOffsetPaginator(data_fn=data_fn),
+            queryset=open_periods,
+            paginator_cls=OffsetPaginator,
+            on_results=lambda x: serialize(x, request.user),
+            count_hits=True,
         )

src/sentry/workflow_engine/endpoints/serializers.py

@@ -16,6 +16,7 @@
 from sentry.api.serializers.rest_framework.base import convert_dict_key_case, snake_to_camel_case
 from sentry.grouping.grouptype import ErrorGroupType
 from sentry.models.group import Group, GroupStatus
+from sentry.models.groupopenperiod import GroupOpenPeriod, get_last_checked_for_open_period
 from sentry.models.options.project_option import ProjectOption
 from sentry.rules.actions.notify_event_service import PLUGINS_WITH_FIRST_PARTY_EQUIVALENTS
 from sentry.rules.history.base import TimeSeriesValue
@@ -682,3 +683,27 @@ def serialize(
             "detectorId": str(obj.detector.id),
             "workflowId": str(obj.workflow.id),
         }
+
+
+class GroupOpenPeriodResponse(TypedDict):
+    id: str
+    start: datetime
+    end: datetime | None
+    duration: timedelta | None
+    isOpen: bool
+    lastChecked: datetime
+
+
+@register(GroupOpenPeriod)
+class GroupOpenPeriodSerializer(Serializer):
+    def serialize(
+        self, obj: GroupOpenPeriod, attrs: Mapping[str, Any], user, **kwargs
+    ) -> GroupOpenPeriodResponse:
+        return {
+            "id": str(obj.id),
+            "start": obj.date_started,
+            "end": obj.date_ended,
+            "duration": obj.date_ended - obj.date_started if obj.date_ended else None,
+            "isOpen": obj.date_ended is None,
+            "lastChecked": get_last_checked_for_open_period(obj.group),
+        }