From 10b54bd6c37d505334c1a65783dcd89de076e3c0 Mon Sep 17 00:00:00 2001
From: Gilbert Szeto <gilbert.szeto@sentry.io>
Date: Thu, 13 Jul 2023 10:07:03 -0700
Subject: [PATCH] fix(helpful-event): sort on whether replay_id or profile_id
 are null or not and not the actual id values (#52777)

Fixes an issue where we sort on the ID values themselves instead of
sorting on whether the replay_id or profile_id are present.
---
 src/sentry/eventstore/snuba/backend.py        | 12 +++++
 .../api/endpoints/test_group_event_details.py | 49 +++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/src/sentry/eventstore/snuba/backend.py b/src/sentry/eventstore/snuba/backend.py
index 93a2860f05902f..1ad415af0edafe 100644
--- a/src/sentry/eventstore/snuba/backend.py
+++ b/src/sentry/eventstore/snuba/backend.py
@@ -103,6 +103,18 @@ def get_events_snql(
                             direction=direction,
                         )
                     )
+                elif order_field_alias in (
+                    Columns.PROFILE_ID.value.alias,
+                    Columns.REPLAY_ID.value.alias,
+                ):
+                    resolved_order_by.append(
+                        OrderBy(
+                            Function(
+                                "if", [Function("isNull", [Column(resolved_column_or_none)]), 0, 1]
+                            ),
+                            direction=direction,
+                        )
+                    )
                 else:
                     resolved_order_by.append(
                         OrderBy(Column(resolved_column_or_none), direction=direction)
diff --git a/tests/sentry/api/endpoints/test_group_event_details.py b/tests/sentry/api/endpoints/test_group_event_details.py
index abd82b718493b7..bfe914040e164a 100644
--- a/tests/sentry/api/endpoints/test_group_event_details.py
+++ b/tests/sentry/api/endpoints/test_group_event_details.py
@@ -175,6 +175,55 @@ def test_get_simple_helpful(self):
         assert response.data["previousEventID"] == self.event_c.event_id
         assert response.data["nextEventID"] is None
 
+    @with_feature("organizations:issue-details-most-helpful-event")
+    def test_get_helpful_replay_id_order(self):
+        replay_id_1 = uuid.uuid4().hex
+        replay_id_2 = uuid.uuid4().hex
+        replay_id_1 = "b" + replay_id_1[1:]
+        replay_id_2 = "a" + replay_id_2[1:]
+
+        self.event_d = self.store_event(
+            data={
+                "event_id": "d" * 32,
+                "environment": "staging",
+                "timestamp": iso_format(before_now(minutes=3)),
+                "fingerprint": ["group-order"],
+                "contexts": {
+                    "replay": {"replay_id": replay_id_1},
+                },
+            },
+            project_id=self.project_1.id,
+        )
+        self.event_e = self.store_event(
+            data={
+                "event_id": "e" * 32,
+                "environment": "staging",
+                "timestamp": iso_format(before_now(minutes=2)),
+                "fingerprint": ["group-order"],
+                "contexts": {
+                    "replay": {"replay_id": replay_id_2},
+                },
+            },
+            project_id=self.project_1.id,
+        )
+        self.event_f = self.store_event(
+            data={
+                "event_id": "f" * 32,
+                "environment": "staging",
+                "timestamp": iso_format(before_now(minutes=1)),
+                "fingerprint": ["group-order"],
+            },
+            project_id=self.project_1.id,
+        )
+
+        url = f"/api/0/issues/{self.event_d.group.id}/events/helpful/"
+        response = self.client.get(url, format="json")
+
+        assert response.status_code == 200, response.content
+        assert response.data["id"] == str(self.event_e.event_id)
+        assert response.data["previousEventID"] == str(self.event_d.event_id)
+        assert response.data["nextEventID"] == str(self.event_f.event_id)
+
     @with_feature("organizations:issue-details-most-helpful-event")
     def test_with_empty_query(self):
         url = f"/api/0/issues/{self.event_a.group.id}/events/helpful/"