diff --git a/src/plugins/expressions/common/expression_functions/specs/map_column.ts b/src/plugins/expressions/common/expression_functions/specs/map_column.ts index 7ea96ee7fdde8..d897e24aaad83 100644 --- a/src/plugins/expressions/common/expression_functions/specs/map_column.ts +++ b/src/plugins/expressions/common/expression_functions/specs/map_column.ts @@ -10,7 +10,7 @@ import { Observable, defer, of, zip } from 'rxjs'; import { map } from 'rxjs/operators'; import { i18n } from '@kbn/i18n'; import { ExpressionFunctionDefinition } from '../types'; -import { Datatable, DatatableColumn, getType } from '../../expression_types'; +import { Datatable, DatatableColumn, DatatableColumnType, getType } from '../../expression_types'; export interface MapColumnArguments { id?: string | null; @@ -103,7 +103,16 @@ export const mapColumn: ExpressionFunctionDefinition< return rows$.pipe( map((rows) => { - const type = getType(rows[0]?.[id]); + let type: DatatableColumnType = 'null'; + if (rows.length) { + for (const row of rows) { + const rowType = getType(row[id]); + if (rowType !== 'null') { + type = rowType; + break; + } + } + } const newColumn: DatatableColumn = { id, name: args.name, diff --git a/src/plugins/expressions/common/expression_functions/specs/math_column.ts b/src/plugins/expressions/common/expression_functions/specs/math_column.ts index 633d912c29502..c59016cd260ab 100644 --- a/src/plugins/expressions/common/expression_functions/specs/math_column.ts +++ b/src/plugins/expressions/common/expression_functions/specs/math_column.ts @@ -9,7 +9,7 @@ import { i18n } from '@kbn/i18n'; import { ExpressionFunctionDefinition } from '../types'; import { math, MathArguments } from './math'; -import { Datatable, DatatableColumn, getType } from '../../expression_types'; +import { Datatable, DatatableColumn, DatatableColumnType, getType } from '../../expression_types'; export type MathColumnArguments = MathArguments & { id: string; @@ -104,7 +104,16 @@ export const mathColumn: ExpressionFunctionDefinition< return { ...row, [args.id]: result }; }); - const type = newRows.length ? getType(newRows[0][args.id]) : 'null'; + let type: DatatableColumnType = 'null'; + if (newRows.length) { + for (const row of newRows) { + const rowType = getType(row[args.id]); + if (rowType !== 'null') { + type = rowType; + break; + } + } + } const newColumn: DatatableColumn = { id: args.id, name: args.name ?? args.id, diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts index bd934745fed72..64a42958ae8a2 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts @@ -10,9 +10,10 @@ import { of, Observable } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; import { Datatable } from '../../../expression_types'; import { mapColumn, MapColumnArguments } from '../map_column'; -import { emptyTable, functionWrapper, testTable } from './utils'; +import { emptyTable, functionWrapper, testTable, tableWithNulls } from './utils'; -const pricePlusTwo = (datatable: Datatable) => of(datatable.rows[0].price + 2); +const pricePlusTwo = (datatable: Datatable) => + of(typeof datatable.rows[0].price === 'number' ? datatable.rows[0].price + 2 : null); describe('mapColumn', () => { const fn = functionWrapper(mapColumn); @@ -219,11 +220,11 @@ describe('mapColumn', () => { }); }); - it('should correctly infer the type fromt he first row if the references column for meta information does not exists', () => { + it('should correctly infer the type from the first row if the references column for meta information does not exists', () => { testScheduler.run(({ expectObservable }) => { expectObservable( runFn( - { ...emptyTable, rows: [...emptyTable.rows, { value: 5 }] }, + { ...emptyTable, rows: [...emptyTable.rows, { price: 5 }] }, { name: 'value', copyMetaFrom: 'time', expression: pricePlusTwo } ) ).toBe('(0|)', [ @@ -236,6 +237,31 @@ describe('mapColumn', () => { meta: expect.objectContaining({ type: 'number' }), }), ], + rows: [{ price: 5, value: 7 }], + }), + ]); + }); + }); + + it('should correctly infer the type from the first non-null row', () => { + testScheduler.run(({ expectObservable }) => { + expectObservable( + runFn(tableWithNulls, { + id: 'value', + name: 'value', + expression: pricePlusTwo, + }) + ).toBe('(0|)', [ + expect.objectContaining({ + type: 'datatable', + columns: [ + ...tableWithNulls.columns, + expect.objectContaining({ + id: 'value', + name: 'value', + meta: expect.objectContaining({ type: 'number' }), + }), + ], }), ]); }); diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts b/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts index e0fb0a3a9f23d..3464736fe0ad3 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/math_column.test.ts @@ -7,7 +7,7 @@ */ import { mathColumn } from '../math_column'; -import { functionWrapper, testTable } from './utils'; +import { functionWrapper, testTable, tableWithNulls } from './utils'; describe('mathColumn', () => { const fn = functionWrapper(mathColumn); @@ -95,4 +95,22 @@ describe('mathColumn', () => { meta: { type: 'date', params: { id: 'number', params: { digits: 2 } } }, }); }); + + it('should correctly infer the type from the first non-null row', () => { + expect( + fn(tableWithNulls, { id: 'value', name: 'value', expression: 'price + 2', onError: 'null' }) + ).toEqual( + expect.objectContaining({ + type: 'datatable', + columns: [ + ...tableWithNulls.columns, + expect.objectContaining({ + id: 'value', + name: 'value', + meta: expect.objectContaining({ type: 'number' }), + }), + ], + }) + ); + }); }); diff --git a/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts b/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts index 60d22d2b8575c..ca41b427a28f7 100644 --- a/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts +++ b/src/plugins/expressions/common/expression_functions/specs/tests/utils.ts @@ -224,4 +224,72 @@ const stringTable: Datatable = { ], }; -export { emptyTable, testTable, stringTable }; +const tableWithNulls: Datatable = { + type: 'datatable', + columns: [ + { + id: 'name', + name: 'name label', + meta: { type: 'string' }, + }, + { + id: 'time', + name: 'time label', + meta: { type: 'date' }, + }, + { + id: 'price', + name: 'price label', + meta: { type: 'number' }, + }, + ], + rows: [ + { + name: 'product1', + time: 1517842800950, // 05 Feb 2018 15:00:00 GMT + price: null, + }, + { + name: 'product1', + time: 1517929200950, // 06 Feb 2018 15:00:00 GMT + price: null, + }, + { + name: 'product1', + time: 1518015600950, // 07 Feb 2018 15:00:00 GMT + price: 420, + }, + { + name: 'product2', + time: 1517842800950, // 05 Feb 2018 15:00:00 GMT + price: 216, + }, + { + name: 'product2', + time: 1517929200950, // 06 Feb 2018 15:00:00 GMT + price: 200, + }, + { + name: 'product2', + time: 1518015600950, // 07 Feb 2018 15:00:00 GMT + price: 190, + }, + { + name: 'product3', + time: 1517842800950, // 05 Feb 2018 15:00:00 GMT + price: null, + }, + { + name: 'product4', + time: 1517842800950, // 05 Feb 2018 15:00:00 GMT + price: 311, + }, + { + name: 'product5', + time: 1517842800950, // 05 Feb 2018 15:00:00 GMT + price: 288, + }, + ], +}; + +export { emptyTable, testTable, stringTable, tableWithNulls }; diff --git a/x-pack/test/functional/apps/lens/formula.ts b/x-pack/test/functional/apps/lens/formula.ts index 38d1f63e946d4..17061f6e37170 100644 --- a/x-pack/test/functional/apps/lens/formula.ts +++ b/x-pack/test/functional/apps/lens/formula.ts @@ -180,7 +180,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(await PageObjects.lens.getErrorCount()).to.eql(0); }); - it('should duplicate a moving average formula and be a valid table', async () => { + it('should duplicate a moving average formula and be a valid table with conditional coloring', async () => { await PageObjects.visualize.navigateToNewVisualization(); await PageObjects.visualize.clickVisType('lens'); await PageObjects.lens.goToTimeRange(); @@ -198,14 +198,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { formula: `moving_average(sum(bytes), window=5`, keepOpen: true, }); + await PageObjects.lens.setTableDynamicColoring('text'); + await PageObjects.header.waitUntilLoadingHasFinished(); + const styleObj = await PageObjects.lens.getDatatableCellStyle(1, 1); + expect(styleObj['background-color']).to.be(undefined); + expect(styleObj.color).not.to.be(undefined); + await PageObjects.lens.closeDimensionEditor(); await PageObjects.lens.dragDimensionToDimension( 'lnsDatatable_metrics > lns-dimensionTrigger', 'lnsDatatable_metrics > lns-empty-dimension' ); - expect(await PageObjects.lens.getDatatableCellText(1, 1)).to.eql('222420'); - expect(await PageObjects.lens.getDatatableCellText(1, 2)).to.eql('222420'); + expect(await PageObjects.lens.getDatatableCellText(1, 1)).to.eql('222,420'); + expect(await PageObjects.lens.getDatatableCellText(1, 2)).to.eql('222,420'); }); it('should keep the formula if the user does not fully transition to a quick function', async () => {