Skip to content

Commit 7cd746b

Browse files
Wylie Conlonelasticmachine
Wylie Conlon
and
elasticmachine
authored
[Lens] Fix missing formatting bug in "break down by" (#63288)
* [Lens] Fix missing formatting bug in "break down by" * Stop showing UUIDs, make logic more explicit Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent f9e8c1b commit 7cd746b

File tree

2 files changed

+254
-63
lines changed

2 files changed

+254
-63
lines changed

‎x-pack/legacy/plugins/lens/public/xy_visualization/xy_expression.test.tsx‎

Lines changed: 233 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -633,70 +633,242 @@ describe('xy_expression', () => {
633633
expect(component.find(BarSeries).prop('enableHistogramMode')).toEqual(false);
634634
});
635635

636-
test('it names the series for multiple accessors', () => {
637-
const { data, args } = sampleArgs();
638-
639-
const component = shallow(
640-
<XYChart
641-
data={data}
642-
args={args}
643-
formatFactory={getFormatSpy}
644-
timeZone="UTC"
645-
chartTheme={{}}
646-
executeTriggerActions={executeTriggerActions}
647-
/>
648-
);
649-
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
650-
651-
expect(
652-
nameFn(
653-
{
654-
seriesKeys: ['a', 'b', 'c', 'd'],
655-
key: '',
656-
specId: 'a',
657-
yAccessor: '',
658-
splitAccessors: new Map(),
636+
describe('provides correct series naming', () => {
637+
const dataWithoutFormats: LensMultiTable = {
638+
type: 'lens_multitable',
639+
tables: {
640+
first: {
641+
type: 'kibana_datatable',
642+
columns: [
643+
{ id: 'a', name: 'a' },
644+
{ id: 'b', name: 'b' },
645+
{ id: 'c', name: 'c' },
646+
{ id: 'd', name: 'd' },
647+
],
648+
rows: [
649+
{ a: 1, b: 2, c: 'I', d: 'Row 1' },
650+
{ a: 1, b: 5, c: 'J', d: 'Row 2' },
651+
],
659652
},
660-
false
661-
)
662-
).toEqual('Label A - Label B - c - Label D');
663-
});
664-
665-
test('it names the series for a single accessor', () => {
666-
const { data, args } = sampleArgs();
667-
668-
const component = shallow(
669-
<XYChart
670-
data={data}
671-
args={{
672-
...args,
673-
layers: [
674-
{
675-
...args.layers[0],
676-
accessors: ['a'],
677-
},
653+
},
654+
};
655+
const dataWithFormats: LensMultiTable = {
656+
type: 'lens_multitable',
657+
tables: {
658+
first: {
659+
type: 'kibana_datatable',
660+
columns: [
661+
{ id: 'a', name: 'a' },
662+
{ id: 'b', name: 'b' },
663+
{ id: 'c', name: 'c' },
664+
{ id: 'd', name: 'd', formatHint: { id: 'custom' } },
665+
],
666+
rows: [
667+
{ a: 1, b: 2, c: 'I', d: 'Row 1' },
668+
{ a: 1, b: 5, c: 'J', d: 'Row 2' },
678669
],
679-
}}
680-
formatFactory={getFormatSpy}
681-
timeZone="UTC"
682-
chartTheme={{}}
683-
executeTriggerActions={executeTriggerActions}
684-
/>
685-
);
686-
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
687-
688-
expect(
689-
nameFn(
690-
{
691-
seriesKeys: ['a', 'b', 'c', 'd'],
692-
key: '',
693-
specId: 'a',
694-
yAccessor: '',
695-
splitAccessors: new Map(),
696670
},
697-
false
698-
)
699-
).toEqual('Label A');
671+
},
672+
};
673+
674+
const nameFnArgs = {
675+
seriesKeys: [],
676+
key: '',
677+
specId: 'a',
678+
yAccessor: '',
679+
splitAccessors: new Map(),
680+
};
681+
682+
const getRenderedComponent = (data: LensMultiTable, args: XYArgs) => {
683+
return shallow(
684+
<XYChart
685+
data={data}
686+
args={args}
687+
formatFactory={getFormatSpy}
688+
timeZone="UTC"
689+
chartTheme={{}}
690+
executeTriggerActions={executeTriggerActions}
691+
/>
692+
);
693+
};
694+
695+
test('simplest xy chart without human-readable name', () => {
696+
const args = createArgsWithLayers();
697+
const newArgs = {
698+
...args,
699+
layers: [
700+
{
701+
...args.layers[0],
702+
accessors: ['a'],
703+
splitAccessor: undefined,
704+
columnToLabel: '',
705+
},
706+
],
707+
};
708+
709+
const component = getRenderedComponent(dataWithoutFormats, newArgs);
710+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
711+
712+
// In this case, the ID is used as the name. This shouldn't happen in practice
713+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('');
714+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['nonsense'] }, false)).toEqual('');
715+
});
716+
717+
test('simplest xy chart with empty name', () => {
718+
const args = createArgsWithLayers();
719+
const newArgs = {
720+
...args,
721+
layers: [
722+
{
723+
...args.layers[0],
724+
accessors: ['a'],
725+
splitAccessor: undefined,
726+
columnToLabel: '{"a":""}',
727+
},
728+
],
729+
};
730+
731+
const component = getRenderedComponent(dataWithoutFormats, newArgs);
732+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
733+
734+
// In this case, the ID is used as the name. This shouldn't happen in practice
735+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('');
736+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['nonsense'] }, false)).toEqual('');
737+
});
738+
739+
test('simplest xy chart with human-readable name', () => {
740+
const args = createArgsWithLayers();
741+
const newArgs = {
742+
...args,
743+
layers: [
744+
{
745+
...args.layers[0],
746+
accessors: ['a'],
747+
splitAccessor: undefined,
748+
columnToLabel: '{"a":"Column A"}',
749+
},
750+
],
751+
};
752+
753+
const component = getRenderedComponent(dataWithoutFormats, newArgs);
754+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
755+
756+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('Column A');
757+
});
758+
759+
test('multiple y accessors', () => {
760+
const args = createArgsWithLayers();
761+
const newArgs = {
762+
...args,
763+
layers: [
764+
{
765+
...args.layers[0],
766+
accessors: ['a', 'b'],
767+
splitAccessor: undefined,
768+
columnToLabel: '{"a": "Label A"}',
769+
},
770+
],
771+
};
772+
773+
const component = getRenderedComponent(dataWithoutFormats, newArgs);
774+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
775+
776+
// This accessor has a human-readable name
777+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('Label A');
778+
// This accessor does not
779+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['b'] }, false)).toEqual('');
780+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['nonsense'] }, false)).toEqual('');
781+
});
782+
783+
test('split series without formatting and single y accessor', () => {
784+
const args = createArgsWithLayers();
785+
const newArgs = {
786+
...args,
787+
layers: [
788+
{
789+
...args.layers[0],
790+
accessors: ['a'],
791+
splitAccessor: 'd',
792+
columnToLabel: '{"a": "Label A"}',
793+
},
794+
],
795+
};
796+
797+
const component = getRenderedComponent(dataWithoutFormats, newArgs);
798+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
799+
800+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'a'] }, false)).toEqual('split1');
801+
});
802+
803+
test('split series with formatting and single y accessor', () => {
804+
const args = createArgsWithLayers();
805+
const newArgs = {
806+
...args,
807+
layers: [
808+
{
809+
...args.layers[0],
810+
accessors: ['a'],
811+
splitAccessor: 'd',
812+
columnToLabel: '{"a": "Label A"}',
813+
},
814+
],
815+
};
816+
817+
const component = getRenderedComponent(dataWithFormats, newArgs);
818+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
819+
820+
convertSpy.mockReturnValueOnce('formatted');
821+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'a'] }, false)).toEqual('formatted');
822+
expect(getFormatSpy).toHaveBeenCalledWith({ id: 'custom' });
823+
});
824+
825+
test('split series without formatting with multiple y accessors', () => {
826+
const args = createArgsWithLayers();
827+
const newArgs = {
828+
...args,
829+
layers: [
830+
{
831+
...args.layers[0],
832+
accessors: ['a', 'b'],
833+
splitAccessor: 'd',
834+
columnToLabel: '{"a": "Label A","b": "Label B"}',
835+
},
836+
],
837+
};
838+
839+
const component = getRenderedComponent(dataWithoutFormats, newArgs);
840+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
841+
842+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'b'] }, false)).toEqual(
843+
'split1 - Label B'
844+
);
845+
});
846+
847+
test('split series with formatting with multiple y accessors', () => {
848+
const args = createArgsWithLayers();
849+
const newArgs = {
850+
...args,
851+
layers: [
852+
{
853+
...args.layers[0],
854+
accessors: ['a', 'b'],
855+
splitAccessor: 'd',
856+
columnToLabel: '{"a": "Label A","b": "Label B"}',
857+
},
858+
],
859+
};
860+
861+
const component = getRenderedComponent(dataWithFormats, newArgs);
862+
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;
863+
864+
convertSpy.mockReturnValueOnce('formatted1').mockReturnValueOnce('formatted2');
865+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'a'] }, false)).toEqual(
866+
'formatted1 - Label A'
867+
);
868+
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'b'] }, false)).toEqual(
869+
'formatted2 - Label B'
870+
);
871+
});
700872
});
701873

702874
test('it set the scale of the x axis according to the args prop', () => {

‎x-pack/legacy/plugins/lens/public/xy_visualization/xy_expression.tsx‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,12 +361,31 @@ export function XYChart({
361361
enableHistogramMode: isHistogram && (seriesType.includes('stacked') || !splitAccessor),
362362
timeZone,
363363
name(d) {
364+
const splitHint = table.columns.find(col => col.id === splitAccessor)?.formatHint;
365+
366+
// For multiple y series, the name of the operation is used on each, either:
367+
// * Key - Y name
368+
// * Formatted value - Y name
364369
if (accessors.length > 1) {
365370
return d.seriesKeys
366-
.map((key: string | number) => columnToLabelMap[key] || key)
371+
.map((key: string | number, i) => {
372+
if (i === 0 && splitHint) {
373+
return formatFactory(splitHint).convert(key);
374+
}
375+
return splitAccessor && i === 0 ? key : columnToLabelMap[key] ?? '';
376+
})
367377
.join(' - ');
368378
}
369-
return columnToLabelMap[d.seriesKeys[0]] ?? d.seriesKeys[0];
379+
380+
// For formatted split series, format the key
381+
// This handles splitting by dates, for example
382+
if (splitHint) {
383+
return formatFactory(splitHint).convert(d.seriesKeys[0]);
384+
}
385+
// This handles both split and single-y cases:
386+
// * If split series without formatting, show the value literally
387+
// * If single Y, the seriesKey will be the acccessor, so we show the human-readable name
388+
return splitAccessor ? d.seriesKeys[0] : columnToLabelMap[d.seriesKeys[0]] ?? '';
370389
},
371390
};
372391

0 commit comments

Comments
 (0)