feat(dashboards): Added widgetType to serializers and model and migration (#30090)

Added a WidgetType to dashboard widgets model in the backend.
Updates Widget and WidgetQuery serializer to check validation for Issue widget queries and Discover widget queries depending on the type of the widget.
Includes a migration to add a widget_type column to dashboard widgets.

migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0252_code_mapping_cascade_delete
+sentry: 0253_add_widget_type
 social_auth: 0001_initial

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

@@ -7,6 +7,7 @@ from sentry.models import (
     DashboardWidget,
     DashboardWidgetDisplayTypes,
     DashboardWidgetQuery,
+    DashboardWidgetTypes,
 )
 
 
@@ -38,6 +39,9 @@ class DashboardWidgetSerializer(Serializer):
             "dateCreated": obj.date_added,
             "dashboardId": str(obj.dashboard_id),
             "queries": attrs["queries"],
+            # Default to discover type if null
+            "widgetType": DashboardWidgetTypes.get_type_name(obj.widget_type)
+            or DashboardWidgetTypes.TYPE_NAMES[0],
         }
 
 

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

@@ -3,6 +3,7 @@ from datetime import datetime, timedelta
 from django.db.models import Max
 from rest_framework import serializers
 
+from sentry.api.issue_search import parse_search_query
 from sentry.api.serializers.rest_framework import CamelSnakeSerializer
 from sentry.discover.arithmetic import ArithmeticError, categorize_columns, resolve_equation_list
 from sentry.exceptions import InvalidSearchQuery
@@ -11,6 +12,7 @@ from sentry.models import (
     DashboardWidget,
     DashboardWidgetDisplayTypes,
     DashboardWidgetQuery,
+    DashboardWidgetTypes,
 )
 from sentry.search.events.fields import get_function_alias, resolve_field_list
 from sentry.search.events.filter import get_filter
@@ -90,6 +92,15 @@ class DashboardWidgetQuerySerializer(CamelSnakeSerializer):
         else:
             resolved_equations = []
 
+        try:
+            parse_search_query(conditions)
+        except InvalidSearchQuery as err:
+            # We don't know if the widget that this query belongs to is an
+            # Issue widget or Discover widget. Pass the error back to the
+            # Widget serializer to decide if whether or not to raise this
+            # error based on the Widget's type
+            data["issue_query_error"] = {"conditions": [f"Invalid conditions: {err}"]}
+
         try:
             # When using the eps/epm functions, they require an interval argument
             # or to provide the start/end so that the interval can be computed.
@@ -104,14 +115,20 @@ class DashboardWidgetQuerySerializer(CamelSnakeSerializer):
 
             snuba_filter = get_filter(conditions, params=params)
         except InvalidSearchQuery as err:
-            raise serializers.ValidationError({"conditions": f"Invalid conditions: {err}"})
+            data["discover_query_error"] = {"conditions": [f"Invalid conditions: {err}"]}
+            return data
 
         if orderby:
             snuba_filter.orderby = get_function_alias(orderby)
         try:
             resolve_field_list(fields, snuba_filter, resolved_equations=resolved_equations)
         except InvalidSearchQuery as err:
-            raise serializers.ValidationError({"fields": f"Invalid fields: {err}"})
+            # We don't know if the widget that this query belongs to is an
+            # Issue widget or Discover widget. Pass the error back to the
+            # Widget serializer to decide if whether or not to raise this
+            # error based on the Widget's type
+            data["discover_query_error"] = {"fields": f"Invalid fields: {err}"}
+
         return data
 
     def _get_attr(self, data, attr, empty_value=None):
@@ -132,10 +149,16 @@ class DashboardWidgetSerializer(CamelSnakeSerializer):
     )
     interval = serializers.CharField(required=False, max_length=10)
     queries = DashboardWidgetQuerySerializer(many=True, required=False)
+    widget_type = serializers.ChoiceField(
+        choices=DashboardWidgetTypes.as_text_choices(), required=False
+    )
 
     def validate_display_type(self, display_type):
         return DashboardWidgetDisplayTypes.get_id_for_type_name(display_type)
 
+    def validate_widget_type(self, widget_type):
+        return DashboardWidgetTypes.get_id_for_type_name(widget_type)
+
     validate_id = validate_id
 
     def validate_interval(self, interval):
@@ -144,6 +167,27 @@ class DashboardWidgetSerializer(CamelSnakeSerializer):
         return interval
 
     def validate(self, data):
+        query_errors = []
+        has_query_error = False
+        if data.get("queries"):
+            # Check each query to see if they have an issue or discover error depending on the type of the widget
+            for query in data.get("queries"):
+                if (
+                    data.get("widget_type") == DashboardWidgetTypes.ISSUE
+                    and "issue_query_error" in query
+                ):
+                    query_errors.append(query["issue_query_error"])
+                    has_query_error = True
+                elif (
+                    "widget_type" not in data
+                    or data.get("widget_type") == DashboardWidgetTypes.DISCOVER
+                ) and "discover_query_error" in query:
+                    query_errors.append(query["discover_query_error"])
+                    has_query_error = True
+                else:
+                    query_errors.append({})
+        if has_query_error:
+            raise serializers.ValidationError({"queries": query_errors})
         if not data.get("id"):
             if not data.get("queries"):
                 raise serializers.ValidationError(
@@ -241,6 +285,9 @@ class DashboardDetailsSerializer(CamelSnakeSerializer):
             display_type=widget_data["display_type"],
             title=widget_data["title"],
             interval=widget_data.get("interval", "5m"),
+            widget_type=widget_data["widget_type"]
+            if "widget_type" in widget_data
+            else DashboardWidgetTypes.DISCOVER,
             order=order,
         )
         new_queries = []
@@ -261,6 +308,7 @@ class DashboardDetailsSerializer(CamelSnakeSerializer):
         widget.title = data.get("title", widget.title)
         widget.display_type = data.get("display_type", widget.display_type)
         widget.interval = data.get("interval", widget.interval)
+        widget.widget_type = data.get("widget_type", widget.widget_type)
         widget.order = order
         widget.save()
 

src/sentry/migrations/0253_add_widget_type.py

@@ -0,0 +1,50 @@
+# Generated by Django 2.2.24 on 2021-11-19 03:32
+
+from django.db import migrations
+
+import sentry.db.models.fields.bounded
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = False
+
+    # This flag is used to decide whether to run this migration in a transaction or not.
+    # By default we prefer to run in a transaction, but for migrations where you want
+    # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
+    # want to create an index concurrently when adding one to an existing table.
+    # You'll also usually want to set this to `False` if you're writing a data
+    # migration, since we don't want the entire migration to run in one long-running
+    # transaction.
+    atomic = True
+
+    dependencies = [
+        ("sentry", "0252_code_mapping_cascade_delete"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.AddField(
+                    model_name="dashboardwidget",
+                    name="widget_type",
+                    field=sentry.db.models.fields.bounded.BoundedPositiveIntegerField(null=True),
+                ),
+            ],
+            state_operations=[
+                migrations.AddField(
+                    model_name="dashboardwidget",
+                    name="widget_type",
+                    field=sentry.db.models.fields.bounded.BoundedPositiveIntegerField(null=True),
+                ),
+            ],
+        )
+    ]

src/sentry/models/dashboard_widget.py

@@ -34,6 +34,16 @@ class TypesClass:
                 return id
 
 
+class DashboardWidgetTypes(TypesClass):
+    DISCOVER = 0
+    ISSUE = 1
+    TYPES = [
+        (DISCOVER, "discover"),
+        (ISSUE, "issue"),
+    ]
+    TYPE_NAMES = [t[1] for t in TYPES]
+
+
 class DashboardWidgetDisplayTypes(TypesClass):
     LINE_CHART = 0
     AREA_CHART = 1
@@ -94,6 +104,7 @@ class DashboardWidget(Model):
     interval = models.CharField(max_length=10, null=True)
     display_type = BoundedPositiveIntegerField(choices=DashboardWidgetDisplayTypes.as_choices())
     date_added = models.DateTimeField(default=timezone.now)
+    widget_type = BoundedPositiveIntegerField(choices=DashboardWidgetTypes.as_choices(), null=True)
 
     class Meta:
         app_label = "sentry"

tests/acceptance/test_organization_dashboards.py

@@ -1,4 +1,9 @@
-from sentry.models import Dashboard, DashboardWidget, DashboardWidgetDisplayTypes
+from sentry.models import (
+    Dashboard,
+    DashboardWidget,
+    DashboardWidgetDisplayTypes,
+    DashboardWidgetTypes,
+)
 from sentry.testutils import AcceptanceTestCase
 from sentry.testutils.helpers.datetime import before_now, iso_format
 
@@ -91,6 +96,7 @@ class OrganizationDashboardsManageAcceptanceTest(AcceptanceTestCase):
             order=0,
             title="Widget 1",
             display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
             interval="1d",
         )
         self.widget_2 = DashboardWidget.objects.create(
@@ -98,6 +104,7 @@ class OrganizationDashboardsManageAcceptanceTest(AcceptanceTestCase):
             order=1,
             title="Widget 2",
             display_type=DashboardWidgetDisplayTypes.TABLE,
+            widget_type=DashboardWidgetTypes.DISCOVER,
             interval="1d",
         )
         self.login_as(self.user)

tests/sentry/api/endpoints/test_organization_dashboard_details.py

@@ -6,6 +6,7 @@ from sentry.models import (
     DashboardWidget,
     DashboardWidgetDisplayTypes,
     DashboardWidgetQuery,
+    DashboardWidgetTypes,
 )
 from sentry.testutils import OrganizationDashboardWidgetTestCase
 
@@ -18,6 +19,7 @@ class OrganizationDashboardDetailsTestCase(OrganizationDashboardWidgetTestCase):
             order=0,
             title="Widget 1",
             display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
             interval="1d",
         )
         self.widget_2 = DashboardWidget.objects.create(
@@ -25,6 +27,7 @@ class OrganizationDashboardDetailsTestCase(OrganizationDashboardWidgetTestCase):
             order=1,
             title="Widget 2",
             display_type=DashboardWidgetDisplayTypes.TABLE,
+            widget_type=DashboardWidgetTypes.DISCOVER,
             interval="1d",
         )
         self.widget_1_data_1 = DashboardWidgetQuery.objects.create(
@@ -166,12 +169,14 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
             order=2,
             title="Widget 3",
             display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
         )
         self.widget_4 = DashboardWidget.objects.create(
             dashboard=self.dashboard,
             order=3,
             title="Widget 4",
             display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
         )
         self.widget_ids = [self.widget_1.id, self.widget_2.id, self.widget_3.id, self.widget_4.id]
 
@@ -674,6 +679,7 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
             ),
             title="Widget 200",
             display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
         )
         response = self.do_request(
             "put",
@@ -694,6 +700,101 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
         assert response.data == ["You cannot update widgets that are not part of this dashboard."]
         self.assert_no_changes()
 
+    def test_add_issue_widget_valid_query(self):
+        data = {
+            "title": "First dashboard",
+            "widgets": [
+                {"id": str(self.widget_1.id)},
+                {
+                    "title": "Issues",
+                    "displayType": "table",
+                    "widgetType": "issue",
+                    "interval": "5m",
+                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:unresolved"}],
+                },
+            ],
+        }
+        response = self.do_request("put", self.url(self.dashboard.id), data=data)
+        assert response.status_code == 200, response.data
+
+    def test_add_issue_widget_invalid_query(self):
+        data = {
+            "title": "First dashboard",
+            "widgets": [
+                {"id": str(self.widget_1.id)},
+                {
+                    "title": "Issues",
+                    "displayType": "table",
+                    "widgetType": "issue",
+                    "interval": "5m",
+                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:())"}],
+                },
+            ],
+        }
+        response = self.do_request("put", self.url(self.dashboard.id), data=data)
+        assert response.status_code == 400, response.data
+        assert b"Parse error" in response.content
+
+    def test_add_discover_widget_invalid_issue_query(self):
+        data = {
+            "title": "First dashboard",
+            "widgets": [
+                {"id": str(self.widget_1.id)},
+                {
+                    "title": "Issues",
+                    "displayType": "table",
+                    "widgetType": "discover",
+                    "interval": "5m",
+                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:unresolved"}],
+                },
+            ],
+        }
+        response = self.do_request("put", self.url(self.dashboard.id), data=data)
+        assert response.status_code == 400, response.data
+        assert b"Invalid conditions" in response.content
+
+    def test_add_multiple_discover_and_issue_widget(self):
+        data = {
+            "title": "First dashboard",
+            "widgets": [
+                {"id": str(self.widget_1.id)},
+                {
+                    "title": "Unresolved Issues",
+                    "displayType": "table",
+                    "widgetType": "issue",
+                    "interval": "5m",
+                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:unresolved"}],
+                },
+                {
+                    "title": "Resolved Issues",
+                    "displayType": "table",
+                    "widgetType": "issue",
+                    "interval": "5m",
+                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:resolved"}],
+                },
+                {
+                    "title": "Transactions",
+                    "displayType": "table",
+                    "widgetType": "discover",
+                    "interval": "5m",
+                    "queries": [
+                        {"name": "", "fields": ["count()"], "conditions": "event.type:transaction"}
+                    ],
+                },
+                {
+                    "title": "Errors",
+                    "displayType": "table",
+                    "widgetType": "discover",
+                    "interval": "5m",
+                    "queries": [
+                        {"name": "", "fields": ["count()"], "conditions": "event.type:error"}
+                    ],
+                },
+            ],
+        }
+        response = self.do_request("put", self.url(self.dashboard.id), data=data)
+        assert response.status_code == 200, response.data
+
 
 class OrganizationDashboardVisitTest(OrganizationDashboardDetailsTestCase):
     def url(self, dashboard_id):

tests/sentry/api/endpoints/test_organization_dashboard_widget_details.py

@@ -220,3 +220,49 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             data=data,
         )
         assert response.status_code == 200, response.data
+
+    def test_valid_issue_query_conditions(self):
+        data = {
+            "title": "Unresolved Issues",
+            "displayType": "table",
+            "widgetType": "issue",
+            "queries": [{"name": "unresolved", "conditions": "is:unresolved", "fields": []}],
+        }
+        response = self.do_request(
+            "post",
+            self.url(),
+            data=data,
+        )
+        assert response.status_code == 200, response.data
+
+    def test_invalid_issue_query_conditions(self):
+        data = {
+            "title": "Unresolved Issues",
+            "displayType": "table",
+            "widgetType": "issue",
+            "queries": [{"name": "unresolved", "conditions": "is:())", "fields": []}],
+        }
+        response = self.do_request(
+            "post",
+            self.url(),
+            data=data,
+        )
+        assert response.status_code == 400, response.data
+        assert "queries" in response.data, response.data
+        assert response.data["queries"][0]["conditions"], response.data
+
+    def test_invalid_issue_query_conditions_in_discover_widget(self):
+        data = {
+            "title": "Unresolved Issues",
+            "displayType": "table",
+            "widgetType": "discover",
+            "queries": [{"name": "unresolved", "conditions": "is:unresolved", "fields": []}],
+        }
+        response = self.do_request(
+            "post",
+            self.url(),
+            data=data,
+        )
+        assert response.status_code == 400, response.data
+        assert "queries" in response.data, response.data
+        assert response.data["queries"][0]["conditions"], response.data

tests/sentry/api/endpoints/test_organization_dashboards.py

@@ -5,6 +5,7 @@ from sentry.models import (
     DashboardTombstone,
     DashboardWidget,
     DashboardWidgetDisplayTypes,
+    DashboardWidgetTypes,
 )
 from sentry.testutils import OrganizationDashboardWidgetTestCase
 from sentry.testutils.helpers.datetime import before_now
@@ -26,6 +27,7 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
             order=0,
             title="Widget 1",
             display_type=DashboardWidgetDisplayTypes.LINE_CHART,
+            widget_type=DashboardWidgetTypes.DISCOVER,
             interval="1d",
         )
 

tests/sentry/deletions/test_organization.py

@@ -8,6 +8,7 @@ from sentry.models import (
     Dashboard,
     DashboardWidget,
     DashboardWidgetQuery,
+    DashboardWidgetTypes,
     Environment,
     ExternalIssue,
     Group,
@@ -59,10 +60,18 @@ class DeleteOrganizationTest(TransactionTestCase):
             organization_id=org.id, title="The Dashboard", created_by=self.user
         )
         widget_1 = DashboardWidget.objects.create(
-            dashboard=dashboard, order=1, title="Widget 1", display_type=0
+            dashboard=dashboard,
+            order=1,
+            title="Widget 1",
+            display_type=0,
+            widget_type=DashboardWidgetTypes.DISCOVER,
         )
         widget_2 = DashboardWidget.objects.create(
-            dashboard=dashboard, order=2, title="Widget 2", display_type=5
+            dashboard=dashboard,
+            order=2,
+            title="Widget 2",
+            display_type=5,
+            widget_type=DashboardWidgetTypes.DISCOVER,
         )
         widget_1_data = DashboardWidgetQuery.objects.create(
             widget=widget_1, order=1, name="Incoming data"