Skip to content
50 changes: 38 additions & 12 deletions static/app/actionCreators/dashboards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
]);
6 changes: 3 additions & 3 deletions static/app/components/modals/dataWidgetViewerModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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={
Expand All @@ -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={{
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/dashboards/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -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: [
Expand All @@ -228,7 +228,7 @@ describe('convertBuilderStateToWidget', () => {

const widget = convertBuilderStateToWidget(mockState);

expect(widget.limit).toBeUndefined();
expect(widget.limit).toBeNull();
});

it('uses explicit axisRange from state', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function convertWidgetToQueryParams(
description,
dataset,
displayType: widget.displayType ?? DisplayType.TABLE,
limit: widget.limit,
limit: widget.limit ?? undefined,
field,
yAxis,
query,
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/dashboards/widgetCard/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ function WidgetCardChart(props: WidgetCardChartProps) {
return (
<TableWrapper>
<AgentsTracesTableWidgetVisualization
limit={widget.limit}
limit={widget.limit ?? undefined}
tableWidths={widget.tableWidths}
dashboardFilters={props.dashboardFilters}
frameless
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function useReleasesSeriesQuery(params: WidgetQueryParams): HookWidgetQue
organization,
pageFilters,
interval,
filteredWidget.limit
filteredWidget.limit ?? undefined
);

return {
Expand Down Expand Up @@ -277,7 +277,7 @@ export function useReleasesTableQuery(params: WidgetQueryParams): HookWidgetQuer
organization,
pageFilters,
undefined, // interval
limit ?? filteredWidget.limit,
limit ?? filteredWidget.limit ?? undefined,
cursor
);

Expand Down
Loading