Skip to content

fix(dashboards): Persist legend selection to URL for new chart widgets #112974

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

Open
gggritso wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(dashboards): Persist legend selection to URL for new chart widgets #112974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion static/app/views/dashboards/widgetCard/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ import {
import {widgetCanUseTimeSeriesVisualization} from 'sentry/views/dashboards/utils/widgetCanUseTimeSeriesVisualization';
import {WidgetCardChartContainer} from 'sentry/views/dashboards/widgetCard/widgetCardChartContainer';
import type {WidgetLegendSelectionState} from 'sentry/views/dashboards/widgetLegendSelectionState';
import type {TabularColumn} from 'sentry/views/dashboards/widgets/common/types';
import type {
LegendSelection,
TabularColumn,
} from 'sentry/views/dashboards/widgets/common/types';
import {Widget} from 'sentry/views/dashboards/widgets/widget/widget';
import {useLLMContext} from 'sentry/views/seerExplorer/contexts/llmContext';
import {registerLLMContext} from 'sentry/views/seerExplorer/contexts/registerLLMContext';
Expand Down Expand Up @@ -356,6 +359,19 @@ function WidgetCard(props: Props) {

const canUseTimeseriesVisualization = widgetCanUseTimeSeriesVisualization(widget);
if (canUseTimeseriesVisualization) {
// Only pass legend selection when there's explicit URL state.
// getWidgetSelectionState returns a default that hides Releases for
// LINE/AREA widgets, which was appropriate for the old overlay-line
// rendering but not for the new bubble markers.
const legendSelectionForWidget = location.query.unselectedSeries
? widgetLegendState.getWidgetSelectionState(widget)
: undefined;

const handleLegendSelectionChange = (legendState: LegendSelection) => {
widgetLegendState.setWidgetSelectionState(legendState, widget);
onLegendSelectChanged?.();
};

return (
<ErrorBoundary customComponent={errorBoundaryHandler}>
<VisuallyCompleteWithData
Expand Down Expand Up @@ -390,6 +406,8 @@ function WidgetCard(props: Props) {
tableItemLimit={tableItemLimit}
widgetInterval={widgetInterval}
showConfidenceWarning={showConfidenceWarning}
legendSelection={legendSelectionForWidget}
onLegendSelectionChange={handleLegendSelectionChange}
/>
</WidgetFrame>
</VisuallyCompleteWithData>
Expand Down
48 changes: 46 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 @@ -31,6 +31,7 @@ import {
import {getChartType} from 'sentry/views/dashboards/utils/getWidgetExploreUrl';
import {matchTimeSeriesToTableRowValue} from 'sentry/views/dashboards/widgetCard/matchTimeSeriesToTableRowValue';
import {transformWidgetSeriesToTimeSeries} from 'sentry/views/dashboards/widgetCard/transformWidgetSeriesToTimeSeries';
import {WidgetLegendNameEncoderDecoder} from 'sentry/views/dashboards/widgetLegendNameEncoderDecoder';
import {
MISSING_DATA_MESSAGE,
NUMBER_MIN_VALUE,
Expand Down Expand Up @@ -103,6 +104,20 @@ export function VisualizationWidget({
}: VisualizationWidgetProps) {
const onWidgetError = useWidgetErrorCallback();

// WidgetLegendSelectionState persists legend selection to the URL with
// keys in `seriesName|~|widgetId` format so each widget's selection is
// tracked independently. TimeSeriesWidgetVisualization uses plain
// series names. Decode on the way in and encode on the way out.
const decodedLegendSelection = legendSelection
? decodeLegendSelection(legendSelection)
: undefined;

const handleLegendSelectionChange = onLegendSelectionChange
? (plain: LegendSelection) => {
onLegendSelectionChange(encodeLegendSelection(plain, widget.id));
}
: undefined;

const {releases: releasesWithDate} = useReleaseStats(selection, {
enabled: showReleaseAs !== 'none',
});
Expand Down Expand Up @@ -157,8 +172,8 @@ export function VisualizationWidget({
isSampled={isSampled}
sampleCount={sampleCount}
onZoom={onZoom}
legendSelection={legendSelection}
onLegendSelectionChange={onLegendSelectionChange}
legendSelection={decodedLegendSelection}
onLegendSelectionChange={handleLegendSelectionChange}
isFullScreen={isFullScreen}
/>
);
Expand Down Expand Up @@ -500,3 +515,32 @@ function renderBreakdownLabel(

return fallbackLabel;
}

/**
* Decodes legend selection keys from `seriesName|~|widgetId` format to
* plain `seriesName` keys used by `TimeSeriesWidgetVisualization`.
*/
function decodeLegendSelection(encoded: LegendSelection): LegendSelection {
const decoded: LegendSelection = {};
for (const key in encoded) {
decoded[WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(key, true)] =
encoded[key]!;
}
return decoded;
}

/**
* Encodes legend selection keys from plain `seriesName` format back to
* `seriesName|~|widgetId` format used by `WidgetLegendSelectionState`.
*/
function encodeLegendSelection(
plain: LegendSelection,
widgetId: string | undefined
): LegendSelection {
const encoded: LegendSelection = {};
for (const key in plain) {
Comment on lines +537 to +541
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 14, 2026

Choose a reason for hiding this comment

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

Bug: encodeLegendSelection is called with an undefined widget.id for unsaved widgets, creating invalid legend keys. Downstream decoding logic using a non-null assertion on widget.id will then fail.
Severity: LOW

Suggested Fix

Add explicit validation in widgetCanUseTimeSeriesVisualization to ensure a widget has an ID before allowing legend state changes. Alternatively, defensively handle undefined IDs in the encoder and decoder functions to prevent creating invalid keys.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/dashboards/widgetCard/visualizationWidget.tsx#L537-L541

Potential issue: The `Widget` type allows `widget.id` to be optional. The function
`widgetCanUseTimeSeriesVisualization` determines if a widget can use the new time series
visualization but does not validate that `widget.id` is present. If a widget without an
ID (e.g., an unsaved widget) meets the criteria, `encodeLegendSelection` is called with
an `undefined` ID. This creates an invalid legend key containing the literal string
"undefined", such as `"seriesName|~|undefined"`. Downstream decoding logic in
`widgetLegendSelectionState.tsx` uses a non-null assertion (`widget.id!`), which will
fail or cause incorrect behavior when it encounters these malformed keys.

Did we get this right? 👍 / 👎 to inform future reviews.

encoded[WidgetLegendNameEncoderDecoder.encodeSeriesNameForLegend(key, widgetId)] =
plain[key]!;
}
return encoded;
}
31 changes: 0 additions & 31 deletions ...c/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesLabel.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,6 @@ import {TimeSeriesFixture} from 'sentry-fixture/timeSeries';
import {formatTimeSeriesLabel} from './formatTimeSeriesLabel';

describe('formatSeriesName', () => {
describe('releases', () => {
it.each([
['p75(span.duration)|~|11762', 'p75(span.duration)'],
['Releases|~|', 'Releases'],
])('Formats %s as %s', (name, result) => {
const timeSeries = TimeSeriesFixture({
yAxis: name,
});

expect(formatTimeSeriesLabel(timeSeries)).toEqual(result);
});
});

describe('aggregates', () => {
it.each([
['user_misery()', 'user_misery()'],
Expand Down Expand Up @@ -70,26 +57,8 @@ describe('formatSeriesName', () => {
});
});

describe('combinations', () => {
it.each([
['equation|p75(measurements.cls) + 1|~|76123', 'p75(measurements.cls) + 1'],
['equation|p75(measurements.cls)|~|76123', 'p75(measurements.cls)'],
])('Formats %s as %s', (name, result) => {
const timeSeries = TimeSeriesFixture({
yAxis: name,
});

expect(formatTimeSeriesLabel(timeSeries)).toEqual(result);
});
});

describe('groupBy', () => {
it.each([
[
'equation|p75(measurements.cls)|~|76123',
[{key: 'release', value: 'v0.0.2'}],
'v0.0.2',
],
['p95(span.duration)', [{key: 'release', value: 'v0.0.2'}], 'v0.0.2'],
['p95(span.duration)', [{key: 'gen_ai.request.model', value: null}], '(no value)'],
[
Expand Down
4 changes: 0 additions & 4 deletions static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesLabel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {t} from 'sentry/locale';
import {maybeEquationAlias, stripEquationPrefix} from 'sentry/utils/discover/fields';
import {formatVersion} from 'sentry/utils/versions/formatVersion';
import {WidgetLegendNameEncoderDecoder} from 'sentry/views/dashboards/widgetLegendNameEncoderDecoder';
import type {TimeSeries} from 'sentry/views/dashboards/widgets/common/types';

export function formatTimeSeriesLabel(timeSeries: TimeSeries): string {
Expand Down Expand Up @@ -34,9 +33,6 @@ export function formatTimeSeriesLabel(timeSeries: TimeSeries): string {

let {yAxis: seriesName} = timeSeries;

// Decode from series name disambiguation
Copy link
Copy Markdown
Contributor

@cursor cursor bot Apr 14, 2026

Choose a reason for hiding this comment

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

Removing decode also removes function name prettification

Low Severity

The removed decodeSeriesNameForLegend call did two things: strip the |~|widgetId suffix (correctly identified as dead code) and prettify aggregate function names via prettifyParsedFunction (e.g., count(span.duration) → count(spans)). By removing the entire call, the prettification side-effect is also lost, causing a subtle label regression in the new chart path for specific aggregates like count(span.duration).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 243cd0f. Configure here.

seriesName = WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(seriesName)!;

// Attempt to parse the `seriesName` as a version. A correct `TimeSeries`
// would have a `yAxis` like `p50(span.duration)` with a `groupBy` like
// `[{key: "release", value: "proj@1.2.3"}]`. `groupBy` was only introduced
Expand Down
Loading