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.7] [Lens] Fix missing formatting bug in "break down by" (#63288) #63528

Merged
merged 7 commits into from
May 13, 2020
Merged

[7.7] [Lens] Fix missing formatting bug in "break down by" (#63288) #63528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a47b5fe
[Lens] Fix missing formatting bug in "break down by" (#63288)
Apr 14, 2020
860f330
Merge branch '7.7' into backport/7.7/pr-63288
elasticmachine Apr 17, 2020
edc7d11
Merge branch '7.7' into backport/7.7/pr-63288
elasticmachine Apr 20, 2020
01c2acd
Merge branch '7.7' into backport/7.7/pr-63288
elasticmachine Apr 21, 2020
e48b6e3
Merge branch '7.7' into backport/7.7/pr-63288
elasticmachine Apr 27, 2020
625ef8a
Merge branch '7.7' into backport/7.7/pr-63288
elasticmachine May 6, 2020
de04ec2
Merge branch '7.7' into backport/7.7/pr-63288
elasticmachine May 13, 2020
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 @@ -359,12 +359,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