feat(discover2) Add support for discover2 saved queries (#14482)

Extend the discover saved query endpoints to accept properties that will
make them compatible with discover2. We've deprecated the array based
conditions, arbitrary groupby, and user defined array based aggregations.

Discover2 adds string based query conditions, persisting the selected
environments, and retaining the field aliases.

Refs SEN-947

src/sentry/api/bases/discoversavedquery.py

@@ -1,53 +0,0 @@
-from __future__ import absolute_import
-from rest_framework import serializers
-from sentry.api.serializers.rest_framework import ListField
-from rest_framework.exceptions import PermissionDenied
-from sentry.models import Project, ProjectStatus
-
-
-class DiscoverSavedQuerySerializer(serializers.Serializer):
-    name = serializers.CharField(required=True)
-    projects = ListField(child=serializers.IntegerField(), required=False, default=[])
-    start = serializers.DateTimeField(required=False, allow_null=True)
-    end = serializers.DateTimeField(required=False, allow_null=True)
-    range = serializers.CharField(required=False, allow_null=True)
-    fields = ListField(child=serializers.CharField(), required=False, allow_null=True)
-    limit = serializers.IntegerField(min_value=0, max_value=1000, required=False, allow_null=True)
-    rollup = serializers.IntegerField(required=False, allow_null=True)
-    orderby = serializers.CharField(required=False, allow_null=True)
-    conditions = ListField(child=ListField(), required=False, allow_null=True)
-    aggregations = ListField(child=ListField(), required=False, default=[])
-    groupby = ListField(child=serializers.CharField(), required=False, allow_null=True)
-
-    def validate_projects(self, projects):
-        organization = self.context["organization"]
-
-        org_projects = set(
-            Project.objects.filter(
-                organization=organization, id__in=projects, status=ProjectStatus.VISIBLE
-            ).values_list("id", flat=True)
-        )
-
-        if set(projects) != org_projects:
-            raise PermissionDenied
-
-        return projects
-
-    def validate(self, data):
-        query = {}
-        query_keys = [
-            "fields",
-            "conditions",
-            "aggregations",
-            "range",
-            "start",
-            "end",
-            "orderby",
-            "limit",
-        ]
-
-        for key in query_keys:
-            if data.get(key) is not None:
-                query[key] = data[key]
-
-        return {"name": data["name"], "project_ids": data["projects"], "query": query}

src/sentry/api/serializers/models/discoversavedquery.py

@@ -10,6 +10,9 @@ class DiscoverSavedQuerySerializer(Serializer):
     def serialize(self, obj, attrs, user, **kwargs):
 
         query_keys = [
+            "fieldnames",
+            "environment",
+            "query",
             "fields",
             "conditions",
             "aggregations",

src/sentry/api/serializers/rest_framework/widget.py

@@ -3,7 +3,7 @@ from __future__ import absolute_import
 from django.db.models import Max
 from rest_framework import serializers
 
-from sentry.api.bases.discoversavedquery import DiscoverSavedQuerySerializer
+from sentry.discover.endpoints.serializers import DiscoverSavedQuerySerializer
 from sentry.api.serializers.rest_framework import JSONField, ListField, ValidationError
 from sentry.models import Widget, WidgetDisplayTypes, WidgetDataSourceTypes
 

src/sentry/discover/endpoints/discover_saved_queries.py

@@ -3,11 +3,11 @@ from __future__ import absolute_import
 from rest_framework.response import Response
 
 from sentry.api.serializers import serialize
-from sentry.api.bases.discoversavedquery import DiscoverSavedQuerySerializer
 from sentry.api.bases import OrganizationEndpoint
 from sentry.discover.models import DiscoverSavedQuery
 from sentry import features
 from sentry.discover.endpoints.bases import DiscoverSavedQueryPermission
+from sentry.discover.endpoints.serializers import DiscoverSavedQuerySerializer
 
 
 class DiscoverSavedQueriesEndpoint(OrganizationEndpoint):

src/sentry/discover/endpoints/discover_saved_query_detail.py

@@ -4,10 +4,10 @@ from rest_framework.response import Response
 from sentry.api.serializers import serialize
 from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.bases import OrganizationEndpoint
-from sentry.api.bases.discoversavedquery import DiscoverSavedQuerySerializer
 from sentry import features
 from sentry.discover.endpoints.bases import DiscoverSavedQueryPermission
 from sentry.discover.models import DiscoverSavedQuery
+from sentry.discover.endpoints.serializers import DiscoverSavedQuerySerializer
 
 
 class DiscoverSavedQueryDetailEndpoint(OrganizationEndpoint):

src/sentry/discover/endpoints/serializers.py

@@ -3,7 +3,9 @@ from __future__ import absolute_import
 import six
 import re
 from rest_framework import serializers
+from rest_framework.exceptions import PermissionDenied
 
+from sentry.models import Project, ProjectStatus
 from sentry.api.fields.empty_integer import EmptyIntegerField
 from sentry.api.serializers.rest_framework import ListField
 from sentry.api.utils import get_date_range_from_params, InvalidParams
@@ -131,3 +133,94 @@ class DiscoverQuerySerializer(serializers.Serializer):
             return [["has", [array_field.group(0), value]], "=", bool_value]
 
         return condition
+
+
+class DiscoverSavedQuerySerializer(serializers.Serializer):
+    name = serializers.CharField(required=True)
+    projects = ListField(child=serializers.IntegerField(), required=False, default=[])
+    start = serializers.DateTimeField(required=False, allow_null=True)
+    end = serializers.DateTimeField(required=False, allow_null=True)
+    range = serializers.CharField(required=False, allow_null=True)
+    fields = ListField(child=serializers.CharField(), required=False, allow_null=True)
+    orderby = serializers.CharField(required=False, allow_null=True)
+
+    # This block of fields is only accepted by discover 1 which omits the version
+    # attribute or has it set to 1
+    rollup = serializers.IntegerField(required=False, allow_null=True)
+    aggregations = ListField(child=ListField(), required=False, allow_null=True)
+    groupby = ListField(child=serializers.CharField(), required=False, allow_null=True)
+    conditions = ListField(child=ListField(), required=False, allow_null=True)
+    limit = serializers.IntegerField(min_value=0, max_value=1000, required=False, allow_null=True)
+
+    # There are multiple versions of saved queries supported.
+    version = serializers.IntegerField(min_value=1, max_value=2, required=False, allow_null=True)
+
+    # Attributes that are only accepted if version = 2
+    environment = ListField(child=serializers.CharField(), required=False, allow_null=True)
+    fieldnames = ListField(child=serializers.CharField(), required=False, allow_null=True)
+    query = serializers.CharField(required=False, allow_null=True)
+
+    disallowed_fields = {
+        1: set(["environment", "fieldnames", "query"]),
+        2: set(["groupby", "rollup", "aggregations", "conditions", "limit"]),
+    }
+
+    def validate_projects(self, projects):
+        organization = self.context["organization"]
+
+        org_projects = set(
+            Project.objects.filter(
+                organization=organization, id__in=projects, status=ProjectStatus.VISIBLE
+            ).values_list("id", flat=True)
+        )
+
+        if set(projects) != org_projects:
+            raise PermissionDenied
+
+        return projects
+
+    def validate(self, data):
+        query = {}
+        query_keys = [
+            "fieldnames",
+            "environment",
+            "query",
+            "version",
+            "fields",
+            "conditions",
+            "aggregations",
+            "range",
+            "start",
+            "end",
+            "orderby",
+            "limit",
+        ]
+
+        for key in query_keys:
+            if data.get(key) is not None:
+                query[key] = data[key]
+
+        version = query.get("version", 1)
+        self.validate_version_fields(version, query)
+        if version == 2:
+            if len(query["fields"]) < 1:
+                raise serializers.ValidationError("You must include at least one field.")
+
+            if query.get("fieldnames") and len(query["fieldnames"]) != len(query["fields"]):
+                raise serializers.ValidationError(
+                    "You must provide an equal number of field names and fields"
+                )
+
+        return {"name": data["name"], "project_ids": data["projects"], "query": query}
+
+    def validate_version_fields(self, version, query):
+        try:
+            not_allowed = self.disallowed_fields[version]
+        except KeyError:
+            raise serializers.ValidationError("Invalid version requested.")
+        bad_fields = set(query.keys()) & not_allowed
+        if bad_fields:
+            raise serializers.ValidationError(
+                "You cannot use the %s attribute(s) with the selected version"
+                % ", ".join(bad_fields)
+            )

tests/snuba/api/endpoints/test_discover_saved_queries.py

@@ -6,9 +6,9 @@ from django.core.urlresolvers import reverse
 from sentry.discover.models import DiscoverSavedQuery
 
 
-class DiscoverSavedQueriesTest(APITestCase, SnubaTestCase):
+class DiscoverSavedQueryBase(APITestCase, SnubaTestCase):
     def setUp(self):
-        super(DiscoverSavedQueriesTest, self).setUp()
+        super(DiscoverSavedQueryBase, self).setUp()
         self.login_as(user=self.user)
         self.org = self.create_organization(owner=self.user)
         self.project_ids = [
@@ -24,6 +24,8 @@ class DiscoverSavedQueriesTest(APITestCase, SnubaTestCase):
 
         model.set_projects(self.project_ids)
 
+
+class DiscoverSavedQueriesTest(DiscoverSavedQueryBase):
     def test_get(self):
         with self.feature("organizations:discover"):
             url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
@@ -53,7 +55,6 @@ class DiscoverSavedQueriesTest(APITestCase, SnubaTestCase):
                     "orderby": "-time",
                 },
             )
-
         assert response.status_code == 201, response.content
         assert response.data["name"] == "New query"
         assert response.data["projects"] == self.project_ids
@@ -77,5 +78,115 @@ class DiscoverSavedQueriesTest(APITestCase, SnubaTestCase):
                     "orderby": "-time",
                 },
             )
-
         assert response.status_code == 403, response.content
+
+    def test_post_cannot_use_version_two_fields(self):
+        with self.feature("organizations:discover"):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "New query",
+                    "projects": self.project_ids,
+                    "fields": ["id"],
+                    "range": "24h",
+                    "limit": 20,
+                    "environment": ["dev"],
+                    "fieldnames": ["event id"],
+                    "aggregations": [],
+                    "orderby": "-time",
+                },
+            )
+        assert response.status_code == 400, response.content
+        assert "cannot use the environment, fieldnames attribute(s)" in response.content
+
+
+class DiscoverSavedQueriesVersion2Test(DiscoverSavedQueryBase):
+    def test_post_invalid_conditions(self):
+        with self.feature("organizations:discover"):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "New query",
+                    "projects": self.project_ids,
+                    "fields": ["title", "count()"],
+                    "range": "24h",
+                    "version": 2,
+                    "conditions": [["field", "=", "value"]],
+                },
+            )
+        assert response.status_code == 400, response.content
+        assert "cannot use the conditions attribute(s)" in response.content
+
+    def test_post_require_selected_fields(self):
+        with self.feature("organizations:discover"):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "New query",
+                    "projects": self.project_ids,
+                    "fields": [],
+                    "range": "24h",
+                    "version": 2,
+                },
+            )
+        assert response.status_code == 400, response.content
+        assert "include at least one field" in response.content
+
+    def test_post_fieldnames_length_mismatch(self):
+        with self.feature("organizations:discover"):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "new query",
+                    "projects": self.project_ids,
+                    "fields": ["event", "count()", "project"],
+                    "fieldnames": ["event", "total"],
+                    "range": "24h",
+                    "version": 2,
+                },
+            )
+        assert response.status_code == 400, response.content
+        assert "equal number of field names and fields" in response.content
+
+    def test_post_success(self):
+        with self.feature("organizations:discover"):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "new query",
+                    "projects": self.project_ids,
+                    "fields": ["title", "count()", "project"],
+                    "environment": ["dev"],
+                    "fieldnames": ["event title", "total", "project"],
+                    "query": "event.type:error browser.name:Firefox",
+                    "range": "24h",
+                    "version": 2,
+                },
+            )
+        assert response.status_code == 201, response.content
+        data = response.data
+        assert data["fields"] == ["title", "count()", "project"]
+        assert data["fieldnames"] == ["event title", "total", "project"]
+        assert data["range"] == "24h"
+        assert data["environment"] == ["dev"]
+        assert data["query"] == "event.type:error browser.name:Firefox"
+
+    def test_post_success_no_fieldnames(self):
+        with self.feature("organizations:discover"):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "new query",
+                    "projects": self.project_ids,
+                    "fields": ["event", "count()", "project"],
+                    "range": "24h",
+                    "version": 2,
+                },
+            )
+        assert response.status_code == 201, response.content