feat(ui): Remove default period/utc from URL params (#12423)

* fix discover with empty dates
* fix bug when navigating back to empty datetime
* fix bug where we update date picker while on saved queries tab and then navigate to new query and try to save, query gets reset with default stats period
* fix saved queries with absolute dates, range mistakenly being applied

Fixes SEN-372

src/sentry/static/sentry/app/actionCreators/globalSelection.jsx

@@ -2,7 +2,7 @@
 import {isEqual, isInteger, omit} from 'lodash';
 import * as Sentry from '@sentry/browser';
 
-import {getParams} from 'app/views/organizationEvents/utils/getParams';
+import {defined} from 'app/utils';
 import {getUtcDateString} from 'app/utils/dates';
 import GlobalSelectionActions from 'app/actions/globalSelectionActions';
 
@@ -170,3 +170,31 @@ function getNewQueryParams(obj, oldQueryParams, {resetParams} = {}) {
 
   return newQuery;
 }
+
+// Filters out params with null values and returns a default
+// `statsPeriod` when necessary.
+//
+// Accepts `period` and `statsPeriod` but will only return `statsPeriod`
+//
+function getParams(params = {}) {
+  const {start, end, period, statsPeriod, ...otherParams} = params;
+
+  // `statsPeriod` takes precendence for now
+  const coercedPeriod = statsPeriod || period;
+
+  // Filter null values
+  return Object.entries({
+    statsPeriod: coercedPeriod,
+    start: coercedPeriod ? null : start,
+    end: coercedPeriod ? null : end,
+    ...otherParams,
+  })
+    .filter(([key, value]) => defined(value))
+    .reduce(
+      (acc, [key, value]) => ({
+        ...acc,
+        [key]: value,
+      }),
+      {}
+    );
+}

src/sentry/static/sentry/app/components/organizations/globalSelectionHeader/index.jsx

@@ -10,7 +10,6 @@ import {
 } from 'app/components/organizations/globalSelectionHeader/constants';
 import {DEFAULT_STATS_PERIOD} from 'app/constants';
 import {callIfFunction} from 'app/utils/callIfFunction';
-import {defined} from 'app/utils';
 import {isEqualWithDates} from 'app/utils/isEqualWithDates';
 import {t} from 'app/locale';
 import {
@@ -221,7 +220,7 @@ class GlobalSelectionHeader extends React.Component {
     const nextQuery = pick(nextProps.location.query, urlParamKeys);
 
     // If no next query is specified keep the previous global selection values
-    if (Object.keys(nextQuery).length === 0) {
+    if (Object.keys(prevQuery).length === 0 && Object.keys(nextQuery).length === 0) {
       return false;
     }
 
@@ -241,11 +240,7 @@ class GlobalSelectionHeader extends React.Component {
       nextProps.location.query
     );
 
-    if (start || end || period || utc) {
-      // Don't attempt to update date if all of these values are empty
-      updateDateTime({start, end, period, utc});
-    }
-
+    updateDateTime({start, end, period, utc});
     updateEnvironments(environment || []);
     updateProjects(project || []);
   };
@@ -281,7 +276,9 @@ class GlobalSelectionHeader extends React.Component {
 
   handleUpdateTime = ({relative: period, start, end, utc} = {}) => {
     const newValueObj = {
-      ...(defined(period) ? {period} : {start, end}),
+      period,
+      start,
+      end,
       utc,
     };
 

src/sentry/static/sentry/app/components/organizations/timeRangeSelector/index.jsx

@@ -6,6 +6,7 @@ import styled from 'react-emotion';
 
 import {DEFAULT_STATS_PERIOD} from 'app/constants';
 import {analytics} from 'app/utils/analytics';
+import {defined, parsePeriodToHours} from 'app/utils';
 import {
   getLocalToSystem,
   getPeriodAgo,
@@ -13,7 +14,6 @@ import {
   getUtcToSystem,
 } from 'app/utils/dates';
 import {getRelativeSummary} from 'app/components/organizations/timeRangeSelector/utils';
-import {parsePeriodToHours} from 'app/utils';
 import {t} from 'app/locale';
 import DateRange from 'app/components/organizations/timeRangeSelector/dateRange';
 import DateSummary from 'app/components/organizations/timeRangeSelector/dateSummary';
@@ -105,7 +105,6 @@ class TimeRangeSelector extends React.PureComponent {
   static defaultProps = {
     showAbsolute: true,
     showRelative: true,
-    utc: getUserTimezone() === 'UTC',
   };
 
   static contextTypes = {
@@ -124,7 +123,9 @@ class TimeRangeSelector extends React.PureComponent {
     }
 
     this.state = {
-      utc: props.utc,
+      // if utc is not null and not undefined, then use value of `props.utc` (it can be false)
+      // otherwise if no value is supplied, the default should be the user's timezone preference
+      utc: defined(props.utc) ? props.utc : getUserTimezone() === 'UTC',
       isOpen: false,
       hasChanges: false,
       start,
@@ -188,8 +189,12 @@ class TimeRangeSelector extends React.PureComponent {
         'hours'
       ).toDate(),
       end: new Date(),
-      utc: this.state.utc,
     };
+
+    if (defined(this.props.utc)) {
+      newDateTime.utc = this.state.utc;
+    }
+
     this.setState({
       hasChanges: true,
       ...newDateTime,
@@ -205,7 +210,6 @@ class TimeRangeSelector extends React.PureComponent {
       relative: value,
       start: null,
       end: null,
-      utc: this.state.utc,
     };
     this.setState(newDateTime);
     this.callCallback(onChange, newDateTime);
@@ -215,11 +219,12 @@ class TimeRangeSelector extends React.PureComponent {
   handleClear = () => {
     const {onChange} = this.props;
     const newDateTime = {
-      relative: DEFAULT_STATS_PERIOD,
+      relative: null,
       start: null,
       end: null,
-      utc: this.state.utc,
+      utc: null,
     };
+    this.setState(newDateTime);
     this.callCallback(onChange, newDateTime);
     this.handleUpdate(newDateTime);
   };
@@ -231,8 +236,12 @@ class TimeRangeSelector extends React.PureComponent {
       relative: null,
       start,
       end,
-      utc: this.state.utc,
     };
+
+    if (defined(this.props.utc)) {
+      newDateTime.utc = this.state.utc;
+    }
+
     this.setState({hasChanges: true, ...newDateTime});
     this.callCallback(onChange, newDateTime);
   };
@@ -241,26 +250,27 @@ class TimeRangeSelector extends React.PureComponent {
     const {onChange} = this.props;
     let {start, end} = this.props;
 
-    if (!start) {
-      start = getDateWithTimezoneInUtc(this.state.start, this.props.utc);
-    }
-
-    if (!end) {
-      end = getDateWithTimezoneInUtc(this.state.end, this.props.utc);
-    }
-
     this.setState(state => {
       const utc = !state.utc;
 
+      if (!start) {
+        start = getDateWithTimezoneInUtc(state.start, state.utc);
+      }
+
+      if (!end) {
+        end = getDateWithTimezoneInUtc(state.end, state.utc);
+      }
+
       analytics('dateselector.utc_changed', {
         utc,
         path: getRouteStringFromRoutes(this.context.router.routes),
         org_id: parseInt(this.props.organization.id, 10),
       });
+
       const newDateTime = {
         relative: null,
-        start: this.props.utc ? getUtcToSystem(start) : getLocalToSystem(start),
-        end: this.props.utc ? getUtcToSystem(end) : getLocalToSystem(end),
+        start: utc ? getLocalToSystem(start) : getUtcToSystem(start),
+        end: utc ? getLocalToSystem(end) : getUtcToSystem(end),
         utc,
       };
       this.callCallback(onChange, newDateTime);
@@ -280,10 +290,10 @@ class TimeRangeSelector extends React.PureComponent {
     const shouldShowRelative = showRelative;
     const isAbsoluteSelected = !!start && !!end;
 
-    const summary = relative ? (
-      getRelativeSummary(relative)
-    ) : (
+    const summary = isAbsoluteSelected ? (
       <DateSummary utc={this.state.utc} start={start} end={end} />
+    ) : (
+      getRelativeSummary(relative || DEFAULT_STATS_PERIOD)
     );
 
     return (
@@ -298,7 +308,10 @@ class TimeRangeSelector extends React.PureComponent {
             <StyledHeaderItem
               icon={<StyledInlineSvg src="icon-calendar" />}
               isOpen={isOpen}
-              hasSelected={this.props.relative !== DEFAULT_STATS_PERIOD}
+              hasSelected={
+                (!!this.props.relative && this.props.relative !== DEFAULT_STATS_PERIOD) ||
+                isAbsoluteSelected
+              }
               hasChanges={this.state.hasChanges}
               onClear={this.handleClear}
               allowClear={true}
@@ -317,7 +330,7 @@ class TimeRangeSelector extends React.PureComponent {
                   {shouldShowRelative && (
                     <RelativeSelector
                       onClick={this.handleSelectRelative}
-                      selected={relative}
+                      selected={relative || DEFAULT_STATS_PERIOD}
                     />
                   )}
                   {shouldShowAbsolute && (

src/sentry/static/sentry/app/stores/globalSelectionStore.jsx

@@ -7,24 +7,20 @@ import {
   LOCAL_STORAGE_KEY,
 } from 'app/components/organizations/globalSelectionHeader/constants';
 import {getStateFromQuery} from 'app/components/organizations/globalSelectionHeader/utils';
-import {DEFAULT_STATS_PERIOD} from 'app/constants';
 import {isEqualWithDates} from 'app/utils/isEqualWithDates';
-import ConfigStore from 'app/stores/configStore';
 import OrganizationsStore from 'app/stores/organizationsStore';
 import GlobalSelectionActions from 'app/actions/globalSelectionActions';
 import localStorage from 'app/utils/localStorage';
 
 const getDefaultSelection = () => {
-  const user = ConfigStore.get('user');
-
   return {
     projects: [],
     environments: [],
     datetime: {
       [DATE_TIME.START]: null,
       [DATE_TIME.END]: null,
-      [DATE_TIME.PERIOD]: DEFAULT_STATS_PERIOD,
-      [DATE_TIME.UTC]: user?.options?.timezone === 'UTC' ? true : null,
+      [DATE_TIME.PERIOD]: null,
+      [DATE_TIME.UTC]: null,
     },
   };
 };

src/sentry/static/sentry/app/views/organizationDashboard/discoverQuery.jsx

@@ -2,6 +2,7 @@ import {isEqual, omit} from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
 
+import {DEFAULT_STATS_PERIOD} from 'app/constants';
 import {getInterval} from 'app/components/charts/utils';
 import {getPeriod} from 'app/utils/getPeriod';
 import {parsePeriodToHours} from 'app/utils';
@@ -88,7 +89,12 @@ class DiscoverQuery extends React.Component {
     if (query.rollup) {
       // getInterval returns a period string depending on current datetime range selected
       // we then use a helper function to parse into hours and then convert back to seconds
-      query.rollup = parsePeriodToHours(getInterval(datetime)) * 60 * 60;
+      query.rollup =
+        parsePeriodToHours(
+          getInterval({...datetime, period: datetime.period || DEFAULT_STATS_PERIOD})
+        ) *
+        60 *
+        60;
     }
 
     return {

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

@@ -8,8 +8,8 @@ import {getUtcDateString} from 'app/utils/dates';
 import {t, tct} from 'app/locale';
 import {updateProjects, updateDateTime} from 'app/actionCreators/globalSelection';
 import BetaTag from 'app/components/betaTag';
-import SentryTypes from 'app/sentryTypes';
 import PageHeading from 'app/components/pageHeading';
+import SentryTypes from 'app/sentryTypes';
 
 import {
   DiscoverContainer,
@@ -116,34 +116,27 @@ export default class OrganizationDiscover extends React.Component {
       // This indicates navigation changes (e.g. back button on browser)
       // We need to update our search store and probably runQuery
       const {projects, range, start, end, utc} = newQuery;
-      let hasChange = false;
 
       if (projects) {
         this.updateProjects(projects);
-        hasChange = true;
       }
 
-      if (range || (end && start)) {
-        this.updateDateTime({
-          period: range || null,
-          start: start || null,
-          end: end || null,
-          utc: typeof utc !== 'undefined' ? utc : null,
-        });
+      this.updateDateTime({
+        period: range || null,
+        start: start || null,
+        end: end || null,
+        utc: typeof utc !== 'undefined' ? utc : null,
+      });
 
-        // These props come from URL string, so will always be in UTC
-        updateDateTime({
-          start: start && new Date(moment.utc(start).local()),
-          end: end && new Date(moment.utc(end).local()),
-          period: range,
-          utc,
-        });
-        hasChange = true;
-      }
+      // These props come from URL string, so will always be in UTC
+      updateDateTime({
+        start: (start && new Date(moment.utc(start).local())) || null,
+        end: (end && new Date(moment.utc(end).local())) || null,
+        period: range || null,
+        utc: typeof utc !== 'undefined' ? utc : null,
+      });
 
-      if (hasChange) {
-        this.runQuery();
-      }
+      this.runQuery();
     }
   }
 
@@ -154,7 +147,7 @@ export default class OrganizationDiscover extends React.Component {
 
   getDateTimeFields = ({period, start, end, utc}) => ({
     range: period || null,
-    utc,
+    utc: typeof utc !== 'undefined' ? utc : null,
     start: (start && getUtcDateString(start)) || null,
     end: (end && getUtcDateString(end)) || null,
   });
@@ -166,6 +159,12 @@ export default class OrganizationDiscover extends React.Component {
   updateDateTime = datetime => {
     const {start, end, range, utc} = this.getDateTimeFields(datetime);
 
+    updateDateTime({
+      start,
+      end,
+      period: range,
+      utc,
+    });
     this.updateFields({start, end, range, utc});
   };
 

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

@@ -96,8 +96,9 @@ class OrganizationDiscoverContainer extends React.Component {
       this.setState({savedQuery: null});
       // Reset querybuilder if we're switching from a saved query
       if (this.props.params.savedQueryId) {
-        const projects = nextProps.selection.projects;
-        this.queryBuilder.reset({projects});
+        const {datetime, projects} = nextProps.selection;
+        const {start, end, period: range} = datetime;
+        this.queryBuilder.reset({projects, range, start, end});
       }
       return;
     }

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

@@ -20,7 +20,6 @@ const DEFAULTS = {
   fields: ['id', 'issue.id', 'project.name', 'platform', 'timestamp'],
   conditions: [],
   aggregations: [],
-  range: DEFAULT_STATS_PERIOD,
   orderby: '-timestamp',
   limit: 1000,
 };
@@ -42,6 +41,11 @@ function applyDefaults(query) {
 export default function createQueryBuilder(initial = {}, organization) {
   const api = new Client();
   let query = applyDefaults(initial);
+
+  if (!query.start && !query.end && !query.range) {
+    query.range = DEFAULT_STATS_PERIOD;
+  }
+
   const defaultProjects = organization.projects
     .filter(projects => projects.isMember)
     .map(project => parseInt(project.id, 10));
@@ -113,6 +117,14 @@ export default function createQueryBuilder(initial = {}, organization) {
     // Default to all projects if none is selected
     const projects = query.projects.length ? query.projects : defaultProjects;
 
+    // Default to DEFAULT_STATS_PERIOD when no date range selected (either relative or absolute)
+    const {range, start, end} = query;
+    const hasAbsolute = start && end;
+    const daterange = {
+      ...(hasAbsolute && {start, end}),
+      ...(range ? {range} : !hasAbsolute && {range: DEFAULT_STATS_PERIOD}),
+    };
+
     // Default to all fields if there are none selected, and no aggregation is
     // specified
     const useDefaultFields = !query.fields.length && !query.aggregations.length;
@@ -126,6 +138,7 @@ export default function createQueryBuilder(initial = {}, organization) {
 
     return {
       ...query,
+      ...daterange,
       projects,
       fields,
     };

src/sentry/static/sentry/app/views/organizationEvents/eventsChart.jsx

@@ -76,12 +76,16 @@ class EventsChart extends React.Component {
     api: PropTypes.object,
     period: PropTypes.string,
     query: PropTypes.string,
+    start: PropTypes.instanceOf(Date),
+    end: PropTypes.instanceOf(Date),
     utc: PropTypes.bool,
     router: PropTypes.object,
   };
 
   render() {
-    const {api, period, utc, query, router, ...props} = this.props;
+    const {api, period, utc, query, router, start, end, ...props} = this.props;
+    // Include previous only on relative dates (defaults to relative if no start and end)
+    const includePrevious = !!period || (!start && !end);
 
     return (
       <ChartZoom router={router} period={period} utc={utc} {...props}>
@@ -90,11 +94,13 @@ class EventsChart extends React.Component {
             {...props}
             api={api}
             period={period}
+            start={start}
+            end={end}
             interval={getInterval(this.props, true)}
             showLoading={false}
             query={query}
             getCategory={DEFAULT_GET_CATEGORY}
-            includePrevious={!!period}
+            includePrevious={includePrevious}
           >
             {({loading, reloading, timeseriesData, previousTimeseriesData}) => {
               return (

tests/js/helpers/mockRouterPush.jsx

@@ -3,12 +3,17 @@ import qs from 'query-string';
 // More closely mocks a router push -- updates wrapper's props/context
 // with updated `router` and calls `wrapper.update()`
 export function mockRouterPush(wrapper, router) {
-  router.push.mockImplementation(({pathname, query}) => {
+  router.push.mockImplementation(({query}) => {
+    const stringifiedQuery = qs.stringify(query);
     const location = {
       ...router.location,
-      query,
-      search: qs.stringify(query),
+
+      // Need to make sure query more closely reflects datatypes in browser
+      // e.g. if we had a param that was boolean, it would get stringified
+      query: qs.parse(stringifiedQuery),
+      search: stringifiedQuery,
     };
+
     const newRouter = {
       router: {
         ...router,