Browse Source

fix(sessions): Sort by sum(session) if available (#35184)

In #33827, we changed the default order-by for the sessions endpoint
(see this comment for reasoning).

However this did not actually change the column used for order-by
because we compared a Field object to a string, which always evaluates
to False.
Joris Bayer 2 years ago
parent
commit
3a0fca73d2

+ 3 - 2
src/sentry/snuba/sessions_v2.py

@@ -296,11 +296,12 @@ class QueryDefinition:
         self.params = params
 
         query_columns = set()
-        for i, field in enumerate(self.fields.values()):
+        for i, (field_name, field) in enumerate(self.fields.items()):
             columns = field.get_snuba_columns(raw_groupby)
-            if i == 0 or field == "sum(session)":  # Prefer first, but sum(session) always wins
+            if i == 0 or field_name == "sum(session)":  # Prefer first, but sum(session) always wins
                 self.primary_column = columns[0]  # Will be used in order by
             query_columns.update(columns)
+
         for groupby in self.groupby:
             query_columns.update(groupby.get_snuba_columns())
         self.query_columns = list(query_columns)

+ 2 - 1
tests/snuba/api/endpoints/test_organization_sessions.py

@@ -775,7 +775,8 @@ class OrganizationSessionsEndpointTest(APITestCase, SnubaTestCase):
                     "project": [-1],
                     "statsPeriod": "3d",
                     "interval": "1d",
-                    "field": ["sum(session)", "count_unique(user)"],
+                    # "user" is the first field, but "session" always wins:
+                    "field": ["count_unique(user)", "sum(session)"],
                     "groupBy": ["project", "release", "environment"],
                 }
             )