🐛 Fix partial rows edge cases

elastic · May 25, 2022 · 0cc33ae · 0cc33ae
1 parent 7a0de80
commit 0cc33ae
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 36 deletions.
diff --git a/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx b/src/plugins/chart_expressions/expression_xy/public/helpers/data_layers.tsx
@@ -105,18 +105,23 @@ export const getFormattedRow = (
   columns: Datatable['columns'],
   columnsFormatters: Record<string, IFieldFormat>,
   xAccessor: string | undefined,
+  splitAccessor: string | undefined,
   xScaleType: XScaleType
 ): { row: Datatable['rows'][number]; formattedColumns: Record<string, true> } =>
   columns.reduce(
     (formattedInfo, { id }) => {
       const record = formattedInfo.row[id];
       if (
-        record != null &&
         // pre-format values for ordinal x axes because there can only be a single x axis formatter on chart level
-        (!isPrimitive(record) || (id === xAccessor && xScaleType === 'ordinal'))
+        !isPrimitive(record) ||
+        (id === xAccessor && xScaleType === 'ordinal')
       ) {
+        const overrideEmptyValue = record == null && splitAccessor;
         return {
-          row: { ...formattedInfo.row, [id]: columnsFormatters[id]!.convert(record) },
+          row: {
+            ...formattedInfo.row,
+            [id]: columnsFormatters[id]!.convert(overrideEmptyValue ? '' : record),
+          },
           formattedColumns: { ...formattedInfo.formattedColumns, [id]: true },
         };
       }
@@ -129,6 +134,7 @@ export const getFormattedTable = (
   table: Datatable,
   formatFactory: FormatFactory,
   xAccessor: string | ExpressionValueVisDimension | undefined,
+  splitAccessor: string | ExpressionValueVisDimension | undefined,
   accessors: Array<string | ExpressionValueVisDimension>,
   xScaleType: XScaleType
 ): { table: Datatable; formattedColumns: Record<string, true> } => {
@@ -146,28 +152,28 @@ export const getFormattedTable = (
     {}
   );
 
-  const formattedTableInfo = table.rows.reduce<{
+  const formattedTableInfo: {
     rows: Datatable['rows'];
     formattedColumns: Record<string, true>;
-  }>(
-    ({ rows: formattedRows, formattedColumns }, row) => {
-      const formattedRowInfo = getFormattedRow(
-        row,
-        table.columns,
-        columnsFormatters,
-        xAccessor ? getAccessorByDimension(xAccessor, table.columns) : undefined,
-        xScaleType
-      );
-      return {
-        rows: [...formattedRows, formattedRowInfo.row],
-        formattedColumns: { ...formattedColumns, ...formattedRowInfo.formattedColumns },
-      };
-    },
-    {
-      rows: [],
-      formattedColumns: {},
-    }
-  );
+  } = {
+    rows: [],
+    formattedColumns: {},
+  };
+  for (const row of table.rows) {
+    const formattedRowInfo = getFormattedRow(
+      row,
+      table.columns,
+      columnsFormatters,
+      xAccessor ? getAccessorByDimension(xAccessor, table.columns) : undefined,
+      splitAccessor ? getAccessorByDimension(splitAccessor, table.columns) : undefined,
+      xScaleType
+    );
+    formattedTableInfo.rows.push(formattedRowInfo.row);
+    formattedTableInfo.formattedColumns = {
+      ...formattedTableInfo.formattedColumns,
+      ...formattedRowInfo.formattedColumns,
+    };
+  }
 
   return {
     table: { ...table, rows: formattedTableInfo.rows },
@@ -186,6 +192,7 @@ export const getFormattedTablesByLayers = (
         table,
         formatFactory,
         xAccessor,
+        splitAccessor,
         [xAccessor, splitAccessor, ...accessors].filter<string | ExpressionValueVisDimension>(
           (a): a is string | ExpressionValueVisDimension => a !== undefined
         ),

diff --git a/...lugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx b/...lugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx
@@ -82,6 +82,12 @@ const GranularityHelpPopover = () => {
   );
 };
 
+function isValidBound(
+  bounds?: { min?: number; max?: number } | undefined
+): bounds is Required<ExtendedBounds> {
+  return boundsValidation(bounds);
+}
+
 function boundsValidation(bounds?: { min?: number; max?: number }) {
   return bounds?.min != null && bounds?.max != null && bounds?.min < bounds?.max;
 }
@@ -122,15 +128,8 @@ const BaseRangeEditor = ({
     });
   const onChangeCustomBoundsWithValidation = useCallback(
     (newBounds: { min?: number; max?: number } | undefined) => {
-      if (
-        newBounds &&
-        newBounds.min != null &&
-        newBounds.max != null &&
-        boundsValidation(newBounds)
-      ) {
-        const bounds = { min: newBounds.min, max: newBounds.max };
-
-        onChangeCustomBounds(bounds);
+      if (!newBounds || isValidBound(newBounds)) {
+        onChangeCustomBounds(newBounds);
       }
     },
     [onChangeCustomBounds]

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx
@@ -174,7 +174,9 @@ export const rangeOperation: OperationDefinition<RangeIndexPatternColumn, 'field
       has_extended_bounds: Boolean(params.includeEmptyRows && params.customBounds),
       min_doc_count: Boolean(params.includeEmptyRows),
       autoExtendBounds: Boolean(params.includeEmptyRows && !params.customBounds),
-      extended_bounds: extendedBoundsToAst(params.customBounds ?? {}),
+      extended_bounds: extendedBoundsToAst(
+        params.includeEmptyRows ? params.customBounds ?? {} : {}
+      ),
     }).toAst();
   },
   paramEditor: ({

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts
@@ -23,7 +23,11 @@ import {
 import { GenericIndexPatternColumn } from './indexpattern';
 import { operationDefinitionMap } from './operations';
 import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from './types';
-import { DateHistogramIndexPatternColumn, RangeIndexPatternColumn } from './operations/definitions';
+import type {
+  DateHistogramIndexPatternColumn,
+  RangeIndexPatternColumn,
+  TermsIndexPatternColumn,
+} from './operations/definitions';
 import { FormattedIndexPatternColumn } from './operations/definitions/column_types';
 import { isColumnFormatted, isColumnOfType } from './operations/definitions/helpers';
 
@@ -265,7 +269,9 @@ function getExpressionForLayer(
       };
     }
 
-    const allDateHistogramFields = Object.values(columns)
+    const configColumns = Object.values(columns);
+
+    const allDateHistogramFields = configColumns
       .map((column) =>
         isColumnOfType<DateHistogramIndexPatternColumn>('date_histogram', column) &&
         !column.params.ignoreTimeRange
@@ -274,6 +280,17 @@ function getExpressionForLayer(
       )
       .filter((field): field is string => Boolean(field));
 
+    // the combination of Interval with extended bounds + Top values (any combination)
+    // requires partial rows to be enabled
+    const shouldEnablePartialRows =
+      configColumns.some((column) => isColumnOfType<TermsIndexPatternColumn>('terms', column)) &&
+      configColumns.some(
+        (column) =>
+          isColumnOfType<RangeIndexPatternColumn>('range', column) &&
+          column.params.includeEmptyRows &&
+          column.params.customBounds
+      );
+
     return {
       type: 'expression',
       chain: [
@@ -287,7 +304,7 @@ function getExpressionForLayer(
           ]),
           aggs,
           metricsAtAllLevels: false,
-          partialRows: false,
+          partialRows: shouldEnablePartialRows,
           timeFields: allDateHistogramFields,
         }).toAst(),
         {