From 3891bf36c1fe8829d66a9bf788998c133aecb07e Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 13 Jun 2019 12:04:32 +0200 Subject: [PATCH 01/10] pipe formatting informations through the expression --- .../common/types/kibana_datatable.ts | 1 + .../interpreter/public/functions/esaggs.ts | 11 ++- .../indexpattern_plugin/rename_columns.ts | 3 +- x-pack/plugins/lens/public/types.ts | 8 --- .../__snapshots__/xy_expression.test.tsx.snap | 18 +++-- .../xy_expression.test.tsx | 70 +++++++++++++++++-- .../xy_visualization_plugin/xy_expression.tsx | 18 ++++- 7 files changed, 104 insertions(+), 25 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/common/types/kibana_datatable.ts b/src/legacy/core_plugins/interpreter/common/types/kibana_datatable.ts index d5622ff50dd83..169ce867f7080 100644 --- a/src/legacy/core_plugins/interpreter/common/types/kibana_datatable.ts +++ b/src/legacy/core_plugins/interpreter/common/types/kibana_datatable.ts @@ -24,6 +24,7 @@ const name = 'kibana_datatable'; interface Column { id: string; name: string; + formatterMapping?: unknown; } interface Row { diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index c64464030f02b..090b3359c7a30 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -99,7 +99,7 @@ export const esaggs = (): ExpressionFunction = await courierRequestHandler({ + const response = await courierRequestHandler({ searchSource, aggs, timeRange: get(context, 'timeRange', null), @@ -112,13 +112,18 @@ export const esaggs = (): ExpressionFunction ({ + columns: response.columns.map((column: any) => ({ id: column.id, name: column.name, + formatterMapping: column.aggConfig.params.field + ? column.aggConfig.params.field.format.toJSON() + : column.aggConfig.type.getFormat().toJSON(), })), }; + + return table; }, }); diff --git a/x-pack/plugins/lens/public/indexpattern_plugin/rename_columns.ts b/x-pack/plugins/lens/public/indexpattern_plugin/rename_columns.ts index 1740d449b62cd..49c17b13e9d52 100644 --- a/x-pack/plugins/lens/public/indexpattern_plugin/rename_columns.ts +++ b/x-pack/plugins/lens/public/indexpattern_plugin/rename_columns.ts @@ -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; diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index dc7f2e04609ef..a8dd23c3f262a 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -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>; - columns: Array<{ id: string; name: string }>; -} - export interface VisualizationProps { dragDropContext: DragContextState; datasource: DatasourcePublicAPI; diff --git a/x-pack/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap b/x-pack/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap index 85e0598114ce8..4392f633380a3 100644 --- a/x-pack/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap +++ b/x-pack/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap @@ -13,12 +13,14 @@ exports[`xy_expression XYChart component it renders area 1`] = ` id="x" position="bottom" showGridLines={false} + tickFormat={[Function]} title="C" /> { + 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 = { @@ -100,6 +113,10 @@ describe('xy_expression', () => { }); describe('XYChart component', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + test('it renders line', () => { const { data, args } = sampleArgs(); @@ -123,5 +140,50 @@ describe('xy_expression', () => { shallow() ).toMatchSnapshot(); }); + + test('it gets the formatter for the x axis', () => { + const { data, args } = sampleArgs(); + + shallow(); + + 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(); + + 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( + + ); + + 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(); + + const tickFormatter = instance + .find(Axis) + .first() + .prop('tickFormat'); + tickFormatter('I'); + + expect(getFormat({}).convert as jest.Mock).toHaveBeenCalledWith('I'); + }); }); }); diff --git a/x-pack/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx b/x-pack/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx index 9b2b9290b54f1..e2937152b6f89 100644 --- a/x-pack/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx +++ b/x-pack/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx @@ -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; @@ -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; + 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 ( @@ -121,6 +133,7 @@ export function XYChart({ data, args }: XYChartProps) { position={x.position} title={x.title} showGridLines={x.showGridlines} + tickFormat={d => xAxisFormatter.convert(d)} /> yAxisFormatter.convert(d)} /> {seriesType === 'line' ? ( From 8c91d6cd0bc2e530b10a50341c85bbda762abeef Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 24 Jun 2019 09:27:42 +0200 Subject: [PATCH 02/10] add robustness --- .../interpreter/public/functions/esaggs.ts | 13 ++++++++++--- .../xy_visualization_plugin/xy_expression.test.tsx | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 090b3359c7a30..c5b0c148a1775 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -52,6 +52,15 @@ interface Arguments { type Return = Promise; +function getFormatter(aggConfig: any) { + if (aggConfig.params && aggConfig.params.field && aggConfig.params.field.format) { + return aggConfig.params.field.format.toJSON(); + } + if (aggConfig.type && aggConfig.type.getFormat()) { + return aggConfig.type.getFormat().toJSON(); + } +} + export const esaggs = (): ExpressionFunction => ({ name, type: 'kibana_datatable', @@ -118,9 +127,7 @@ export const esaggs = (): ExpressionFunction ({ id: column.id, name: column.name, - formatterMapping: column.aggConfig.params.field - ? column.aggConfig.params.field.format.toJSON() - : column.aggConfig.type.getFormat().toJSON(), + formatterMapping: getFormatter(column.aggConfig), })), }; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx index f35fe1c68f07e..e8d2fccbf8c4a 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx @@ -17,8 +17,8 @@ import { xyChart, XYChart } from './xy_expression'; 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'; +import { getFormat } from '../../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; +import { KibanaDatatable } from 'src/legacy/core_plugins/interpreter/common'; function sampleArgs() { const data: KibanaDatatable = { From e692f9c4fc55391a0496015615883ec85371c65d Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 24 Jun 2019 12:05:55 +0200 Subject: [PATCH 03/10] fix import path --- .../lens/public/xy_visualization_plugin/xy_expression.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx index e2937152b6f89..cad99cd91669c 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx @@ -19,7 +19,7 @@ import { import { ExpressionFunction, KibanaDatatable } from 'src/legacy/core_plugins/interpreter/types'; import { XYArgs } from './types'; import { RenderFunction } from './plugin'; -import { getFormat } from '../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; +import { getFormat } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; export interface XYChartProps { data: KibanaDatatable; From eeaa86d59e224c271479bfd209108c8db3517d2d Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 24 Jun 2019 14:41:38 +0200 Subject: [PATCH 04/10] fix references in tests --- .../xy_expression.test.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx index e8d2fccbf8c4a..507d12bf4cff9 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx @@ -4,20 +4,23 @@ * you may not use this file except in compliance with the Elastic License. */ -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 }; -}); +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 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 { getFormat } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; import { KibanaDatatable } from 'src/legacy/core_plugins/interpreter/common'; function sampleArgs() { From 76b25ff824e5c2a48647092b7224b6e525cc2044 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 8 Aug 2019 07:11:20 +0200 Subject: [PATCH 05/10] use same formats as in pipeline loader --- .../interpreter/public/functions/esaggs.ts | 42 +++++++++++++++---- .../expression.tsx | 18 +++++++- 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 1d34591436daa..77a17162d71ce 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -36,6 +36,7 @@ import chrome from 'ui/chrome'; const courierRequestHandlerProvider = CourierRequestHandlerProvider; const courierRequestHandler = courierRequestHandlerProvider().handler; +import { AggConfig } from 'ui/vis'; import { ExpressionFunction } from '../../types'; import { KibanaContext, KibanaDatatable } from '../../common'; @@ -52,14 +53,37 @@ interface Arguments { type Return = Promise; -function getFormatter(aggConfig: any) { - if (aggConfig.params && aggConfig.params.field && aggConfig.params.field.format) { - return aggConfig.params.field.format.toJSON(); - } - if (aggConfig.type && aggConfig.type.getFormat()) { - return aggConfig.type.getFormat().toJSON(); - } -} +// TODO copied from build_pipeline, this should be unified +const createFormat = (agg: AggConfig) => { + const format: any = agg.params.field ? agg.params.field.format.toJSON() : {}; + const formats: any = { + date_range: () => ({ id: 'string' }), + percentile_ranks: () => ({ id: 'percent' }), + count: () => ({ id: 'number' }), + cardinality: () => ({ id: 'number' }), + date_histogram: () => ({ + id: 'date', + params: { + pattern: agg.buckets.getScaledDateFormat(), + }, + }), + terms: () => ({ + id: 'terms', + params: { + id: format.id, + otherBucketLabel: agg.params.otherBucketLabel, + missingBucketLabel: agg.params.missingBucketLabel, + ...format.params, + }, + }), + range: () => ({ + id: 'range', + params: { id: format.id, ...format.params }, + }), + }; + + return formats[agg.type.name] ? formats[agg.type.name]() : format; +}; export const esaggs = (): ExpressionFunction => ({ name, @@ -127,7 +151,7 @@ export const esaggs = (): ExpressionFunction ({ id: column.id, name: column.name, - formatterMapping: getFormatter(column.aggConfig), + formatterMapping: createFormat(column.aggConfig), })), }; diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx index 076bb1c20ac10..133e2644c6c29 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx @@ -11,6 +11,7 @@ import { EuiBasicTable } from '@elastic/eui'; import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types'; import { KibanaDatatable, LensMultiTable } from '../types'; import { RenderFunction } from '../interpreter_types'; +import { getFormat } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; export interface DatatableColumns { columnIds: string[]; @@ -121,6 +122,11 @@ export const datatableRenderer: RenderFunction = { function DatatableComponent(props: DatatableProps) { const [firstTable] = Object.values(props.data.tables); + const formatters: Record> = {}; + + firstTable.columns.forEach(column => { + formatters[column.id] = getFormat(column.formatterMapping || undefined); + }); return ( !!field)} - items={firstTable ? firstTable.rows : []} + items={ + firstTable + ? firstTable.rows.map(row => { + const formattedRow: Record = {}; + Object.entries(formatters).forEach(([columnId, formatter]) => { + formattedRow[columnId] = formatter.convert(row[columnId]); + }); + return formattedRow; + }) + : [] + } /> ); } From ef698ac41ed85f30b5e23adc191821f9a2fc4846 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Thu, 8 Aug 2019 07:46:57 +0200 Subject: [PATCH 06/10] fix bugs and make formats optional --- .../interpreter/public/functions/esaggs.ts | 21 ++++++++++++++----- .../indexpattern_plugin/to_expression.ts | 1 + x-pack/legacy/plugins/lens/public/types.ts | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 77a17162d71ce..15f7057e573a9 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -48,6 +48,7 @@ interface Arguments { index: string | null; metricsAtAllLevels: boolean; partialRows: boolean; + includeFormats: boolean; aggConfigs: string; } @@ -110,6 +111,11 @@ export const esaggs = (): ExpressionFunction ({ - id: column.id, - name: column.name, - formatterMapping: createFormat(column.aggConfig), - })), + columns: response.columns.map((column: any) => { + const cleanedColumn: KibanaDatatable['columns'][0] = { + id: column.id, + name: column.name, + }; + if (args.includeFormats) { + cleanedColumn.formatterMapping = createFormat(column.aggConfig); + } + return cleanedColumn; + }), }; return table; diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts index 46095cd434335..e39222bc8059b 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts @@ -74,6 +74,7 @@ function getExpressionForLayer( index="${indexPattern.id}" metricsAtAllLevels=false partialRows=false + includeFormats=true aggConfigs='${JSON.stringify(aggs)}' | lens_rename_columns idMap='${JSON.stringify(idMap)}'`; } diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index f2bffb96132d4..34aeb6fd4a148 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -172,7 +172,7 @@ export interface LensMultiTable { export interface KibanaDatatable { type: 'kibana_datatable'; rows: Array>; - columns: Array<{ id: string; name: string; formatterMapping: unknown; }>; + columns: Array<{ id: string; name: string; formatterMapping: unknown }>; } export interface VisualizationProps { From eaa93f61d5bc23d05a07977c03bc9788bcb6f2da Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Fri, 9 Aug 2019 16:14:12 +0200 Subject: [PATCH 07/10] clean up field formatters --- .../interpreter/public/functions/esaggs.ts | 42 ++-------- .../loader/pipeline_helpers/build_pipeline.ts | 39 +-------- .../loader/pipeline_helpers/utilities.ts | 71 +++++++++++++--- .../expression_types/kibana_datatable.ts | 3 +- .../expression.tsx | 20 ++--- .../datatable_visualization_plugin/plugin.tsx | 20 ++++- .../editor_frame_plugin/merge_tables.ts | 4 +- .../indexpattern_plugin/filter_ratio.ts | 3 +- .../indexpattern_plugin/indexpattern.test.tsx | 1 + .../indexpattern_plugin/to_expression.ts | 2 +- .../public/persistence/saved_object_store.ts | 2 +- x-pack/legacy/plugins/lens/public/types.ts | 10 +-- .../__snapshots__/xy_expression.test.tsx.snap | 76 +++++++++-------- .../public/xy_visualization_plugin/plugin.tsx | 17 +++- .../xy_expression.test.tsx | 81 ++++++++++--------- .../xy_visualization_plugin/xy_expression.tsx | 26 +++--- 16 files changed, 221 insertions(+), 196 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 15f7057e573a9..c788d4a88d781 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -36,7 +36,7 @@ import chrome from 'ui/chrome'; const courierRequestHandlerProvider = CourierRequestHandlerProvider; const courierRequestHandler = courierRequestHandlerProvider().handler; -import { AggConfig } from 'ui/vis'; +import { createFormat } from 'ui/visualize/loader/pipeline_helpers/utilities'; import { ExpressionFunction } from '../../types'; import { KibanaContext, KibanaDatatable } from '../../common'; @@ -48,44 +48,12 @@ interface Arguments { index: string | null; metricsAtAllLevels: boolean; partialRows: boolean; - includeFormats: boolean; + includeFormatHints: boolean; aggConfigs: string; } type Return = Promise; -// TODO copied from build_pipeline, this should be unified -const createFormat = (agg: AggConfig) => { - const format: any = agg.params.field ? agg.params.field.format.toJSON() : {}; - const formats: any = { - date_range: () => ({ id: 'string' }), - percentile_ranks: () => ({ id: 'percent' }), - count: () => ({ id: 'number' }), - cardinality: () => ({ id: 'number' }), - date_histogram: () => ({ - id: 'date', - params: { - pattern: agg.buckets.getScaledDateFormat(), - }, - }), - terms: () => ({ - id: 'terms', - params: { - id: format.id, - otherBucketLabel: agg.params.otherBucketLabel, - missingBucketLabel: agg.params.missingBucketLabel, - ...format.params, - }, - }), - range: () => ({ - id: 'range', - params: { id: format.id, ...format.params }, - }), - }; - - return formats[agg.type.name] ? formats[agg.type.name]() : format; -}; - export const esaggs = (): ExpressionFunction => ({ name, type: 'kibana_datatable', @@ -111,7 +79,7 @@ export const esaggs = (): ExpressionFunction { - const createFormat = (agg: AggConfig): SchemaFormat => { - const format: SchemaFormat = agg.params.field ? agg.params.field.format.toJSON() : {}; - const formats: any = { - date_range: () => ({ id: 'string' }), - percentile_ranks: () => ({ id: 'percent' }), - count: () => ({ id: 'number' }), - cardinality: () => ({ id: 'number' }), - date_histogram: () => ({ - id: 'date', - params: { - pattern: agg.buckets.getScaledDateFormat(), - }, - }), - terms: () => ({ - id: 'terms', - params: { - id: format.id, - otherBucketLabel: agg.params.otherBucketLabel, - missingBucketLabel: agg.params.missingBucketLabel, - ...format.params, - }, - }), - range: () => ({ - id: 'range', - params: { id: format.id, ...format.params }, - }), - }; - - return formats[agg.type.name] ? formats[agg.type.name]() : format; - }; - const createSchemaConfig = (accessor: number, agg: AggConfig): SchemaConfig => { if (agg.type.name === 'date_histogram') { agg.params.timeRange = timeRange; diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts index 20128eb5a3a64..e440d3678a6d9 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts @@ -28,12 +28,29 @@ import chrome from '../../../chrome'; // @ts-ignore import { fieldFormats } from '../../../registry/field_formats'; +interface TermsFieldFormatParams { + otherBucketLabel: string; + missingBucketLabel: string; + id: string; +} + +function isTermsFieldFormat( + serializedFieldFormat: SerializedFieldFormat +): serializedFieldFormat is SerializedFieldFormat { + return serializedFieldFormat.id === 'terms'; +} + +export interface SerializedFieldFormat { + id?: string; + params?: TParams; +} + const config = chrome.getUiSettingsClient(); const getConfig = (...args: any[]): any => config.get(...args); const getDefaultFieldFormat = () => ({ convert: identity }); -const getFieldFormat = (id: string, params: object) => { +const getFieldFormat = (id: string | undefined, params: object = {}) => { const Format = fieldFormats.byId[id]; if (Format) { return new Format(params, getConfig); @@ -42,7 +59,42 @@ const getFieldFormat = (id: string, params: object) => { } }; -export const getFormat = (mapping: any) => { +export type FieldFormat = any; + +export const createFormat = (agg: AggConfig): SerializedFieldFormat => { + const format: SerializedFieldFormat = agg.params.field ? agg.params.field.format.toJSON() : {}; + const formats: Record SerializedFieldFormat> = { + date_range: () => ({ id: 'string' }), + percentile_ranks: () => ({ id: 'percent' }), + count: () => ({ id: 'number' }), + cardinality: () => ({ id: 'number' }), + date_histogram: () => ({ + id: 'date', + params: { + pattern: agg.buckets.getScaledDateFormat(), + }, + }), + terms: () => ({ + id: 'terms', + params: { + id: format.id, + otherBucketLabel: agg.params.otherBucketLabel, + missingBucketLabel: agg.params.missingBucketLabel, + ...format.params, + }, + }), + range: () => ({ + id: 'range', + params: { id: format.id, ...format.params }, + }), + }; + + return formats[agg.type.name] ? formats[agg.type.name]() : format; +}; + +export type FormatFactory = (mapping?: SerializedFieldFormat) => FieldFormat; + +export const getFormat: FormatFactory = (mapping: SerializedFieldFormat = {}): FieldFormat => { if (!mapping) { return getDefaultFieldFormat(); } @@ -59,16 +111,17 @@ export const getFormat = (mapping: any) => { }); }); return new RangeFormat(); - } else if (id === 'terms') { + } else if (isTermsFieldFormat(mapping) && mapping.params) { + const params = mapping.params; return { getConverterFor: (type: string) => { - const format = getFieldFormat(mapping.params.id, mapping.params); + const format = getFieldFormat(params.id, mapping.params); return (val: string) => { if (val === '__other__') { - return mapping.params.otherBucketLabel; + return params.otherBucketLabel; } if (val === '__missing__') { - return mapping.params.missingBucketLabel; + return params.missingBucketLabel; } const parsedUrl = { origin: window.location.origin, @@ -79,12 +132,12 @@ export const getFormat = (mapping: any) => { }; }, convert: (val: string, type: string) => { - const format = getFieldFormat(mapping.params.id, mapping.params); + const format = getFieldFormat(params.id, mapping.params); if (val === '__other__') { - return mapping.params.otherBucketLabel; + return params.otherBucketLabel; } if (val === '__missing__') { - return mapping.params.missingBucketLabel; + return params.missingBucketLabel; } const parsedUrl = { origin: window.location.origin, diff --git a/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts b/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts index 169ce867f7080..f44819ebd8c3e 100644 --- a/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts +++ b/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts @@ -18,13 +18,14 @@ */ import { map } from 'lodash'; +import { SerializedFieldFormat } from 'ui/visualize/loader/pipeline_helpers/utilities'; const name = 'kibana_datatable'; interface Column { id: string; name: string; - formatterMapping?: unknown; + formatHint?: SerializedFieldFormat; } interface Row { diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx index 133e2644c6c29..0e53ee59761f5 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/expression.tsx @@ -8,10 +8,10 @@ import React from 'react'; import ReactDOM from 'react-dom'; import { i18n } from '@kbn/i18n'; import { EuiBasicTable } from '@elastic/eui'; -import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types'; -import { KibanaDatatable, LensMultiTable } from '../types'; +import { ExpressionFunction, KibanaDatatable } from 'src/legacy/core_plugins/interpreter/types'; +import { LensMultiTable } from '../types'; import { RenderFunction } from '../interpreter_types'; -import { getFormat } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; +import { FormatFactory } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; export interface DatatableColumns { columnIds: string[]; @@ -107,7 +107,9 @@ export const datatableColumns: ExpressionFunction< }, }; -export const datatableRenderer: RenderFunction = { +export const getDatatableRenderer = ( + formatFactory: FormatFactory +): RenderFunction => ({ name: 'lens_datatable_renderer', displayName: i18n.translate('xpack.lens.datatable.visualizationName', { defaultMessage: 'Datatable', @@ -116,16 +118,16 @@ export const datatableRenderer: RenderFunction = { validate: () => {}, reuseDomNode: true, render: async (domNode: Element, config: DatatableProps, _handlers: unknown) => { - ReactDOM.render(, domNode); + ReactDOM.render(, domNode); }, -}; +}); -function DatatableComponent(props: DatatableProps) { +function DatatableComponent(props: DatatableProps & { formatFactory: FormatFactory }) { const [firstTable] = Object.values(props.data.tables); - const formatters: Record> = {}; + const formatters: Record> = {}; firstTable.columns.forEach(column => { - formatters[column.id] = getFormat(column.formatterMapping || undefined); + formatters[column.id] = props.formatFactory(column.formatHint); }); return ( diff --git a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/plugin.tsx b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/plugin.tsx index 7bcddae13e1ac..52f4f99513e7a 100644 --- a/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/plugin.tsx +++ b/x-pack/legacy/plugins/lens/public/datatable_visualization_plugin/plugin.tsx @@ -6,6 +6,7 @@ // import { Registry } from '@kbn/interpreter/target/common'; import { CoreSetup } from 'src/core/public'; +import { getFormat, FormatFactory } from 'ui/visualize/loader/pipeline_helpers/utilities'; import { datatableVisualization } from './visualization'; import { @@ -13,19 +14,29 @@ import { functionsRegistry, } from '../../../../../../src/legacy/core_plugins/interpreter/public/registries'; import { InterpreterSetup, RenderFunction } from '../interpreter_types'; -import { datatable, datatableColumns, datatableRenderer } from './expression'; +import { datatable, datatableColumns, getDatatableRenderer } from './expression'; export interface DatatableVisualizationPluginSetupPlugins { interpreter: InterpreterSetup; + // TODO this is a simulated NP plugin. + // Once field formatters are actually migrated, the actual shim can be used + fieldFormat: { + formatFactory: FormatFactory; + }; } class DatatableVisualizationPlugin { constructor() {} - setup(_core: CoreSetup | null, { interpreter }: DatatableVisualizationPluginSetupPlugins) { + setup( + _core: CoreSetup | null, + { interpreter, fieldFormat }: DatatableVisualizationPluginSetupPlugins + ) { interpreter.functionsRegistry.register(() => datatableColumns); interpreter.functionsRegistry.register(() => datatable); - interpreter.renderersRegistry.register(() => datatableRenderer as RenderFunction); + interpreter.renderersRegistry.register( + () => getDatatableRenderer(fieldFormat.formatFactory) as RenderFunction + ); return datatableVisualization; } @@ -41,5 +52,8 @@ export const datatableVisualizationSetup = () => renderersRegistry, functionsRegistry, }, + fieldFormat: { + formatFactory: getFormat, + }, }); export const datatableVisualizationStop = () => plugin.stop(); diff --git a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/merge_tables.ts b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/merge_tables.ts index c7747ace106fd..2b7e35876bb63 100644 --- a/x-pack/legacy/plugins/lens/public/editor_frame_plugin/merge_tables.ts +++ b/x-pack/legacy/plugins/lens/public/editor_frame_plugin/merge_tables.ts @@ -5,8 +5,8 @@ */ import { i18n } from '@kbn/i18n'; -import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types'; -import { LensMultiTable, KibanaDatatable } from '../types'; +import { ExpressionFunction, KibanaDatatable } from 'src/legacy/core_plugins/interpreter/types'; +import { LensMultiTable } from '../types'; interface MergeTables { layerIds: string[]; diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/filter_ratio.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/filter_ratio.ts index 1fe57f42fc987..854284f98a8d0 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/filter_ratio.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/filter_ratio.ts @@ -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 FilterRatioKey { id: string; diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx index b7d761b42030a..2ab26455a9f7a 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/indexpattern.test.tsx @@ -243,6 +243,7 @@ describe('IndexPattern Data Source', () => { index=\\"1\\" metricsAtAllLevels=false partialRows=false + includeFormatHints=true aggConfigs='[{\\"id\\":\\"col1\\",\\"enabled\\":true,\\"type\\":\\"count\\",\\"schema\\":\\"metric\\",\\"params\\":{}},{\\"id\\":\\"col2\\",\\"enabled\\":true,\\"type\\":\\"date_histogram\\",\\"schema\\":\\"segment\\",\\"params\\":{\\"field\\":\\"timestamp\\",\\"useNormalizedEsInterval\\":true,\\"interval\\":\\"1d\\",\\"drop_partials\\":false,\\"min_doc_count\\":1,\\"extended_bounds\\":{}}}]' | lens_rename_columns idMap='{\\"col-0-col1\\":\\"col1\\",\\"col-1-col2\\":\\"col2\\"}'" `); }); diff --git a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts index e39222bc8059b..71e8473aa6839 100644 --- a/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts +++ b/x-pack/legacy/plugins/lens/public/indexpattern_plugin/to_expression.ts @@ -74,7 +74,7 @@ function getExpressionForLayer( index="${indexPattern.id}" metricsAtAllLevels=false partialRows=false - includeFormats=true + includeFormatHints=true aggConfigs='${JSON.stringify(aggs)}' | lens_rename_columns idMap='${JSON.stringify(idMap)}'`; } diff --git a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts index 5846565dd05a7..b87ca1aa4f868 100644 --- a/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/legacy/plugins/lens/public/persistence/saved_object_store.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { SavedObjectAttributes } from 'target/types/server'; import { Filter } from '@kbn/es-query'; import { Query } from 'src/plugins/data/common'; +import { SavedObjectAttributes } from 'src/core/server'; export interface Document { id?: string; diff --git a/x-pack/legacy/plugins/lens/public/types.ts b/x-pack/legacy/plugins/lens/public/types.ts index 34aeb6fd4a148..6deab82c5658e 100644 --- a/x-pack/legacy/plugins/lens/public/types.ts +++ b/x-pack/legacy/plugins/lens/public/types.ts @@ -6,7 +6,7 @@ import { Ast } from '@kbn/interpreter/common'; import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; -import { Query } from 'src/plugins/data/common'; +import { Query, KibanaDatatable } from 'src/plugins/data/common'; import { DragContextState } from './drag_drop'; import { Document } from './persistence'; @@ -167,14 +167,6 @@ export interface LensMultiTable { tables: Record; } -// This is a temporary type definition, to be replaced with -// the official Kibana Datatable type definition. -export interface KibanaDatatable { - type: 'kibana_datatable'; - rows: Array>; - columns: Array<{ id: string; name: string; formatterMapping: unknown }>; -} - export interface VisualizationProps { dragDropContext: DragContextState; frame: FramePublicAPI; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap index ed1bbf8c696d6..deaf7254d6789 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/__snapshots__/xy_expression.test.tsx.snap @@ -16,26 +16,26 @@ exports[`xy_expression XYChart component it renders area 1`] = ` position="bottom" showGridLines={false} tickFormat={[Function]} - title="C" + title="" /> -`; \ No newline at end of file +`; diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/plugin.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/plugin.tsx index b16c26f6dee45..81098b0a420b2 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/plugin.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/plugin.tsx @@ -5,6 +5,7 @@ */ import { CoreSetup } from 'src/core/public'; +import { getFormat, FormatFactory } from 'ui/visualize/loader/pipeline_helpers/utilities'; import { xyVisualization } from './xy_visualization'; import { @@ -12,23 +13,30 @@ import { functionsRegistry, } from '../../../../../../src/legacy/core_plugins/interpreter/public/registries'; import { InterpreterSetup, RenderFunction } from '../interpreter_types'; -import { xyChart, xyChartRenderer } from './xy_expression'; +import { xyChart, getXyChartRenderer } from './xy_expression'; import { legendConfig, xConfig, layerConfig } from './types'; export interface XyVisualizationPluginSetupPlugins { interpreter: InterpreterSetup; + // TODO this is a simulated NP plugin. + // Once field formatters are actually migrated, the actual shim can be used + fieldFormat: { + formatFactory: FormatFactory; + }; } class XyVisualizationPlugin { constructor() {} - setup(_core: CoreSetup | null, { interpreter }: XyVisualizationPluginSetupPlugins) { + setup(_core: CoreSetup | null, { interpreter, fieldFormat }: XyVisualizationPluginSetupPlugins) { interpreter.functionsRegistry.register(() => legendConfig); interpreter.functionsRegistry.register(() => xConfig); interpreter.functionsRegistry.register(() => layerConfig); interpreter.functionsRegistry.register(() => xyChart); - interpreter.renderersRegistry.register(() => xyChartRenderer as RenderFunction); + interpreter.renderersRegistry.register( + () => getXyChartRenderer(fieldFormat.formatFactory) as RenderFunction + ); return xyVisualization; } @@ -44,5 +52,8 @@ export const xyVisualizationSetup = () => renderersRegistry, functionsRegistry, }, + fieldFormat: { + formatFactory: getFormat, + }, }); export const xyVisualizationStop = () => plugin.stop(); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx index cc1321e2856eb..ac858e65e53cb 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx @@ -4,19 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -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 { Axis } from '@elastic/charts'; -import { getFormat } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; import { AreaSeries, BarSeries, Position, LineSeries, Settings, ScaleType } from '@elastic/charts'; import { xyChart, XYChart } from './xy_expression'; import { LensMultiTable } from '../types'; @@ -34,10 +22,10 @@ function sampleArgs() { { id: 'a', name: 'a', - formatterMapping: { id: 'number', params: { pattern: '0,0.000' } }, + formatHint: { 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' } }, + { id: 'b', name: 'b', formatHint: { id: 'number', params: { pattern: '000,0' } } }, + { id: 'c', name: 'c', formatHint: { id: 'string' } }, ], rows: [{ a: 1, b: 2, c: 'I' }, { a: 1, b: 5, c: 'J' }], }, @@ -112,8 +100,13 @@ describe('xy_expression', () => { }); describe('XYChart component', () => { + let getFormatSpy: jest.Mock; + let convertSpy: jest.Mock; + beforeEach(() => { - jest.clearAllMocks(); + convertSpy = jest.fn(x => x); + getFormatSpy = jest.fn(); + getFormatSpy.mockReturnValue({ convert: convertSpy }); }); test('it renders line', () => { @@ -123,6 +116,7 @@ describe('xy_expression', () => { ); expect(component).toMatchSnapshot(); @@ -135,6 +129,7 @@ describe('xy_expression', () => { ); expect(component).toMatchSnapshot(); @@ -147,6 +142,7 @@ describe('xy_expression', () => { ); expect(component).toMatchSnapshot(); @@ -159,6 +155,7 @@ describe('xy_expression', () => { ); expect(component).toMatchSnapshot(); @@ -172,6 +169,7 @@ describe('xy_expression', () => { ); expect(component).toMatchSnapshot(); @@ -185,6 +183,7 @@ describe('xy_expression', () => { ); expect(component).toMatchSnapshot(); @@ -202,6 +201,7 @@ describe('xy_expression', () => { isHorizontal: true, layers: [{ ...args.layers[0], seriesType: 'bar_stacked' }], }} + formatFactory={getFormatSpy} /> ); expect(component).toMatchSnapshot(); @@ -213,28 +213,28 @@ describe('xy_expression', () => { test('it rewrites the rows based on provided labels', () => { const { data, args } = sampleArgs(); - const component = shallow(); + const component = shallow(); expect(component.find(LineSeries).prop('data')).toEqual([ - { 'Label A': 1, 'Label B': 2, c: 3 }, - { 'Label A': 1, 'Label B': 5, c: 4 }, + { 'Label A': 1, 'Label B': 2, c: 'I' }, + { 'Label A': 1, 'Label B': 5, c: 'J' }, ]); }); test('it uses labels as Y accessors', () => { const { data, args } = sampleArgs(); - const component = shallow(); + const component = shallow(); expect(component.find(LineSeries).prop('yAccessors')).toEqual(['Label A', 'Label B']); }); - test('it indicates a linear scale for a numeric X axis', () => { + test('it indicates an ordinal scale for a string X axis', () => { const { data, args } = sampleArgs(); - const component = shallow(); - expect(component.find(LineSeries).prop('xScaleType')).toEqual(ScaleType.Linear); + const component = shallow(); + expect(component.find(LineSeries).prop('xScaleType')).toEqual(ScaleType.Ordinal); }); - test('it indicates an ordinal scale for a string X axis', () => { + test('it indicates a linear scale for a numeric X axis', () => { const { args } = sampleArgs(); const data: LensMultiTable = { @@ -243,41 +243,44 @@ describe('xy_expression', () => { first: { type: 'kibana_datatable', columns: [{ id: 'a', name: 'a' }, { id: 'b', name: 'b' }, { id: 'c', name: 'c' }], - rows: [{ a: 1, b: 2, c: 'Hello' }, { a: 6, b: 5, c: 'World' }], + rows: [{ a: 1, b: 2, c: 3 }, { a: 6, b: 5, c: 9 }], }, }, }; - const component = shallow(); - expect(component.find(LineSeries).prop('xScaleType')).toEqual(ScaleType.Ordinal); + const component = shallow(); + expect(component.find(LineSeries).prop('xScaleType')).toEqual(ScaleType.Linear); }); test('it gets the formatter for the x axis', () => { const { data, args } = sampleArgs(); - shallow(); + shallow(); - expect(getFormat as jest.Mock).toHaveBeenCalledWith({ id: 'string' }); + expect(getFormatSpy).toHaveBeenCalledWith({ id: 'string' }); }); test('it gets a default formatter for y if there are multiple y accessors', () => { const { data, args } = sampleArgs(); - shallow(); + shallow(); - expect(getFormat as jest.Mock).toHaveBeenCalledTimes(2); - expect(getFormat as jest.Mock).toHaveBeenCalledWith({ id: 'number' }); + expect(getFormatSpy).toHaveBeenCalledTimes(2); + expect(getFormatSpy).toHaveBeenCalledWith({ id: 'number' }); }); test('it gets the formatter for the y axis if there is only one accessor', () => { const { data, args } = sampleArgs(); shallow( - + ); - - expect(getFormat as jest.Mock).toHaveBeenCalledTimes(2); - expect(getFormat as jest.Mock).toHaveBeenCalledWith({ + expect(getFormatSpy).toHaveBeenCalledTimes(2); + expect(getFormatSpy).toHaveBeenCalledWith({ id: 'number', params: { pattern: '0,0.000' }, }); @@ -286,7 +289,9 @@ describe('xy_expression', () => { test('it should pass the formatter function to the axis', () => { const { data, args } = sampleArgs(); - const instance = shallow(); + const instance = shallow( + + ); const tickFormatter = instance .find(Axis) @@ -294,7 +299,7 @@ describe('xy_expression', () => { .prop('tickFormat'); tickFormatter('I'); - expect(getFormat({}).convert as jest.Mock).toHaveBeenCalledWith('I'); + expect(convertSpy).toHaveBeenCalledWith('I'); }); }); }); diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx index 5cb83e49a0507..a247476a7d820 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx @@ -22,7 +22,7 @@ import { I18nProvider } from '@kbn/i18n/react'; import { ExpressionFunction } from 'src/legacy/core_plugins/interpreter/types'; import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiText, IconType } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; -import { getFormat } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; +import { FormatFactory } from '../../../../../../src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities'; import { LensMultiTable } from '../types'; import { XYArgs, SeriesType, visualizationTypes } from './types'; import { RenderFunction } from '../interpreter_types'; @@ -86,7 +86,7 @@ export interface XYChartProps { args: XYArgs; } -export const xyChartRenderer: RenderFunction = { +export const getXyChartRenderer = (formatFactory: FormatFactory): RenderFunction => ({ name: 'lens_xy_chart_renderer', displayName: 'XY Chart', help: 'X/Y Chart Renderer', @@ -95,18 +95,24 @@ export const xyChartRenderer: RenderFunction = { render: async (domNode: Element, config: XYChartProps, _handlers: unknown) => { ReactDOM.render( - + , domNode ); }, -}; +}); function getIconForSeriesType(seriesType: SeriesType): IconType { return visualizationTypes.find(c => c.id === seriesType)!.icon || 'empty'; } -export function XYChart({ data, args }: XYChartProps) { +export function XYChart({ + data, + args, + formatFactory, +}: XYChartProps & { + formatFactory: FormatFactory; +}) { const { legend, layers, isHorizontal } = args; if (Object.values(data.tables).every(table => table.rows.length === 0)) { @@ -130,18 +136,18 @@ export function XYChart({ data, args }: XYChartProps) { const xAxisColumn = Object.values(data.tables)[0].columns.find( ({ id }) => id === layers[0].xAccessor - )!; - const xAxisFormatter = getFormat(xAxisColumn ? xAxisColumn.formatterMapping : undefined); + ); + const xAxisFormatter = formatFactory(xAxisColumn && xAxisColumn.formatHint); - let yAxisFormatter: ReturnType; + let yAxisFormatter: ReturnType; if (layers.length === 1 && layers[0].accessors.length === 1) { const firstYAxisColumn = Object.values(data.tables)[0].columns.find( ({ id }) => id === layers[0].accessors[0] ); - yAxisFormatter = getFormat(firstYAxisColumn ? firstYAxisColumn.formatterMapping : undefined); + yAxisFormatter = formatFactory(firstYAxisColumn && firstYAxisColumn.formatHint); } else { // TODO if there are multiple y axes, try to merge the formatters for the axis - yAxisFormatter = getFormat({ id: 'number' }); + yAxisFormatter = formatFactory({ id: 'number' }); } return ( From 554857954addb35eb6a78762134abf3f9bb1343e Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Sun, 11 Aug 2019 11:20:23 +0200 Subject: [PATCH 08/10] fix type error --- .../visualize/loader/pipeline_helpers/build_pipeline.ts | 3 ++- .../public/visualize/loader/pipeline_helpers/utilities.ts | 6 +----- .../common/expressions/expression_types/kibana_datatable.ts | 6 +++++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts index 54540256737b8..22e41dbe757b4 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts @@ -23,7 +23,8 @@ import { setBounds } from 'ui/agg_types/buckets/date_histogram'; import { SearchSource } from 'ui/courier'; import { AggConfig, Vis, VisParams, VisState } from 'ui/vis'; import moment from 'moment'; -import { SerializedFieldFormat, createFormat } from './utilities'; +import { SerializedFieldFormat } from 'src/plugins/data/common'; +import { createFormat } from './utilities'; interface SchemaConfigParams { precision?: number; diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts index e440d3678a6d9..cec5ca39712d9 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts @@ -20,6 +20,7 @@ import { i18n } from '@kbn/i18n'; import { identity } from 'lodash'; import { AggConfig, Vis } from 'ui/vis'; +import { SerializedFieldFormat } from 'src/plugins/data/common'; // @ts-ignore import { FieldFormat } from '../../../../field_formats/field_format'; // @ts-ignore @@ -40,11 +41,6 @@ function isTermsFieldFormat( return serializedFieldFormat.id === 'terms'; } -export interface SerializedFieldFormat { - id?: string; - params?: TParams; -} - const config = chrome.getUiSettingsClient(); const getConfig = (...args: any[]): any => config.get(...args); diff --git a/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts b/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts index f44819ebd8c3e..571e9ec7ff1e9 100644 --- a/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts +++ b/src/plugins/data/common/expressions/expression_types/kibana_datatable.ts @@ -18,10 +18,14 @@ */ import { map } from 'lodash'; -import { SerializedFieldFormat } from 'ui/visualize/loader/pipeline_helpers/utilities'; const name = 'kibana_datatable'; +export interface SerializedFieldFormat { + id?: string; + params?: TParams; +} + interface Column { id: string; name: string; From 498f836c40d324e0fba61c7ba57f9f3be48e5327 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Sun, 11 Aug 2019 11:27:34 +0200 Subject: [PATCH 09/10] clean up formatters --- .../xy_visualization_plugin/xy_expression.test.tsx | 1 - .../public/xy_visualization_plugin/xy_expression.tsx | 11 ++++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx index ac858e65e53cb..a3d8a3b3f0133 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.test.tsx @@ -279,7 +279,6 @@ describe('xy_expression', () => { formatFactory={getFormatSpy} /> ); - expect(getFormatSpy).toHaveBeenCalledTimes(2); expect(getFormatSpy).toHaveBeenCalledWith({ id: 'number', params: { pattern: '0,0.000' }, diff --git a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx index a247476a7d820..37680bbe8e3d9 100644 --- a/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx +++ b/x-pack/legacy/plugins/lens/public/xy_visualization_plugin/xy_expression.tsx @@ -134,20 +134,21 @@ export function XYChart({ ); } + // use formatting hint of first x axis column to format ticks const xAxisColumn = Object.values(data.tables)[0].columns.find( ({ id }) => id === layers[0].xAccessor ); const xAxisFormatter = formatFactory(xAxisColumn && xAxisColumn.formatHint); - let yAxisFormatter: ReturnType; + // use default number formatter for y axis and use formatting hint if there is just a single y column + let yAxisFormatter = formatFactory({ id: 'number' }); if (layers.length === 1 && layers[0].accessors.length === 1) { const firstYAxisColumn = Object.values(data.tables)[0].columns.find( ({ id }) => id === layers[0].accessors[0] ); - yAxisFormatter = formatFactory(firstYAxisColumn && firstYAxisColumn.formatHint); - } else { - // TODO if there are multiple y axes, try to merge the formatters for the axis - yAxisFormatter = formatFactory({ id: 'number' }); + if (firstYAxisColumn && firstYAxisColumn.formatHint) { + yAxisFormatter = formatFactory(firstYAxisColumn.formatHint); + } } return ( From 1b761fb2dcdf3ee2cd46757d745e7fbd98438b2b Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Tue, 13 Aug 2019 08:31:59 +0200 Subject: [PATCH 10/10] code hygiene --- .../core_plugins/interpreter/public/functions/esaggs.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index c788d4a88d781..bb8feabb159cd 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -119,7 +119,7 @@ export const esaggs = (): ExpressionFunction { @@ -133,7 +133,5 @@ export const esaggs = (): ExpressionFunction