Browse Source

fix(discover): Defer autorunning queries until tags are loaded (#11837)

Fixes a bug where conditions and aggregations that contained dynamically
loaded tag values would be dropped since these are validated upon running
a query.
Lyn Nagara 6 years ago
parent
commit
e4e8aa26c3

+ 8 - 9
src/sentry/static/sentry/app/views/organizationDiscover/discover.jsx

@@ -71,15 +71,6 @@ export default class OrganizationDiscover extends React.Component {
     };
   }
 
-  componentDidMount() {
-    const {savedQuery, location} = this.props;
-
-    // Run query if there is *any* querystring
-    if (savedQuery || (location && !!location.search)) {
-      this.runQuery();
-    }
-  }
-
   componentWillReceiveProps(nextProps) {
     const {
       queryBuilder,
@@ -87,9 +78,17 @@ export default class OrganizationDiscover extends React.Component {
       savedQuery,
       isEditingSavedQuery,
       params,
+      isLoading,
     } = nextProps;
     const {resultManager} = this.state;
 
+    // Run query on isLoading change if there is a querystring or saved search
+    const loadingStatusChanged = isLoading !== this.props.isLoading;
+    if (loadingStatusChanged && (savedQuery || !!search)) {
+      this.runQuery();
+      return;
+    }
+
     if (savedQuery && savedQuery !== this.props.savedQuery) {
       this.setState({view: 'saved'});
       this.runQuery();

+ 2 - 1
src/sentry/static/sentry/app/views/organizationDiscover/index.jsx

@@ -44,6 +44,7 @@ class OrganizationDiscoverContainer extends React.Component {
     const {organization} = context;
 
     const query = getQueryFromQueryString(search);
+
     if (query.hasOwnProperty('projects')) {
       // Update global store with projects from querystring
       updateProjects(query.projects);
@@ -60,7 +61,7 @@ class OrganizationDiscoverContainer extends React.Component {
         period: query.range || null,
       });
     } else {
-      // Update query with global projects
+      // Update query with global datetime values
       query.start = props.selection.datetime.start;
       query.end = props.selection.datetime.end;
       query.range = props.selection.datetime.period;

+ 13 - 4
tests/js/spec/views/organizationDiscover/discover.spec.jsx

@@ -31,28 +31,33 @@ describe('Discover', function() {
       queryBuilder.fetch = jest.fn(() => Promise.resolve(mockResponse));
     });
 
-    it('auto-runs saved query', async function() {
+    it('auto-runs saved query after tags are loaded', async function() {
       const savedQuery = TestStubs.DiscoverSavedQuery();
       wrapper = mount(
         <Discover
+          location={{}}
           queryBuilder={queryBuilder}
           organization={organization}
           savedQuery={savedQuery}
           params={{savedQueryId: savedQuery.id}}
           updateSavedQueryData={jest.fn()}
           toggleEditMode={jest.fn()}
-          isLoading={false}
+          isLoading={true}
         />,
         TestStubs.routerContext([{organization}])
       );
       await tick();
+      expect(wrapper.state().data.baseQuery.query).toBe(null);
+      expect(wrapper.state().data.baseQuery.data).toBe(null);
+      wrapper.setProps({isLoading: false});
+      await tick();
       expect(wrapper.state().data.baseQuery.query).toEqual(queryBuilder.getExternal());
       expect(wrapper.state().data.baseQuery.data).toEqual(
         expect.objectContaining({data: mockResponse.data})
       );
     });
 
-    it('auto-runs when there is a query string', async function() {
+    it('auto-runs when there is a query string after tags are loaded', async function() {
       wrapper = mount(
         <Discover
           location={{
@@ -63,11 +68,15 @@ describe('Discover', function() {
           organization={organization}
           updateSavedQueryData={jest.fn()}
           toggleEditMode={jest.fn()}
-          isLoading={false}
+          isLoading={true}
         />,
         TestStubs.routerContext([{organization}])
       );
       await tick();
+      expect(wrapper.state().data.baseQuery.query).toBe(null);
+      expect(wrapper.state().data.baseQuery.data).toBe(null);
+      wrapper.setProps({isLoading: false});
+      await tick();
       expect(wrapper.state().data.baseQuery.query).toEqual(queryBuilder.getExternal());
       expect(wrapper.state().data.baseQuery.data).toEqual(
         expect.objectContaining({data: mockResponse.data})

+ 3 - 1
tests/js/spec/views/organizationDiscover/index.spec.jsx

@@ -29,13 +29,15 @@ describe('OrganizationDiscoverContainer', function() {
         method: 'POST',
         body: {
           data: [{tags_key: 'tag1', count: 5}, {tags_key: 'tag2', count: 1}],
+          timing: {},
+          meta: [],
         },
       });
       wrapper = mount(
         <OrganizationDiscoverContainer
           location={{query: {}, search: ''}}
           params={{}}
-          selection={{datetime: {}}}
+          selection={{projects: [], environments: [], datetime: {}}}
         />,
         TestStubs.routerContext([{organization}])
       );