Browse Source

fix(events-facets): Only pop tags on first page (#51370)

Part of the conditional for popping a tag from the list was missing. It
should only be popped on the first page because of injecting projects.
Nar Saynorath 1 year ago
parent
commit
1d3c9119af

+ 1 - 1
src/sentry/snuba/discover.py

@@ -599,7 +599,7 @@ def get_facets(
     # Rescale the results if we're sampling
     multiplier = 1 / sample_rate if sample_rate is not None else 1
 
-    if fetch_projects and len(top_tags) == per_page:
+    if fetch_projects and len(top_tags) == per_page and cursor == 0:
         top_tags.pop()
 
     top_tag_results = []

+ 105 - 0
tests/snuba/api/endpoints/test_organization_events_facets.py

@@ -808,3 +808,108 @@ class OrganizationEventsFacetsEndpointTest(SnubaTestCase, APITestCase):
             {"count": 1, "name": "error", "value": "error"},
         ]
         self.assert_facet(response, "level", expected)
+
+    def test_multiple_pages_with_single_project(self):
+        test_project = self.create_project()
+        test_tags = {str(i): str(i) for i in range(22)}
+
+        self.store_event(
+            data={"event_id": uuid4().hex, "timestamp": self.min_ago_iso, "tags": test_tags},
+            project_id=test_project.id,
+        )
+
+        # Test the default query fetches the first 10 results
+        with self.feature(self.features):
+            response = self.client.get(self.url, format="json", data={"project": test_project.id})
+            links = requests.utils.parse_header_links(
+                response.get("link", "").rstrip(">").replace(">,<", ",<")
+            )
+
+        assert response.status_code == 200, response.content
+        assert links[1]["results"] == "true"  # There are more results to be fetched
+        assert links[1]["cursor"] == "0:10:0"
+        assert len(response.data) == 10
+
+        # Get the next page
+        with self.feature(self.features):
+            response = self.client.get(
+                self.url,
+                format="json",
+                data={"project": test_project.id, "cursor": links[1]["cursor"]},
+            )
+            links = requests.utils.parse_header_links(
+                response.get("link", "").rstrip(">").replace(">,<", ",<")
+            )
+
+        assert response.status_code == 200, response.content
+        assert links[1]["results"] == "true"  # There are more tags to fetch
+        assert len(response.data) == 10
+
+        # Get the next page
+        with self.feature(self.features):
+            response = self.client.get(
+                self.url,
+                format="json",
+                data={"project": test_project.id, "cursor": links[1]["cursor"]},
+            )
+            links = requests.utils.parse_header_links(
+                response.get("link", "").rstrip(">").replace(">,<", ",<")
+            )
+
+        assert response.status_code == 200, response.content
+        assert links[1]["results"] == "false"  # There should be no more tags to fetch
+        assert len(response.data) == 3
+
+    def test_multiple_pages_with_multiple_projects(self):
+        test_project = self.create_project()
+        test_project2 = self.create_project()
+        test_tags = {str(i): str(i) for i in range(22)}  # At least 3 pages worth of information
+
+        self.store_event(
+            data={"event_id": uuid4().hex, "timestamp": self.min_ago_iso, "tags": test_tags},
+            project_id=test_project.id,
+        )
+
+        # Test the default query fetches the first 10 results
+        with self.feature(self.features):
+            response = self.client.get(
+                self.url, format="json", data={"project": [test_project.id, test_project2.id]}
+            )
+            links = requests.utils.parse_header_links(
+                response.get("link", "").rstrip(">").replace(">,<", ",<")
+            )
+
+        assert response.status_code == 200, response.content
+        assert links[1]["results"] == "true"  # There are more results to be fetched
+        assert links[1]["cursor"] == "0:10:0"
+        assert len(response.data) == 10
+
+        # Get the next page
+        with self.feature(self.features):
+            response = self.client.get(
+                self.url,
+                format="json",
+                data={"project": [test_project.id, test_project2.id], "cursor": links[1]["cursor"]},
+            )
+            links = requests.utils.parse_header_links(
+                response.get("link", "").rstrip(">").replace(">,<", ",<")
+            )
+
+        assert response.status_code == 200, response.content
+        assert links[1]["results"] == "true"  # There are more tags to fetch
+        assert len(response.data) == 10
+
+        # Get the next page
+        with self.feature(self.features):
+            response = self.client.get(
+                self.url,
+                format="json",
+                data={"project": [test_project.id, test_project2.id], "cursor": links[1]["cursor"]},
+            )
+            links = requests.utils.parse_header_links(
+                response.get("link", "").rstrip(">").replace(">,<", ",<")
+            )
+
+        assert response.status_code == 200, response.content
+        assert links[1]["results"] == "false"  # There should be no more tags to fetch
+        assert len(response.data) == 4  # 4 because projects and levels were added to the base 22