Browse Source

fix(dashboard-duration-chart-axis-duplication): Removed duplication. (#49855)

Fixes the [Duplication of axis labels in Dashboard Widget Line
Charts](https://github.com/getsentry/sentry/issues/49615).

Uses a combination of duration unit conversion during formatting of
labels and setting a minInterval for the y axis.

1. When there is a chance for a transition between units in the y axis
labels, stick to the smaller unit:
  
Before:
<img width="343" alt="Screenshot 2023-05-26 at 10 26 32 AM"
src="https://github.com/getsentry/sentry/assets/60121741/9dbe7027-812b-4d42-9381-d92c5ab29e0c">
  
After: 
<img width="434" alt="Screenshot 2023-05-26 at 10 26 56 AM"
src="https://github.com/getsentry/sentry/assets/60121741/179890f1-5c17-4654-b723-8e300fda6bc6">

2. Sets the minInterval to be the Minimum fixed value of the unit
chosen. Therefore there are no decimals involved when formatting the
axis labels.

3. Added a test.

---------

Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Abdkhan14 1 year ago
parent
commit
6abf8deb08

+ 19 - 6
static/app/views/dashboards/widgetCard/chart.tsx

@@ -27,6 +27,7 @@ import {EChartDataZoomHandler, EChartEventHandler} from 'sentry/types/echarts';
 import {
   axisLabelFormatter,
   axisLabelFormatterUsingAggregateOutputType,
+  getDurationUnit,
   tooltipFormatter,
 } from 'sentry/utils/discover/charts';
 import {getFieldFormatter} from 'sentry/utils/discover/fieldRenderers';
@@ -380,6 +381,17 @@ class WidgetCardChart extends Component<WidgetCardChartProps, State> {
 
     const axisField = widget.queries[0]?.aggregates?.[0] ?? 'count()';
     const axisLabel = isEquation(axisField) ? getEquation(axisField) : axisField;
+
+    // Check to see if all series output types are the same. If not, then default to number.
+    const outputType =
+      timeseriesResultsTypes && new Set(Object.values(timeseriesResultsTypes)).size === 1
+        ? timeseriesResultsTypes[axisLabel]
+        : 'number';
+    const isDurationChart = outputType === 'duration';
+    const durationUnit = isDurationChart
+      ? timeseriesResults && getDurationUnit(timeseriesResults, legendOptions)
+      : undefined;
+
     const chartOptions = {
       autoHeightResize,
       grid: {
@@ -408,16 +420,17 @@ class WidgetCardChart extends Component<WidgetCardChartProps, State> {
           color: theme.chartLabel,
           formatter: (value: number) => {
             if (timeseriesResultsTypes) {
-              // Check to see if all series output types are the same. If not, then default to number.
-              const outputType =
-                new Set(Object.values(timeseriesResultsTypes)).size === 1
-                  ? timeseriesResultsTypes[axisLabel]
-                  : 'number';
-              return axisLabelFormatterUsingAggregateOutputType(value, outputType);
+              return axisLabelFormatterUsingAggregateOutputType(
+                value,
+                outputType,
+                undefined,
+                durationUnit
+              );
             }
             return axisLabelFormatter(value, aggregateOutputType(axisLabel));
           },
         },
+        minInterval: durationUnit ?? 0,
       },
     };
 

+ 97 - 1
static/app/views/dashboards/widgetCard/index.spec.tsx

@@ -11,6 +11,7 @@ import * as modal from 'sentry/actionCreators/modal';
 import {Client} from 'sentry/api';
 import * as LineChart from 'sentry/components/charts/lineChart';
 import SimpleTableChart from 'sentry/components/charts/simpleTableChart';
+import {MINUTE, SECOND} from 'sentry/utils/formatters';
 import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
 import {DisplayType, Widget, WidgetType} from 'sentry/views/dashboards/types';
 import WidgetCard from 'sentry/views/dashboards/widgetCard';
@@ -650,7 +651,7 @@ describe('Dashboards > WidgetCard', function () {
           },
           units: {
             time: null,
-            p95_measurements_custom: 'minute',
+            p95_measurements_custom: 'millisecond',
           },
           isMetricsData: true,
           tips: {},
@@ -700,6 +701,101 @@ describe('Dashboards > WidgetCard', function () {
     expect(yAxis.axisLabel.formatter(24, 'p95(measurements.custom)')).toEqual('24ms');
   });
 
+  it('renders label in seconds when there is a transition from seconds to minutes in the y axis', async function () {
+    const spy = jest.spyOn(LineChart, 'LineChart');
+    const eventsStatsMock = MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events-stats/',
+      body: {
+        data: [
+          [
+            1658262600,
+            [
+              {
+                count: 40 * SECOND,
+              },
+            ],
+          ],
+          [
+            1658262601,
+            [
+              {
+                count: 50 * SECOND,
+              },
+            ],
+          ],
+          [
+            1658262602,
+            [
+              {
+                count: MINUTE,
+              },
+            ],
+          ],
+          [
+            1658262603,
+            [
+              {
+                count: 1.3 * MINUTE,
+              },
+            ],
+          ],
+        ],
+        meta: {
+          fields: {
+            time: 'date',
+            p50_transaction_duration: 'duration',
+          },
+          units: {
+            time: null,
+            p50_transaction_duration: 'millisecond',
+          },
+          isMetricsData: false,
+          tips: {},
+        },
+      },
+    });
+
+    renderWithProviders(
+      <WidgetCard
+        api={api}
+        organization={organization}
+        widget={{
+          title: '',
+          interval: '5m',
+          widgetType: WidgetType.DISCOVER,
+          displayType: DisplayType.LINE,
+          queries: [
+            {
+              conditions: '',
+              name: '',
+              fields: [],
+              columns: [],
+              aggregates: ['p50(transaction.duration)'],
+              orderby: '',
+            },
+          ],
+        }}
+        selection={selection}
+        isEditing={false}
+        onDelete={() => undefined}
+        onEdit={() => undefined}
+        onDuplicate={() => undefined}
+        renderErrorMessage={() => undefined}
+        showContextMenu
+        widgetLimitReached={false}
+      />
+    );
+    await waitFor(function () {
+      expect(eventsStatsMock).toHaveBeenCalled();
+    });
+    const {yAxis} = spy.mock.calls.pop()?.[0] ?? {};
+    expect(yAxis).toBeDefined();
+
+    // @ts-ignore
+    expect(yAxis.axisLabel.formatter(60000, 'p50(transaction.duration)')).toEqual('60s');
+    expect((yAxis as any).minInterval).toEqual(SECOND);
+  });
+
   it('displays indexed badge in preview mode', async function () {
     renderWithProviders(
       <WidgetCard