Browse Source

feat(ui): Add actual data to chart in Incident Rules [SEN-895] (#14271)

This adds data to the chart when creating or modifying an incident rule
Billy Vong 5 years ago
parent
commit
c7ab5bbef9

+ 1 - 0
package.json

@@ -140,6 +140,7 @@
     "jest-canvas-mock": "^2.1.0",
     "jest-junit": "^3.4.1",
     "mockdate": "2.0.2",
+    "object.fromentries": "^2.0.0",
     "prettier": "1.16.4",
     "react-test-renderer": "16.7.0",
     "source-map-loader": "^0.2.4",

+ 9 - 7
src/sentry/static/sentry/app/actionCreators/events.tsx

@@ -49,13 +49,15 @@ export const doEventsRequest = (
   }: Options
 ): Promise<EventsStats> => {
   const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period);
-  const urlQuery = {
-    interval,
-    project,
-    environment,
-    query,
-    yAxis,
-  };
+  const urlQuery = Object.fromEntries(
+    Object.entries({
+      interval,
+      project,
+      environment,
+      query,
+      yAxis,
+    }).filter(([, value]) => typeof value !== 'undefined')
+  );
 
   // Doubling period for absolute dates is not accurate unless starting and
   // ending times are the same (at least for daily intervals). This is

+ 11 - 4
src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx

@@ -27,11 +27,10 @@ type RenderProps = {
   timeAggregatedData?: Series | {};
 };
 
-type EventsRequestProps = {
+type EventsRequestPartialProps = {
   // TODO(ts): Update when we type `app/api`
   api: object;
   organization: Organization;
-  timeAggregationSeriesName: string;
 
   project?: number[];
   environment?: string[];
@@ -44,13 +43,18 @@ type EventsRequestProps = {
   query?: string;
   includePrevious?: boolean;
   includeTransformedData?: boolean;
-  includeTimeAggregation?: boolean;
   loading?: boolean;
   showLoading?: boolean;
   yAxis?: 'event_count' | 'user_count';
   children: (renderProps: RenderProps) => React.ReactNode;
 };
 
+type TimeAggregationProps =
+  | {includeTimeAggregation: true; timeAggregationSeriesName: string}
+  | {includeTimeAggregation?: false; timeAggregationSeriesName?: undefined};
+
+type EventsRequestProps = TimeAggregationProps & EventsRequestPartialProps;
+
 type EventsRequestState = {
   reloading: boolean;
   timeseriesData: null | EventsStats;
@@ -266,7 +270,10 @@ class EventsRequest extends React.PureComponent<EventsRequestProps, EventsReques
   /**
    * Aggregate all counts for each time stamp
    */
-  transformAggregatedTimeseries = (data: EventsStatsData, seriesName: string): Series => {
+  transformAggregatedTimeseries = (
+    data: EventsStatsData,
+    seriesName: string = ''
+  ): Series => {
     return {
       seriesName,
       data: this.calculateTotalsPerTimestamp(data),

+ 18 - 16
src/sentry/static/sentry/app/views/settings/projectIncidentRules/chart.tsx

@@ -3,20 +3,15 @@ import {debounce, maxBy} from 'lodash';
 import React from 'react';
 import styled from 'react-emotion';
 
-import {ReactEchartsRef} from 'app/types/echarts';
+import {ReactEchartsRef, Series, SeriesDataUnit} from 'app/types/echarts';
 import {Panel} from 'app/components/panels';
 import Graphic from 'app/components/charts/components/graphic';
 import LineChart from 'app/components/charts/lineChart';
 import space from 'app/styles/space';
 
-type DataArray = Array<[number, number]>;
-
 type Props = {
-  data: Array<{
-    seriesName: string;
-    dataArray: DataArray;
-  }>;
-  onChangeUpperBound: Function;
+  data: Series[];
+  onChangeUpperBound: (upperBound: number) => void;
   upperBound: number;
 };
 type State = {
@@ -35,21 +30,27 @@ export default class IncidentRulesChart extends React.Component<Props, State> {
   };
 
   componentDidUpdate(prevProps: Props) {
-    const {data, upperBound} = this.props;
     if (
-      upperBound !== prevProps.upperBound &&
-      this.chartRef &&
-      data.length &&
-      data[0].dataArray
+      this.props.upperBound !== prevProps.upperBound ||
+      this.props.data !== prevProps.data
     ) {
-      this.updateChartAxis(upperBound, data[0].dataArray);
+      this.handleUpdateChartAxis();
     }
   }
 
   chartRef: null | ECharts = null;
 
-  updateChartAxis = debounce((upperBound, dataArray: DataArray) => {
-    const max = maxBy(dataArray, ([_ts, number]) => number);
+  // If we have ref to chart and data, try to update chart axis so that
+  // upperBound is visible in chart
+  handleUpdateChartAxis = () => {
+    const {data, upperBound} = this.props;
+    if (this.chartRef && data.length && data[0].data) {
+      this.updateChartAxis(upperBound, data[0].data);
+    }
+  };
+
+  updateChartAxis = debounce((upperBound, dataArray: SeriesDataUnit[]) => {
+    const max = maxBy(dataArray, ({value}) => value);
     if (typeof max !== 'undefined' && upperBound > max) {
       // We need to force update after we set a new yAxis max because `convertToPixel` will
       // can return a negitive position (probably because yAxisMax is not synced with chart yet)
@@ -74,6 +75,7 @@ export default class IncidentRulesChart extends React.Component<Props, State> {
     if (ref && typeof ref.getEchartsInstance === 'function' && !this.chartRef) {
       this.chartRef = ref.getEchartsInstance();
       const width = this.chartRef.getWidth();
+      this.handleUpdateChartAxis();
       if (width !== this.state.width) {
         this.setState({
           width,

+ 33 - 21
src/sentry/static/sentry/app/views/settings/projectIncidentRules/ruleForm.tsx

@@ -2,17 +2,24 @@ import {debounce} from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
 
-import {EventsStatsData} from 'app/types';
+import {EventsStatsData, Organization, Project} from 'app/types';
 import {PanelAlert} from 'app/components/panels';
 import {t} from 'app/locale';
+import EventsRequest from 'app/views/events/utils/eventsRequest';
 import Form from 'app/views/settings/components/forms/form';
 import JsonForm from 'app/views/settings/components/forms/jsonForm';
+import withApi from 'app/utils/withApi';
+import withOrganization from 'app/utils/withOrganization';
+import withProject from 'app/utils/withProject';
 
 import IncidentRulesChart from './chart';
 import {AlertRuleAggregations, AlertRuleThresholdType} from './constants';
 
 type Props = {
+  api: any;
   data: EventsStatsData;
+  organization: Organization;
+  project: Project;
 };
 
 type State = {
@@ -43,27 +50,26 @@ class RuleForm extends React.Component<Props, State> {
     this.setState({upperBound});
     this.context.form.setValue('alertThreshold', Math.round(upperBound));
   };
-
   render() {
+    const {api, organization, project} = this.props;
+
     return (
       <React.Fragment>
-        <IncidentRulesChart
-          onChangeUpperBound={this.handleChangeUpperBound}
-          upperBound={this.state.upperBound}
-          data={[
-            {
-              seriesName: 'Test',
-              dataArray: this.props.data.map(([ts, val]) => {
-                return [
-                  ts * 1000,
-                  val.length
-                    ? val.reduce((acc, {count} = {count: 0}) => acc + (count || 0), 0)
-                    : 0,
-                ];
-              }),
-            },
-          ]}
-        />
+        <EventsRequest
+          api={api}
+          organization={organization}
+          project={[parseInt(project.id, 10)]}
+          interval="10m"
+          includePrevious={false}
+        >
+          {({timeseriesData}) => (
+            <IncidentRulesChart
+              onChangeUpperBound={this.handleChangeUpperBound}
+              upperBound={this.state.upperBound}
+              data={timeseriesData}
+            />
+          )}
+        </EventsRequest>
         <JsonForm
           renderHeader={() => {
             return (
@@ -157,6 +163,9 @@ class RuleForm extends React.Component<Props, State> {
 }
 
 type RuleFormContainerProps = {
+  api: any;
+  organization: Organization;
+  project: Project;
   orgId: string;
   projectId: string;
   incidentRuleId?: string;
@@ -164,6 +173,9 @@ type RuleFormContainerProps = {
   onSubmitSuccess?: Function;
 };
 function RuleFormContainer({
+  api,
+  organization,
+  project,
   orgId,
   projectId,
   incidentRuleId,
@@ -185,9 +197,9 @@ function RuleFormContainer({
       saveOnBlur={false}
       onSubmitSuccess={onSubmitSuccess}
     >
-      <RuleForm {...props} />
+      <RuleForm api={api} project={project} organization={organization} {...props} />
     </Form>
   );
 }
 
-export default RuleFormContainer;
+export default withApi(withOrganization(withProject(RuleFormContainer)));

+ 5 - 0
tests/js/setup.js

@@ -5,6 +5,7 @@ import Adapter from 'enzyme-adapter-react-16';
 import Enzyme from 'enzyme';
 import MockDate from 'mockdate';
 import PropTypes from 'prop-types';
+import fromEntries from 'object.fromentries';
 
 import ConfigStore from 'app/stores/configStore';
 import theme from 'app/utils/theme';
@@ -13,6 +14,10 @@ import {loadFixtures} from './helpers/loadFixtures';
 
 export * from './helpers/select';
 
+// We need this polyfill for testing only because
+// typescript handles it for main application
+fromEntries.shim();
+
 /**
  * Enzyme configuration
  */