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.

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

[Lens] Field formatter support #38874

Merged
merged 18 commits into from
Aug 13, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const name = 'kibana_datatable';
interface Column {
id: string;
name: string;
formatterMapping?: unknown;
}

interface Row {
Expand Down
11 changes: 8 additions & 3 deletions src/legacy/core_plugins/interpreter/public/functions/esaggs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export const esaggs = (): ExpressionFunction<typeof name, Context, Arguments, Re
searchSource.setField('index', indexPattern);
searchSource.setField('size', 0);

const response: Pick<KibanaDatatable, 'columns' | 'rows'> = await courierRequestHandler({
const response = await courierRequestHandler({
searchSource,
aggs,
timeRange: get(context, 'timeRange', null),
Expand All @@ -112,13 +112,18 @@ export const esaggs = (): ExpressionFunction<typeof name, Context, Arguments, Re
queryFilter,
});

return {
const table: KibanaDatatable = {
type: 'kibana_datatable',
rows: response.rows,
columns: response.columns.map(column => ({
columns: response.columns.map((column: any) => ({
id: column.id,
name: column.name,
formatterMapping: column.aggConfig.params.field
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not complete yet (there might be cases where this way of retrieving the formatters fails), but it works as a proof of concept for simple visualizations in Lens. As soon as we decide to follow this approach, I will clean up here

? column.aggConfig.params.field.format.toJSON()
: column.aggConfig.type.getFormat().toJSON(),
})),
};

return table;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the intermediate table variable is not required, can the table be returned directly?

},
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
*/

import { i18n } from '@kbn/i18n';
import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types';
import { KibanaDatatable } from '../types';
import { ExpressionFunction, KibanaDatatable } from 'src/legacy/core_plugins/interpreter/types';

interface RemapArgs {
idMap: string;
Expand Down
8 changes: 0 additions & 8 deletions x-pack/plugins/lens/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,6 @@ export interface TableSpecColumn {
// TableSpec is managed by visualizations
export type TableSpec = TableSpecColumn[];

// This is a temporary type definition, to be replaced with
// the official Kibana Datatable type definition.
export interface KibanaDatatable {
type: 'kibana_datatable';
rows: Array<Record<string, unknown>>;
columns: Array<{ id: string; name: string }>;
}

export interface VisualizationProps<T = unknown> {
dragDropContext: DragContextState;
datasource: DatasourcePublicAPI;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,31 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Position } from '@elastic/charts';
jest.mock('../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities', () => {
const formatterSpy = jest.fn();
const getFormatSpy = jest.fn(() => {
return { convert: formatterSpy };
});
return { getFormat: getFormatSpy };
});

import { Position, Axis } from '@elastic/charts';
import { xyChart, XYChart } from './xy_expression';
import { KibanaDatatable } from '../types';
import React from 'react';
import { shallow } from 'enzyme';
import { XYArgs, LegendConfig, legendConfig, XConfig, xConfig, YConfig, yConfig } from './types';
import { getFormat } from '../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities';
import { KibanaDatatable } from 'src/legacy/core_plugins/interpreter/types';

function sampleArgs() {
const data: KibanaDatatable = {
type: 'kibana_datatable',
columns: [{ id: 'a', name: 'a' }, { id: 'b', name: 'b' }, { id: 'c', name: 'c' }],
rows: [{ a: 1, b: 2, c: 3 }, { a: 1, b: 5, c: 4 }],
columns: [
{ id: 'a', name: 'a', formatterMapping: { id: 'number', params: { pattern: '0,0.000' } } },
{ id: 'b', name: 'b', formatterMapping: { id: 'number', params: { pattern: '000,0' } } },
{ id: 'c', name: 'c', formatterMapping: { id: 'string' } },
],
rows: [{ a: 1, b: 2, c: 'I' }, { a: 1, b: 5, c: 'J' }],
};

const args: XYArgs = {
Expand Down Expand Up @@ -100,6 +113,10 @@ describe('xy_expression', () => {
});

describe('XYChart component', () => {
beforeEach(() => {
jest.clearAllMocks();
});

test('it renders line', () => {
const { data, args } = sampleArgs();

Expand All @@ -123,5 +140,50 @@ describe('xy_expression', () => {
shallow(<XYChart data={data} args={{ ...args, seriesType: 'area' }} />)
).toMatchSnapshot();
});

test('it gets the formatter for the x axis', () => {
const { data, args } = sampleArgs();

shallow(<XYChart data={{ ...data }} args={{ ...args }} />);

expect(getFormat as jest.Mock).toHaveBeenCalledWith({ id: 'string' });
});

test('it gets a default formatter for y if there are multiple y accessors', () => {
const { data, args } = sampleArgs();

shallow(<XYChart data={{ ...data }} args={{ ...args }} />);

expect(getFormat as jest.Mock).toHaveBeenCalledTimes(2);
expect(getFormat as jest.Mock).toHaveBeenCalledWith({ id: 'number' });
});

test('it gets the formatter for the y axis if there is only one accessor', () => {
const { data, args } = sampleArgs();

shallow(
<XYChart data={{ ...data }} args={{ ...args, y: { ...args.y, accessors: ['a'] } }} />
);

expect(getFormat as jest.Mock).toHaveBeenCalledTimes(2);
expect(getFormat as jest.Mock).toHaveBeenCalledWith({
id: 'number',
params: { pattern: '0,0.000' },
});
});

test('it should pass the formatter function to the axis', () => {
const { data, args } = sampleArgs();

const instance = shallow(<XYChart data={{ ...data }} args={{ ...args }} />);

const tickFormatter = instance
.find(Axis)
.first()
.prop('tickFormat');
tickFormatter('I');

expect(getFormat({}).convert as jest.Mock).toHaveBeenCalledWith('I');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import {
AreaSeries,
BarSeries,
} from '@elastic/charts';
import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types';
import { ExpressionFunction, KibanaDatatable } from 'src/legacy/core_plugins/interpreter/types';
import { XYArgs } from './types';
import { KibanaDatatable } from '../types';
import { RenderFunction } from './plugin';
import { getFormat } from '../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities';

export interface XYChartProps {
data: KibanaDatatable;
Expand Down Expand Up @@ -112,6 +112,18 @@ export function XYChart({ data, args }: XYChartProps) {
data: data.rows,
};

const xAxisColumn = data.columns.find(({ id }) => id === x.accessor);
const xAxisFormatter = getFormat(xAxisColumn ? xAxisColumn.formatterMapping : undefined);

let yAxisFormatter: ReturnType<typeof getFormat>;
if (y.accessors.length === 1) {
const firstYAxisColumn = data.columns.find(({ id }) => id === y.accessors[0]);
yAxisFormatter = getFormat(firstYAxisColumn ? firstYAxisColumn.formatterMapping : undefined);
} else {
// TODO if there are multiple y axes, try to merge the formatters for the axis
yAxisFormatter = getFormat({ id: 'number' });
}

return (
<Chart className="lnsChart">
<Settings showLegend={legend.isVisible} legendPosition={legend.position} />
Expand All @@ -121,13 +133,15 @@ export function XYChart({ data, args }: XYChartProps) {
position={x.position}
title={x.title}
showGridLines={x.showGridlines}
tickFormat={d => xAxisFormatter.convert(d)}
/>

<Axis
id={getAxisId('y')}
position={y.position}
title={y.title}
showGridLines={y.showGridlines}
tickFormat={d => yAxisFormatter.convert(d)}
/>

{seriesType === 'line' ? (
Expand Down