fix(discover2): Fix event details pagination (#16742)

src/sentry/api/endpoints/organization_event_details.py

@@ -54,6 +54,7 @@ class OrganizationEventDetailsEndpoint(OrganizationEventsEndpointBase):
                 event=event,
                 query=request.query_params.get("query"),
                 params=params,
+                organization=organization,
                 reference_event=reference,
                 referrer="api.organization-event-details",
             )

src/sentry/snuba/discover.py

@@ -437,7 +437,7 @@ def get_id(result):
         return result[1]
 
 
-def get_pagination_ids(event, query, params, reference_event=None, referrer=None):
+def get_pagination_ids(event, query, params, organization, reference_event=None, referrer=None):
     """
     High-level API for getting pagination data for an event + filter
 
@@ -458,12 +458,38 @@ def get_pagination_ids(event, query, params, reference_event=None, referrer=None
         if ref_conditions:
             snuba_filter.conditions.extend(ref_conditions)
 
-    return PaginationResult(
-        next=get_id(eventstore.get_next_event_id(event, filter=snuba_filter)),
-        previous=get_id(eventstore.get_prev_event_id(event, filter=snuba_filter)),
-        latest=get_id(eventstore.get_latest_event_id(event, filter=snuba_filter)),
-        oldest=get_id(eventstore.get_earliest_event_id(event, filter=snuba_filter)),
-    )
+    result = {
+        "next": eventstore.get_next_event_id(event, filter=snuba_filter),
+        "previous": eventstore.get_prev_event_id(event, filter=snuba_filter),
+        "latest": eventstore.get_latest_event_id(event, filter=snuba_filter),
+        "oldest": eventstore.get_earliest_event_id(event, filter=snuba_filter),
+    }
+
+    # translate project ids to slugs
+
+    project_ids = set([tuple[0] for tuple in result.values() if tuple])
+
+    project_slugs = {}
+    projects = Project.objects.filter(
+        id__in=list(project_ids), organization=organization, status=ProjectStatus.VISIBLE
+    ).values("id", "slug")
+
+    for project in projects:
+        project_slugs[project["id"]] = project["slug"]
+
+    def into_pagination_record(project_slug_event_id):
+
+        if not project_slug_event_id:
+            return None
+
+        project_id = int(project_slug_event_id[0])
+
+        return "{}:{}".format(project_slugs[project_id], project_slug_event_id[1])
+
+    for key, value in result.items():
+        result[key] = into_pagination_record(value)
+
+    return PaginationResult(**result)
 
 
 def get_facets(query, params, limit=10, referrer=None):

src/sentry/static/sentry/app/views/eventsV2/eventDetails/pagination.tsx

@@ -36,13 +36,11 @@ function buildTargets(
 
   const links: {[k in keyof LinksType]?: any} = {};
 
-  Object.entries(urlMap).forEach(([key, value]) => {
+  Object.entries(urlMap).forEach(([key, eventSlug]) => {
     // If the urlMap has no value we want to skip this link as it is 'disabled';
-    if (!value) {
+    if (!eventSlug) {
       links[key] = null;
     } else {
-      const eventSlug = `${event.projectSlug}:${value}`;
-
       links[key] = {
         pathname: generateEventDetailsRoute({eventSlug, orgSlug: organization.slug}),
         query: eventView.generateQueryStringObject(),

tests/sentry/snuba/test_discover.py

@@ -1166,11 +1166,16 @@ class CreateReferenceEventConditionsTest(SnubaTestCase, TestCase):
         assert result == [["issue.id", "=", event.group_id]]
 
 
+def format_project_event(project_slug, event_id):
+    return "{}:{}".format(project_slug, event_id)
+
+
 class GetPaginationIdsTest(SnubaTestCase, TestCase):
     def setUp(self):
         super(GetPaginationIdsTest, self).setUp()
 
         self.project = self.create_project()
+        self.project_2 = self.create_project()
         self.min_ago = before_now(minutes=1)
         self.day_ago = before_now(days=1)
 
@@ -1207,6 +1212,17 @@ class GetPaginationIdsTest(SnubaTestCase, TestCase):
             },
             project_id=self.project.id,
         )
+        self.store_event(
+            data={
+                "event_id": "e" * 32,
+                "message": "very bad",
+                "type": "default",
+                "platform": "python",
+                "timestamp": iso_format(before_now(minutes=2, seconds=30)),
+                "tags": {"foo": "1"},
+            },
+            project_id=self.project_2.id,
+        )
         self.event = eventstore.get_event_by_id(self.project.id, "b" * 32)
 
     def test_no_related_events(self):
@@ -1214,6 +1230,7 @@ class GetPaginationIdsTest(SnubaTestCase, TestCase):
             self.event,
             "foo:bar",
             {"project_id": [self.project.id], "start": self.min_ago, "end": self.day_ago},
+            self.organization,
         )
         assert result.previous is None
         assert result.next is None
@@ -1226,6 +1243,7 @@ class GetPaginationIdsTest(SnubaTestCase, TestCase):
                 self.event,
                 "foo:(11",
                 {"project_id": [self.project.id], "end": self.min_ago, "start": self.day_ago},
+                self.organization,
             )
 
     def test_matching_conditions(self):
@@ -1233,11 +1251,12 @@ class GetPaginationIdsTest(SnubaTestCase, TestCase):
             self.event,
             "foo:1",
             {"project_id": [self.project.id], "end": self.min_ago, "start": self.day_ago},
+            self.organization,
         )
-        assert result.previous == "a" * 32
-        assert result.next == "c" * 32
-        assert result.oldest == "a" * 32
-        assert result.latest == "c" * 32
+        assert result.previous == format_project_event(self.project.slug, "a" * 32)
+        assert result.next == format_project_event(self.project.slug, "c" * 32)
+        assert result.oldest == format_project_event(self.project.slug, "a" * 32)
+        assert result.latest == format_project_event(self.project.slug, "c" * 32)
 
     def test_reference_event_matching(self):
         # Create an event that won't match the reference
@@ -1259,12 +1278,13 @@ class GetPaginationIdsTest(SnubaTestCase, TestCase):
             self.event,
             "foo:1",
             {"project_id": [self.project.id], "end": self.min_ago, "start": self.day_ago},
+            self.organization,
             reference_event=reference,
         )
-        assert result.previous == "a" * 32
-        assert result.next == "c" * 32
-        assert result.oldest == "a" * 32
-        assert result.latest == "c" * 32
+        assert result.previous == format_project_event(self.project.slug, "a" * 32)
+        assert result.next == format_project_event(self.project.slug, "c" * 32)
+        assert result.oldest == format_project_event(self.project.slug, "a" * 32)
+        assert result.latest == format_project_event(self.project.slug, "c" * 32)
 
     def test_date_params_included(self):
         # Create an event that is outside the date range
@@ -1283,11 +1303,29 @@ class GetPaginationIdsTest(SnubaTestCase, TestCase):
             self.event,
             "foo:1",
             {"project_id": [self.project.id], "end": self.min_ago, "start": self.day_ago},
+            self.organization,
         )
-        assert result.previous == "a" * 32
-        assert result.next == "c" * 32
-        assert result.oldest == "a" * 32
-        assert result.latest == "c" * 32
+        assert result.previous == format_project_event(self.project.slug, "a" * 32)
+        assert result.next == format_project_event(self.project.slug, "c" * 32)
+        assert result.oldest == format_project_event(self.project.slug, "a" * 32)
+        assert result.latest == format_project_event(self.project.slug, "c" * 32)
+
+    def test_multi_projects(self):
+        result = discover.get_pagination_ids(
+            self.event,
+            "foo:1",
+            {
+                "project_id": [self.project.id, self.project_2.id],
+                "end": self.min_ago,
+                "start": self.day_ago,
+            },
+            self.organization,
+        )
+
+        assert result.previous == format_project_event(self.project.slug, "a" * 32)
+        assert result.next == format_project_event(self.project_2.slug, "e" * 32)
+        assert result.oldest == format_project_event(self.project.slug, "a" * 32)
+        assert result.latest == format_project_event(self.project.slug, "c" * 32)
 
 
 class GetFacetsTest(SnubaTestCase, TestCase):

tests/snuba/api/endpoints/test_organization_event_details.py

@@ -8,6 +8,10 @@ from sentry.testutils.helpers.datetime import iso_format, before_now
 from sentry.models import Group
 
 
+def format_project_event(project_slug, event_id):
+    return "{}:{}".format(project_slug, event_id)
+
+
 class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
     def setUp(self):
         super(OrganizationEventDetailsEndpointTest, self).setUp()
@@ -17,6 +21,7 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
 
         self.login_as(user=self.user)
         self.project = self.create_project()
+        self.project_2 = self.create_project()
 
         self.store_event(
             data={
@@ -63,7 +68,7 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
         assert response.status_code == 200, response.content
         assert response.data["id"] == "a" * 32
         assert response.data["previousEventID"] is None
-        assert response.data["nextEventID"] == "b" * 32
+        assert response.data["nextEventID"] == format_project_event(self.project.slug, "b" * 32)
         assert response.data["projectSlug"] == self.project.slug
 
     def test_simple_out_of_range_pagination(self):
@@ -126,6 +131,43 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
             response = self.client.get(url, format="json")
         assert response.status_code == 200
 
+    def test_multi_project_pagination(self):
+        self.store_event(
+            data={
+                "event_id": "d" * 32,
+                "message": "hello world",
+                "timestamp": iso_format(before_now(minutes=2, seconds=30)),
+                "fingerprint": ["group-3"],
+            },
+            project_id=self.project_2.id,
+        )
+        url = reverse(
+            "sentry-api-0-organization-event-details",
+            kwargs={
+                "organization_slug": self.project_2.organization.slug,
+                "project_slug": self.project_2.slug,
+                "event_id": "d" * 32,
+            },
+        )
+
+        with self.feature("organizations:discover-basic"):
+            response = self.client.get(
+                url,
+                data={
+                    "field": ["project", "count(id)", "count_unique(issue.id)"],
+                    "statsPeriod": "24h",
+                    "sort": "-count_id",
+                },
+                format="json",
+            )
+
+        assert response.status_code == 200, response.content
+        assert response.data["id"] == "d" * 32
+        assert response.data["previousEventID"] == format_project_event(self.project.slug, "a" * 32)
+        assert response.data["nextEventID"] == format_project_event(self.project.slug, "b" * 32)
+        assert response.data["latestEventID"] == format_project_event(self.project.slug, "c" * 32)
+        assert response.data["oldestEventID"] == format_project_event(self.project.slug, "a" * 32)
+
     def test_no_access_missing_feature(self):
         url = reverse(
             "sentry-api-0-organization-event-details",
@@ -225,10 +267,18 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
         with self.feature("organizations:discover-basic"):
             response = self.client.get(url, format="json", data={"field": ["message", "count()"]})
         assert response.data["eventID"] == "b" * 32
-        assert response.data["nextEventID"] == "c" * 32, "c is newer & matches message"
-        assert response.data["previousEventID"] == "d" * 32, "d is older & matches message"
-        assert response.data["oldestEventID"] == "e" * 32, "e is oldest matching message"
-        assert response.data["latestEventID"] == "1" * 32, "1 is newest matching message"
+        assert response.data["nextEventID"] == format_project_event(
+            self.project.slug, "c" * 32
+        ), "c is newer & matches message"
+        assert response.data["previousEventID"] == format_project_event(
+            self.project.slug, "d" * 32
+        ), "d is older & matches message"
+        assert response.data["oldestEventID"] == format_project_event(
+            self.project.slug, "e" * 32
+        ), "e is oldest matching message"
+        assert response.data["latestEventID"] == format_project_event(
+            self.project.slug, "1" * 32
+        ), "1 is newest matching message"
 
     def test_event_links_with_date_range(self):
         # Create older in and out of range events
@@ -256,10 +306,18 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
                 url, format="json", data={"field": ["message", "count()"], "statsPeriod": "7d"}
             )
         assert response.data["eventID"] == "b" * 32
-        assert response.data["nextEventID"] == "c" * 32, "c is newer & matches message + range"
-        assert response.data["previousEventID"] == "2" * 32, "d is older & matches message + range"
-        assert response.data["oldestEventID"] == "2" * 32, "3 is outside range, no match"
-        assert response.data["latestEventID"] == "c" * 32, "c is newest matching message"
+        assert response.data["nextEventID"] == format_project_event(
+            self.project.slug, "c" * 32
+        ), "c is newer & matches message + range"
+        assert response.data["previousEventID"] == format_project_event(
+            self.project.slug, "2" * 32
+        ), "d is older & matches message + range"
+        assert response.data["oldestEventID"] == format_project_event(
+            self.project.slug, "2" * 32
+        ), "3 is outside range, no match"
+        assert response.data["latestEventID"] == format_project_event(
+            self.project.slug, "c" * 32
+        ), "c is newest matching message"
 
     def test_event_links_with_tag_fields(self):
         # Create events that overlap with other event messages but
@@ -308,8 +366,12 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
         assert response.data["eventID"] == "1" * 32
         assert response.data["previousEventID"] is None, "no matching tags"
         assert response.data["oldestEventID"] is None, "no older matching events"
-        assert response.data["nextEventID"] == "2" * 32, "2 is older and has matching tags "
-        assert response.data["latestEventID"] == "2" * 32, "2 is oldest matching message"
+        assert response.data["nextEventID"] == format_project_event(
+            self.project.slug, "2" * 32
+        ), "2 is older and has matching tags "
+        assert response.data["latestEventID"] == format_project_event(
+            self.project.slug, "2" * 32
+        ), "2 is oldest matching message"
 
     def test_event_links_with_transaction_events(self):
         prototype = {
@@ -346,8 +408,8 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
                 data={"field": ["important", "count()"], "query": "transaction.duration:>2"},
             )
         assert response.status_code == 200
-        assert response.data["nextEventID"] == "d" * 32
-        assert response.data["previousEventID"] == "f" * 32
+        assert response.data["nextEventID"] == format_project_event(self.project.slug, "d" * 32)
+        assert response.data["previousEventID"] == format_project_event(self.project.slug, "f" * 32)
 
     def test_event_links_with_transaction_events_aggregate_fields(self):
         prototype = {
@@ -387,8 +449,8 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
                 },
             )
         assert response.status_code == 200
-        assert response.data["nextEventID"] == "d" * 32
-        assert response.data["previousEventID"] == "f" * 32
+        assert response.data["nextEventID"] == format_project_event(self.project.slug, "d" * 32)
+        assert response.data["previousEventID"] == format_project_event(self.project.slug, "f" * 32)
 
     def test_event_links_with_transaction_events_aggregate_conditions(self):
         prototype = {
@@ -428,5 +490,5 @@ class OrganizationEventDetailsEndpointTest(APITestCase, SnubaTestCase):
                 },
             )
         assert response.status_code == 200
-        assert response.data["nextEventID"] == "d" * 32
-        assert response.data["previousEventID"] == "f" * 32
+        assert response.data["nextEventID"] == format_project_event(self.project.slug, "d" * 32)
+        assert response.data["previousEventID"] == format_project_event(self.project.slug, "f" * 32)