Browse Source

feat(new-widget-builder-experience): Limit field shows at most 10 lines (#33607)

Nar Saynorath 2 years ago
parent
commit
99fa3da562

+ 4 - 0
static/app/actionCreators/events.tsx

@@ -20,6 +20,7 @@ type Options = {
   comparisonDelta?: number;
   end?: DateString;
   environment?: Readonly<string[]>;
+  excludeOther?: boolean;
   field?: string[];
   generatePathname?: (org: OrganizationSummary) => string;
   includePrevious?: boolean;
@@ -47,6 +48,7 @@ type Options = {
  * @param {Object} options.organization Organization object
  * @param {Number[]} options.project List of project ids
  * @param {String[]} options.environment List of environments to query for
+ * @param {Boolean} options.excludeOther Exclude the "Other" series when making a topEvents query
  * @param {String[]} options.team List of teams to query for
  * @param {String} options.period Time period to query for, in the format: <integer><units> where units are "d" or "h"
  * @param {String} options.interval Time interval to group results in, in the format: <integer><units> where units are "d", "h", "m", "s"
@@ -82,6 +84,7 @@ export const doEventsRequest = (
     queryBatching,
     generatePathname,
     queryExtras,
+    excludeOther,
   }: Options
 ): Promise<EventsStats | MultiSeriesEventsStats> => {
   const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period);
@@ -100,6 +103,7 @@ export const doEventsRequest = (
       partial: partial ? '1' : undefined,
       withoutZerofill: withoutZerofill ? '1' : undefined,
       referrer: referrer ? referrer : 'api.organization-event-stats',
+      excludeOther: excludeOther ? '1' : undefined,
     }).filter(([, value]) => typeof value !== 'undefined')
   );
 

+ 13 - 2
static/app/views/dashboardsV2/widgetBuilder/buildSteps/sortByStep/index.tsx

@@ -1,3 +1,4 @@
+import {useEffect} from 'react';
 import styled from '@emotion/styled';
 
 import {generateOrderOptions} from 'sentry/components/dashboards/widgetQueriesForm';
@@ -10,7 +11,7 @@ import {DisplayType, WidgetQuery, WidgetType} from 'sentry/views/dashboardsV2/ty
 import {generateIssueWidgetOrderOptions} from 'sentry/views/dashboardsV2/widgetBuilder/issueWidget/utils';
 import {IssueSortOptions} from 'sentry/views/issueList/utils';
 
-import {DataSet, RESULTS_LIMIT, SortDirection} from '../../utils';
+import {DataSet, getResultsLimit, SortDirection} from '../../utils';
 import {BuildStep} from '../buildStep';
 
 import {SortBySelectors} from './sortBySelectors';
@@ -41,6 +42,16 @@ export function SortByStep({
   onLimitChange,
 }: Props) {
   const orderBy = queries[0].orderby;
+  const maxLimit = getResultsLimit(queries.length, queries[0].aggregates.length);
+
+  useEffect(() => {
+    if (!limit) {
+      return;
+    }
+    if (limit > maxLimit) {
+      onLimitChange(maxLimit);
+    }
+  }, [limit, maxLimit]);
 
   if (widgetBuilderNewDesign) {
     return (
@@ -62,7 +73,7 @@ export function SortByStep({
               <ResultsLimitSelector
                 name="resultsLimit"
                 menuPlacement="auto"
-                options={[...Array(RESULTS_LIMIT).keys()].map(resultLimit => {
+                options={[...Array(maxLimit).keys()].map(resultLimit => {
                   const value = resultLimit + 1;
                   return {
                     label: tn('Limit to %s result', 'Limit to %s results', value),

+ 5 - 1
static/app/views/dashboardsV2/widgetBuilder/utils.tsx

@@ -27,7 +27,7 @@ import {FlatValidationError, ValidationError} from '../utils';
 
 // Used in the widget builder to limit the number of lines plotted in the chart
 export const DEFAULT_RESULTS_LIMIT = 5;
-export const RESULTS_LIMIT = 10;
+const RESULTS_LIMIT = 10;
 
 // Both dashboards and widgets use the 'new' keyword when creating
 export const NEW_DASHBOARD_ID = 'new';
@@ -338,3 +338,7 @@ export function filterPrimaryOptions({
     option.value.kind
   );
 }
+
+export function getResultsLimit(numQueries, numYAxes) {
+  return Math.floor(RESULTS_LIMIT / (numQueries * numYAxes));
+}

+ 21 - 15
static/app/views/dashboardsV2/widgetBuilder/widgetBuilder.tsx

@@ -75,6 +75,7 @@ import {
   DataSet,
   DEFAULT_RESULTS_LIMIT,
   getParsedDefaultWidgetQuery,
+  getResultsLimit,
   mapErrors,
   NEW_DASHBOARD_ID,
   normalizeQueries,
@@ -504,16 +505,6 @@ function WidgetBuilder({
       const newState = cloneDeep(prevState);
       set(newState, `queries.${queryIndex}`, newQuery);
       set(newState, 'userHasModified', true);
-
-      if (widgetBuilderNewDesign && isTimeseriesChart && queryIndex === 0) {
-        const groupByFields = newQuery.columns.filter(field => !(field === 'equation|'));
-
-        if (groupByFields.length === 0) {
-          set(newState, 'limit', undefined);
-        } else {
-          set(newState, 'limit', newState.limit ?? DEFAULT_RESULTS_LIMIT);
-        }
-      }
       return {...newState, errors: undefined};
     });
   }
@@ -605,7 +596,14 @@ function WidgetBuilder({
       if (groupByFields.length === 0) {
         set(newState, 'limit', undefined);
       } else {
-        set(newState, 'limit', newState.limit ?? DEFAULT_RESULTS_LIMIT);
+        set(
+          newState,
+          'limit',
+          Math.min(
+            newState.limit ?? DEFAULT_RESULTS_LIMIT,
+            getResultsLimit(newQueries.length, newQueries[0].aggregates.length)
+          )
+        );
       }
     }
 
@@ -617,13 +615,14 @@ function WidgetBuilder({
 
     const newState = cloneDeep(state);
 
-    state.queries.forEach((query, index) => {
+    const newQueries = state.queries.map(query => {
       const newQuery = cloneDeep(query);
       newQuery.columns = fieldStrings;
-      set(newState, `queries.${index}`, newQuery);
+      return newQuery;
     });
 
     set(newState, 'userHasModified', true);
+    set(newState, 'queries', newQueries);
 
     if (widgetBuilderNewDesign && isTimeseriesChart) {
       const groupByFields = newState.queries[0].columns.filter(
@@ -632,7 +631,14 @@ function WidgetBuilder({
       if (groupByFields.length === 0) {
         set(newState, 'limit', undefined);
       } else {
-        set(newState, 'limit', newState.limit ?? DEFAULT_RESULTS_LIMIT);
+        set(
+          newState,
+          'limit',
+          Math.min(
+            newState.limit ?? DEFAULT_RESULTS_LIMIT,
+            getResultsLimit(newQueries.length, newQueries[0].aggregates.length)
+          )
+        );
       }
     }
 
@@ -640,7 +646,7 @@ function WidgetBuilder({
   }
 
   function handleLimitChange(newLimit: number) {
-    setState({...state, limit: newLimit});
+    setState(prevState => ({...prevState, limit: newLimit}));
   }
 
   function handleSortByChange(newSortBy: string) {

+ 5 - 0
static/app/views/dashboardsV2/widgetCard/widgetQueries.tsx

@@ -417,6 +417,11 @@ class WidgetQueries extends React.Component<Props, State> {
           requestData.topEvents = widget.limit ?? TOP_N;
           // Aggregates need to be in fields as well
           requestData.field = [...query.columns, ...query.aggregates];
+
+          // The "Other" series is only included when there is one
+          // y-axis and one query
+          requestData.excludeOther =
+            query.aggregates.length !== 1 || widget.queries.length !== 1;
         }
       }
       return doEventsRequest(api, requestData);

+ 87 - 0
tests/js/spec/views/dashboardsV2/widgetBuilder/widgetBuilder.spec.tsx

@@ -1305,6 +1305,93 @@ describe('WidgetBuilder', function () {
     expect(tagsMock).not.toHaveBeenCalled();
   });
 
+  it('excludes the Other series when grouping and using multiple y-axes', async function () {
+    renderTestComponent({
+      orgFeatures: [...defaultOrgFeatures, 'new-widget-builder-experience-design'],
+      query: {
+        displayType: DisplayType.LINE,
+      },
+    });
+
+    await selectEvent.select(await screen.findByText('Select group'), 'project');
+
+    userEvent.click(screen.getByText('Add Overlay'));
+    await selectEvent.select(screen.getByText('(Required)'), /count_unique/);
+
+    await waitFor(() => {
+      expect(eventsStatsMock).toBeCalledWith(
+        '/organizations/org-slug/events-stats/',
+        expect.objectContaining({
+          query: expect.objectContaining({excludeOther: '1'}),
+        })
+      );
+    });
+  });
+
+  it('excludes the Other series when grouping and using multiple queries', async function () {
+    renderTestComponent({
+      orgFeatures: [...defaultOrgFeatures, 'new-widget-builder-experience-design'],
+      query: {
+        displayType: DisplayType.LINE,
+      },
+    });
+
+    await selectEvent.select(await screen.findByText('Select group'), 'project');
+    userEvent.click(screen.getByText('Add Query'));
+
+    await waitFor(() => {
+      expect(eventsStatsMock).toBeCalledWith(
+        '/organizations/org-slug/events-stats/',
+        expect.objectContaining({
+          query: expect.objectContaining({excludeOther: '1'}),
+        })
+      );
+    });
+  });
+
+  it('includes Other series when there is only one query and one y-axis', async function () {
+    renderTestComponent({
+      orgFeatures: [...defaultOrgFeatures, 'new-widget-builder-experience-design'],
+      query: {
+        displayType: DisplayType.LINE,
+      },
+    });
+
+    await selectEvent.select(await screen.findByText('Select group'), 'project');
+
+    await waitFor(() => {
+      expect(eventsStatsMock).toBeCalledWith(
+        '/organizations/org-slug/events-stats/',
+        expect.objectContaining({
+          query: expect.not.objectContaining({excludeOther: '1'}),
+        })
+      );
+    });
+  });
+
+  it('decreases the limit when more y-axes and queries are added', async function () {
+    renderTestComponent({
+      orgFeatures: [...defaultOrgFeatures, 'new-widget-builder-experience-design'],
+      query: {
+        displayType: DisplayType.LINE,
+      },
+    });
+
+    await selectEvent.select(await screen.findByText('Select group'), 'project');
+
+    screen.getByText('Limit to 5 results');
+
+    userEvent.click(screen.getByText('Add Query'));
+    userEvent.click(screen.getByText('Add Query'));
+
+    screen.getByText('Limit to 3 results');
+
+    userEvent.click(screen.getByText('Add Overlay'));
+    userEvent.click(screen.getByText('Add Overlay'));
+
+    await screen.findByText('Limit to 1 result');
+  });
+
   describe('Sort by selectors', function () {
     it('renders', async function () {
       renderTestComponent({