[Lens] Formula can be colored by value (elastic#103572)

* [Lens] Formula can be colored by value * Fix mapColumn test * Fix tests
kibanamachine · Jun 30, 2021 · 4495fc6 · 4495fc6
1 parent f4b29da
commit 4495fc6
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 13 deletions.
diff --git 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<Datatable>(
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 () => {