Browse Source

fix(widget-builder): Do widget validation with columns and aggregates as fields (#33986)

There was code to construct columns and aggregates from fields to account for
stale frontend during the transition to split up fields, but enough time has passed
that we can use the columns and aggregates as the source of truth for validation
Nar Saynorath 2 years ago
parent
commit
a88dd006ae

+ 3 - 20
src/sentry/api/serializers/rest_framework/dashboard.py

@@ -141,9 +141,10 @@ class DashboardWidgetQuerySerializer(CamelSnakeSerializer):
         conditions = self._get_attr(data, "conditions", "")
         orderby = self._get_attr(data, "orderby", "")
         is_table = is_table_display_type(self.context.get("displayType"))
+        columns = self._get_attr(data, "columns", []).copy()
+        aggregates = self._get_attr(data, "aggregates", []).copy()
+        fields = columns + aggregates
 
-        # TODO(dam): Use columns and aggregates for validation
-        fields = self._get_attr(data, "fields", []).copy()
         injected_orderby_equation = None
         if is_equation(orderby.lstrip("-")):
             injected_orderby_equation = orderby.lstrip("-")
@@ -156,24 +157,6 @@ class DashboardWidgetQuerySerializer(CamelSnakeSerializer):
         else:
             equations, fields = categorize_columns(fields)
 
-        # TODO(dam): Temp code while we are sure adoption
-        # of frontend code that sends this data is high enough
-        columns = self._get_attr(data, "columns", []).copy()
-        aggregates = self._get_attr(data, "aggregates", []).copy()
-
-        if not columns and not aggregates:
-            # If the orderby is an equation, it was injected into the fields
-            # so it needs to be ignored when filling out columns and aggregates
-            iterable_fields = fields[:-1] if injected_orderby_equation else fields
-            for field in iterable_fields:
-                if is_aggregate(field):
-                    aggregates.append(field)
-                else:
-                    columns.append(field)
-
-            data["columns"] = columns
-            data["aggregates"] = aggregates
-
         try:
             parse_search_query(conditions)
         except InvalidSearchQuery as err:

+ 211 - 24
tests/sentry/api/endpoints/test_organization_dashboard_details.py

@@ -267,7 +267,13 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "displayType": "world_map",
                     "interval": "5m",
                     "queries": [
-                        {"name": "Errors", "fields": ["count()"], "conditions": "event.type:error"}
+                        {
+                            "name": "Errors",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "event.type:error",
+                        }
                     ],
                 },
                 {
@@ -278,6 +284,8 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                         {
                             "name": "Errors",
                             "fields": ["count()", "project"],
+                            "columns": ["project"],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:error",
                         }
                     ],
@@ -335,10 +343,26 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
         data = {
             "title": "First dashboard",
             "widgets": [
-                {"id": str(self.widget_1.id)},
-                {"id": str(self.widget_2.id)},
-                {"id": str(self.widget_3.id)},
-                {"id": str(self.widget_4.id)},
+                {
+                    "id": str(self.widget_1.id),
+                    "columns": [],
+                    "aggregates": [],
+                },
+                {
+                    "id": str(self.widget_2.id),
+                    "columns": [],
+                    "aggregates": [],
+                },
+                {
+                    "id": str(self.widget_3.id),
+                    "columns": [],
+                    "aggregates": [],
+                },
+                {
+                    "id": str(self.widget_4.id),
+                    "columns": [],
+                    "aggregates": [],
+                },
                 {
                     "title": "Error Counts by Country",
                     "displayType": "world_map",
@@ -347,6 +371,7 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                         {
                             "name": "Errors",
                             "fields": [],
+                            "columns": [],
                             "aggregates": ["count()"],
                             "conditions": "event.type:error",
                         }
@@ -389,7 +414,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                 {
                     "displayType": "line",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": ""}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "",
+                        }
+                    ],
                 },
             ],
         }
@@ -406,7 +439,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "displayType": "line",
                     "interval": "5m",
                     "limit": None,
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": ""}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "",
+                        }
+                    ],
                 },
                 {
                     "title": "Duration Distribution",
@@ -421,6 +462,12 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                                 "p75(transaction.duration)",
                                 "p95(transaction.duration)",
                             ],
+                            "columns": [],
+                            "aggregates": [
+                                "p50(transaction.duration)",
+                                "p75(transaction.duration)",
+                                "p95(transaction.duration)",
+                            ],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -454,6 +501,12 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                                 "p75(transaction.duration)",
                                 "p95(transaction.duration)",
                             ],
+                            "columns": [],
+                            "aggregates": [
+                                "p50(transaction.duration)",
+                                "p75(transaction.duration)",
+                                "p95(transaction.duration)",
+                            ],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -482,6 +535,12 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                                 "p75(transaction.duration)",
                                 "p95(transaction.duration)",
                             ],
+                            "columns": [],
+                            "aggregates": [
+                                "p50(transaction.duration)",
+                                "p75(transaction.duration)",
+                                "p95(transaction.duration)",
+                            ],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -501,7 +560,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                 {
                     "title": "Errors",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": ""}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "",
+                        }
+                    ],
                 },
             ],
         }
@@ -522,6 +589,8 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                         {
                             "name": "Errors",
                             "fields": ["p95(transaction.duration)"],
+                            "columns": [],
+                            "aggregates": ["p95(transaction.duration)"],
                             "conditions": "foo: bar:",
                         }
                     ],
@@ -541,7 +610,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "title": "Invalid fields",
                     "displayType": "line",
                     "interval": "5m",
-                    "queries": [{"name": "Errors", "fields": ["wrong()"], "conditions": ""}],
+                    "queries": [
+                        {
+                            "name": "Errors",
+                            "fields": ["wrong()"],
+                            "columns": [],
+                            "aggregates": ["wrong()"],
+                            "conditions": "",
+                        }
+                    ],
                 },
             ],
         }
@@ -557,7 +634,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                 {
                     "title": "Invalid fields",
                     "displayType": "line",
-                    "queries": [{"name": "Errors", "fields": ["p95(user)"], "conditions": ""}],
+                    "queries": [
+                        {
+                            "name": "Errors",
+                            "fields": ["p95(user)"],
+                            "columns": [],
+                            "aggregates": ["p95(user)"],
+                            "conditions": "",
+                        }
+                    ],
                 },
             ],
         }
@@ -578,6 +663,8 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                         {
                             "name": "Durations",
                             "fields": ["p95(transaction.duration)"],
+                            "columns": [],
+                            "aggregates": ["p95(transaction.duration)"],
                             "conditions": "",
                         }
                     ],
@@ -612,10 +699,16 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "id": str(self.widget_1.id),
                     "title": "New title",
                     "queries": [
-                        {"id": str(self.widget_1_data_1.id)},
+                        {
+                            "id": str(self.widget_1_data_1.id),
+                            "columns": [],
+                            "aggregates": [],
+                        },
                         {
                             "name": "transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         },
                     ],
@@ -648,6 +741,8 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                             "id": str(self.widget_1_data_1.id),
                             "name": "transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         },
                     ],
@@ -674,8 +769,16 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "id": str(self.widget_1.id),
                     "title": "New title",
                     "queries": [
-                        {"id": str(self.widget_1_data_2.id)},
-                        {"id": str(self.widget_1_data_1.id)},
+                        {
+                            "id": str(self.widget_1_data_2.id),
+                            "columns": [],
+                            "aggregates": [],
+                        },
+                        {
+                            "id": str(self.widget_1_data_1.id),
+                            "columns": [],
+                            "aggregates": [],
+                        },
                     ],
                 },
                 {"id": str(self.widget_2.id)},
@@ -700,7 +803,13 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                 {
                     "id": str(self.widget_1.id),
                     "title": "New title",
-                    "queries": [{"id": str(self.widget_2_data_1.id)}],
+                    "queries": [
+                        {
+                            "id": str(self.widget_2_data_1.id),
+                            "columns": [],
+                            "aggregates": [],
+                        }
+                    ],
                 },
             ],
         }
@@ -717,6 +826,8 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "queries": [
                         {
                             "fields": ["title", "count()"],
+                            "columns": ["title"],
+                            "aggregates": ["count()"],
                             "conditions": "",
                             "orderby": "message",
                         }
@@ -739,7 +850,13 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "title": "Errors over time",
                     "displayType": "line",
                     "queries": [
-                        {"name": "Errors", "fields": ["count()"], "conditions": "event.type:error"}
+                        {
+                            "name": "Errors",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "event.type:error",
+                        }
                     ],
                 },
                 {"id": str(self.widget_4.id)},
@@ -764,7 +881,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "id": str(self.widget_1.id),
                     "title": "Invalid fields",
                     "displayType": "line",
-                    "queries": [{"name": "Errors", "fields": ["p95(user)"], "conditions": ""}],
+                    "queries": [
+                        {
+                            "name": "Errors",
+                            "fields": ["p95(user)"],
+                            "columns": [],
+                            "aggregates": ["p95(user)"],
+                            "conditions": "",
+                        }
+                    ],
                 },
             ],
         }
@@ -780,7 +905,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "id": str(self.widget_1.id),
                     "title": "Invalid fields",
                     "displayType": "line",
-                    "queries": [{"name": "Errors", "fields": ["p95()"], "conditions": "foo: bar:"}],
+                    "queries": [
+                        {
+                            "name": "Errors",
+                            "fields": ["p95()"],
+                            "columns": [],
+                            "aggregates": ["p95()"],
+                            "conditions": "foo: bar:",
+                        }
+                    ],
                 },
             ],
         }
@@ -926,6 +1059,8 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                         {
                             "name": "transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         },
                     ],
@@ -1009,7 +1144,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "displayType": "table",
                     "widgetType": "issue",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:unresolved"}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "is:unresolved",
+                        }
+                    ],
                 },
             ],
         }
@@ -1026,7 +1169,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "displayType": "table",
                     "widgetType": "issue",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:())"}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "is:())",
+                        }
+                    ],
                 },
             ],
         }
@@ -1044,7 +1195,15 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "displayType": "table",
                     "widgetType": "discover",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:unresolved"}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "is:unresolved",
+                        }
+                    ],
                 },
             ],
         }
@@ -1062,14 +1221,30 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "displayType": "table",
                     "widgetType": "issue",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:unresolved"}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "is:unresolved",
+                        }
+                    ],
                 },
                 {
                     "title": "Resolved Issues",
                     "displayType": "table",
                     "widgetType": "issue",
                     "interval": "5m",
-                    "queries": [{"name": "", "fields": ["count()"], "conditions": "is:resolved"}],
+                    "queries": [
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "is:resolved",
+                        }
+                    ],
                 },
                 {
                     "title": "Transactions",
@@ -1077,7 +1252,13 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "widgetType": "discover",
                     "interval": "5m",
                     "queries": [
-                        {"name": "", "fields": ["count()"], "conditions": "event.type:transaction"}
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "event.type:transaction",
+                        }
                     ],
                 },
                 {
@@ -1086,7 +1267,13 @@ class OrganizationDashboardDetailsPutTest(OrganizationDashboardDetailsTestCase):
                     "widgetType": "discover",
                     "interval": "5m",
                     "queries": [
-                        {"name": "", "fields": ["count()"], "conditions": "event.type:error"}
+                        {
+                            "name": "",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "event.type:error",
+                        }
                     ],
                 },
             ],

+ 125 - 10
tests/sentry/api/endpoints/test_organization_dashboard_widget_details.py

@@ -16,11 +16,19 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Errors over time",
             "displayType": "line",
             "queries": [
-                {"name": "errors", "conditions": "event.type:error", "fields": ["count()"]},
+                {
+                    "name": "errors",
+                    "conditions": "event.type:error",
+                    "fields": ["count()"],
+                    "columns": [],
+                    "aggregates": ["count()"],
+                },
                 {
                     "name": "errors",
                     "conditions": "(level:error OR title:*Error*) !release:latest",
                     "fields": ["count()"],
+                    "columns": [],
+                    "aggregates": ["count()"],
                     "orderby": "count()",
                 },
             ],
@@ -41,7 +49,13 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Invalid query",
             "displayType": "line",
             "queries": [
-                {"name": "errors", "conditions": "event.type: tag:foo", "fields": ["count()"]}
+                {
+                    "name": "errors",
+                    "conditions": "event.type: tag:foo",
+                    "fields": ["count()"],
+                    "columns": [],
+                    "aggregates": ["count()"],
+                }
             ],
         }
         response = self.do_request(
@@ -62,7 +76,13 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Invalid query",
             "displayType": "line",
             "queries": [
-                {"name": "errors", "conditions": "event.type:error", "fields": ["p95(user)"]}
+                {
+                    "name": "errors",
+                    "conditions": "event.type:error",
+                    "fields": ["p95(user)"],
+                    "columns": [],
+                    "aggregates": ["p95(user)"],
+                }
             ],
         }
         response = self.do_request(
@@ -79,7 +99,13 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Invalid query",
             "displayType": "cats",
             "queries": [
-                {"name": "errors", "conditions": "event.type:error", "fields": ["count()"]}
+                {
+                    "name": "errors",
+                    "conditions": "event.type:error",
+                    "fields": ["count()"],
+                    "columns": [],
+                    "aggregates": ["count()"],
+                }
             ],
         }
         response = self.do_request(
@@ -99,6 +125,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "errors",
                     "conditions": "event.type:error",
                     "fields": ["equation|count()"],
+                    "columns": [],
+                    "aggregates": ["equation|count()"],
                 }
             ],
         }
@@ -119,6 +147,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "errors",
                     "conditions": "event.type:error",
                     "fields": ["equation|count() * 2"],
+                    "columns": [],
+                    "aggregates": ["equation|count() * 2"],
                 }
             ],
         }
@@ -138,6 +168,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "errors",
                     "conditions": "event.type:error",
                     "fields": ["equation|count() * 2"],
+                    "columns": [],
+                    "aggregates": ["equation|count() * 2"],
                     "orderby": "equation[0]",
                 }
             ],
@@ -158,6 +190,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "errors",
                     "conditions": "event.type:error",
                     "fields": ["equation|count() * 2"],
+                    "columns": [],
+                    "aggregates": ["equation|count() * 2"],
                     "orderby": "equation[999999]",
                 }
             ],
@@ -179,6 +213,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "errors",
                     "conditions": "event.type:error",
                     "fields": [""],
+                    "columns": [],
+                    "aggregates": [],
                     "orderby": "equation[0]",
                 }
             ],
@@ -200,6 +236,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "errors",
                     "conditions": "event.type:error",
                     "fields": ["equation|count() * 2"],
+                    "columns": [],
+                    "aggregates": ["equation|count() * 2"],
                 }
             ],
         }
@@ -215,7 +253,16 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
         data = {
             "title": "EPM Big Number",
             "displayType": "big_number",
-            "queries": [{"name": "", "fields": ["epm()"], "conditions": "", "orderby": ""}],
+            "queries": [
+                {
+                    "name": "",
+                    "fields": ["epm()"],
+                    "columns": [],
+                    "aggregates": ["epm()"],
+                    "conditions": "",
+                    "orderby": "",
+                }
+            ],
         }
         response = self.do_request(
             "post",
@@ -240,6 +287,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                 {
                     "name": "",
                     "fields": ["epm()"],
+                    "columns": [],
+                    "aggregates": ["epm()"],
                     "conditions": f"project:{self.project.name}",
                     "orderby": "",
                 }
@@ -277,6 +326,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                 {
                     "name": "",
                     "fields": ["epm()"],
+                    "columns": [],
+                    "aggregates": ["epm()"],
                     "conditions": f"issue:{event.group.qualified_short_id}",
                     "orderby": "",
                 }
@@ -294,7 +345,15 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Unresolved Issues",
             "displayType": "table",
             "widgetType": "issue",
-            "queries": [{"name": "unresolved", "conditions": "is:unresolved", "fields": []}],
+            "queries": [
+                {
+                    "name": "unresolved",
+                    "conditions": "is:unresolved",
+                    "fields": [],
+                    "columns": [],
+                    "aggregates": [],
+                }
+            ],
         }
         response = self.do_request(
             "post",
@@ -308,7 +367,15 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Unresolved Issues",
             "displayType": "table",
             "widgetType": "issue",
-            "queries": [{"name": "unresolved", "conditions": "is:())", "fields": []}],
+            "queries": [
+                {
+                    "name": "unresolved",
+                    "conditions": "is:())",
+                    "fields": [],
+                    "columns": [],
+                    "aggregates": [],
+                }
+            ],
         }
         response = self.do_request(
             "post",
@@ -324,7 +391,15 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "title": "Unresolved Issues",
             "displayType": "table",
             "widgetType": "discover",
-            "queries": [{"name": "unresolved", "conditions": "is:unresolved", "fields": []}],
+            "queries": [
+                {
+                    "name": "unresolved",
+                    "conditions": "is:unresolved",
+                    "fields": [],
+                    "columns": [],
+                    "aggregates": [],
+                }
+            ],
         }
         response = self.do_request(
             "post",
@@ -341,7 +416,14 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "displayType": "table",
             "widgetType": "discover",
             "queries": [
-                {"name": "", "conditions": "", "fields": [], "orderby": "equation|count() * 2"}
+                {
+                    "name": "",
+                    "conditions": "",
+                    "fields": [],
+                    "columns": [],
+                    "aggregates": [],
+                    "orderby": "equation|count() * 2",
+                }
             ],
         }
         response = self.do_request(
@@ -357,7 +439,14 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
             "displayType": "table",
             "widgetType": "discover",
             "queries": [
-                {"name": "", "conditions": "", "fields": [], "orderby": "-equation|count() * 2"}
+                {
+                    "name": "",
+                    "conditions": "",
+                    "fields": [],
+                    "columns": [],
+                    "aggregates": [],
+                    "orderby": "-equation|count() * 2",
+                }
             ],
         }
         response = self.do_request(
@@ -377,6 +466,8 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
                     "name": "",
                     "conditions": "",
                     "fields": [],
+                    "columns": [],
+                    "aggregates": [],
                     "orderby": "-equation|thisIsNotARealEquation() * 42",
                 }
             ],
@@ -388,3 +479,27 @@ class OrganizationDashboardWidgetDetailsTestCase(OrganizationDashboardWidgetTest
         )
         assert response.status_code == 400, response.data
         assert "queries" in response.data, response.data
+
+    def test_save_with_no_fields_and_orderby(self):
+        data = {
+            "title": "Test Query",
+            "displayType": "line",
+            "widgetType": "discover",
+            "limit": 5,
+            "queries": [
+                {
+                    "name": "",
+                    "conditions": "",
+                    "fields": ["count()"],
+                    "columns": ["project"],
+                    "aggregates": ["count()"],
+                    "orderby": "-project",
+                }
+            ],
+        }
+        response = self.do_request(
+            "post",
+            self.url(),
+            data=data,
+        )
+        assert response.status_code == 200, response.data

+ 34 - 41
tests/sentry/api/endpoints/test_organization_dashboards.py

@@ -247,6 +247,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -257,7 +259,13 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                     "interval": "5m",
                     "title": "Error count()",
                     "queries": [
-                        {"name": "Errors", "fields": ["count()"], "conditions": "event.type:error"}
+                        {
+                            "name": "Errors",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "event.type:error",
+                        }
                     ],
                     "layout": {"x": 1, "y": 0, "w": 1, "h": 1, "minH": 2},
                 },
@@ -294,6 +302,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -326,6 +336,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -363,6 +375,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -382,6 +396,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                 {
                     "name": "Transactions",
                     "fields": ["count()"],
+                    "columns": [],
+                    "aggregates": ["count()"],
                     "conditions": "event.type:transaction",
                 }
             ],
@@ -423,6 +439,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -445,6 +463,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -455,45 +475,6 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
         response = self.do_request("post", self.url, data=data)
         assert response.status_code == 400, response.data
 
-    def test_post_widgets_with_null_columns_and_aggregates_succeeds_and_sets_value(self):
-        data = {
-            "title": "Dashboard with null agg and cols",
-            "widgets": [
-                {
-                    "displayType": "line",
-                    "interval": "5m",
-                    "title": "Transaction count()",
-                    "queries": [
-                        {
-                            "name": "Transactions",
-                            "fields": ["count()"],
-                            "columns": None,
-                            "aggregates": None,
-                            "conditions": "event.type:transaction",
-                        }
-                    ],
-                    "layout": {"x": 0, "y": 0, "w": 1, "h": 1, "minH": 2},
-                },
-            ],
-        }
-        response = self.do_request("post", self.url, data=data)
-        assert response.status_code == 201, response.data
-        dashboard = Dashboard.objects.get(
-            organization=self.organization, title="Dashboard with null agg and cols"
-        )
-        assert dashboard.created_by == self.user
-
-        widgets = self.get_widgets(dashboard.id)
-        assert len(widgets) == 1
-
-        for expected_widget, actual_widget in zip(data["widgets"], widgets):
-            self.assert_serialized_widget(expected_widget, actual_widget)
-            queries = actual_widget.dashboardwidgetquery_set.all()
-            for expected_query, actual_query in zip(expected_widget["queries"], queries):
-                expected_query["columns"] = []
-                expected_query["aggregates"] = ["count()"]
-                self.assert_serialized_widget_query(expected_query, actual_query)
-
     def test_add_widget_with_limit(self):
         data = {
             "title": "Dashboard from Post",
@@ -507,6 +488,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -517,7 +500,13 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                     "limit": 5,
                     "title": "Error count()",
                     "queries": [
-                        {"name": "Errors", "fields": ["count()"], "conditions": "event.type:error"}
+                        {
+                            "name": "Errors",
+                            "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
+                            "conditions": "event.type:error",
+                        }
                     ],
                 },
             ],
@@ -545,6 +534,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],
@@ -568,6 +559,8 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
                         {
                             "name": "Transactions",
                             "fields": ["count()"],
+                            "columns": [],
+                            "aggregates": ["count()"],
                             "conditions": "event.type:transaction",
                         }
                     ],