Browse Source

fix(sort): Fix some better priority bugs (#49211)

Move the feature checking into GET to fix
[SENTRY-11AQ](https://sentry.sentry.io/issues/4185726772/?project=1&query=ContentNotRenderedError&referrer=issue-stream&statsPeriod=14d&stream_index=0)
and add `aggregate_kwargs` to the query base to fix
[SENTRY-11AT](https://sentry.sentry.io/issues/4185934163/events/7054dba61c1a4c13bc128f09e837d351/?project=1&referrer=issue-list&statsPeriod=14d).
Colleen O'Rourke 1 year ago
parent
commit
e6d29314b8

+ 9 - 12
src/sentry/api/endpoints/organization_group_index.py

@@ -181,18 +181,7 @@ class OrganizationGroupIndexEndpoint(OrganizationEventsEndpointBase):
                 assert "environment" not in extra_query_kwargs
                 query_kwargs.update(extra_query_kwargs)
 
-            if query_kwargs["sort_by"] == "better priority":
-                if not features.has(
-                    "organizations:issue-list-better-priority-sort",
-                    organization,
-                    actor=request.user,
-                ):
-                    return self.respond(
-                        {
-                            "error": "This organization does not have the better priority sort feature."
-                        },
-                        status=403,
-                    )
+            if query_kwargs["sort_by"] == "betterPriority":
                 query_kwargs["aggregate_kwargs"] = self.build_better_priority_sort_kwargs(request)
 
             query_kwargs["environments"] = environments if environments else None
@@ -250,6 +239,14 @@ class OrganizationGroupIndexEndpoint(OrganizationEventsEndpointBase):
         :qparam list expand: an optional list of strings to opt in to additional data. Supports `inbox`
         :qparam list collapse: an optional list of strings to opt out of certain pieces of data. Supports `stats`, `lifetime`, `base`
         """
+
+        if request.GET.get("sort") == "betterPriority" and not features.has(
+            "organizations:issue-list-better-priority-sort", organization, actor=request.user
+        ):
+            return Response(
+                {"detail": "This organization does not have the better priority sort feature."},
+                status=400,
+            )
         stats_period = request.GET.get("groupStatsPeriod")
         try:
             start, end = get_date_range_from_stats_period(request.GET)

+ 2 - 0
src/sentry/search/base.py

@@ -10,6 +10,7 @@ ANY = object()
 if TYPE_CHECKING:
     from sentry.api.event_search import SearchFilter
     from sentry.models import Environment, Group, Project
+    from sentry.search.snuba.executors import PrioritySortWeights
     from sentry.utils.cursors import Cursor, CursorResult
 
 
@@ -36,5 +37,6 @@ class SearchBackend(Service):
         max_hits: Optional[int] = None,
         referrer: Optional[str] = None,
         actor: Optional[Any] = None,
+        aggregate_kwargs: Optional[PrioritySortWeights] = None,
     ) -> CursorResult[Group]:
         raise NotImplementedError

+ 1 - 1
src/sentry/search/snuba/executors.py

@@ -501,7 +501,7 @@ class PostgresSnubaQueryExecutor(AbstractQueryExecutor):
         # We don't need a corresponding snuba field here, since this sort only happens
         # in Postgres
         "inbox": "",
-        "better priority": "better_priority",
+        "betterPriority": "better_priority",
     }
 
     aggregation_defs = {

+ 1 - 1
tests/snuba/api/endpoints/test_organization_group_index.py

@@ -161,7 +161,7 @@ class GroupListTest(APITestCase, SnubaTestCase):
         }
 
         response = self.get_success_response(
-            sort="better priority",
+            sort="betterPriority",
             query="is:unresolved",
             limit=25,
             start=iso_format(before_now(days=1)),

+ 2 - 2
tests/snuba/search/test_backend.py

@@ -361,7 +361,7 @@ class EventsSnubaSearchTest(SharedSnubaTest):
         with self.feature("organizations:issue-list-better-priority-sort"):
             weights: PrioritySortWeights = {"log_level": 5, "frequency": 5, "has_stacktrace": 5}
             results = self.make_query(
-                sort_by="better priority",
+                sort_by="betterPriority",
                 aggregate_kwargs=weights,
             )
         assert list(results) == [self.group2, self.group1]
@@ -399,7 +399,7 @@ class EventsSnubaSearchTest(SharedSnubaTest):
         with self.feature("organizations:issue-list-better-priority-sort"):
             weights: PrioritySortWeights = {"log_level": 5, "frequency": 5, "has_stacktrace": 5}
             results = self.make_query(
-                sort_by="better priority",
+                sort_by="betterPriority",
                 projects=[new_project],
                 aggregate_kwargs=weights,
             )