Skip to content

chore(aci): remove lastChecked from open period serializer #103868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 24, 2025
Merged

chore(aci): remove lastChecked from open period serializer #103868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf5c829
remove lastChecked from open period serializer
mifu67 Nov 21, 2025
c66c842
update tests
mifu67 Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/sentry/workflow_engine/endpoints/serializers/group_open_period_serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from sentry.api.serializers import Serializer, register, serialize
from sentry.incidents.utils.process_update_helpers import calculate_event_date_from_update_date
from sentry.models.groupopenperiod import GroupOpenPeriod, get_last_checked_for_open_period
from sentry.models.groupopenperiod import GroupOpenPeriod
Copy link
Contributor

@cursor cursor bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Frontend still expects removed lastChecked field

The lastChecked field was removed from GroupOpenPeriodResponse and the serializer, but the frontend code at static/app/views/issueDetails/streamline/sidebar/firstLastSeenSection.tsx still accesses openPeriods?.[0]?.lastChecked to determine the last seen time. This causes the frontend to always fall back to group.lastSeen instead of the intended value, breaking the "Last seen" display for issues using open period checks.

Fix in Cursor Fix in Web

Copy link
Member

@malwilley malwilley Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, it falls back the group.lastSeen if it's not there, though we should followup and remove it from the expected type

from sentry.models.groupopenperiodactivity import GroupOpenPeriodActivity, OpenPeriodActivityType
from sentry.types.group import PriorityLevel

Expand All @@ -22,7 +22,6 @@ class GroupOpenPeriodResponse(TypedDict):
start: datetime
end: datetime | None
isOpen: bool
lastChecked: datetime
activities: list[GroupOpenPeriodActivityResponse] | None


Expand Down Expand Up @@ -72,6 +71,5 @@ def serialize(
else None
),
isOpen=obj.date_ended is None,
lastChecked=get_last_checked_for_open_period(obj.group),
activities=attrs.get("activities"),
)
40 changes: 0 additions & 40 deletions tests/sentry/workflow_engine/endpoints/test_organization_open_periods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from datetime import timedelta

from django.utils import timezone

from sentry.incidents.grouptype import MetricIssue
Expand Down Expand Up @@ -81,32 +79,6 @@ def test_open_periods_group_id(self) -> None:
def test_validation_error_when_missing_params(self) -> None:
self.get_error_response(*self.get_url_args(), status_code=400)

def test_open_periods_new_group_with_last_checked(self) -> None:
alert_rule = self.create_alert_rule(
organization=self.organization,
projects=[self.project],
name="Test Alert Rule",
)
last_checked = timezone.now() - timedelta(seconds=alert_rule.snuba_query.time_window)

response = self.get_success_response(
*self.get_url_args(), qs_params={"groupId": self.group.id}
)
assert response.status_code == 200, response.content
assert len(response.data) == 1
resp = response.data[0]
assert resp["id"] == str(self.group_open_period.id)
assert resp["start"] == self.group.first_seen
assert resp["end"] is None
assert resp["isOpen"] is True
assert resp["lastChecked"] >= last_checked
assert resp["activities"][0] == {
"id": str(self.opened_gopa.id),
"type": OpenPeriodActivityType.OPENED.to_str(),
"value": PriorityLevel(self.group.priority).to_str(),
"dateCreated": self.opened_gopa.date_added,
}

def test_open_periods_resolved_group(self) -> None:
self.group.status = GroupStatus.RESOLVED
self.group.save()
Expand Down Expand Up @@ -138,9 +110,6 @@ def test_open_periods_resolved_group(self) -> None:
assert resp["start"] == self.group.first_seen
assert resp["end"] == resolved_time
assert resp["isOpen"] is False
assert resp["lastChecked"].replace(second=0, microsecond=0) == activity.datetime.replace(
second=0, microsecond=0
)
assert len(resp["activities"]) == 2
assert resp["activities"][0] == {
"id": str(self.opened_gopa.id),
Expand Down Expand Up @@ -223,9 +192,6 @@ def test_open_periods_unresolved_group(self) -> None:
assert resp["start"] == unresolved_time
assert resp["end"] == second_resolved_time
assert resp["isOpen"] is False
assert resp["lastChecked"].replace(second=0, microsecond=0) == second_resolved_time.replace(
second=0, microsecond=0
)
assert len(resp["activities"]) == 2
assert resp["activities"][0] == {
"id": str(opened_gopa2.id),
Expand All @@ -244,9 +210,6 @@ def test_open_periods_unresolved_group(self) -> None:
assert resp2["start"] == self.group.first_seen
assert resp2["end"] == resolved_time
assert resp2["isOpen"] is False
assert resp2["lastChecked"].replace(second=0, microsecond=0) == resolved_time.replace(
second=0, microsecond=0
)
assert len(resp2["activities"]) == 2
assert resp2["activities"][0] == {
"id": str(self.opened_gopa.id),
Expand Down Expand Up @@ -317,6 +280,3 @@ def test_open_periods_limit(self) -> None:
assert resp["start"] == unresolved_time
assert resp["end"] == second_resolved_time
assert resp["isOpen"] is False
assert resp["lastChecked"].replace(second=0, microsecond=0) == second_resolved_time.replace(
second=0, microsecond=0
)
Loading