Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions static/app/views/dashboards/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,13 @@ export const usesTimeSeriesData = (displayType?: DisplayType) => {
].includes(displayType);
};

export function doesDisplayTypeSupportThresholds(displayType?: DisplayType): boolean {
if (!displayType) {
return false;
}
return displayType === DisplayType.BIG_NUMBER || usesTimeSeriesData(displayType);
}

// Custom widgets that fetch their own data (and don't use genericWidgetQueries)
// handle error state and loading state on their own
export const widgetFetchesOwnData = (widgetType: DisplayType) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ import {
type DashboardFilters,
type Widget,
} from 'sentry/views/dashboards/types';
import {usesTimeSeriesData} from 'sentry/views/dashboards/utils';
import {
doesDisplayTypeSupportThresholds,
usesTimeSeriesData,
} from 'sentry/views/dashboards/utils';
import {animationTransitionSettings} from 'sentry/views/dashboards/widgetBuilder/components/common/animationSettings';
import WidgetBuilderDatasetSelector from 'sentry/views/dashboards/widgetBuilder/components/datasetSelector';
import WidgetBuilderFilterBar from 'sentry/views/dashboards/widgetBuilder/components/filtersBar';
Expand Down Expand Up @@ -429,8 +432,7 @@ function WidgetBuilderSlideout({
/>
</Section>
)}
{(state.displayType === DisplayType.BIG_NUMBER ||
usesTimeSeriesData(state.displayType)) && (
{doesDisplayTypeSupportThresholds(state.displayType) && (
<Section>
<ThresholdsSection
dataType={thresholdMetaState?.dataType}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,40 @@ describe('useWidgetBuilderState', () => {
expect(result.current.state.selectedAggregate).toBeUndefined();
});

it('resets thresholds when the display type is switched', () => {
it('preserves thresholds when switching to a display type that supports thresholds', () => {
mockedUsedLocation.mockReturnValue(
LocationFixture({
query: {
dataset: WidgetType.ERRORS,
displayType: DisplayType.BIG_NUMBER,
thresholds: '{"max_values":{"max1":200,"max2":300},"unit":"milliseconds"}',
},
})
);

const {result} = renderHook(() => useWidgetBuilderState(), {
wrapper: WidgetBuilderProvider,
});

expect(result.current.state.thresholds).toEqual({
max_values: {max1: 200, max2: 300},
unit: 'milliseconds',
});

act(() => {
result.current.dispatch({
type: BuilderStateAction.SET_DISPLAY_TYPE,
payload: DisplayType.LINE,
});
});

expect(result.current.state.thresholds).toEqual({
max_values: {max1: 200, max2: 300},
unit: 'milliseconds',
});
});

it('resets thresholds when switching to a display type that does not support thresholds', () => {
mockedUsedLocation.mockReturnValue(
LocationFixture({
query: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import {
WidgetType,
type LinkedDashboard,
} from 'sentry/views/dashboards/types';
import {usesTimeSeriesData} from 'sentry/views/dashboards/utils';
import {
doesDisplayTypeSupportThresholds,
usesTimeSeriesData,
} from 'sentry/views/dashboards/utils';
import type {ThresholdsConfig} from 'sentry/views/dashboards/widgetBuilder/buildSteps/thresholdsStep/thresholds';
import {
DISABLED_SORT,
Expand Down Expand Up @@ -437,7 +440,9 @@ function useWidgetBuilderState(): {
);
}
}
setThresholds(undefined, options);
if (!doesDisplayTypeSupportThresholds(action.payload)) {
setThresholds(undefined, options);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thresholds preserved while aggregate resets

Medium Severity

On SET_DISPLAY_TYPE, thresholds now persist for threshold-capable display types, but selectedAggregate still gets cleared unconditionally. This can leave thresholds values out of sync with the widget’s metric/field (and its unit/type), causing the thresholds UI and saved payload to represent an invalid or misleading configuration after a display type switch.

Fix in Cursor Fix in Web

Copy link
Contributor Author

@DominikB2014 DominikB2014 Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fair, i guess it's better to reset the threshold if the aggregates change, but the user will also have a clear idea what's going on in the widget preview.

setSelectedAggregate(undefined, options);
setLinkedDashboards([], options);
break;
Expand Down
Loading