diff --git a/src/sentry/models/groupresolution.py b/src/sentry/models/groupresolution.py index d76c88de109b..fb05d155d571 100644 --- a/src/sentry/models/groupresolution.py +++ b/src/sentry/models/groupresolution.py @@ -19,6 +19,18 @@ from sentry.utils import metrics +def release_order_date(date_added, date_released=None): + """The date a release is ordered by. + + Mirrors `COALESCE(date_released, date_added)`, the ordering the releases + endpoints and `most_recent_release` already use. `date_added` alone is when + Sentry first saw the release, which is not when it shipped: a release row + created by an incoming event from an old build carries a recent + `date_added` and so outranks the release that actually fixed the issue. + """ + return date_released or date_added + + @cell_silo_model class GroupResolution(Model): """ @@ -81,14 +93,17 @@ def compare_release_dates_for_in_next_release(res_release, res_release_datetime, Helper function that compares release versions based on date for `GroupResolution.Type.in_next_release` """ - return res_release == release.id or res_release_datetime > release.date_added + return res_release == release.id or res_release_datetime > release_order_date( + release.date_added, release.date_released + ) try: ( res_type, res_release, res_release_version, - res_release_datetime, + res_release_date_added, + res_release_date_released, current_release_version, ) = ( cls.objects.filter(group=group) @@ -98,12 +113,15 @@ def compare_release_dates_for_in_next_release(res_release, res_release_datetime, "release__id", "release__version", "release__date_added", + "release__date_released", "current_release_version", )[0] ) except IndexError: return False + res_release_datetime = release_order_date(res_release_date_added, res_release_date_released) + # if no release is present, we assume we've gone from "no release" to "some release" # in application configuration, and thus this must be older if not release: @@ -155,7 +173,10 @@ def compare_release_dates_for_in_next_release(res_release, res_release_datetime, return compare_release_dates_for_in_next_release( res_release=current_release_obj.id, - res_release_datetime=current_release_obj.date_added, + res_release_datetime=release_order_date( + current_release_obj.date_added, + current_release_obj.date_released, + ), release=release, ) except Release.DoesNotExist: @@ -194,6 +215,8 @@ def compare_release_dates_for_in_next_release(res_release, res_release_datetime, ... # Fallback to older model if semver comparison fails due to whatever reason - return res_release_datetime >= release.date_added + return res_release_datetime >= release_order_date( + release.date_added, release.date_released + ) else: raise NotImplementedError diff --git a/tests/sentry/models/test_groupresolution.py b/tests/sentry/models/test_groupresolution.py index 4875f0b616bc..75a8b0579744 100644 --- a/tests/sentry/models/test_groupresolution.py +++ b/tests/sentry/models/test_groupresolution.py @@ -16,6 +16,13 @@ def setUp(self) -> None: self.group = self.create_group() self.old_semver_release = self.create_release(version="foo_package@1.0") self.new_semver_release = self.create_release(version="foo_package@2.0") + # Added after new_release, but finalized with an earlier ship date -- + # the shape a straggler event from an old build produces. + self.late_registered_old_release = self.create_release( + version="c", + date_added=timezone.now(), + date_released=timezone.now() - timedelta(minutes=60), + ) def test_in_next_release_with_new_release(self) -> None: GroupResolution.objects.create( @@ -172,6 +179,30 @@ def test_in_release_with_old_release(self) -> None: ) assert GroupResolution.has_resolution(self.group, self.old_release) + def test_in_release_with_late_registered_old_release(self) -> None: + """A release finalized as older must not clear a newer resolution.""" + GroupResolution.objects.create( + release=self.new_release, group=self.group, type=GroupResolution.Type.in_release + ) + assert GroupResolution.has_resolution(self.group, self.late_registered_old_release) + + def test_in_next_release_with_late_registered_old_release(self) -> None: + GroupResolution.objects.create( + release=self.new_release, + group=self.group, + type=GroupResolution.Type.in_next_release, + ) + assert GroupResolution.has_resolution(self.group, self.late_registered_old_release) + + def test_in_release_resolved_in_a_late_registered_release(self) -> None: + """The resolution's own release is ordered by its ship date too.""" + GroupResolution.objects.create( + release=self.late_registered_old_release, + group=self.group, + type=GroupResolution.Type.in_release, + ) + assert not GroupResolution.has_resolution(self.group, self.new_release) + def test_for_semver_in_release_with_new_release(self) -> None: GroupResolution.objects.create( release=self.old_semver_release, group=self.group, type=GroupResolution.Type.in_release