feat(discover) Add a display property to saved queries and EventView (#18161)

We want the new display mode selector to be persistent with the saved
query. This means we'll need it in both EventView and the saved query
models. The initial display menu will include:

* None - No additional options
* Releases - The release markers
* Previous - Previous period data

And in the future we'll add a 'top5' option to allow top5 mode
to be entered.

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

@@ -24,6 +24,7 @@ class DiscoverSavedQuerySerializer(Serializer):
             "orderby",
             "limit",
             "yAxis",
+            "display",
         ]
 
         data = {

src/sentry/discover/endpoints/serializers.py

@@ -162,9 +162,10 @@ class DiscoverSavedQuerySerializer(serializers.Serializer):
     query = serializers.CharField(required=False, allow_null=True)
     widths = ListField(child=serializers.CharField(), required=False, allow_null=True)
     yAxis = serializers.CharField(required=False, allow_null=True)
+    display = serializers.CharField(required=False, allow_null=True)
 
     disallowed_fields = {
-        1: set(["environment", "query", "yAxis"]),
+        1: set(["environment", "query", "yAxis", "display"]),
         2: set(["groupby", "rollup", "aggregations", "conditions", "limit"]),
     }
 
@@ -200,6 +201,7 @@ class DiscoverSavedQuerySerializer(serializers.Serializer):
             "limit",
             "widths",
             "yAxis",
+            "display",
         ]
 
         for key in query_keys:

src/sentry/static/sentry/app/types/index.tsx

@@ -938,6 +938,7 @@ export type NewQuery = {
   environment?: Readonly<string[]>;
   tags?: Readonly<string[]>;
   yAxis?: string;
+  display?: string;
   createdBy?: User;
 };
 

src/sentry/static/sentry/app/utils/discover/eventView.tsx

@@ -256,6 +256,7 @@ class EventView {
   statsPeriod: string | undefined;
   environment: Readonly<string[]>;
   yAxis: string | undefined;
+  display: string | undefined;
   createdBy: User | undefined;
 
   constructor(props: {
@@ -270,6 +271,7 @@ class EventView {
     statsPeriod: string | undefined;
     environment: Readonly<string[]>;
     yAxis: string | undefined;
+    display: string | undefined;
     createdBy: User | undefined;
   }) {
     const fields: Field[] = Array.isArray(props.fields) ? props.fields : [];
@@ -283,7 +285,6 @@ class EventView {
       .filter((sortKey): sortKey is string => !!sortKey);
 
     const sort = sorts.find(currentSort => sortKeys.includes(currentSort.field));
-
     sorts = sort ? [sort] : [];
 
     const id = props.id !== null && props.id !== void 0 ? String(props.id) : void 0;
@@ -299,6 +300,7 @@ class EventView {
     this.statsPeriod = props.statsPeriod;
     this.environment = environment;
     this.yAxis = props.yAxis;
+    this.display = props.display;
     this.createdBy = props.createdBy;
   }
 
@@ -317,6 +319,7 @@ class EventView {
       statsPeriod: decodeScalar(statsPeriod),
       environment: collectQueryStringByKey(location.query, 'environment'),
       yAxis: decodeScalar(location.query.yAxis),
+      display: decodeScalar(location.query.display),
       createdBy: undefined,
     });
   }
@@ -357,8 +360,6 @@ class EventView {
 
       return {field, width};
     });
-    const yAxis = saved.yAxis;
-
     // normalize datetime selection
     const {start, end, statsPeriod} = getParams({
       start: saved.start,
@@ -382,7 +383,8 @@ class EventView {
         },
         'environment'
       ),
-      yAxis,
+      yAxis: saved.yAxis,
+      display: saved.display,
       createdBy: saved.createdBy,
     });
   }
@@ -398,6 +400,7 @@ class EventView {
       'project',
       'environment',
       'yAxis',
+      'display',
     ];
 
     for (const key of keys) {
@@ -447,6 +450,7 @@ class EventView {
       range: this.statsPeriod,
       environment: this.environment,
       yAxis: this.yAxis,
+      display: this.display,
     };
 
     if (!newQuery.query) {
@@ -484,6 +488,7 @@ class EventView {
       tag: undefined,
       query: undefined,
       yAxis: undefined,
+      display: undefined,
     };
 
     for (const field of EXTERNAL_QUERY_STRING_KEYS) {
@@ -504,6 +509,7 @@ class EventView {
       project: this.project,
       query: this.query,
       yAxis: this.yAxis,
+      display: this.display,
     };
 
     for (const field of EXTERNAL_QUERY_STRING_KEYS) {
@@ -560,6 +566,7 @@ class EventView {
       statsPeriod: this.statsPeriod,
       environment: this.environment,
       yAxis: this.yAxis,
+      display: this.display,
       createdBy: this.createdBy,
     });
   }
@@ -798,21 +805,6 @@ class EventView {
     return newEventView;
   }
 
-  withMovedColumn({fromIndex, toIndex}: {fromIndex: number; toIndex: number}): EventView {
-    if (fromIndex === toIndex) {
-      return this;
-    }
-
-    const newEventView = this.clone();
-
-    const fields = [...newEventView.fields];
-    fields.splice(toIndex, 0, fields.splice(fromIndex, 1)[0]);
-
-    newEventView.fields = fields;
-
-    return newEventView;
-  }
-
   getSorts(): TableColumnSort<React.ReactText>[] {
     return this.sorts.map(
       sort =>

tests/js/spec/utils/discover/eventView.spec.jsx

@@ -32,6 +32,7 @@ describe('EventView constructor', function() {
       statsPeriod: undefined,
       environment: [],
       yAxis: undefined,
+      display: undefined,
     });
   });
 });
@@ -52,6 +53,7 @@ describe('EventView.fromLocation()', function() {
         statsPeriod: '14d',
         environment: ['staging'],
         yAxis: 'p95',
+        display: 'previous',
       },
     };
 
@@ -72,6 +74,7 @@ describe('EventView.fromLocation()', function() {
       statsPeriod: '14d',
       environment: ['staging'],
       yAxis: 'p95',
+      display: 'previous',
     });
   });
 
@@ -181,6 +184,7 @@ describe('EventView.fromSavedQuery()', function() {
       end: '2019-10-02T00:00:00',
       orderby: '-id',
       environment: ['staging'],
+      display: 'previous',
     };
     const eventView = EventView.fromSavedQuery(saved);
 
@@ -200,6 +204,7 @@ describe('EventView.fromSavedQuery()', function() {
       statsPeriod: '14d',
       environment: ['staging'],
       yAxis: undefined,
+      display: 'previous',
     });
 
     const eventView2 = EventView.fromSavedQuery({
@@ -521,6 +526,7 @@ describe('EventView.generateQueryStringObject()', function() {
       start: null,
       end: undefined,
       yAxis: undefined,
+      display: 'previous',
     });
 
     const expected = {
@@ -532,6 +538,7 @@ describe('EventView.generateQueryStringObject()', function() {
       query: '',
       project: [],
       environment: [],
+      display: 'previous',
     };
 
     expect(eventView.generateQueryStringObject()).toEqual(expected);
@@ -553,6 +560,7 @@ describe('EventView.generateQueryStringObject()', function() {
       statsPeriod: '14d',
       environment: ['staging'],
       yAxis: 'count()',
+      display: 'releases',
     };
 
     const eventView = new EventView(state);
@@ -570,6 +578,7 @@ describe('EventView.generateQueryStringObject()', function() {
       statsPeriod: '14d',
       environment: ['staging'],
       yAxis: 'count()',
+      display: 'releases',
     };
 
     expect(eventView.generateQueryStringObject()).toEqual(expected);
@@ -611,6 +620,7 @@ describe('EventView.getEventsAPIPayload()', function() {
       project: [567],
       environment: ['prod'],
       yAxis: 'users',
+      display: 'releases',
     });
 
     expect(eventView.getEventsAPIPayload({})).toEqual({
@@ -864,6 +874,7 @@ describe('EventView.getFacetsAPIPayload()', function() {
         sort: 'the world',
         project: '1234',
         environment: ['staging'],
+        display: 'releases',
       },
     };
 
@@ -893,6 +904,7 @@ describe('EventView.toNewQuery()', function() {
     end: '2019-10-02T00:00:00',
     statsPeriod: '14d',
     environment: ['staging'],
+    display: 'releases',
   };
 
   it('outputs the right fields', function() {
@@ -913,6 +925,7 @@ describe('EventView.toNewQuery()', function() {
       end: '2019-10-02T00:00:00',
       range: '14d',
       environment: ['staging'],
+      display: 'releases',
     };
 
     expect(output).toEqual(expected);
@@ -941,6 +954,7 @@ describe('EventView.toNewQuery()', function() {
       end: '2019-10-02T00:00:00',
       range: '14d',
       environment: ['staging'],
+      display: 'releases',
     };
 
     expect(output).toEqual(expected);
@@ -969,6 +983,7 @@ describe('EventView.toNewQuery()', function() {
       end: '2019-10-02T00:00:00',
       range: '14d',
       environment: ['staging'],
+      display: 'releases',
     };
 
     expect(output).toEqual(expected);
@@ -1046,6 +1061,7 @@ describe('EventView.clone()', function() {
       end: '2019-10-02T00:00:00',
       statsPeriod: '14d',
       environment: ['staging'],
+      display: 'releases',
     };
 
     const eventView = new EventView(state);
@@ -1659,46 +1675,6 @@ describe('EventView.withDeletedColumn()', function() {
   });
 });
 
-describe('EventView.withMovedColumn()', function() {
-  const state = {
-    id: '1234',
-    name: 'best query',
-    fields: [{field: 'count()'}, {field: 'project.id'}],
-    sorts: generateSorts(['count']),
-    query: 'event.type:error',
-    project: [42],
-    start: '2019-10-01T00:00:00',
-    end: '2019-10-02T00:00:00',
-    statsPeriod: '14d',
-    environment: ['staging'],
-  };
-
-  it('returns itself when attempting to move column to the same placement', function() {
-    const eventView = new EventView(state);
-
-    const eventView2 = eventView.withMovedColumn({fromIndex: 0, toIndex: 0});
-
-    expect(eventView2 === eventView).toBeTruthy();
-    expect(eventView).toMatchObject(state);
-  });
-
-  it('move column', function() {
-    const eventView = new EventView(state);
-
-    const eventView2 = eventView.withMovedColumn({fromIndex: 0, toIndex: 1});
-
-    expect(eventView2 !== eventView).toBeTruthy();
-    expect(eventView).toMatchObject(state);
-
-    const nextState = {
-      ...state,
-      fields: [...state.fields].reverse(),
-    };
-
-    expect(eventView2).toMatchObject(nextState);
-  });
-});
-
 describe('EventView.getSorts()', function() {
   it('returns fields', function() {
     const eventView = new EventView({
@@ -1875,6 +1851,7 @@ describe('EventView.isEqualTo()', function() {
       statsPeriod: '14d',
       environment: ['staging'],
       yAxis: 'fam',
+      display: 'releases',
     };
 
     const eventView = new EventView(state);
@@ -1926,6 +1903,7 @@ describe('EventView.isEqualTo()', function() {
       statsPeriod: '14d',
       environment: ['staging'],
       yAxis: 'fam',
+      display: 'releases',
     };
 
     const differences = {
@@ -1940,6 +1918,7 @@ describe('EventView.isEqualTo()', function() {
       statsPeriod: '24d',
       environment: [],
       yAxis: 'ok boomer',
+      display: 'previous',
     };
     const eventView = new EventView(state);
 
@@ -1962,6 +1941,7 @@ describe('EventView.getResultsViewUrlTarget()', function() {
     end: '2019-10-02T00:00:00',
     statsPeriod: '14d',
     environment: ['staging'],
+    display: 'previous',
   };
   const organization = TestStubs.Organization();
 
@@ -1971,6 +1951,7 @@ describe('EventView.getResultsViewUrlTarget()', function() {
     expect(result.pathname).toEqual('/organizations/org-slug/discover/results/');
     expect(result.query.query).toEqual(state.query);
     expect(result.query.project).toEqual(state.project);
+    expect(result.query.display).toEqual(state.display);
   });
 });
 
@@ -2337,20 +2318,6 @@ describe('isAPIPayloadSimilar', function() {
 
       expect(results).toBe(false);
     });
-
-    it('is similar when a column is moved', function() {
-      const thisEventView = new EventView(state);
-      const location = {};
-      const thisAPIPayload = thisEventView.getEventsAPIPayload(location);
-
-      const otherEventView = thisEventView.withMovedColumn({fromIndex: 0, toIndex: 1});
-      const otherLocation = {};
-      const otherAPIPayload = otherEventView.getEventsAPIPayload(otherLocation);
-
-      const results = isAPIPayloadSimilar(thisAPIPayload, otherAPIPayload);
-
-      expect(results).toBe(true);
-    });
   });
 
   describe('getFacetsAPIPayload', function() {

tests/snuba/api/endpoints/test_discover_saved_queries.py

@@ -269,6 +269,7 @@ class DiscoverSavedQueriesVersion2Test(DiscoverSavedQueryBase):
                     "query": "event.type:error browser.name:Firefox",
                     "range": "24h",
                     "yAxis": "count(id)",
+                    "display": "releases",
                     "version": 2,
                 },
             )
@@ -279,6 +280,7 @@ class DiscoverSavedQueriesVersion2Test(DiscoverSavedQueryBase):
         assert data["environment"] == ["dev"]
         assert data["query"] == "event.type:error browser.name:Firefox"
         assert data["yAxis"] == "count(id)"
+        assert data["display"] == "releases"
         assert data["version"] == 2
 
     def test_post_all_projects(self):