Skip to content

fix(widget-builder): Prevent crash when threshold max_values is undefined #124855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TkDodo merged 4 commits into from
Sep 18, 2026
Merged

fix(widget-builder): Prevent crash when threshold max_values is undefined #124855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
93ab6b0
fix(widget-builder): Prevent crash when threshold max_values is undef…
sentry[bot] Sep 17, 2026
61ac9b7
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Sep 17, 2026
5681dcf
fix: widen type of thresholdsConfig to have values be potentially und…
TkDodo Sep 18, 2026
c9d704a
fix: optional chaining check
TkDodo Sep 18, 2026
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
2 changes: 1 addition & 1 deletion static/app/chartcuterie/dashboardsWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const makeDashboardsWidgetCharts = (
const {thresholds} = data.widget;
if (
thresholds &&
(defined(thresholds.max_values.max1) || defined(thresholds.max_values.max2))
(defined(thresholds.max_values?.max1) || defined(thresholds.max_values?.max2))
) {
extraPlottables.push(
new Thresholds({
Expand Down
23 changes: 23 additions & 0 deletions static/app/views/dashboards/widgetBuilder/buildSteps/thresholdsStep/thresholds.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,29 @@ const exampleThresholdsConfig: ThresholdsConfig = {
};

describe('Widget Builder > ThresholdsStep', () => {
it('renders without crashing when max_values is undefined', async () => {
const onChange = jest.fn();
const configWithoutMaxValues = {
max_values: undefined,
unit: null,
} as unknown as ThresholdsConfig;
render(
<Thresholds
thresholdsConfig={configWithoutMaxValues}
onThresholdChange={onChange}
onUnitChange={onChange}
errors={{}}
/>
);

// Component should render with empty max value inputs instead of throwing
expect(await screen.findByLabelText('First Minimum')).toBeInTheDocument();
expect(screen.getByLabelText('First Maximum', {selector: 'input'})).toHaveValue(null);
expect(screen.getByLabelText('Second Maximum', {selector: 'input'})).toHaveValue(
null
);
});

it('renders thresholds step', async () => {
const onChange = jest.fn();
render(
Expand Down
6 changes: 3 additions & 3 deletions static/app/views/dashboards/widgetBuilder/buildSteps/thresholdsStep/thresholds.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ enum ThresholdMaxKeys {
type ThresholdMaxValues = Partial<Record<ThresholdMaxKeys, number>>;

export type ThresholdsConfig = {
max_values: ThresholdMaxValues;
max_values: ThresholdMaxValues | undefined;
unit: string | null;
preferredPolarity?: Polarity;
};
Expand Down Expand Up @@ -102,8 +102,8 @@ export function Thresholds({
onPolarityChange,
}: ThresholdsStepProps) {
const theme = useTheme();
const maxOneValue = thresholdsConfig?.max_values[ThresholdMaxKeys.MAX_1] ?? '';
const maxTwoValue = thresholdsConfig?.max_values[ThresholdMaxKeys.MAX_2] ?? '';
const maxOneValue = thresholdsConfig?.max_values?.[ThresholdMaxKeys.MAX_1] ?? '';
const maxTwoValue = thresholdsConfig?.max_values?.[ThresholdMaxKeys.MAX_2] ?? '';
const unit = thresholdsConfig?.unit ?? dataUnit;
const unitOptions = getThresholdUnitSelectOptions(dataType);

Expand Down
5 changes: 1 addition & 4 deletions ...c/app/views/dashboards/widgetBuilder/buildSteps/thresholdsStep/thresholdsHoverWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ type Props = {
};

export function ThresholdsHoverWrapper({children, thresholds, type}: Props) {
const {
unit,
max_values: {max1, max2},
} = thresholds;
const {unit, max_values: {max1, max2} = {}} = thresholds;
const theme = useTheme();
const formattedUnit =
unit && (type === 'duration' ? `${unit}s` : `/${unit.split('/')[1]}`);
Expand Down
3 changes: 2 additions & 1 deletion static/app/views/dashboards/widgetBuilder/components/thresholds.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export function ThresholdsSection({
}

if (newThresholds) {
newThresholds.max_values ??= {};
if (value) {
newThresholds.max_values[maxKey] = Number(value);
} else {
Expand All @@ -92,7 +93,7 @@ export function ThresholdsSection({

// Check if the value cleared all of the max values
if (
newThresholds &&
newThresholds?.max_values &&
Object.values(newThresholds.max_values).every(
nextMaxValue => !defined(nextMaxValue)
)
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/dashboards/widgetCard/visualizationWidget.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@ function VisualizationWidgetContent({
);

if (
defined(widget.thresholds?.max_values.max1) ||
defined(widget.thresholds?.max_values.max2)
defined(widget.thresholds?.max_values?.max1) ||
defined(widget.thresholds?.max_values?.max2)
) {
plottables.push(
new Thresholds({
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/dashboards/widgets/bigNumberWidget/bigNumberWidgetVisualization.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ function BigNumberWidgetVisualizationInner(props: BigNumberWidgetVisualizationPr
return (
<Wrapper>
<NumberAndDifferenceContainer>
{defined(props.thresholds?.max_values.max1) &&
{defined(props.thresholds?.max_values?.max1) &&
defined(props.thresholds?.max_values.max2) && (
<ThresholdsIndicator
preferredPolarity={props.preferredPolarity}
thresholds={{
unit: props.thresholds.unit ?? undefined,
max_values: {
max1: props.thresholds.max_values.max1,
max1: props.thresholds.max_values?.max1,
max2: props.thresholds.max_values.max2,
},
}}
Expand Down
14 changes: 7 additions & 7 deletions static/app/views/dashboards/widgets/timeSeriesWidget/plottables/thresholds.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,20 @@ export class Thresholds implements Plottable {
this.thresholds = {
...thresholds,
max_values: {
max1: thresholds.max_values.max1
max1: thresholds.max_values?.max1
? normalizeUnit(thresholds.max_values.max1, thresholdUnit, dataType)
: thresholds.max_values.max1,
max2: thresholds.max_values.max2
: thresholds.max_values?.max1,
max2: thresholds.max_values?.max2
? normalizeUnit(thresholds.max_values.max2, thresholdUnit, dataType)
: thresholds.max_values.max2,
: thresholds.max_values?.max2,
},
};
} else {
this.thresholds = thresholds;
}

this.showLabels = options.showLabels ?? false;
this.isEmpty = !this.thresholds.max_values.max1 && !this.thresholds.max_values.max2;
this.isEmpty = !this.thresholds.max_values?.max1 && !this.thresholds.max_values?.max2;
}

toMarkArea(
Expand All @@ -95,7 +95,7 @@ export class Thresholds implements Plottable {
}

toMarkAreas(theme: Theme, maxOffset = this.maxOffset) {
const {max1, max2} = this.thresholds.max_values;
const {max1, max2} = this.thresholds.max_values ?? {};
const isHigherBetter = this.thresholds.preferredPolarity === '+';

const colorOrder = isHigherBetter
Expand Down Expand Up @@ -160,7 +160,7 @@ export class Thresholds implements Plottable {
}

toMarkLines(theme: Theme, maxOffset = this.maxOffset) {
const {max1, max2} = this.thresholds.max_values;
const {max1, max2} = this.thresholds.max_values ?? {};
const isHigherBetter = this.thresholds.preferredPolarity === '+';

// For '-' (lower is better): Good (green), Meh (yellow), Poor (red) bottom-to-top
Expand Down
Loading