feat(widget-builder): Deprecate Top 5 Events in v2 Widget Builder (#34497)

The functionality from the Top 5 Events display type is covered by using
an area chart with a grouping and limit. This frontend change opens
saved top 5 widgets as area charts and uses area charts for Add to Dashboard

static/app/views/dashboardsV2/widgetBuilder/buildSteps/visualizationStep.tsx

@@ -8,7 +8,6 @@ import {TableCell} from 'sentry/components/charts/simpleTableChart';
 import Field from 'sentry/components/forms/field';
 import SelectControl from 'sentry/components/forms/selectControl';
 import {PanelAlert} from 'sentry/components/panels';
-import Tag from 'sentry/components/tag';
 import {DEFAULT_DEBOUNCE_DURATION} from 'sentry/constants';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
@@ -42,6 +41,10 @@ export function VisualizationStep({
 
   const previousWidget = usePrevious(widget);
 
+  // Disabling for now because we use debounce to avoid excessively hitting
+  // our endpoints, but useCallback wants an inline function and not one
+  // returned from debounce
+  // eslint-disable-next-line react-hooks/exhaustive-deps
   const debounceWidget = useCallback(
     debounce((value: Widget, shouldCancelUpdates: boolean) => {
       if (shouldCancelUpdates) {
@@ -62,19 +65,10 @@ export function VisualizationStep({
     return () => {
       shouldCancelUpdates = true;
     };
-  }, [widget, previousWidget]);
+  }, [widget, previousWidget, debounceWidget]);
 
   const displayOptions = Object.keys(displayTypes).map(value => ({
-    label:
-      organization.features.includes('new-widget-builder-experience-design') &&
-      value === DisplayType.TOP_N ? (
-        <DisplayOptionLabel>
-          {displayTypes[value]}
-          <Tag type="info">{t('deprecated')}</Tag>
-        </DisplayOptionLabel>
-      ) : (
-        displayTypes[value]
-      ),
+    label: displayTypes[value],
     value,
   }));
 
@@ -142,9 +136,3 @@ const VisualizationWrapper = styled('div')<{displayType: DisplayType}>`
       }
     `};
 `;
-
-const DisplayOptionLabel = styled('span')`
-  display: flex;
-  justify-content: space-between;
-  width: calc(100% - ${space(1)});
-`;

static/app/views/dashboardsV2/widgetBuilder/utils.tsx

@@ -56,7 +56,6 @@ export const displayTypes = {
   [DisplayType.TABLE]: t('Table'),
   [DisplayType.WORLD_MAP]: t('World Map'),
   [DisplayType.BIG_NUMBER]: t('Big Number'),
-  [DisplayType.TOP_N]: t('Top 5 Events'),
 };
 
 export function mapErrors(

static/app/views/dashboardsV2/widgetBuilder/widgetBuilder.tsx

@@ -235,10 +235,13 @@ function WidgetBuilder({
   const [state, setState] = useState<State>(() => {
     const defaultState: State = {
       title: defaultTitle ?? t('Custom Widget'),
-      displayType: displayType ?? DisplayType.TABLE,
+      displayType:
+        (widgetBuilderNewDesign && displayType === DisplayType.TOP_N
+          ? DisplayType.AREA
+          : displayType) ?? DisplayType.TABLE,
       interval: '5m',
       queries: [],
-      limit,
+      limit: limit ? Number(limit) : undefined,
       errors: undefined,
       loading: !!notDashboardsOrigin,
       dashboards: [],
@@ -265,7 +268,13 @@ function WidgetBuilder({
         defaultState.queries = [{...defaultWidgetQuery}];
       }
 
-      if (![DisplayType.TABLE, DisplayType.TOP_N].includes(defaultState.displayType)) {
+      if (
+        ![DisplayType.TABLE, DisplayType.TOP_N].includes(defaultState.displayType) &&
+        !(
+          getIsTimeseriesChart(defaultState.displayType) &&
+          defaultState.queries[0].columns.length
+        )
+      ) {
         defaultState.queries[0].orderby = '';
       }
     } else {
@@ -291,16 +300,40 @@ function WidgetBuilder({
 
     if (isEditing && isValidWidgetIndex) {
       const widgetFromDashboard = filteredDashboardWidgets[widgetIndexNum];
-      setState({
-        title: widgetFromDashboard.title,
-        displayType: widgetFromDashboard.displayType,
-        interval: widgetFromDashboard.interval,
-        queries: normalizeQueries({
-          displayType: widgetFromDashboard.displayType,
+
+      let queries;
+      let newDisplayType = widgetFromDashboard.displayType;
+      let newLimit = widgetFromDashboard.limit;
+      if (widgetFromDashboard.displayType === DisplayType.TOP_N) {
+        newLimit = DEFAULT_RESULTS_LIMIT;
+        newDisplayType = DisplayType.AREA;
+
+        queries = normalizeQueries({
+          displayType: newDisplayType,
           queries: widgetFromDashboard.queries,
           widgetType: widgetFromDashboard.widgetType ?? WidgetType.DISCOVER,
           widgetBuilderNewDesign,
-        }),
+        }).map(query => ({
+          ...query,
+          // Use the last aggregate because that's where the y-axis is stored
+          aggregates: query.aggregates.length
+            ? [query.aggregates[query.aggregates.length - 1]]
+            : [],
+        }));
+      } else {
+        queries = normalizeQueries({
+          displayType: newDisplayType,
+          queries: widgetFromDashboard.queries,
+          widgetType: widgetFromDashboard.widgetType ?? WidgetType.DISCOVER,
+          widgetBuilderNewDesign,
+        });
+      }
+
+      setState({
+        title: widgetFromDashboard.title,
+        displayType: newDisplayType,
+        interval: widgetFromDashboard.interval,
+        queries,
         errors: undefined,
         loading: false,
         dashboards: [],
@@ -308,13 +341,35 @@ function WidgetBuilder({
         dataSet: widgetFromDashboard.widgetType
           ? WIDGET_TYPE_TO_DATA_SET[widgetFromDashboard.widgetType]
           : DataSet.EVENTS,
-        limit: widgetFromDashboard.limit,
+        limit: newLimit,
       });
       setWidgetToBeUpdated(widgetFromDashboard);
     }
+    // This should only run once on mount
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
   useEffect(() => {
+    async function fetchDashboards() {
+      const promise: Promise<DashboardListItem[]> = api.requestPromise(
+        `/organizations/${organization.slug}/dashboards/`,
+        {
+          method: 'GET',
+          query: {sort: 'myDashboardsAndRecentlyViewed'},
+        }
+      );
+
+      try {
+        const dashboards = await promise;
+        setState(prevState => ({...prevState, dashboards, loading: false}));
+      } catch (error) {
+        const errorMessage = t('Unable to fetch dashboards');
+        addErrorMessage(errorMessage);
+        handleXhrErrorResponse(errorMessage)(error);
+        setState(prevState => ({...prevState, loading: false}));
+      }
+    }
+
     if (notDashboardsOrigin) {
       fetchDashboards();
     }
@@ -328,11 +383,19 @@ function WidgetBuilder({
         },
       }));
     }
-  }, [source]);
+  }, [
+    api,
+    dashboard.id,
+    dashboard.title,
+    notDashboardsOrigin,
+    organization.slug,
+    source,
+    widgetBuilderNewDesign,
+  ]);
 
   useEffect(() => {
     fetchOrgMembers(api, organization.slug, selection.projects?.map(String));
-  }, [selection.projects]);
+  }, [selection.projects, api, organization.slug]);
 
   const widgetType =
     state.dataSet === DataSet.EVENTS
@@ -897,26 +960,6 @@ function WidgetBuilder({
     }
   }
 
-  async function fetchDashboards() {
-    const promise: Promise<DashboardListItem[]> = api.requestPromise(
-      `/organizations/${organization.slug}/dashboards/`,
-      {
-        method: 'GET',
-        query: {sort: 'myDashboardsAndRecentlyViewed'},
-      }
-    );
-
-    try {
-      const dashboards = await promise;
-      setState(prevState => ({...prevState, dashboards, loading: false}));
-    } catch (error) {
-      const errorMessage = t('Unable to fetch dashboards');
-      addErrorMessage(errorMessage);
-      handleXhrErrorResponse(errorMessage)(error);
-      setState(prevState => ({...prevState, loading: false}));
-    }
-  }
-
   function submitFromSelectedDashboard(widgetData: Widget) {
     if (!state.selectedDashboard) {
       return;

static/app/views/dashboardsV2/widgetBuilder/widgetLibrary/index.tsx

@@ -8,7 +8,7 @@ import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {DisplayType} from 'sentry/views/dashboardsV2/types';
 import {
-  DEFAULT_WIDGETS,
+  getTopNConvertedDefaultWidgets,
   WidgetTemplate,
 } from 'sentry/views/dashboardsV2/widgetLibrary/data';
 
@@ -28,6 +28,7 @@ export function WidgetLibrary({
   widgetBuilderNewDesign,
 }: Props) {
   const theme = useTheme();
+  const defaultWidgets = getTopNConvertedDefaultWidgets();
 
   function getLibrarySelectionHandler(
     widget: OverwriteWidgetModalProps['widget'],
@@ -51,8 +52,8 @@ export function WidgetLibrary({
     <Fragment>
       <Header>{t('Widget Library')}</Header>
       <WidgetLibraryWrapper>
-        {DEFAULT_WIDGETS.map((widget, index) => {
-          const iconColor = theme.charts.getColorPalette(DEFAULT_WIDGETS.length - 2)[
+        {defaultWidgets.map((widget, index) => {
+          const iconColor = theme.charts.getColorPalette(defaultWidgets.length - 2)[
             index
           ];
 

static/app/views/dashboardsV2/widgetLibrary/data.tsx

@@ -1,4 +1,5 @@
 import {t} from 'sentry/locale';
+import {TOP_N} from 'sentry/utils/discover/types';
 
 import {DisplayType, Widget, WidgetType} from '../types';
 
@@ -166,3 +167,16 @@ export const DEFAULT_WIDGETS: Readonly<Array<WidgetTemplate>> = [
     ],
   },
 ];
+
+export function getTopNConvertedDefaultWidgets(): Readonly<Array<WidgetTemplate>> {
+  return DEFAULT_WIDGETS.map(widget => {
+    if (widget.displayType === DisplayType.TOP_N) {
+      return {
+        ...widget,
+        displayType: DisplayType.AREA,
+        limit: TOP_N,
+      };
+    }
+    return widget;
+  });
+}

static/app/views/eventsV2/utils.tsx

@@ -35,7 +35,7 @@ import {
   measurementType,
   TRACING_FIELDS,
 } from 'sentry/utils/discover/fields';
-import {DisplayModes} from 'sentry/utils/discover/types';
+import {DisplayModes, TOP_N} from 'sentry/utils/discover/types';
 import {getTitle} from 'sentry/utils/events';
 import localStorage from 'sentry/utils/localStorage';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
@@ -562,9 +562,11 @@ export function eventViewToWidgetQuery({
   eventView,
   yAxis,
   displayType,
+  widgetBuilderNewDesign,
 }: {
   displayType: DisplayType;
   eventView: EventView;
+  widgetBuilderNewDesign?: boolean;
   yAxis?: string | string[];
 }) {
   const fields = eventView.fields.map(({field}) => field);
@@ -574,7 +576,7 @@ export function eventViewToWidgetQuery({
   let orderby = '';
   // The orderby should only be set to sort.field if it is a Top N query
   // since the query uses all of the fields, or if the ordering is used in the y-axis
-  if (sort) {
+  if (sort && displayType !== DisplayType.WORLD_MAP) {
     let orderbyFunction = '';
     const aggregateFields = [...queryYAxis, ...aggregates];
     for (let i = 0; i < aggregateFields.length; i++) {
@@ -589,9 +591,20 @@ export function eventViewToWidgetQuery({
       orderby = `${sort.kind === 'desc' ? '-' : ''}${bareOrderby}`;
     }
   }
+  let newAggregates = aggregates;
+
+  if (widgetBuilderNewDesign && displayType !== DisplayType.TABLE) {
+    newAggregates = queryYAxis;
+  } else if (!widgetBuilderNewDesign) {
+    newAggregates = [
+      ...(displayType === DisplayType.TOP_N ? aggregates : []),
+      ...queryYAxis,
+    ];
+  }
+
   const widgetQuery: WidgetQuery = {
     name: '',
-    aggregates: [...(displayType === DisplayType.TOP_N ? aggregates : []), ...queryYAxis],
+    aggregates: newAggregates,
     columns: [...(displayType === DisplayType.TOP_N ? columns : [])],
     fields: [...(displayType === DisplayType.TOP_N ? fields : []), ...queryYAxis],
     conditions: eventView.query,
@@ -621,6 +634,9 @@ export function handleAddQueryToDashboard({
     eventView,
     displayType,
     yAxis,
+    widgetBuilderNewDesign: organization.features.includes(
+      'new-widget-builder-experience-design'
+    ),
   });
 
   if (organization.features.includes('new-widget-builder-experience-design')) {
@@ -645,9 +661,23 @@ export function handleAddQueryToDashboard({
       },
       widget: {
         title: query?.name ?? eventView.name,
-        displayType,
-        queries: [defaultWidgetQuery],
+        displayType:
+          organization.features.includes('new-widget-builder-experience-design') &&
+          displayType === DisplayType.TOP_N
+            ? DisplayType.AREA
+            : displayType,
+        queries: [
+          {
+            ...defaultWidgetQuery,
+            aggregates: [...(typeof yAxis === 'string' ? [yAxis] : yAxis ?? ['count()'])],
+          },
+        ],
         interval: eventView.interval,
+        limit:
+          organization.features.includes('new-widget-builder-experience-design') &&
+          displayType === DisplayType.TOP_N
+            ? Number(eventView.topEvents) || TOP_N
+            : undefined,
       },
       router,
       widgetAsQueryParams,
@@ -692,6 +722,9 @@ export function constructAddQueryToDashboardLink({
     eventView,
     displayType,
     yAxis,
+    widgetBuilderNewDesign: organization.features.includes(
+      'new-widget-builder-experience-design'
+    ),
   });
   const defaultTitle =
     query?.name ?? (eventView.name !== 'All Events' ? eventView.name : undefined);
@@ -707,7 +740,16 @@ export function constructAddQueryToDashboardLink({
       defaultWidgetQuery: urlEncode(defaultWidgetQuery),
       defaultTableColumns: defaultTableFields,
       defaultTitle,
-      displayType,
+      displayType:
+        organization.features.includes('new-widget-builder-experience-design') &&
+        displayType === DisplayType.TOP_N
+          ? DisplayType.AREA
+          : displayType,
+      limit:
+        organization.features.includes('new-widget-builder-experience-design') &&
+        displayType === DisplayType.TOP_N
+          ? Number(eventView.topEvents) || TOP_N
+          : undefined,
     },
   };
 }

tests/js/spec/views/dashboardsV2/widgetBuilder/buildSteps/visualizationStep.spec.tsx

@@ -140,40 +140,4 @@ describe('VisualizationStep', function () {
 
     await screen.findByText(/we've automatically adjusted your results/i);
   });
-
-  it('displays a deprecated tag next to the top 5 display option', async function () {
-    mockRequests(organization.slug);
-
-    render(
-      <WidgetBuilder
-        route={{}}
-        router={router}
-        routes={router.routes}
-        routeParams={router.params}
-        location={router.location}
-        dashboard={{
-          id: 'new',
-          title: 'Dashboard',
-          createdBy: undefined,
-          dateCreated: '2020-01-01T00:00:00.000Z',
-          widgets: [],
-        }}
-        onSave={jest.fn()}
-        params={{
-          orgId: organization.slug,
-          dashboardId: 'new',
-        }}
-      />,
-      {
-        context: routerContext,
-        organization,
-      }
-    );
-
-    expect(screen.queryByText('deprecated')).not.toBeInTheDocument();
-    userEvent.click(await screen.findByText('Table'));
-    screen.getByText('deprecated');
-    userEvent.click(screen.getByText('Top 5 Events'));
-    screen.getByText('deprecated');
-  });
 });

tests/js/spec/views/dashboardsV2/widgetBuilder/widgetBuilder.spec.tsx

@@ -912,47 +912,6 @@ describe('WidgetBuilder', function () {
     });
   });
 
-  it('should automatically add columns for top n widget charts according to the URL params', async function () {
-    const defaultWidgetQuery = {
-      name: '',
-      fields: ['title', 'count()', 'count_unique(user)', 'epm()', 'count()'],
-      columns: ['title'],
-      aggregates: ['count()', 'count_unique(user)', 'epm()', 'count()'],
-      conditions: 'tag:value',
-      orderby: '',
-    };
-
-    renderTestComponent({
-      query: {
-        source: DashboardWidgetSource.DISCOVERV2,
-        defaultWidgetQuery: urlEncode(defaultWidgetQuery),
-        displayType: DisplayType.TOP_N,
-        defaultTableColumns: ['title', 'count()', 'count_unique(user)', 'epm()'],
-      },
-    });
-
-    //  Top N display
-    expect(await screen.findByText('Top 5 Events')).toBeInTheDocument();
-
-    // No delete button as there is only one field.
-    expect(screen.queryByLabelText('Remove query')).not.toBeInTheDocument();
-
-    // Restricting to a single query
-    expect(screen.queryByLabelText('Add Query')).not.toBeInTheDocument();
-
-    // Restricting to a single y-axis
-    expect(screen.queryByLabelText('Add Overlay')).not.toBeInTheDocument();
-
-    expect(screen.getByText('Choose what to plot in the y-axis')).toBeInTheDocument();
-
-    expect(screen.getByText('Sort by a column')).toBeInTheDocument();
-
-    expect(screen.getByText('title')).toBeInTheDocument();
-    expect(screen.getAllByText('count()')).toHaveLength(2);
-    expect(screen.getByText('count_unique(…)')).toBeInTheDocument();
-    expect(screen.getByText('user')).toBeInTheDocument();
-  });
-
   it('should use defaultWidgetQuery Y-Axis and Conditions if given a defaultWidgetQuery', async function () {
     const defaultWidgetQuery = {
       name: '',
@@ -994,51 +953,6 @@ describe('WidgetBuilder', function () {
     expect(await screen.findByText('Bar Chart')).toBeInTheDocument();
   });
 
-  it('correctly defaults fields and orderby when in Top N display', async function () {
-    const defaultWidgetQuery = {
-      fields: ['title', 'count()', 'count_unique(user)'],
-      columns: ['title'],
-      aggregates: ['count()', 'count_unique(user)'],
-      orderby: '-count_unique(user)',
-    };
-
-    renderTestComponent({
-      query: {
-        source: DashboardWidgetSource.DISCOVERV2,
-        defaultWidgetQuery: urlEncode(defaultWidgetQuery),
-        displayType: DisplayType.TOP_N,
-        defaultTableColumns: ['title', 'count()'],
-      },
-    });
-
-    userEvent.click(await screen.findByText('Top 5 Events'));
-
-    expect(screen.getByText('count()')).toBeInTheDocument();
-    expect(screen.getByText('count_unique(…)')).toBeInTheDocument();
-    expect(screen.getByText('user')).toBeInTheDocument();
-
-    // Sort by a column
-    expect(screen.getByText('Sort by a column')).toBeInTheDocument();
-    expect(screen.getByText('count_unique(user) desc')).toBeInTheDocument();
-  });
-
-  it('limits TopN display to one query when switching from another visualization', async () => {
-    renderTestComponent();
-
-    userEvent.click(await screen.findByText('Table'));
-    userEvent.click(screen.getByText('Bar Chart'));
-    userEvent.click(screen.getByLabelText('Add Query'));
-    userEvent.click(screen.getByLabelText('Add Query'));
-    expect(
-      screen.getAllByPlaceholderText('Search for events, users, tags, and more')
-    ).toHaveLength(3);
-    userEvent.click(screen.getByText('Bar Chart'));
-    userEvent.click(await screen.findByText('Top 5 Events'));
-    expect(
-      screen.getByPlaceholderText('Search for events, users, tags, and more')
-    ).toBeInTheDocument();
-  });
-
   it('deletes the widget when the modal is confirmed', async () => {
     const handleSave = jest.fn();
     const widget: Widget = {
@@ -1659,16 +1573,6 @@ describe('WidgetBuilder', function () {
 
       // SortBy step shall no longer be visible
       expect(screen.queryByText('Sort by a y-axis')).not.toBeInTheDocument();
-
-      // Update visualization to be "Top 5 Events"
-      userEvent.click(screen.getByText('Line Chart'));
-      userEvent.click(screen.getByText('Top 5 Events'));
-
-      // Tabular visualizations display "Choose your columns" step
-      expect(await screen.findByText('Choose your columns')).toBeInTheDocument();
-
-      // SortBy step shall be visible
-      expect(screen.getByText('Sort by a y-axis')).toBeInTheDocument();
     });
 
     it('allows for sorting by a custom equation', async function () {
@@ -2218,7 +2122,7 @@ describe('WidgetBuilder', function () {
     });
   });
 
-  it('opens top-N widgets as top-N display', async function () {
+  it('opens top-N widgets as area display', async function () {
     const widget: Widget = {
       id: '1',
       title: 'Errors over time',
@@ -2252,7 +2156,7 @@ describe('WidgetBuilder', function () {
       },
     });
 
-    expect(await screen.findByText('Top 5 Events')).toBeInTheDocument();
+    expect(await screen.findByText('Area Chart')).toBeInTheDocument();
   });
 
   it('Update table header values (field alias)', async function () {

tests/js/spec/views/eventsV2/queryList.spec.jsx

@@ -283,6 +283,7 @@ describe('EventsV2 > QueryList', function () {
               display: DisplayModes.TOP5,
               orderby: 'test',
               fields: ['test', 'count()'],
+              yAxis: ['count()'],
             }),
           ]}
           pageLinks=""
@@ -315,10 +316,11 @@ describe('EventsV2 > QueryList', function () {
         expect.objectContaining({
           widget: {
             title: 'Saved query #1',
-            displayType: DisplayType.TOP_N,
+            displayType: DisplayType.AREA,
+            limit: 5,
             queries: [
               {
-                aggregates: ['count()', 'count()'],
+                aggregates: ['count()'],
                 columns: ['test'],
                 conditions: '',
                 fields: ['test', 'count()', 'count()'],
@@ -331,8 +333,8 @@ describe('EventsV2 > QueryList', function () {
             defaultTableColumns: ['test', 'count()'],
             defaultTitle: 'Saved query #1',
             defaultWidgetQuery:
-              'name=&aggregates=count()%2Ccount()&columns=test&fields=test%2Ccount()%2Ccount()&conditions=&orderby=test',
-            displayType: DisplayType.TOP_N,
+              'name=&aggregates=count()&columns=test&fields=test%2Ccount()%2Ccount()&conditions=&orderby=test',
+            displayType: DisplayType.AREA,
             source: DashboardWidgetSource.DISCOVERV2,
           }),
         })

tests/js/spec/views/eventsV2/savedQuery/index.spec.tsx

@@ -361,6 +361,7 @@ describe('EventsV2 > SaveQueryButtonGroup', function () {
         display: DisplayModes.TOP5,
         orderby: 'test',
         fields: ['test', 'count()'],
+        topEvents: '2',
       });
       mount(
         testData.router.location,
@@ -378,10 +379,11 @@ describe('EventsV2 > SaveQueryButtonGroup', function () {
           expect.objectContaining({
             widget: {
               title: 'Saved query #1',
-              displayType: DisplayType.TOP_N,
+              displayType: DisplayType.AREA,
+              limit: 2,
               queries: [
                 {
-                  aggregates: ['count()', 'count()'],
+                  aggregates: ['count()'],
                   columns: ['test'],
                   conditions: '',
                   fields: ['test', 'count()', 'count()'],
@@ -394,8 +396,8 @@ describe('EventsV2 > SaveQueryButtonGroup', function () {
               defaultTableColumns: ['test', 'count()'],
               defaultTitle: 'Saved query #1',
               defaultWidgetQuery:
-                'name=&aggregates=count()%2Ccount()&columns=test&fields=test%2Ccount()%2Ccount()&conditions=&orderby=test',
-              displayType: DisplayType.TOP_N,
+                'name=&aggregates=count()&columns=test&fields=test%2Ccount()%2Ccount()&conditions=&orderby=test',
+              displayType: DisplayType.AREA,
               source: DashboardWidgetSource.DISCOVERV2,
             }),
           })