Browse Source

feat(dashboard): Allow linking from percentage tables to events (#11744)

Allow clicking in percentage table charts to events (with specific groupby condition per row)
Billy Vong 6 years ago
parent
commit
12f71f4753

+ 60 - 11
src/sentry/static/sentry/app/components/charts/percentageTableChart.jsx

@@ -1,5 +1,7 @@
+import {Link} from 'react-router';
 import PropTypes from 'prop-types';
 import React from 'react';
+import classNames from 'classnames';
 import styled from 'react-emotion';
 
 import {PanelItem} from 'app/components/panels';
@@ -55,6 +57,17 @@ class PercentageTableChart extends React.Component {
 
     extraTitle: PropTypes.node,
     onRowClick: PropTypes.func,
+
+    // Class name for header
+    headerClassName: PropTypes.string,
+
+    // Class name for rows
+    rowClassName: PropTypes.string,
+
+    // If this is a function and returns a truthy value, then the row will be a link
+    // to the return value of this function
+    getRowLink: PropTypes.func,
+
     data: PropTypes.arrayOf(
       PropTypes.shape({
         name: PropTypes.node,
@@ -78,7 +91,15 @@ class PercentageTableChart extends React.Component {
   };
 
   render() {
-    const {title, countTitle, extraTitle, data} = this.props;
+    const {
+      rowClassName,
+      headerClassName,
+      getRowLink,
+      title,
+      countTitle,
+      extraTitle,
+      data,
+    } = this.props;
 
     return (
       <TableChart
@@ -96,7 +117,14 @@ class PercentageTableChart extends React.Component {
           </React.Fragment>,
         ])}
         renderRow={({items, rowIndex, ...other}) => (
-          <Row onClick={this.handleRowClick} data={data} rowIndex={rowIndex}>
+          <Row
+            dataRowClassName={rowClassName}
+            headerRowClassName={headerClassName}
+            getRowLink={getRowLink}
+            onClick={this.handleRowClick}
+            data={data}
+            rowIndex={rowIndex}
+          >
             <NameAndCountContainer>
               {items[0]}
               <div>{items[1]}</div>
@@ -126,15 +154,36 @@ class PercentageTableChart extends React.Component {
   }
 }
 
-const Row = styled(function RowComponent({className, data, rowIndex, onClick, children}) {
-  return (
-    <div
-      className={className}
-      onClick={e => typeof onClick === 'function' && onClick(data[rowIndex], e)}
-    >
-      {children}
-    </div>
-  );
+const Row = styled(function RowComponent({
+  headerRowClassName,
+  dataRowClassName,
+  className,
+  data,
+  getRowLink,
+  rowIndex,
+  onClick,
+  children,
+}) {
+  const isLink = typeof getRowLink === 'function' && rowIndex > -1;
+  const linkPath = isLink && getRowLink(data[rowIndex]);
+  const Component = isLink ? Link : 'div';
+  const rowProps = {
+    className: classNames(
+      className,
+      rowIndex > -1 && dataRowClassName,
+      rowIndex === -1 && headerRowClassName
+    ),
+    children,
+    ...(linkPath && {
+      to: linkPath,
+    }),
+    ...(!isLink &&
+      typeof onClick === 'function' && {
+        onClick: e => onClick(data[rowIndex], e),
+      }),
+  };
+
+  return <Component {...rowProps} />;
 })`
   display: flex;
   flex: 1;

+ 39 - 0
src/sentry/static/sentry/app/views/organizationDashboard/utils/getEventsUrlFromDiscoverQueryWithConditions.jsx

@@ -0,0 +1,39 @@
+/**
+ * Generate a URL to the events page for a discover query that
+ * contains a condition.
+ *
+ * @param {Object} query The discover query object
+ * @param {String[]} values A list of strings that represent field values
+ *   e.g. if the query has multiple fields (browser, device), values could be ["Chrome", "iPhone"]
+ * @return {String} Returns a url to the "events" page with any discover conditions tranformed to search query syntax
+ */
+import {zipWith} from 'lodash';
+
+import {OPERATOR} from 'app/views/organizationDiscover/data';
+import {getEventsUrlPathFromDiscoverQuery} from 'app/views/organizationDashboard/utils/getEventsUrlPathFromDiscoverQuery';
+
+export function getEventsUrlFromDiscoverQueryWithConditions({
+  values,
+  query,
+  selection,
+  organization,
+}) {
+  return getEventsUrlPathFromDiscoverQuery({
+    organization,
+    selection,
+    query: {
+      ...query,
+      conditions: [
+        ...query.conditions,
+        // For each `field`, create a condition that joins it with each `rowObject.name` value (separated by commas)
+        // e.g. fields: ['browser', 'device'],  rowObject.name: "Chrome, iPhone"
+        //      ----> [['browser', '=', 'Chrome'], ['device', '=', 'iPhone']]
+        ...zipWith(query.fields, values, (field, value) => [
+          field,
+          OPERATOR.EQUAL,
+          value === null ? '""' : `"${value}"`,
+        ]),
+      ],
+    },
+  });
+}

+ 30 - 2
src/sentry/static/sentry/app/views/organizationDashboard/widgetChart.jsx

@@ -1,12 +1,20 @@
+import {css} from 'react-emotion';
 import {isEqual} from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
 
+import {WIDGET_DISPLAY} from 'app/views/organizationDashboard/constants';
 import {getChartComponent} from 'app/views/organizationDashboard/utils/getChartComponent';
 import {getData} from 'app/views/organizationDashboard/utils/getData';
+import {getEventsUrlFromDiscoverQueryWithConditions} from 'app/views/organizationDashboard/utils/getEventsUrlFromDiscoverQueryWithConditions';
 import ChartZoom from 'app/components/charts/chartZoom';
 import ReleaseSeries from 'app/components/charts/releaseSeries';
 import SentryTypes from 'app/sentryTypes';
+import theme from 'app/utils/theme';
+
+const tableRowCss = css`
+  color: ${theme.textColor};
+`;
 
 /**
  * Component that decides what Chart to render
@@ -50,7 +58,8 @@ class WidgetChart extends React.Component {
   }
 
   render() {
-    const {results, releases, widget} = this.props;
+    const {organization, results, releases, selection, widget} = this.props;
+    const isTable = widget.type === WIDGET_DISPLAY.TABLE;
 
     // get visualization based on widget data
     const ChartComponent = getChartComponent(widget);
@@ -58,12 +67,30 @@ class WidgetChart extends React.Component {
     // get data func based on query
     const chartData = getData(results, widget);
 
+    const extra = {
+      ...(isTable && {
+        rowClassName: tableRowCss,
+        getRowLink: rowObject => {
+          // Table Charts don't support multiple queries
+          const [query] = widget.queries.discover;
+
+          return getEventsUrlFromDiscoverQueryWithConditions({
+            values: rowObject.fieldValues,
+            query,
+            organization,
+            selection,
+          });
+        },
+      }),
+    };
+
     // Releases can only be added to time charts
     if (widget.includeReleases) {
       return (
         <ReleaseSeries releases={releases}>
           {({releaseSeries}) =>
             this.renderZoomableChart(ChartComponent, {
+              ...extra,
               ...chartData,
               series: [...chartData.series, ...releaseSeries],
             })}
@@ -73,11 +100,12 @@ class WidgetChart extends React.Component {
 
     if (chartData.isGroupedByDate) {
       return this.renderZoomableChart(ChartComponent, {
+        ...extra,
         ...chartData,
       });
     }
 
-    return <ChartComponent {...chartData} />;
+    return <ChartComponent {...extra} {...chartData} />;
   }
 }
 

+ 3 - 2
src/sentry/static/sentry/app/views/organizationDiscover/result/utils.jsx

@@ -1,8 +1,8 @@
 /*eslint no-use-before-define: ["error", { "functions": false }]*/
-import React from 'react';
-import styled from 'react-emotion';
 import {orderBy} from 'lodash';
 import Papa from 'papaparse';
+import React from 'react';
+import styled from 'react-emotion';
 
 import {NUMBER_OF_SERIES_BY_DAY} from '../data';
 
@@ -67,6 +67,7 @@ export function getChartDataForWidget(data, query, options = {}) {
         const obj = {
           value: res[aggregation[2]],
           name: fields.map(field => `${res[field]}`).join(', '),
+          fieldValues: fields.map(field => res[field]),
         };
 
         if (options.includePercentages && total) {

+ 83 - 0
tests/js/spec/views/organizationDashboard/utils/getEventsUrlFromDiscoverQueryWithConditions.spec.jsx

@@ -0,0 +1,83 @@
+import {getEventsUrlFromDiscoverQueryWithConditions} from 'app/views/organizationDashboard/utils/getEventsUrlFromDiscoverQueryWithConditions';
+
+describe('getEventsUrlFromDiscoverQueryWithConditions', function() {
+  const organization = TestStubs.Organization();
+
+  it('single field', function() {
+    const query = {
+      fields: ['browser.name'],
+      conditions: [],
+      aggregations: ['count()', null, 'count'],
+      limit: 1000,
+      orderby: 'count',
+    };
+    expect(
+      getEventsUrlFromDiscoverQueryWithConditions({
+        organization,
+        selection: {
+          datetime: {
+            start: null,
+            end: null,
+            period: '14d',
+          },
+        },
+        query,
+        values: ['Chrome'],
+      })
+    ).toBe(
+      '/organizations/org-slug/events/?query=browser.name%3A%22Chrome%22&statsPeriod=14d'
+    );
+  });
+
+  it('multiple fields', function() {
+    const query = {
+      fields: ['browser.name', 'device'],
+      conditions: [],
+      aggregations: ['count()', null, 'count'],
+      limit: 1000,
+      orderby: 'count',
+    };
+    expect(
+      getEventsUrlFromDiscoverQueryWithConditions({
+        organization,
+        selection: {
+          datetime: {
+            start: null,
+            end: null,
+            period: '14d',
+          },
+        },
+        query,
+        values: ['Chrome', 'iPhone'],
+      })
+    ).toBe(
+      '/organizations/org-slug/events/?query=browser.name%3A%22Chrome%22%20device%3A%22iPhone%22&statsPeriod=14d'
+    );
+  });
+
+  it('handles null values and spaces', function() {
+    const query = {
+      fields: ['browser.name', 'device'],
+      conditions: [],
+      aggregations: ['count()', null, 'count'],
+      limit: 1000,
+      orderby: 'count',
+    };
+    expect(
+      getEventsUrlFromDiscoverQueryWithConditions({
+        organization,
+        selection: {
+          datetime: {
+            start: null,
+            end: null,
+            period: '14d',
+          },
+        },
+        query,
+        values: [null, 'iPhone X'],
+      })
+    ).toBe(
+      '/organizations/org-slug/events/?query=browser.name%3A%22%22%20device%3A%22iPhone%20X%22&statsPeriod=14d'
+    );
+  });
+});

+ 29 - 9
tests/js/spec/views/organizationDiscover/result/utils.spec.jsx

@@ -60,24 +60,44 @@ describe('Utils', function() {
       fields: ['project.id', 'environment'],
     };
 
-    it('returns chart data with percentages', function() {
+    it('returns chart data for widgets with percentages', function() {
       const expected = [
         {
           seriesName: 'count',
           data: [
-            {value: 2, percentage: 16.67, name: '5, null'},
-            {value: 2, percentage: 16.67, name: '5, staging'},
-            {value: 2, percentage: 16.67, name: '5, alpha'},
-            {value: 6, percentage: 50, name: '5, production'},
+            {value: 2, percentage: 16.67, name: '5, null', fieldValues: [5, null]},
+            {
+              value: 2,
+              percentage: 16.67,
+              name: '5, staging',
+              fieldValues: [5, 'staging'],
+            },
+            {value: 2, percentage: 16.67, name: '5, alpha', fieldValues: [5, 'alpha']},
+            {
+              value: 6,
+              percentage: 50,
+              name: '5, production',
+              fieldValues: [5, 'production'],
+            },
           ],
         },
         {
           seriesName: 'uniq_id',
           data: [
-            {value: 1, percentage: 5.56, name: '5, null'},
-            {value: 3, percentage: 16.67, name: '5, staging'},
-            {value: 4, percentage: 22.22, name: '5, alpha'},
-            {value: 10, percentage: 55.56, name: '5, production'},
+            {value: 1, percentage: 5.56, name: '5, null', fieldValues: [5, null]},
+            {
+              value: 3,
+              percentage: 16.67,
+              name: '5, staging',
+              fieldValues: [5, 'staging'],
+            },
+            {value: 4, percentage: 22.22, name: '5, alpha', fieldValues: [5, 'alpha']},
+            {
+              value: 10,
+              percentage: 55.56,
+              name: '5, production',
+              fieldValues: [5, 'production'],
+            },
           ],
         },
       ];