Skip to content

fix(dashboards): use null instead of undefined for widget limits on TABLE/BIG_NUMBER#112921

Open
gggritso wants to merge 7 commits intomasterfrom
georgegritsouk/dain-1330-ui-allows-table-widgets-with-invalid-limits
Open

fix(dashboards): use null instead of undefined for widget limits on TABLE/BIG_NUMBER#112921
gggritso wants to merge 7 commits intomasterfrom
georgegritsouk/dain-1330-ui-allows-table-widgets-with-invalid-limits

Conversation

@gggritso
Copy link
Copy Markdown
Member

@gggritso gggritso commented Apr 14, 2026

When a user changes a widget from a chart type (e.g., CATEGORICAL_BAR with limit=25) to TABLE via the widget builder, convertBuilderStateToWidget was setting limit: undefined. Since JSON.stringify drops undefined values, the backend never received the limit field and kept the stale value via data.get("limit", widget.limit). On subsequent dashboard saves (e.g., rearranging widgets), the stale limit was sent as a real number, triggering "Maximum limit for this display type is {N}" validation errors.

The fix uses null instead of undefined for TABLE and BIG_NUMBER widget limits, so the value survives JSON serialization and the backend actually clears it. The backend serializer already has allow_null=True on the limit field.

I had to update a bunch of types to fall back to undefined if null is provided which adds a bit of confusion. I'm feeling like maybe a cleaner approach is to actually enforce that a limit must be provided, but that seems better for a future PR.

Additionally, _enforceWidgetLimit (the safety net for dashboard-level saves like rearranging/duplicating) previously skipped TABLE/BIG_NUMBER entirely and didn't cap existing limits. It now:

  • Clears limits for TABLE/BIG_NUMBER (sets null)
  • Caps chart limits to display-type maximum (CATEGORICAL_BAR=25, others use getResultsLimit)

I also filed a separate ticket to harden the API against changing the widget type and preserving an illegal limit

Fixes DAIN-1330

…ABLE/BIG_NUMBER

TABLE and BIG_NUMBER widgets should never have limits, but changing a
chart widget to TABLE via the builder set limit to undefined, which
JSON.stringify drops. The backend then kept the stale DB value, causing
"Maximum limit for this display type" errors on subsequent saves.

Use null so the value survives serialization and the backend clears it.
Also strengthen _enforceWidgetLimit to clear limits for TABLE/BIG_NUMBER
and cap chart limits to their display-type maximum.

Fixes DAIN-1330
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 14, 2026

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 14, 2026
@sentry
Copy link
Copy Markdown
Contributor

sentry bot commented Apr 14, 2026

Sentry Snapshot Testing

Name Added Removed Modified Renamed Unchanged Status
sentry-frontend
sentry-frontend
0 0 0 0 204 ✅ Unchanged

Follow-up to the Widget.limit type widening (number | null). Places
that pass widget.limit to components or functions expecting
number | undefined now use ?? undefined to coerce null.
TEXT widgets have no queries, so accessing queries[0].aggregates
would crash. Add TEXT to the early-return alongside TABLE/BIG_NUMBER.
@gggritso gggritso marked this pull request as ready for review April 14, 2026 17:19
@gggritso gggritso requested a review from a team as a code owner April 14, 2026 17:19
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 585bda6. Configure here.

…limit check

- Add AGENTS_TRACES_TABLE to the early-return list in _enforceWidgetLimit
  (table-like widget was falling through to chart capping logic)
- Guard against empty queries array before accessing queries[0]
- Use defined() instead of !== undefined for limit check in
  dataWidgetViewerModal (null !== undefined is true)
The limit cap in _enforceWidgetLimit should only apply to chart widgets
that have grouping columns, since limits are only relevant for Top N
behavior. Non-grouped chart widgets should pass through unchanged.
Instead of maintaining a growing exclusion list of display types that
don't use limits (TABLE, BIG_NUMBER, TEXT, AGENTS_TRACES_TABLE, WHEEL,
DETAILS, SERVER_TREE, RAGE_AND_DEAD_CLICKS...), flip to an allowlist
of the chart types that actually use limits: AREA, BAR, LINE, TOP_N,
CATEGORICAL_BAR. Everything else gets limit cleared to null.
Document why DISPLAY_TYPES_WITH_LIMITS is an allowlist — other types
either fetch their own data (see widgetFetchesOwnData) or don't use
limits (TABLE, BIG_NUMBER, WHEEL, DETAILS).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants