feat(discover): Use URL parameters for each visualization

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

@@ -22,7 +22,8 @@ import {
   SavedQueryWrapper,
 } from './styles';
 import {
-  getQueryStringFromQuery,
+  areQueriesEqual,
+  getQueryObjectFromQuery,
   getQueryFromQueryString,
   deleteSavedQuery,
   updateSavedQuery,
@@ -67,19 +68,19 @@ export default class OrganizationDiscover extends React.Component {
 
   componentDidMount() {
     if (this.props.savedQuery) {
-      this.runQuery();
+      this.runQuery({keepVisual: true});
     }
   }
 
   componentWillReceiveProps(nextProps) {
     const {
       queryBuilder,
-      location: {search},
+      location: {search, query},
       savedQuery,
       isEditingSavedQuery,
       params,
     } = nextProps;
-    const currentSearch = this.props.location.search;
+    const currentQuery = this.props.location.query;
     const {resultManager} = this.state;
 
     if (savedQuery && savedQuery !== this.props.savedQuery) {
@@ -92,7 +93,9 @@ export default class OrganizationDiscover extends React.Component {
       return;
     }
 
-    if (currentSearch === search) {
+    // Don't compare `visual` since there is no need to re-query if queries are equal
+    // but views are changing
+    if (areQueriesEqual(currentQuery, query)) {
       return;
     }
 
@@ -177,8 +180,11 @@ export default class OrganizationDiscover extends React.Component {
     this.runQuery();
   };
 
-  runQuery = () => {
-    const {queryBuilder, organization} = this.props;
+  // runQuery can get called from many different paths - only keep visual in URL params
+  // when the option is passed. We want it to reset in most cases (but not when running query
+  // when initially mounted)
+  runQuery = ({keepVisual} = {}) => {
+    const {queryBuilder, organization, location} = this.props;
     const {resultManager} = this.state;
 
     // Track query for analytics
@@ -212,7 +218,10 @@ export default class OrganizationDiscover extends React.Component {
         if (shouldRedirect) {
           browserHistory.push({
             pathname: `/organizations/${organization.slug}/discover/`,
-            search: getQueryStringFromQuery(queryBuilder.getInternal()),
+            query: {
+              ...getQueryObjectFromQuery(queryBuilder.getInternal()),
+              visual: keepVisual && location.query.visual,
+            },
           });
         }
 

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

@@ -1,19 +1,16 @@
-import React from 'react';
-import PropTypes from 'prop-types';
 import {throttle} from 'lodash';
+import {withRouter} from 'react-router';
+import PropTypes from 'prop-types';
+import React from 'react';
 
-import SentryTypes from 'app/sentryTypes';
 import {t} from 'app/locale';
-import getDynamicText from 'app/utils/getDynamicText';
 import BarChart from 'app/components/charts/barChart';
-import LineChart from 'app/components/charts/lineChart';
 import InlineSvg from 'app/components/inlineSvg';
+import LineChart from 'app/components/charts/lineChart';
 import PageHeading from 'app/components/pageHeading';
+import SentryTypes from 'app/sentryTypes';
+import getDynamicText from 'app/utils/getDynamicText';
 
-import {getChartData, getChartDataByDay, getRowsPageRange, downloadAsCsv} from './utils';
-import Table from './table';
-import Pagination from './pagination';
-import VisualizationsToggle from './visualizationsToggle';
 import {
   HeadingContainer,
   ResultSummary,
@@ -25,19 +22,26 @@ import {
   ResultSummaryAndButtons,
 } from '../styles';
 import {NUMBER_OF_SERIES_BY_DAY} from '../data';
+import {getChartData, getChartDataByDay, getRowsPageRange, downloadAsCsv} from './utils';
+import Pagination from './pagination';
+import Table from './table';
+import VisualizationsToggle from './visualizationsToggle';
+
+const DEFAULT_VIEW = 'table';
 
-export default class Result extends React.Component {
+class Result extends React.Component {
   static propTypes = {
+    location: PropTypes.object,
     data: PropTypes.object.isRequired,
     savedQuery: SentryTypes.DiscoverSavedQuery, // Provided if it's a saved search
     onFetchPage: PropTypes.func.isRequired,
     onToggleEdit: PropTypes.func,
   };
 
-  constructor() {
-    super();
+  constructor(props) {
+    super(props);
     this.state = {
-      view: 'table',
+      view: props.location.query.visual || DEFAULT_VIEW,
       height: null,
       width: null,
     };
@@ -50,6 +54,17 @@ export default class Result extends React.Component {
   componentWillReceiveProps(nextProps) {
     const {baseQuery, byDayQuery} = nextProps.data;
 
+    if (nextProps.location.query.visual) {
+      this.setState({
+        view: nextProps.location.query.visual,
+      });
+    } else if (this.props.location.query.visual) {
+      // Going from selected -> none (e.g. clicking back), should show default
+      this.setState({
+        view: DEFAULT_VIEW,
+      });
+    }
+
     if (!byDayQuery.data && ['line-by-day', 'bar-by-day'].includes(this.state.view)) {
       this.setState({
         view: 'table',
@@ -88,12 +103,6 @@ export default class Result extends React.Component {
 
   throttledUpdateDimensions = throttle(this.updateDimensions, 200, {trailing: true});
 
-  handleToggleVisualizations = opt => {
-    this.setState({
-      view: opt,
-    });
-  };
-
   renderToggle() {
     const {baseQuery, byDayQuery} = this.props.data;
 
@@ -116,7 +125,6 @@ export default class Result extends React.Component {
       <div>
         <VisualizationsToggle
           options={options}
-          handleChange={this.handleToggleVisualizations}
           handleCsvDownload={handleCsvDownload}
           visualization={this.state.view}
         />
@@ -269,3 +277,6 @@ export default class Result extends React.Component {
     );
   }
 }
+
+export default withRouter(Result);
+export {Result};

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

@@ -1,9 +1,11 @@
+import {Link, withRouter} from 'react-router';
 import PropTypes from 'prop-types';
 import React from 'react';
 import classNames from 'classnames';
+
+import {t} from 'app/locale';
 import DropdownLink from 'app/components/dropdownLink';
 import MenuItem from 'app/components/menuItem';
-import {t} from 'app/locale';
 
 import {
   ResultViewActions,
@@ -20,18 +22,31 @@ class VisualizationsToggle extends React.Component {
         name: PropTypes.string,
       })
     ).isRequired,
-    handleChange: PropTypes.func.isRequired,
     handleCsvDownload: PropTypes.func.isRequired,
     visualization: PropTypes.string.isRequired,
   };
 
+  getUrl = id => {
+    const {location} = this.props;
+
+    return {
+      ...location,
+      query: {
+        ...location.query,
+        visual: id,
+      },
+    };
+  };
+
   getMenuItem = opt => {
+    const {location, visualization} = this.props;
     return (
       <MenuItem
         key={opt.id}
-        onSelect={this.props.handleChange}
+        to={location.pathname}
+        query={{...location.query, visual: opt.id}}
         eventKey={opt.id}
-        isActive={opt.id === this.props.visualization}
+        isActive={opt.id === visualization}
       >
         {opt.name}
       </MenuItem>
@@ -42,7 +57,7 @@ class VisualizationsToggle extends React.Component {
     const active = opt.id === this.props.visualization;
     return (
       <li key={opt.id} className={classNames({active})}>
-        <a onClick={() => this.props.handleChange(opt.id)}>{opt.name}</a>
+        <Link to={this.getUrl(opt.id)}>{opt.name}</Link>
       </li>
     );
   };
@@ -81,4 +96,4 @@ class VisualizationsToggle extends React.Component {
   }
 }
 
-export default VisualizationsToggle;
+export default withRouter(VisualizationsToggle);

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

@@ -1,21 +1,25 @@
+import {isEqual, pick} from 'lodash';
 import moment from 'moment';
+
 import {Client} from 'app/api';
-import {isValidAggregation} from './aggregations/utils';
+import {defined} from 'app/utils';
+
 import {NON_SNUBA_FIELDS} from './data';
+import {isValidAggregation} from './aggregations/utils';
 
-export function getQueryFromQueryString(queryString) {
-  const validQueryKeys = new Set([
-    'projects',
-    'fields',
-    'conditions',
-    'aggregations',
-    'range',
-    'start',
-    'end',
-    'orderby',
-    'limit',
-  ]);
+const validQueryKeys = new Set([
+  'projects',
+  'fields',
+  'conditions',
+  'aggregations',
+  'range',
+  'start',
+  'end',
+  'orderby',
+  'limit',
+]);
 
+export function getQueryFromQueryString(queryString) {
   const result = {};
   let parsedQuery = queryString;
   parsedQuery = parsedQuery.replace(/^\?|\/$/g, '').split('&');
@@ -31,12 +35,19 @@ export function getQueryFromQueryString(queryString) {
   return result;
 }
 
-export function getQueryStringFromQuery(query) {
-  const queryProperties = Object.entries(query).map(([key, value]) => {
-    return key + '=' + encodeURIComponent(JSON.stringify(value));
-  });
+export function getQueryObjectFromQuery(query) {
+  // Filter out undefined/null values
+  const queryProperties = Object.entries(query)
+    .filter(([key, value]) => defined(value))
+    .map(([key, value]) => {
+      return [key, JSON.stringify(value)];
+    })
+    .reduce((acc, [key, val]) => {
+      acc[key] = val;
+      return acc;
+    }, {});
 
-  return `?${queryProperties.join('&')}`;
+  return queryProperties;
 }
 
 export function getOrderbyFields(queryBuilder) {
@@ -99,6 +110,11 @@ export function getView(params, requestedView) {
   }
 }
 
+export function areQueriesEqual(query, other) {
+  const validKeys = Array.from(validQueryKeys);
+  return isEqual(pick(query, validKeys), pick(other, validKeys));
+}
+
 /**
  * Takes a saved query and strips associated query metadata in order to match
  * our internal representation of queries.