diff --git a/static/app/actionCreators/dashboards.tsx b/static/app/actionCreators/dashboards.tsx index 3be7ed6fc4a958..4b3f77744f57c1 100644 --- a/static/app/actionCreators/dashboards.tsx +++ b/static/app/actionCreators/dashboards.tsx @@ -15,6 +15,7 @@ import {getStarredDashboardsQueryKey} from 'sentry/views/dashboards/hooks/useGet import { DashboardFilter, DisplayType, + MAX_CATEGORICAL_BAR_LIMIT, type DashboardDetails, type DashboardListItem, type Widget, @@ -354,34 +355,59 @@ export function validateWidget( } /** - * Enforces a limit on the widget if it is a chart and has a grouping + * Enforces valid limits on widgets before saving to the backend. * - * This ensures that widgets from previously created dashboards will have - * a limit applied properly when editing old dashboards that did not have - * this validation in place. + * - TABLE and BIG_NUMBER widgets should never have limits — clear any stale ones. + * - Chart widgets with grouping must have a limit, capped to the display type's max. + * + * Uses `null` (not `undefined`) so the value survives JSON.stringify and + * reaches the backend, which will clear the stale DB value. */ function _enforceWidgetLimit(widget: Widget) { - if ( - widget.displayType === DisplayType.TABLE || - widget.displayType === DisplayType.BIG_NUMBER - ) { + if (!DISPLAY_TYPES_WITH_LIMITS.has(widget.displayType)) { + return {...widget, limit: null}; + } + + if (widget.queries.length === 0) { return widget; } + let maxLimit: number; + if (widget.displayType === DisplayType.CATEGORICAL_BAR) { + maxLimit = MAX_CATEGORICAL_BAR_LIMIT; + } else { + maxLimit = getResultsLimit( + widget.queries.length, + widget.queries[0]!.aggregates.length + ); + } + const hasColumns = widget.queries.some(query => query.columns.length > 0); + if (hasColumns && !defined(widget.limit)) { // The default we historically assign for charts with a grouping is 5, // continue using that default unless there are conditions which make 5 // too large to automatically apply. - const maxLimit = getResultsLimit( - widget.queries.length, - widget.queries[0]!.aggregates.length - ); return { ...widget, limit: Math.min(maxLimit, TOP_N), }; } + if (hasColumns && defined(widget.limit) && widget.limit > maxLimit) { + return {...widget, limit: maxLimit}; + } + return widget; } + +// Chart types where `limit` controls the Top N grouping cap. +// Other display types either fetch their own data (see `widgetFetchesOwnData`) +// or don't use limits (TABLE, BIG_NUMBER, WHEEL, DETAILS). +const DISPLAY_TYPES_WITH_LIMITS = new Set([ + DisplayType.AREA, + DisplayType.BAR, + DisplayType.LINE, + DisplayType.TOP_N, + DisplayType.CATEGORICAL_BAR, +]); diff --git a/static/app/components/modals/dataWidgetViewerModal.tsx b/static/app/components/modals/dataWidgetViewerModal.tsx index 3f889cd5ae6f53..ca55660d5eacc3 100644 --- a/static/app/components/modals/dataWidgetViewerModal.tsx +++ b/static/app/components/modals/dataWidgetViewerModal.tsx @@ -288,7 +288,7 @@ function DataWidgetViewerModal(props: Props) { // Top N widget charts (including widgets with limits) results rely on the sorting of the query // Set the orderby of the widget chart to match the location query params const primaryWidget = - widget.displayType === DisplayType.TOP_N || widget.limit !== undefined + widget.displayType === DisplayType.TOP_N || defined(widget.limit) ? {...widget, queries: sortedQueries} : widget; const api = useApi(); @@ -658,7 +658,7 @@ function DataWidgetViewerModal(props: Props) { selection={modalSelection} dashboardFilters={dashboardFilters} widget={primaryWidget} - tableItemLimit={widget.limit} + tableItemLimit={widget.limit ?? undefined} onZoom={onZoom} isFullScreen showConfidenceWarning={ @@ -675,7 +675,7 @@ function DataWidgetViewerModal(props: Props) { dashboardFilters={dashboardFilters} // Top N charts rely on the orderby of the table widget={primaryWidget} - tableItemLimit={widget.limit} + tableItemLimit={widget.limit ?? undefined} onZoom={onZoom} onLegendSelectChanged={onLegendSelectChanged} legendOptions={{ diff --git a/static/app/views/dashboards/types.tsx b/static/app/views/dashboards/types.tsx index 8de3739854d693..dea8325a54f046 100644 --- a/static/app/views/dashboards/types.tsx +++ b/static/app/views/dashboards/types.tsx @@ -168,7 +168,7 @@ export type Widget = { layout?: WidgetLayout | null; legendType?: LegendType | null; // Used to define 'topEvents' when fetching time-series data for a widget - limit?: number; + limit?: number | null; // Used for table widget column widths, currently is not saved tableWidths?: number[]; tempId?: string; diff --git a/static/app/views/dashboards/widgetBuilder/components/widgetPreview.tsx b/static/app/views/dashboards/widgetBuilder/components/widgetPreview.tsx index 5f4909a8c4b49f..fa77e307dc8784 100644 --- a/static/app/views/dashboards/widgetBuilder/components/widgetPreview.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/widgetPreview.tsx @@ -129,7 +129,7 @@ export function WidgetPreview({ // dashboard state to be added onWidgetSplitDecision={() => {}} // onWidgetSplitDecision={onWidgetSplitDecision} - tableItemLimit={widget.limit} + tableItemLimit={widget.limit ?? undefined} showConfidenceWarning={ widget.widgetType === WidgetType.SPANS || widget.widgetType === WidgetType.TRACEMETRICS || diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx index e9af15be4bacb6..0d511983d2e2bd 100644 --- a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.spec.tsx @@ -200,7 +200,7 @@ describe('convertBuilderStateToWidget', () => { expect(widget.queries[0]!.orderby).toBe(''); }); - it('sets limit to undefined for table widgets', () => { + it('sets limit to null for table widgets so it survives JSON serialization', () => { const mockState: WidgetBuilderState = { displayType: DisplayType.TABLE, fields: [ @@ -213,10 +213,10 @@ describe('convertBuilderStateToWidget', () => { const widget = convertBuilderStateToWidget(mockState); - expect(widget.limit).toBeUndefined(); + expect(widget.limit).toBeNull(); }); - it('sets limit to undefined for big number widgets', () => { + it('sets limit to null for big number widgets so it survives JSON serialization', () => { const mockState: WidgetBuilderState = { displayType: DisplayType.BIG_NUMBER, fields: [ @@ -228,7 +228,7 @@ describe('convertBuilderStateToWidget', () => { const widget = convertBuilderStateToWidget(mockState); - expect(widget.limit).toBeUndefined(); + expect(widget.limit).toBeNull(); }); it('uses explicit axisRange from state', () => { diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts index f2241a082c7d5d..c43393f77c1a09 100644 --- a/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts +++ b/static/app/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget.ts @@ -136,7 +136,7 @@ export function convertBuilderStateToWidget(state: WidgetBuilderState): Widget { const limit = [DisplayType.BIG_NUMBER, DisplayType.TABLE].includes( state.displayType ?? DisplayType.TABLE ) - ? undefined + ? null : state.limit; return { diff --git a/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts b/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts index 0dbd86a8f41dc6..f72087d54f7e38 100644 --- a/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts +++ b/static/app/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams.ts @@ -62,7 +62,7 @@ export function convertWidgetToQueryParams( description, dataset, displayType: widget.displayType ?? DisplayType.TABLE, - limit: widget.limit, + limit: widget.limit ?? undefined, field, yAxis, query, diff --git a/static/app/views/dashboards/widgetCard/chart.tsx b/static/app/views/dashboards/widgetCard/chart.tsx index 408d7164831e22..267adbf722a294 100644 --- a/static/app/views/dashboards/widgetCard/chart.tsx +++ b/static/app/views/dashboards/widgetCard/chart.tsx @@ -263,7 +263,7 @@ function WidgetCardChart(props: WidgetCardChartProps) { return (