feat(discover): Disable saving/updating queries (#19005)

* feat(discover): Disable saving/updating queries

- This will disable the button group when the query is invalid (ie.
  status 4xx)
- This also prevents the actual saving when the query is invalid as
  well.

src/sentry/discover/endpoints/serializers.py

@@ -8,6 +8,7 @@ from rest_framework.exceptions import PermissionDenied
 from sentry.models import Project, ProjectStatus
 from sentry.discover.models import KeyTransaction, MAX_KEY_TRANSACTIONS
 from sentry.api.fields.empty_integer import EmptyIntegerField
+from sentry.api.event_search import get_filter, InvalidSearchQuery
 from sentry.api.serializers.rest_framework import ListField
 from sentry.api.utils import get_date_range_from_params, InvalidParams
 from sentry.constants import ALL_ACCESS_PROJECTS
@@ -214,6 +215,12 @@ class DiscoverSavedQuerySerializer(serializers.Serializer):
             if len(query["fields"]) < 1:
                 raise serializers.ValidationError("You must include at least one field.")
 
+        if "query" in query:
+            try:
+                get_filter(query["query"])
+            except InvalidSearchQuery as err:
+                raise serializers.ValidationError("Cannot save invalid query: {}".format(err))
+
         if data["projects"] == ALL_ACCESS_PROJECTS:
             data["projects"] = []
             query["all_projects"] = True

src/sentry/static/sentry/app/views/eventsV2/results.tsx

@@ -44,6 +44,7 @@ type Props = {
 type State = {
   eventView: EventView;
   error: string;
+  errorCode: number;
   totalValues: null | number;
 };
 
@@ -56,6 +57,7 @@ class Results extends React.Component<Props, State> {
   state = {
     eventView: EventView.fromLocation(this.props.location),
     error: '',
+    errorCode: 200,
     totalValues: null,
   };
 
@@ -221,13 +223,13 @@ class Results extends React.Component<Props, State> {
     );
   }
 
-  setError = (error: string) => {
-    this.setState({error});
+  setError = (error: string, errorCode: number) => {
+    this.setState({error, errorCode});
   };
 
   render() {
     const {organization, location, router, api} = this.props;
-    const {eventView, error, totalValues} = this.state;
+    const {eventView, error, errorCode, totalValues} = this.state;
     const query = location.query.query || '';
     const title = this.getDocumentTitle();
 
@@ -236,6 +238,7 @@ class Results extends React.Component<Props, State> {
         <StyledPageContent>
           <LightWeightNoProjectMessage organization={organization}>
             <ResultsHeader
+              errorCode={errorCode}
               organization={organization}
               location={location}
               eventView={eventView}

src/sentry/static/sentry/app/views/eventsV2/resultsHeader.tsx

@@ -20,6 +20,7 @@ type Props = {
   api: Client;
   organization: Organization;
   location: Location;
+  errorCode: number;
   eventView: EventView;
 };
 
@@ -61,7 +62,7 @@ class ResultsHeader extends React.Component<Props, State> {
   }
 
   render() {
-    const {organization, location, eventView} = this.props;
+    const {organization, location, errorCode, eventView} = this.props;
     const {savedQuery, loading} = this.state;
 
     const renderDisabled = p => (
@@ -105,7 +106,7 @@ class ResultsHeader extends React.Component<Props, State> {
                 eventView={eventView}
                 savedQuery={savedQuery}
                 savedQueryLoading={loading}
-                disabled={!hasFeature}
+                disabled={!hasFeature || (errorCode >= 400 && errorCode < 500)}
               />
             )}
           </Feature>

src/sentry/static/sentry/app/views/eventsV2/table/index.tsx

@@ -20,7 +20,7 @@ type TableProps = {
   eventView: EventView;
   organization: Organization;
   tags: {[key: string]: Tag};
-  setError: (msg: string) => void;
+  setError: (msg: string, code: number) => void;
   title: string;
 };
 
@@ -81,7 +81,7 @@ class Table extends React.PureComponent<TableProps, TableState> {
     const url = `/organizations/${organization.slug}/eventsv2/`;
     const tableFetchID = Symbol('tableFetchID');
     const apiPayload = eventView.getEventsAPIPayload(location);
-    setError('');
+    setError('', 200);
 
     this.setState({isLoading: true, tableFetchID});
     metric.mark({name: `discover-events-start-${apiPayload.query}`});
@@ -130,7 +130,7 @@ class Table extends React.PureComponent<TableProps, TableState> {
           pageLinks: null,
           tableData: null,
         });
-        setError(message);
+        setError(message, err.status);
       });
   };
 

tests/snuba/api/endpoints/test_discover_saved_queries.py

@@ -298,3 +298,20 @@ class DiscoverSavedQueriesVersion2Test(DiscoverSavedQueryBase):
             )
         assert response.status_code == 201, response.content
         assert response.data["projects"] == [-1]
+
+    def test_save_invalid_query(self):
+        with self.feature(self.feature_name):
+            url = reverse("sentry-api-0-discover-saved-queries", args=[self.org.slug])
+            response = self.client.post(
+                url,
+                {
+                    "name": "Bad query",
+                    "projects": [-1],
+                    "fields": ["title", "count()"],
+                    "range": "24h",
+                    "query": "spaceAfterColon: 1",
+                    "version": 2,
+                },
+            )
+        assert response.status_code == 400, response.content
+        assert not DiscoverSavedQuery.objects.filter(name="Bad query").exists()