Skip to content

Commit

Permalink
[Lens] Fix missing formatting bug in "break down by" (elastic#63288) (e…
Browse files Browse the repository at this point in the history 
…lastic#63527)

* [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>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
@elasticmachine
Wylie Conlon and elasticmachine committed Apr 15, 2020
1 parent 16c5771 commit 0da8f78
Show file tree
Hide file tree
Showing 2 changed files with 254 additions and 63 deletions.
294 changes: 233 additions & 61 deletions x-pack/legacy/plugins/lens/public/xy_visualization/xy_expression.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,70 +633,242 @@ describe('xy_expression', () => {
expect(component.find(BarSeries).prop('enableHistogramMode')).toEqual(false);
});

test('it names the series for multiple accessors', () => {
const { data, args } = sampleArgs();

const component = shallow(
<XYChart
data={data}
args={args}
formatFactory={getFormatSpy}
timeZone="UTC"
chartTheme={{}}
executeTriggerActions={executeTriggerActions}
/>
);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

expect(
nameFn(
{
seriesKeys: ['a', 'b', 'c', 'd'],
key: '',
specId: 'a',
yAccessor: '',
splitAccessors: new Map(),
describe('provides correct series naming', () => {
const dataWithoutFormats: LensMultiTable = {
type: 'lens_multitable',
tables: {
first: {
type: 'kibana_datatable',
columns: [
{ id: 'a', name: 'a' },
{ id: 'b', name: 'b' },
{ id: 'c', name: 'c' },
{ id: 'd', name: 'd' },
],
rows: [
{ a: 1, b: 2, c: 'I', d: 'Row 1' },
{ a: 1, b: 5, c: 'J', d: 'Row 2' },
],
},
false
)
).toEqual('Label A - Label B - c - Label D');
});

test('it names the series for a single accessor', () => {
const { data, args } = sampleArgs();

const component = shallow(
<XYChart
data={data}
args={{
...args,
layers: [
{
...args.layers[0],
accessors: ['a'],
},
},
};
const dataWithFormats: LensMultiTable = {
type: 'lens_multitable',
tables: {
first: {
type: 'kibana_datatable',
columns: [
{ id: 'a', name: 'a' },
{ id: 'b', name: 'b' },
{ id: 'c', name: 'c' },
{ id: 'd', name: 'd', formatHint: { id: 'custom' } },
],
rows: [
{ a: 1, b: 2, c: 'I', d: 'Row 1' },
{ a: 1, b: 5, c: 'J', d: 'Row 2' },
],
}}
formatFactory={getFormatSpy}
timeZone="UTC"
chartTheme={{}}
executeTriggerActions={executeTriggerActions}
/>
);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

expect(
nameFn(
{
seriesKeys: ['a', 'b', 'c', 'd'],
key: '',
specId: 'a',
yAccessor: '',
splitAccessors: new Map(),
},
false
)
).toEqual('Label A');
},
};

const nameFnArgs = {
seriesKeys: [],
key: '',
specId: 'a',
yAccessor: '',
splitAccessors: new Map(),
};

const getRenderedComponent = (data: LensMultiTable, args: XYArgs) => {
return shallow(
<XYChart
data={data}
args={args}
formatFactory={getFormatSpy}
timeZone="UTC"
chartTheme={{}}
executeTriggerActions={executeTriggerActions}
/>
);
};

test('simplest xy chart without human-readable name', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a'],
splitAccessor: undefined,
columnToLabel: '',
},
],
};

const component = getRenderedComponent(dataWithoutFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

// In this case, the ID is used as the name. This shouldn't happen in practice
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('');
expect(nameFn({ ...nameFnArgs, seriesKeys: ['nonsense'] }, false)).toEqual('');
});

test('simplest xy chart with empty name', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a'],
splitAccessor: undefined,
columnToLabel: '{"a":""}',
},
],
};

const component = getRenderedComponent(dataWithoutFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

// In this case, the ID is used as the name. This shouldn't happen in practice
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('');
expect(nameFn({ ...nameFnArgs, seriesKeys: ['nonsense'] }, false)).toEqual('');
});

test('simplest xy chart with human-readable name', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a'],
splitAccessor: undefined,
columnToLabel: '{"a":"Column A"}',
},
],
};

const component = getRenderedComponent(dataWithoutFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('Column A');
});

test('multiple y accessors', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a', 'b'],
splitAccessor: undefined,
columnToLabel: '{"a": "Label A"}',
},
],
};

const component = getRenderedComponent(dataWithoutFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

// This accessor has a human-readable name
expect(nameFn({ ...nameFnArgs, seriesKeys: ['a'] }, false)).toEqual('Label A');
// This accessor does not
expect(nameFn({ ...nameFnArgs, seriesKeys: ['b'] }, false)).toEqual('');
expect(nameFn({ ...nameFnArgs, seriesKeys: ['nonsense'] }, false)).toEqual('');
});

test('split series without formatting and single y accessor', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a'],
splitAccessor: 'd',
columnToLabel: '{"a": "Label A"}',
},
],
};

const component = getRenderedComponent(dataWithoutFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'a'] }, false)).toEqual('split1');
});

test('split series with formatting and single y accessor', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a'],
splitAccessor: 'd',
columnToLabel: '{"a": "Label A"}',
},
],
};

const component = getRenderedComponent(dataWithFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

convertSpy.mockReturnValueOnce('formatted');
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'a'] }, false)).toEqual('formatted');
expect(getFormatSpy).toHaveBeenCalledWith({ id: 'custom' });
});

test('split series without formatting with multiple y accessors', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a', 'b'],
splitAccessor: 'd',
columnToLabel: '{"a": "Label A","b": "Label B"}',
},
],
};

const component = getRenderedComponent(dataWithoutFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'b'] }, false)).toEqual(
'split1 - Label B'
);
});

test('split series with formatting with multiple y accessors', () => {
const args = createArgsWithLayers();
const newArgs = {
...args,
layers: [
{
...args.layers[0],
accessors: ['a', 'b'],
splitAccessor: 'd',
columnToLabel: '{"a": "Label A","b": "Label B"}',
},
],
};

const component = getRenderedComponent(dataWithFormats, newArgs);
const nameFn = component.find(LineSeries).prop('name') as SeriesNameFn;

convertSpy.mockReturnValueOnce('formatted1').mockReturnValueOnce('formatted2');
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'a'] }, false)).toEqual(
'formatted1 - Label A'
);
expect(nameFn({ ...nameFnArgs, seriesKeys: ['split1', 'b'] }, false)).toEqual(
'formatted2 - Label B'
);
});
});

test('it set the scale of the x axis according to the args prop', () => {
Expand Down
23 changes: 21 additions & 2 deletions x-pack/legacy/plugins/lens/public/xy_visualization/xy_expression.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,12 +361,31 @@ export function XYChart({
enableHistogramMode: isHistogram && (seriesType.includes('stacked') || !splitAccessor),
timeZone,
name(d) {
const splitHint = table.columns.find(col => col.id === splitAccessor)?.formatHint;

// For multiple y series, the name of the operation is used on each, either:
// * Key - Y name
// * Formatted value - Y name
if (accessors.length > 1) {
return d.seriesKeys
.map((key: string | number) => columnToLabelMap[key] || key)
.map((key: string | number, i) => {
if (i === 0 && splitHint) {
return formatFactory(splitHint).convert(key);
}
return splitAccessor && i === 0 ? key : columnToLabelMap[key] ?? '';
})
.join(' - ');
}
return columnToLabelMap[d.seriesKeys[0]] ?? d.seriesKeys[0];

// For formatted split series, format the key
// This handles splitting by dates, for example
if (splitHint) {
return formatFactory(splitHint).convert(d.seriesKeys[0]);
}
// This handles both split and single-y cases:
// * If split series without formatting, show the value literally
// * If single Y, the seriesKey will be the acccessor, so we show the human-readable name
return splitAccessor ? d.seriesKeys[0] : columnToLabelMap[d.seriesKeys[0]] ?? '';
},
};

Expand Down

0 comments on commit 0da8f78

Please sign in to comment.