Skip to content
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

[7.x] [Lens] Fix missing formatting bug in "break down by" (#63288) #63527

Merged
merged 1 commit into from
Apr 15, 2020
Merged

[7.x] [Lens] Fix missing formatting bug in "break down by" (#63288) #63527

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
294 changes: 233 additions & 61 deletions x-pack/legacy/plugins/lens/public/xy_visualization/xy_expression.test.tsx
View file
Open in desktop
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
Open in desktop
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