fix(trends): Fix default period and remove trends query when navigating away (#20930)

* fix(trends): Fix default period when clicking 'by trends'

Clicking the 'by trends' tab was calling 14d by default correctly, but wasn't updating the location. Also added some tests to assert what is getting modified in the query

This will remove the entire trends query to fix the errors the appended queries cause on the different tabs when navigating there from trends.

src/sentry/static/sentry/app/utils/tokenizeSearch.tsx

@@ -128,6 +128,11 @@ export class QueryResults {
     return this.tagValues[key];
   }
 
+  hasTags(key: string) {
+    const tags = this.getTags(key);
+    return tags && tags.length;
+  }
+
   removeTag(key: string) {
     this.tokens = this.tokens.filter(token => token.key !== key);
     delete this.tagValues[key];

src/sentry/static/sentry/app/views/performance/landing.tsx

@@ -27,6 +27,7 @@ import withGlobalSelection from 'app/utils/withGlobalSelection';
 import withOrganization from 'app/utils/withOrganization';
 import withProjects from 'app/utils/withProjects';
 import {tokenizeSearch, stringifyQueryObject} from 'app/utils/tokenizeSearch';
+import {decodeScalar} from 'app/utils/queryString';
 
 import {generatePerformanceEventView, DEFAULT_STATS_PERIOD} from './data';
 import Table from './table';
@@ -179,18 +180,38 @@ class PerformanceLanding extends React.Component<Props, State> {
       ...location.query,
     };
 
+    const query = decodeScalar(location.query.query) || '';
+    const conditions = tokenizeSearch(query);
+
     // This is a temporary change for trends to test adding a default count to increase relevancy
     if (viewKey === FilterViews.TRENDS) {
-      if (!newQuery.query) {
-        newQuery.query =
-          'count():>1000 transaction.duration:>0 p50():>0 avg(transaction.duration):>0';
+      const hasStartAndEnd = newQuery.start && newQuery.end;
+      if (!newQuery.statsPeriod && !hasStartAndEnd) {
+        newQuery.statsPeriod = DEFAULT_TRENDS_STATS_PERIOD;
+      }
+      if (!query) {
+        conditions.setTag('count()', ['>1000']);
+        conditions.setTag('transaction.duration', ['>0']);
       }
-      if (!newQuery.query.includes('count()')) {
-        newQuery.query += 'count():>1000';
+      if (!conditions.hasTags('count()')) {
+        conditions.setTag('count()', ['>1000']);
       }
-      if (!newQuery.query.includes('transaction.duration')) {
-        newQuery.query += ' transaction.duration:>0';
+      if (!conditions.hasTags('transaction.duration')) {
+        conditions.setTag('transaction.duration', ['>0']);
       }
+
+      newQuery.query = stringifyQueryObject(conditions);
+    }
+
+    const isNavigatingAwayFromTrends =
+      viewKey !== FilterViews.TRENDS && location.query.view === FilterViews.TRENDS;
+
+    if (isNavigatingAwayFromTrends) {
+      // This stops errors from occurring when navigating to other views since we are appending aggregates to the trends view
+      conditions.removeTag('count()');
+      conditions.removeTag('transaction.duration');
+
+      newQuery.query = stringifyQueryObject(conditions);
     }
 
     ReactRouter.browserHistory.push({
@@ -211,6 +232,7 @@ class PerformanceLanding extends React.Component<Props, State> {
                     key={viewKey}
                     barId={viewKey}
                     size="small"
+                    data-test-id={'landing-header-' + viewKey.toLowerCase()}
                     onClick={() => this.handleViewChange(viewKey)}
                   >
                     {this.getViewLabel(viewKey)}

tests/js/spec/views/performance/landing.spec.jsx

@@ -1,15 +1,17 @@
 import React from 'react';
+import {browserHistory} from 'react-router';
 
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {mountWithTheme} from 'sentry-test/enzyme';
 
 import ProjectsStore from 'app/stores/projectsStore';
-import PerformanceLanding from 'app/views/performance/landing';
+import PerformanceLanding, {FilterViews} from 'app/views/performance/landing';
+
+const FEATURES = ['transaction-event', 'performance-view'];
 
 function initializeData(projects, query) {
-  const features = ['transaction-event', 'performance-view'];
   const organization = TestStubs.Organization({
-    features,
+    features: FEATURES,
     projects,
   });
   const initialData = initializeOrg({
@@ -24,8 +26,30 @@ function initializeData(projects, query) {
   return initialData;
 }
 
+function initializeTrendsData(query) {
+  const features = [...FEATURES, 'trends'];
+  const projects = [
+    TestStubs.Project({id: '1', firstTransactionEvent: false}),
+    TestStubs.Project({id: '2', firstTransactionEvent: true}),
+  ];
+  const organization = TestStubs.Organization({
+    features,
+    projects,
+  });
+
+  const initialData = initializeOrg({
+    organization,
+    router: {
+      location: {query},
+    },
+  });
+  ProjectsStore.loadInitialData(initialData.organization.projects);
+  return initialData;
+}
+
 describe('Performance > Landing', function() {
   beforeEach(function() {
+    browserHistory.push.mockReset();
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/projects/',
       body: [],
@@ -88,6 +112,13 @@ describe('Performance > Landing', function() {
         count: 2,
       },
     });
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events-trends/',
+      body: {
+        stats: {},
+        events: {meta: {}, data: []},
+      },
+    });
   });
 
   afterEach(function() {
@@ -191,4 +222,89 @@ describe('Performance > Landing', function() {
       })
     );
   });
+
+  it('Sets default period when navigating to trends when stats period is not set', async function() {
+    const data = initializeTrendsData({query: 'tag:value'});
+    const wrapper = mountWithTheme(
+      <PerformanceLanding
+        organization={data.organization}
+        location={data.router.location}
+      />,
+      data.routerContext
+    );
+    await tick();
+    wrapper.update();
+
+    const trendsLink = wrapper.find('[data-test-id="landing-header-trends"]').at(0);
+    trendsLink.simulate('click');
+
+    expect(browserHistory.push).toHaveBeenCalledWith(
+      expect.objectContaining({
+        query: {
+          query: 'tag:value count():>1000 transaction.duration:>0',
+          statsPeriod: '14d',
+          view: 'TRENDS',
+        },
+      })
+    );
+  });
+
+  it('Navigating to trends does not modify statsPeriod when already set', async function() {
+    const data = initializeTrendsData({
+      query: 'count():>500 transaction.duration:>10',
+      statsPeriod: '24h',
+    });
+
+    const wrapper = mountWithTheme(
+      <PerformanceLanding
+        organization={data.organization}
+        location={data.router.location}
+      />,
+      data.routerContext
+    );
+    await tick();
+    wrapper.update();
+
+    const trendsLink = wrapper.find('[data-test-id="landing-header-trends"]').at(0);
+    trendsLink.simulate('click');
+
+    expect(browserHistory.push).toHaveBeenCalledWith(
+      expect.objectContaining({
+        query: {
+          query: 'count():>500 transaction.duration:>10',
+          statsPeriod: '24h',
+          view: 'TRENDS',
+        },
+      })
+    );
+  });
+
+  it('Navigating away from trends will reset query', async function() {
+    const data = initializeTrendsData({view: FilterViews.TRENDS});
+
+    const wrapper = mountWithTheme(
+      <PerformanceLanding
+        organization={data.organization}
+        location={data.router.location}
+      />,
+      data.routerContext
+    );
+    await tick();
+    wrapper.update();
+
+    const byTransactionLink = wrapper
+      .find('[data-test-id="landing-header-all_transactions"]')
+      .at(0);
+    byTransactionLink.simulate('click');
+
+    expect(browserHistory.push).toHaveBeenNthCalledWith(
+      1,
+      expect.objectContaining({
+        query: {
+          query: '',
+          view: 'ALL_TRANSACTIONS',
+        },
+      })
+    );
+  });
 });