fix(discover): Fix results not updating after changing a field [SEN-410] (#12562)

Results were not re-rendering how you would expect it to (not exactly sure what was going on but in Results.shouldComponentUpdate, nextProps was equal to current props) )

Fixes SEN-410

src/sentry/static/sentry/app/views/organizationDiscover/discover.jsx

@@ -43,6 +43,7 @@ import createResultManager from './resultManager';
 export default class OrganizationDiscover extends React.Component {
   static propTypes = {
     organization: SentryTypes.Organization.isRequired,
+    location: PropTypes.object.isRequired,
     queryBuilder: PropTypes.object.isRequired,
     // savedQuery and isEditingSavedQuery are provided if it's a saved query
     savedQuery: SentryTypes.DiscoverSavedQuery,
@@ -225,6 +226,10 @@ export default class OrganizationDiscover extends React.Component {
         if (shouldRedirect) {
           browserHistory.push({
             pathname: `/organizations/${organization.slug}/discover/`,
+            // This is kind of a hack, but this causes a re-render in result where this.props == nextProps after
+            // a query has completed... we don't preserve `state` when we update browser history, so
+            // if this is present in `Result.shouldComponentUpdate` then should perform a render
+            state: 'fetching',
             // Don't drop "visualization" from querystring
             search: getQueryStringFromQuery(queryBuilder.getInternal(), {
               ...(location.query.visualization && {
@@ -357,6 +362,7 @@ export default class OrganizationDiscover extends React.Component {
       savedQuery,
       toggleEditMode,
       isLoading,
+      location,
       utc,
     } = this.props;
 
@@ -434,6 +440,7 @@ export default class OrganizationDiscover extends React.Component {
           <BodyContent>
             {shouldDisplayResult && (
               <Result
+                location={location}
                 utc={utc}
                 data={data}
                 savedQuery={savedQuery}

src/sentry/static/sentry/app/views/organizationDiscover/result/index.jsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import {browserHistory, withRouter} from 'react-router';
+import {browserHistory} from 'react-router';
 import PropTypes from 'prop-types';
 import {throttle, isEqual} from 'lodash';
 
@@ -41,6 +41,7 @@ import {
 class Result extends React.Component {
   static propTypes = {
     data: PropTypes.object.isRequired,
+    location: PropTypes.object.isRequired,
     savedQuery: SentryTypes.DiscoverSavedQuery, // Provided if it's a saved search
     onFetchPage: PropTypes.func.isRequired,
     onToggleEdit: PropTypes.func,
@@ -78,7 +79,12 @@ class Result extends React.Component {
   }
 
   shouldComponentUpdate(nextProps, nextState) {
-    return !isEqual(nextProps, this.props) || !isEqual(nextState, this.state);
+    return (
+      (nextProps.location.state === 'fetching' &&
+        nextProps.location.state !== this.props.location.state) ||
+      !isEqual(nextProps, this.props) ||
+      !isEqual(nextState, this.state)
+    );
   }
 
   componentWillUnmount() {
@@ -299,5 +305,4 @@ class Result extends React.Component {
   }
 }
 
-export {Result};
-export default withRouter(Result);
+export default Result;

tests/js/spec/views/organizationDiscover/discover.spec.jsx

@@ -35,7 +35,7 @@ describe('Discover', function() {
       const savedQuery = TestStubs.DiscoverSavedQuery();
       wrapper = mount(
         <Discover
-          location={{}}
+          location={{query: {}}}
           queryBuilder={queryBuilder}
           organization={organization}
           savedQuery={savedQuery}
@@ -61,6 +61,7 @@ describe('Discover', function() {
       wrapper = mount(
         <Discover
           location={{
+            query: {},
             search:
               'projects=%5B%5D&fields=%5B%22id%22%2C%22issue.id%22%2C%22project.name%22%2C%22platform%22%2C%22timestamp%22%5D&conditions=%5B%5D&aggregations=%5B%5D&range=%227d%22&orderby=%22-timestamp%22&limit=1000&start=null&end=null',
           }}
@@ -87,6 +88,7 @@ describe('Discover', function() {
       wrapper = mount(
         <Discover
           location={{
+            query: {},
             search: '',
           }}
           queryBuilder={queryBuilder}
@@ -110,7 +112,7 @@ describe('Discover', function() {
           queryBuilder={queryBuilder}
           organization={organization}
           updateSavedQueryData={jest.fn()}
-          location={{search: ''}}
+          location={{query: {}, search: ''}}
           params={{}}
           toggleEditMode={jest.fn()}
           isLoading={false}
@@ -136,7 +138,7 @@ describe('Discover', function() {
           queryBuilder={queryBuilder}
           organization={organization}
           updateSavedQueryData={jest.fn()}
-          location={{search: ''}}
+          location={{query: {}, search: ''}}
           params={{}}
           toggleEditMode={jest.fn()}
           isLoading={false}
@@ -147,6 +149,7 @@ describe('Discover', function() {
       expect(wrapper.find('TimeRangeSelector').text()).toEqual('Last 14 days');
       wrapper.setProps({
         location: {
+          query: {},
           search:
             'projects=%5B%5D&fields=%5B%22id%22%2C%22issue.id%22%2C%22project.name%22%2C%22platform%22%2C%22timestamp%22%5D&conditions=%5B%5D&aggregations=%5B%5D&range=%227d%22&orderby=%22-timestamp%22&limit=1000&start=null&end=null',
         },
@@ -181,6 +184,7 @@ describe('Discover', function() {
 
       wrapper = mount(
         <Discover
+          location={{query: {}}}
           queryBuilder={queryBuilder}
           organization={organization}
           params={{}}
@@ -241,6 +245,7 @@ describe('Discover', function() {
 
       wrapper = mount(
         <Discover
+          location={{query: {}}}
           queryBuilder={queryBuilder}
           organization={organization}
           updateSavedQueryData={jest.fn()}
@@ -311,6 +316,7 @@ describe('Discover', function() {
     it('can be saved', function() {
       const wrapper = mount(
         <Discover
+          location={{query: {}}}
           queryBuilder={queryBuilder}
           organization={organization}
           updateSavedQueryData={jest.fn()}
@@ -343,6 +349,7 @@ describe('Discover', function() {
         browserHistory.push.mockImplementation(function({search}) {
           wrapper.setProps({
             location: {
+              query: {},
               search: search || '',
             },
           });
@@ -417,6 +424,7 @@ describe('Discover', function() {
         const prevCallCount = queryBuilder.reset.mock.calls.length;
         wrapper.setProps({
           location: {
+            query: {},
             search: '?fields=[]',
           },
         });
@@ -437,7 +445,10 @@ describe('Discover', function() {
           params={{savedQueryId: savedQuery.id}}
           updateSavedQueryData={jest.fn()}
           view="saved"
-          location={{search: ''}}
+          location={{
+            query: {},
+            search: '',
+          }}
           toggleEditMode={jest.fn()}
           isLoading={false}
         />,
@@ -517,7 +528,10 @@ describe('Discover', function() {
         <Discover
           queryBuilder={queryBuilder}
           organization={organization}
-          location={{location: '?fields=something'}}
+          location={{
+            query: {},
+            location: '?fields=something',
+          }}
           updateSavedQueryData={jest.fn()}
           toggleEditMode={jest.fn()}
           isLoading={false}
@@ -558,6 +572,7 @@ describe('Discover', function() {
       browserHistory.push.mockImplementation(function({search}) {
         wrapper.setProps({
           location: {
+            query: {},
             search: search || '',
           },
         });
@@ -567,7 +582,10 @@ describe('Discover', function() {
         <Discover
           queryBuilder={queryBuilder}
           organization={organization}
-          location={{location: ''}}
+          location={{
+            query: {},
+            location: '',
+          }}
           updateSavedQueryData={jest.fn()}
           toggleEditMode={jest.fn()}
           isLoading={false}
@@ -611,6 +629,7 @@ describe('Discover', function() {
       });
       wrapper = mount(
         <Discover
+          location={{query: {}}}
           queryBuilder={queryBuilder}
           organization={organization}
           updateSavedQueryData={jest.fn()}

tests/js/spec/views/organizationDiscover/result/index.spec.jsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import {mount, shallow} from 'enzyme';
 
-import {Result} from 'app/views/organizationDiscover/result';
+import Result from 'app/views/organizationDiscover/result';
 import createQueryBuilder from 'app/views/organizationDiscover/queryBuilder';
 
 describe('Result', function() {