From c4460dc568069991a100090ee93cdaf5afd21055 Mon Sep 17 00:00:00 2001 From: Leon Sorokin Date: Thu, 21 Aug 2025 12:56:27 -0500 Subject: [PATCH] Transformations: Fix XYChart legend toggle with Organize -> Partition (#109771) --- .../partitionByValues.test.ts | 46 +++++++++++ .../partitionByValues/partitionByValues.ts | 81 ++++++++++++++++++- public/app/plugins/panel/xychart/utils.ts | 39 +-------- 3 files changed, 128 insertions(+), 38 deletions(-) diff --git a/public/app/features/transformers/partitionByValues/partitionByValues.test.ts b/public/app/features/transformers/partitionByValues/partitionByValues.test.ts index 2431f5a2f5e..d04f65e4dbe 100644 --- a/public/app/features/transformers/partitionByValues/partitionByValues.test.ts +++ b/public/app/features/transformers/partitionByValues/partitionByValues.test.ts @@ -50,6 +50,52 @@ describe('Partition by values transformer', () => { expect(partitioned[1].fields[1].values).toEqual(['China', 'China', 'China']); }); + it('should partition by one field and apply auto-naming to displayName', () => { + const source = [ + toDataFrame({ + name: 'XYZ', + refId: 'A', + fields: [ + { + name: 'model', + type: FieldType.string, + config: { displayName: 'myModel' }, + values: ['E1', 'E2', 'C1', 'E3', 'C2', 'C3'], + }, + { name: 'region', type: FieldType.string, values: ['Europe', 'Europe', 'China', 'Europe', 'China', 'China'] }, + ], + }), + ]; + + const config: PartitionByValuesTransformerOptions = { + fields: ['region'], + keepFields: true, + naming: { + asLabels: false, + }, + }; + + let partitioned = partitionByValuesTransformer.transformer(config, ctx)(source); + + expect(partitioned.length).toEqual(2); + + expect(partitioned[0].length).toEqual(3); + expect(partitioned[0].name).toEqual('Europe'); + expect(partitioned[0].fields[0].name).toEqual('model'); + expect(partitioned[0].fields[0].config).toEqual({ displayName: 'Europe myModel' }); + expect(partitioned[0].fields[1].name).toEqual('region'); + expect(partitioned[0].fields[0].values).toEqual(['E1', 'E2', 'E3']); + expect(partitioned[0].fields[1].values).toEqual(['Europe', 'Europe', 'Europe']); + + expect(partitioned[1].length).toEqual(3); + expect(partitioned[1].name).toEqual('China'); + expect(partitioned[1].fields[0].name).toEqual('model'); + expect(partitioned[1].fields[0].config).toEqual({ displayName: 'China myModel' }); + expect(partitioned[1].fields[1].name).toEqual('region'); + expect(partitioned[1].fields[0].values).toEqual(['C1', 'C2', 'C3']); + expect(partitioned[1].fields[1].values).toEqual(['China', 'China', 'China']); + }); + it('should partition by multiple fields', () => { const source = [ toDataFrame({ diff --git a/public/app/features/transformers/partitionByValues/partitionByValues.ts b/public/app/features/transformers/partitionByValues/partitionByValues.ts index 4cef11b0673..89ee7d4c901 100644 --- a/public/app/features/transformers/partitionByValues/partitionByValues.ts +++ b/public/app/features/transformers/partitionByValues/partitionByValues.ts @@ -1,3 +1,4 @@ +import { cloneDeep } from 'lodash'; import { map } from 'rxjs'; import { @@ -7,6 +8,7 @@ import { getFieldMatcher, DataTransformContext, FieldMatcher, + cacheFieldDisplayNames, } from '@grafana/data'; import { getMatcherConfig, noopTransformer } from '@grafana/data/internal'; import { t } from '@grafana/i18n'; @@ -98,7 +100,7 @@ export const getPartitionByValuesTransformer: () => SynchronousDataTransformerIn }); // Split a single frame dataset into multiple frames based on values in a set of fields -export function partitionByValues( +function _partitionByValues( frame: DataFrame, matcher: FieldMatcher, options?: PartitionByValuesTransformerOptions @@ -163,7 +165,7 @@ export function partitionByValues( return { name: f.name, type: f.type, - config: f.config, + config: cloneDeep(f.config), labels: { ...f.labels, ...fieldLabels, @@ -174,3 +176,78 @@ export function partitionByValues( }; }); } + +// since this transformation splits one frame into multiple, we end up with duplicate field names across all frames +// this is normally okay since getFieldDisplayName() -> calculateFieldDisplayName() avoids creating duplicate names +// by using other sources of entropy such as refIds, frame names, field labels, and increments. + +// however, this does *not* work if a field has been renamed by the user or datasource (config.displayName or config.displayNameFromDS). +// Organize fields transformation or field overrides are common places where this happens. +// in this situation the auto-namer is skipped, and we end up with multiple fields named exactly the same. + +// consequently, onToggleSeriesVisibility() (from usePanelContext) does not have a unique field name to use for applying a +// fieldMatcher that controls field.config.hideFrom.viz + +// so what we need to do to make this work is either make field.name unique or make field.config.displayName unique. +// since field.name might need to be used for data links and subsequent drill down queries, we cannot overwrite it. +// therefore, the code below [unfortunately] has to modify field.config.displayName by using calculateFieldDisplayName() logic. +// this will have the side-effect of the displayName being a more verbose variant of what the user indicated, except in panels that +// know how to remove common prefixes/suffixes from field names in tooltip and legend rendering (like XYChart) +export function partitionByValues( + frame: DataFrame, + matcher: FieldMatcher, + options?: PartitionByValuesTransformerOptions +) { + // remember original field names, we'll need to restore them later + let fieldNames: Record = {}; + + let frame2 = { + ...frame, + + fields: frame.fields.map((f) => { + let f2 = f; + + let renameTo = f.config.displayNameFromDS ?? f.config.displayName; + + if (renameTo) { + f2 = { + ...f, + config: { + ...f.config, + }, + state: { + ...f.state, + }, + }; + + fieldNames[renameTo] = f.name; + f2.name = renameTo; + + delete f2.config.displayName; + delete f2.config.displayNameFromDS; + delete f2.state?.displayName; + } + + return f2; + }), + }; + + let frames2 = _partitionByValues(frame2, matcher, options); + + cacheFieldDisplayNames(frames2); + + // restore original field names + frames2.forEach((frame) => { + frame.fields.forEach((field) => { + if (field.name in fieldNames) { + field.name = fieldNames[field.name] ?? field.name; + field.config.displayName = field.state!.displayName!; + } + + delete field.state?.displayName; + delete field.state?.multipleFrames; + }); + }); + + return frames2; +} diff --git a/public/app/plugins/panel/xychart/utils.ts b/public/app/plugins/panel/xychart/utils.ts index 3526f7c9d6d..039f559e14f 100644 --- a/public/app/plugins/panel/xychart/utils.ts +++ b/public/app/plugins/panel/xychart/utils.ts @@ -62,8 +62,8 @@ export function prepSeries( let xMatcher = getFieldMatcher( seriesCfg.x?.matcher ?? { - id: FieldMatcherID.byTypes, - options: new Set(['number', 'time']), + id: FieldMatcherID.byType, + options: 'number', } ); let yMatcher = getFieldMatcher( @@ -123,40 +123,7 @@ export function prepSeries( // if we match non-excluded y, create series if (yMatcher(field, frame, frames) && !field.config.custom?.hideFrom?.viz) { let y = field; - - let name = seriesCfg.name?.fixed; - - if (name == null) { - // if the displayed field name is likely to have a common prefix or suffix - // (such as those from Partition by values transformation) - const likelyHasCommonParts = - frames.length > 1 && (frame.name != null || Object.keys(y.labels ?? {}).length > 0); - - // if the field was explictly (re)named using config.displayName or config.displayNameFromDS - // we still want to retain any frame name prefix or suffix so that autoNameSeries() can - // properly detect + strip common parts across all series... - const { displayName, displayNameFromDS } = y.config; - const hasExplicitName = displayName != null || displayNameFromDS != null; - - if (likelyHasCommonParts && hasExplicitName) { - // ...and a hacky way to do this is to temp remove the explicit name, get the auto name, then revert - const stateDisplayName = y.state!.displayName; - - // clear config and cache - y.config.displayName = y.config.displayNameFromDS = y.state!.displayName = undefined; - // get default/calculated display name (maybe use calculateFieldDisplayName() here instead?) - name = getFieldDisplayName(y, frame, frames); - // replace original field name with explicit one - name = name.replace(y.name, (displayNameFromDS ?? displayName)!); - - // revert - y.config.displayName = displayName; - y.config.displayNameFromDS = displayNameFromDS; - y.state!.displayName = stateDisplayName; - } else { - name = getFieldDisplayName(y, frame, frames); - } - } + let name = seriesCfg.name?.fixed ?? getFieldDisplayName(y, frame, frames); let ser: XYSeries = { // these typically come from y field