feat(ui) Add consistent ordering to multi series charts (#18463)

Use the server-side order information to sort multi-series results. This
enables charts and tooltips to have consistent ordering with the table
shown below. This change also impacts multiple yaxis results so I needed
to change performance charts too.

src/sentry/static/sentry/app/actionCreators/events.tsx

@@ -5,7 +5,7 @@ import {Client} from 'app/api';
 import {URL_PARAM} from 'app/constants/globalSelectionHeader';
 import {canIncludePreviousPeriod} from 'app/views/events/utils/canIncludePreviousPeriod';
 import {getPeriod} from 'app/utils/getPeriod';
-import {EventsStats, Organization, YAxisEventsStats} from 'app/types';
+import {EventsStats, Organization, MultiSeriesEventsStats} from 'app/types';
 
 function getBaseUrl(org: Organization, keyTransactions: boolean | undefined) {
   if (keyTransactions) {
@@ -67,7 +67,7 @@ export const doEventsRequest = (
     topEvents,
     orderby,
   }: Options
-): Promise<EventsStats | YAxisEventsStats> => {
+): Promise<EventsStats | MultiSeriesEventsStats> => {
   const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period);
   const urlQuery = Object.fromEntries(
     Object.entries({

src/sentry/static/sentry/app/types/index.tsx

@@ -305,14 +305,15 @@ export type Event = ({type: string} & SentryEventBase) | SentryTransactionEvent;
 
 export type EventsStatsData = [number, {count: number}[]][];
 
+// API response format for a single series
 export type EventsStats = {
   data: EventsStatsData;
   totals?: {count: number};
+  order?: number;
 };
 
-export type YAxisEventsStats = {
-  [yAxisName: string]: EventsStats;
-};
+// API response format for multiple series
+export type MultiSeriesEventsStats = {[seriesName: string]: EventsStats};
 
 /**
  * Avatars are a more primitive version of User.

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

@@ -188,7 +188,7 @@ class EventsChart extends React.Component {
           </ErrorPanel>
         );
       }
-      const seriesData = results ? Object.values(results) : timeseriesData;
+      const seriesData = results ? results : timeseriesData;
 
       return (
         <TransitionChart loading={loading} reloading={reloading}>

src/sentry/static/sentry/app/views/events/utils/eventsRequest.tsx

@@ -3,7 +3,12 @@ import omitBy from 'lodash/omitBy';
 import PropTypes from 'prop-types';
 import React from 'react';
 
-import {Organization, EventsStats, YAxisEventsStats, EventsStatsData} from 'app/types';
+import {
+  Organization,
+  EventsStats,
+  MultiSeriesEventsStats,
+  EventsStatsData,
+} from 'app/types';
 import {Series, SeriesDataUnit} from 'app/types/echarts';
 import {Client} from 'app/api';
 import {doEventsRequest} from 'app/actionCreators/events';
@@ -34,11 +39,8 @@ type LoadingStatus = {
   errored: boolean;
 };
 
-// API response format for multiple series
-type MultiSeriesData = {[seriesName: string]: EventsStats};
-
 // Chart format for multiple series.
-type MultiSeriesResults = {[seriesName: string]: Series};
+type MultiSeriesResults = Series[];
 
 type RenderProps = LoadingStatus & TimeSeriesData & {results?: MultiSeriesResults};
 
@@ -148,16 +150,16 @@ type EventsRequestProps = DefaultProps & TimeAggregationProps & EventsRequestPar
 type EventsRequestState = {
   reloading: boolean;
   errored: boolean;
-  timeseriesData: null | EventsStats | YAxisEventsStats;
+  timeseriesData: null | EventsStats | MultiSeriesEventsStats;
 };
 
 const propNamesToIgnore = ['api', 'children', 'organization', 'loading'];
 const omitIgnoredProps = (props: EventsRequestProps) =>
   omitBy(props, (_value, key) => propNamesToIgnore.includes(key));
 
-function isMultiSeriesData(
-  data: MultiSeriesData | EventsStats | null
-): data is MultiSeriesData {
+function isMultiSeriesStats(
+  data: MultiSeriesEventsStats | EventsStats | null
+): data is MultiSeriesEventsStats {
   return data !== null && data.data === undefined && data.totals === undefined;
 }
 
@@ -235,7 +237,7 @@ class EventsRequest extends React.PureComponent<EventsRequestProps, EventsReques
 
   fetchData = async () => {
     const {api, ...props} = this.props;
-    let timeseriesData: EventsStats | YAxisEventsStats | null = null;
+    let timeseriesData: EventsStats | MultiSeriesEventsStats | null = null;
 
     this.setState(state => ({
       reloading: state.timeseriesData !== null,
@@ -391,21 +393,23 @@ class EventsRequest extends React.PureComponent<EventsRequestProps, EventsReques
       return <LoadingPanel data-test-id="events-request-loading" />;
     }
 
-    if (isMultiSeriesData(timeseriesData)) {
+    if (isMultiSeriesStats(timeseriesData)) {
       // Convert multi-series results into chartable series. Multi series results
       // are created when multiple yAxis are used or a topEvents request is made.
       // Convert the timeseries data into a multi-series result set.
       // As the server will have replied with a map like:
       // {[titleString: string]: EventsStats}
-      const results: MultiSeriesResults = Object.fromEntries(
-        Object.keys(timeseriesData).map((seriesName: string): [string, Series] => {
+      const results: MultiSeriesResults = Object.keys(timeseriesData)
+        .map((seriesName: string): [number, Series] => {
           const seriesData: EventsStats = timeseriesData[seriesName];
-          return [
-            seriesName,
-            this.transformTimeseriesData(seriesData.data, seriesName)[0],
-          ];
+          const transformed = this.transformTimeseriesData(
+            seriesData.data,
+            seriesName
+          )[0];
+          return [seriesData.order || 0, transformed];
         })
-      );
+        .sort((a, b) => a[0] - b[0])
+        .map(item => item[1]);
 
       return children({
         loading,

src/sentry/static/sentry/app/views/performance/charts/index.tsx

@@ -98,6 +98,9 @@ class Container extends React.Component<Props> {
               if (!results) {
                 return <LoadingPanel data-test-id="events-request-loading" />;
               }
+              const resultMap = Object.fromEntries(
+                results.map(item => [item.seriesName, item])
+              );
 
               return (
                 <ChartsGrid>
@@ -108,7 +111,7 @@ class Container extends React.Component<Props> {
                           <Chart
                             loading={loading || reloading}
                             yAxis={yAxis.label}
-                            data={results[yAxis.value]}
+                            data={resultMap[yAxis.value]}
                             router={router}
                             statsPeriod={globalSelection.statsPeriod}
                             utc={utc === 'true'}

tests/js/spec/views/events/utils/eventsRequest.spec.jsx

@@ -398,22 +398,21 @@ describe('EventsRequest', function() {
       await tick();
       wrapper.update();
 
-      const expectedData = {
-        seriesName: expect.anything(),
-        data: [
-          {name: expect.anything(), value: 400},
-          {name: expect.anything(), value: 123},
-        ],
+      const generateExpected = name => {
+        return {
+          seriesName: name,
+          data: [
+            {name: expect.anything(), value: 400},
+            {name: expect.anything(), value: 123},
+          ],
+        };
       };
 
       expect(mock).toHaveBeenLastCalledWith(
         expect.objectContaining({
           loading: false,
 
-          results: {
-            'apdex()': expect.objectContaining(expectedData),
-            'rpm()': expect.objectContaining(expectedData),
-          },
+          results: [generateExpected('rpm()'), generateExpected('apdex()')],
         })
       );
     });
@@ -468,22 +467,24 @@ describe('EventsRequest', function() {
       await tick();
       wrapper.update();
 
-      const expectedData = {
-        seriesName: expect.anything(),
-        data: [
-          {name: expect.anything(), value: 400},
-          {name: expect.anything(), value: 123},
-        ],
+      const generateExpected = name => {
+        return {
+          seriesName: name,
+          data: [
+            {name: expect.anything(), value: 400},
+            {name: expect.anything(), value: 123},
+          ],
+        };
       };
 
       expect(mock).toHaveBeenLastCalledWith(
         expect.objectContaining({
           loading: false,
 
-          results: {
-            'project1,error': expect.objectContaining(expectedData),
-            'project1,warning': expect.objectContaining(expectedData),
-          },
+          results: [
+            generateExpected('project1,error'),
+            generateExpected('project1,warning'),
+          ],
         })
       );
     });