Browse Source

feat(discover-homepage): Homepage query without changes shows reset (#39686)

When the homepage is first accessed, we should show "Reset Discover
Home" because there are no changes and the user can restore the
default setting at this point
Nar Saynorath 2 years ago
parent
commit
cbd39e0f79

+ 6 - 0
static/app/actionCreators/discoverHomepageQueries.tsx

@@ -1,6 +1,12 @@
 import {Client} from 'sentry/api';
 import {NewQuery, SavedQuery} from 'sentry/types';
 
+export function fetchHomepageQuery(api: Client, orgId: string): Promise<SavedQuery> {
+  return api.requestPromise(`/organizations/${orgId}/discover/homepage/`, {
+    method: 'GET',
+  });
+}
+
 export function updateHomepageQuery(
   api: Client,
   orgId: string,

+ 42 - 0
static/app/views/eventsV2/homepage.spec.tsx

@@ -146,4 +146,46 @@ describe('Discover > Homepage', () => {
     expect(screen.queryByText(/Created by:/)).not.toBeInTheDocument();
     expect(screen.queryByText(/Last edited:/)).not.toBeInTheDocument();
   });
+
+  it('shows the Reset Discover Home button on initial load', async () => {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/discover/homepage/',
+      method: 'GET',
+      statusCode: 200,
+      body: {
+        id: '2',
+        name: 'homepage query',
+        projects: [],
+        version: 2,
+        expired: false,
+        dateCreated: '2021-04-08T17:53:25.195782Z',
+        dateUpdated: '2021-04-09T12:13:18.567264Z',
+        createdBy: {
+          id: '2',
+        },
+        environment: [],
+        fields: ['environment'],
+        widths: ['-1'],
+        range: '14d',
+        orderby: '-environment',
+        display: 'previous',
+        query: 'event.type:error',
+        topEvents: '5',
+      },
+    });
+
+    render(
+      <Homepage
+        organization={organization}
+        location={initialData.router.location}
+        router={initialData.router}
+        setSavedQuery={jest.fn()}
+        loading={false}
+      />,
+      {context: initialData.routerContext, organization: initialData.organization}
+    );
+
+    expect(await screen.findByText('Reset Discover Home')).toBeInTheDocument();
+    expect(screen.queryByText('Use as Discover Home')).not.toBeInTheDocument();
+  });
 });

+ 32 - 4
static/app/views/eventsV2/results.spec.jsx

@@ -2,7 +2,7 @@ import {browserHistory} from 'react-router';
 
 import {enforceActOnUseLegacyStoreHook, mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {act, render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
 import {triggerPress} from 'sentry-test/utils';
 
 import * as PageFilterPersistence from 'sentry/components/organizations/pageFilters/persistence';
@@ -195,6 +195,28 @@ describe('Results', function () {
         orderby: '-user.display',
       },
     });
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/discover/homepage/',
+      method: 'GET',
+      statusCode: 200,
+      body: {
+        id: '2',
+        name: '',
+        projects: [],
+        version: 2,
+        expired: false,
+        dateCreated: '2021-04-08T17:53:25.195782Z',
+        dateUpdated: '2021-04-09T12:13:18.567264Z',
+        createdBy: {
+          id: '2',
+        },
+        environment: [],
+        fields: ['title', 'event.type', 'project', 'user.display', 'timestamp'],
+        widths: ['-1', '-1', '-1', '-1', '-1'],
+        range: '24h',
+        orderby: '-user.display',
+      },
+    });
   });
 
   afterEach(function () {
@@ -1708,11 +1730,11 @@ describe('Results', function () {
     );
   });
 
-  it('updates the homepage query with up to date eventView when Use as Discover Home is clicked', () => {
+  it('updates the homepage query with up to date eventView when Use as Discover Home is clicked', async () => {
     const mockHomepageUpdate = MockApiClient.addMockResponse({
       url: '/organizations/org-slug/discover/homepage/',
       method: 'PUT',
-      statusCode: 204,
+      statusCode: 200,
     });
     const organization = TestStubs.Organization({
       features: [
@@ -1726,7 +1748,7 @@ describe('Results', function () {
       organization,
       router: {
         // These fields take priority and should be sent in the request
-        location: {query: {field: ['title', 'user']}},
+        location: {query: {field: ['title', 'user'], id: '1'}},
       },
     });
 
@@ -1741,6 +1763,9 @@ describe('Results', function () {
       {context: initialData.routerContext, organization}
     );
 
+    await waitFor(() =>
+      expect(screen.getByRole('button', {name: /use as discover home/i})).toBeEnabled()
+    );
     userEvent.click(screen.getByText('Use as Discover Home'));
 
     expect(mockHomepageUpdate).toHaveBeenCalledWith(
@@ -1802,6 +1827,9 @@ describe('Results', function () {
       {context: initialData.routerContext, organization}
     );
 
+    await waitFor(() =>
+      expect(screen.getByRole('button', {name: /use as discover home/i})).toBeEnabled()
+    );
     userEvent.click(screen.getByText('Use as Discover Home'));
     expect(await screen.findByText('Reset Discover Home')).toBeInTheDocument();
 

+ 19 - 2
static/app/views/eventsV2/resultsHeader.tsx

@@ -3,6 +3,7 @@ import {InjectedRouter} from 'react-router';
 import styled from '@emotion/styled';
 import {Location} from 'history';
 
+import {fetchHomepageQuery} from 'sentry/actionCreators/discoverHomepageQueries';
 import {fetchSavedQuery} from 'sentry/actionCreators/discoverSavedQueries';
 import {Client} from 'sentry/api';
 import * as Layout from 'sentry/components/layouts/thirds';
@@ -43,8 +44,16 @@ class ResultsHeader extends Component<Props, State> {
   };
 
   componentDidMount() {
-    if (this.props.eventView.id) {
+    const {eventView, isHomepage} = this.props;
+    const {loading} = this.state;
+    if (!isHomepage && eventView.id) {
       this.fetchData();
+    } else if (eventView.id === undefined && loading) {
+      // If this is a new query, there's nothing to load
+      this.setState({loading: false});
+    }
+    if (isHomepage) {
+      this.fetchHomepageQueryData();
     }
   }
 
@@ -68,6 +77,14 @@ class ResultsHeader extends Component<Props, State> {
     }
   }
 
+  fetchHomepageQueryData() {
+    const {api, organization} = this.props;
+    this.setState({loading: true});
+    fetchHomepageQuery(api, organization.slug).then(homepageQuery => {
+      this.setState({homepageQuery, loading: false});
+    });
+  }
+
   renderAuthor() {
     const {eventView, isHomepage} = this.props;
     const {savedQuery} = this.state;
@@ -125,7 +142,7 @@ class ResultsHeader extends Component<Props, State> {
             organization={organization}
             eventView={eventView}
             savedQuery={savedQuery}
-            savedQueryLoading={loading}
+            queryDataLoading={loading}
             disabled={errorCode >= 400 && errorCode < 500}
             updateCallback={() => this.fetchData()}
             yAxis={yAxis}

+ 2 - 2
static/app/views/eventsV2/savedQuery/index.spec.tsx

@@ -36,7 +36,7 @@ function mount(
       updateCallback={() => {}}
       yAxis={yAxis}
       router={router}
-      savedQueryLoading={false}
+      queryDataLoading={false}
       setSavedQuery={jest.fn()}
       setHomepageQuery={jest.fn()}
     />
@@ -64,7 +64,7 @@ function generateWrappedComponent(
         updateCallback={() => {}}
         yAxis={yAxis}
         router={router}
-        savedQueryLoading={false}
+        queryDataLoading={false}
         setSavedQuery={mockSetSavedQuery}
         setHomepageQuery={jest.fn()}
       />

+ 6 - 3
static/app/views/eventsV2/savedQuery/index.tsx

@@ -114,9 +114,9 @@ type Props = DefaultProps & {
   location: Location;
   organization: Organization;
   projects: Project[];
+  queryDataLoading: boolean;
   router: InjectedRouter;
   savedQuery: SavedQuery | undefined;
-  savedQueryLoading: boolean;
   setHomepageQuery: (homepageQuery?: SavedQuery) => void;
   setSavedQuery: (savedQuery: SavedQuery) => void;
   updateCallback: () => void;
@@ -134,7 +134,7 @@ type State = {
 
 class SavedQueryButtonGroup extends PureComponent<Props, State> {
   static getDerivedStateFromProps(nextProps: Readonly<Props>, prevState: State): State {
-    const {eventView: nextEventView, savedQuery, savedQueryLoading, yAxis} = nextProps;
+    const {eventView: nextEventView, savedQuery, queryDataLoading, yAxis} = nextProps;
 
     // For a new unsaved query
     if (!savedQuery) {
@@ -145,7 +145,7 @@ class SavedQueryButtonGroup extends PureComponent<Props, State> {
       };
     }
 
-    if (savedQueryLoading) {
+    if (queryDataLoading) {
       return prevState;
     }
 
@@ -403,6 +403,7 @@ class SavedQueryButtonGroup extends PureComponent<Props, State> {
       isHomepage,
       setHomepageQuery,
       homepageQuery,
+      queryDataLoading,
     } = this.props;
     if (
       homepageQuery &&
@@ -426,6 +427,7 @@ class SavedQueryButtonGroup extends PureComponent<Props, State> {
               });
             }
           }}
+          disabled={queryDataLoading}
         >
           {t('Reset Discover Home')}
           <FeatureBadge type="alpha" />
@@ -447,6 +449,7 @@ class SavedQueryButtonGroup extends PureComponent<Props, State> {
             setHomepageQuery(updatedHomepageQuery);
           }
         }}
+        disabled={queryDataLoading}
       >
         {t('Use as Discover Home')}
         <FeatureBadge type="alpha" />