[Canvas] XY. Step 1. Remove all specific logic, related to aggType. (#…

…111876) * Removed logic, related to BUCKET_TYPES. * removed aggType connected code. * Added `id` to `xy_dimension` to avoid direct comparison with aggId * Removed all checks of seriesParams at chart. * removed aggId and aggType from chart * removed all aggId/aggTypes from tests/mocks. * moved `get_agg_id.ts` util to the vislib * clarified the code, related to isSimpleField logic. * added a comment. * Fixed error at percentile agg. * Fixed render_all_series failure of tests. * Added tests for new behavior. * changed the way of handling `enableHistogramMode`. * updated snapshots. * Fixed shard_delay test. * removed all todos from xy. * removed latest todos. * Changed the type of tickFormatter. Co-authored-by: Kibana Machine <[email protected]>
elastic · Sep 22, 2021 · a35b18a · a35b18a
1 parent aa1372f
commit a35b18a
Show file tree

Hide file tree

Showing 29 changed files with 255 additions and 213 deletions.
diff --git a/src/plugins/vis_types/vislib/public/__snapshots__/to_ast.test.ts.snap b/src/plugins/vis_types/vislib/public/__snapshots__/to_ast.test.ts.snap
diff --git a/src/plugins/vis_types/vislib/public/__snapshots__/to_ast_pie.test.ts.snap b/src/plugins/vis_types/vislib/public/__snapshots__/to_ast_pie.test.ts.snap
diff --git a/src/plugins/vis_types/vislib/public/vislib/helpers/point_series/_add_to_siri.ts b/src/plugins/vis_types/vislib/public/vislib/helpers/point_series/_add_to_siri.ts
@@ -5,10 +5,8 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
-
-import { getAggId } from '../../../../../xy/public';
 import type { Dimension } from '../../../../../xy/public';
-
+import { getAggId } from './get_agg_id';
 import { Point } from './_get_point';
 
 export interface Serie {

diff --git a/.../vis_types/xy/public/config/get_agg_id.ts → ...vislib/helpers/point_series/get_agg_id.ts b/.../vis_types/xy/public/config/get_agg_id.ts → ...vislib/helpers/point_series/get_agg_id.ts
diff --git a/src/plugins/vis_types/xy/kibana.json b/src/plugins/vis_types/xy/kibana.json
@@ -3,7 +3,7 @@
   "version": "kibana",
   "ui": true,
   "server": false,
-  "requiredPlugins": ["charts", "data", "expressions", "visualizations", "usageCollection"],
+  "requiredPlugins": ["charts", "data", "expressions", "visualizations", "usageCollection", "fieldFormats"],
   "requiredBundles": ["kibanaUtils", "visDefaultEditor"],
   "extraPublicDirs": ["common/index"],
   "owner": {

diff --git a/src/plugins/vis_types/xy/public/__snapshots__/to_ast.test.ts.snap b/src/plugins/vis_types/xy/public/__snapshots__/to_ast.test.ts.snap
diff --git a/src/plugins/vis_types/xy/public/components/detailed_tooltip.mock.ts b/src/plugins/vis_types/xy/public/components/detailed_tooltip.mock.ts
@@ -17,8 +17,7 @@ export const aspects = {
         pattern: 'YYYY-MM-DD HH:mm',
       },
     },
-    aggType: 'date_histogram',
-    aggId: '3',
+    id: '3',
     params: {
       date: true,
       intervalESUnit: 'h',
@@ -35,8 +34,7 @@ export const aspects = {
       format: {
         id: 'number',
       },
-      aggType: 'count',
-      aggId: '1',
+      id: '1',
       params: {},
     },
   ],
@@ -53,8 +51,7 @@ export const aspectsWithSplitColumn = {
         pattern: 'YYYY-MM-DD HH:mm',
       },
     },
-    aggType: 'date_histogram',
-    aggId: '3',
+    id: '3',
     params: {
       date: true,
       intervalESUnit: 'h',
@@ -71,8 +68,7 @@ export const aspectsWithSplitColumn = {
       format: {
         id: 'number',
       },
-      aggType: 'count',
-      aggId: '1',
+      id: '1',
       params: {},
     },
   ],
@@ -88,8 +84,7 @@ export const aspectsWithSplitColumn = {
         missingBucketLabel: 'Missing',
       },
     },
-    aggType: 'terms',
-    aggId: '4',
+    id: '4',
     params: {},
   },
 };
@@ -105,8 +100,7 @@ export const aspectsWithSplitRow = {
         pattern: 'YYYY-MM-DD HH:mm',
       },
     },
-    aggType: 'date_histogram',
-    aggId: '3',
+    id: '3',
     params: {
       date: true,
       intervalESUnit: 'h',
@@ -123,8 +117,7 @@ export const aspectsWithSplitRow = {
       format: {
         id: 'number',
       },
-      aggType: 'count',
-      aggId: '1',
+      id: '1',
       params: {},
     },
   ],
@@ -140,8 +133,7 @@ export const aspectsWithSplitRow = {
         missingBucketLabel: 'Missing',
       },
     },
-    aggType: 'terms',
-    aggId: '4',
+    id: '4',
     params: {},
   },
 };

diff --git a/src/plugins/vis_types/xy/public/components/detailed_tooltip.tsx b/src/plugins/vis_types/xy/public/components/detailed_tooltip.tsx
@@ -17,9 +17,13 @@ import {
 } from '@elastic/charts';
 
 import { Aspects } from '../types';
+import {
+  applyFormatterIfSimpleField,
+  applyFormatter,
+  COMPLEX_SPLIT_ACCESSOR,
+} from '../utils/accessors';
 
 import './_detailed_tooltip.scss';
-import { COMPLEX_SPLIT_ACCESSOR, isRangeAggType } from '../utils/accessors';
 
 interface TooltipData {
   label: string;
@@ -34,10 +38,10 @@ export const getTooltipData = (
   const data: TooltipData[] = [];
 
   if (header) {
-    const xFormatter = isRangeAggType(aspects.x.aggType) ? null : aspects.x.formatter;
     data.push({
       label: aspects.x.title,
-      value: xFormatter ? xFormatter(header.value) : `${header.value}`,
+      // already formatted while executing accessor on such a complex field type as `*_range`
+      value: `${applyFormatterIfSimpleField(aspects.x, header.value)}`,
     });
   }
 
@@ -47,14 +51,14 @@ export const getTooltipData = (
   if (yAccessor) {
     data.push({
       label: yAccessor.title,
-      value: yAccessor.formatter ? yAccessor.formatter(value.value) : `${value.value}`,
+      value: `${applyFormatter(yAccessor, value.value)}`,
     });
   }
 
   if (aspects.z && !isNil(value.markValue)) {
     data.push({
       label: aspects.z.title,
-      value: aspects.z.formatter ? aspects.z.formatter(value.markValue) : `${value.markValue}`,
+      value: `${applyFormatterIfSimpleField(aspects.z, value.markValue)}`,
     });
   }
 

diff --git a/src/plugins/vis_types/xy/public/config/get_aspects.ts b/src/plugins/vis_types/xy/public/config/get_aspects.ts
@@ -14,13 +14,10 @@ import { DatatableColumn } from '../../../../expressions/public';
 
 import { Aspect, Dimension, Aspects, Dimensions } from '../types';
 import { getFormatService } from '../services';
-import { getAggId } from './get_agg_id';
 
 export function getEmptyAspect(): Aspect {
   return {
     accessor: null,
-    aggId: null,
-    aggType: null,
     title: i18n.translate('visTypeXy.aggResponse.allDocsTitle', {
       defaultMessage: 'All docs',
     }),
@@ -76,14 +73,13 @@ function getAspectsFromDimension(
 
 const getAspect = (
   { id: accessor, name: title }: DatatableColumn,
-  { accessor: column, format, params, aggType }: Dimension
+  { accessor: column, format, params, id }: Dimension
 ): Aspect => ({
   accessor,
   column,
   title,
   format,
-  aggType,
-  aggId: getAggId(accessor),
   formatter: (value: any) => getFormatService().deserialize(format).convert(value),
   params,
+  id,
 });
diff --git a/src/plugins/vis_types/xy/public/config/get_axis.ts b/src/plugins/vis_types/xy/public/config/get_axis.ts
@@ -6,13 +6,9 @@
  * Side Public License, v 1.
  */
 
-import { identity, isNil } from 'lodash';
-
+import { isNil } from 'lodash';
 import { AxisSpec, TickFormatter, YDomainRange, ScaleType as ECScaleType } from '@elastic/charts';
-
 import { LabelRotation } from '../../../../charts/public';
-import { BUCKET_TYPES } from '../../../../data/public';
-
 import {
   Aspect,
   CategoryAxis,
@@ -27,13 +23,15 @@ import {
   YScaleType,
   SeriesParam,
 } from '../types';
+import { isSimpleField } from '../utils/accessors';
 
 export function getAxis<S extends XScaleType | YScaleType>(
   { type, title: axisTitle, labels, scale: axisScale, ...axis }: CategoryAxis,
   { categoryLines, valueAxis }: Grid,
-  { params, format, formatter, title: fallbackTitle = '', aggType }: Aspect,
+  { params, format, formatter, title: fallbackTitle = '', accessor }: Aspect,
   seriesParams: SeriesParam[],
-  isDateHistogram = false
+  isDateHistogram = false,
+  shouldApplyFormatter = false
 ): AxisConfig<S> {
   const isCategoryAxis = type === AxisType.Category;
   // Hide unassigned axis, not supported in elastic charts
@@ -50,9 +48,10 @@ export function getAxis<S extends XScaleType | YScaleType>(
     : {
         show: valueAxis === axis.id,
       };
-  // Date range formatter applied on xAccessor
-  const tickFormatter =
-    aggType === BUCKET_TYPES.DATE_RANGE || aggType === BUCKET_TYPES.RANGE ? identity : formatter;
+
+  const tickFormatter: TickFormatter = (v) =>
+    isSimpleField(format) || shouldApplyFormatter ? formatter?.(v) ?? v : v;
+
   const ticks: TickOptions = {
     formatter: tickFormatter,
     labelFormatter: getLabelFormatter(labels.truncate, tickFormatter),
@@ -61,6 +60,7 @@ export function getAxis<S extends XScaleType | YScaleType>(
     showOverlappingLabels: !labels.filter,
     showDuplicates: !labels.filter,
   };
+
   const scale = getScale<S>(axisScale, params, format, isCategoryAxis);
   const title = axisTitle.text || fallbackTitle;
   const fallbackRotation =
@@ -76,7 +76,7 @@ export function getAxis<S extends XScaleType | YScaleType>(
     scale,
     style: getAxisStyle(ticks, title, fallbackRotation),
     domain: getAxisDomain(scale, isCategoryAxis),
-    integersOnly: aggType === 'count',
+    integersOnly: params?.integersOnly ?? false,
   };
 }
 

diff --git a/src/plugins/vis_types/xy/public/config/get_config.ts b/src/plugins/vis_types/xy/public/config/get_config.ts
@@ -9,12 +9,12 @@
 import { ScaleContinuousType } from '@elastic/charts';
 
 import { Datatable } from '../../../../expressions/public';
-import { BUCKET_TYPES } from '../../../../data/public';
-import { DateHistogramParams } from '../../../../visualizations/public';
-
+import { KBN_FIELD_TYPES } from '../../../../data/public';
 import {
   Aspect,
   AxisConfig,
+  AxisMode,
+  ChartMode,
   SeriesParam,
   VisConfig,
   VisParams,
@@ -42,23 +42,28 @@ export function getConfig(table: Datatable, params: VisParams): VisConfig {
     fillOpacity,
   } = params;
   const aspects = getAspects(table.columns, params.dimensions);
+  const yAxes = params.valueAxes.map((a) =>
+    // shouldApplyFormatter = true, because no formatter was applied to this axis values before
+    // and will be not applied in the future
+    getAxis<YScaleType>(a, params.grid, aspects.y[0], params.seriesParams, false, true)
+  );
+
+  const enableHistogramMode =
+    (params.enableHistogramMode ?? false) &&
+    shouldEnableHistogramMode(params.seriesParams, aspects.y, yAxes);
+
+  const timeChartFieldTypes: string[] = [KBN_FIELD_TYPES.DATE, KBN_FIELD_TYPES.DATE_RANGE];
+  const isTimeChart = timeChartFieldTypes.includes(aspects.x.format?.id ?? '');
+
   const xAxis = getAxis<XScaleType>(
     params.categoryAxes[0],
     params.grid,
     aspects.x,
     params.seriesParams,
-    params.dimensions.x?.aggType === BUCKET_TYPES.DATE_HISTOGRAM
+    aspects.x.format?.id === KBN_FIELD_TYPES.DATE
   );
+
   const tooltip = getTooltip(aspects, params);
-  const yAxes = params.valueAxes.map((a) =>
-    // uses first y aspect in array for formatting axis
-    getAxis<YScaleType>(a, params.grid, aspects.y[0], params.seriesParams)
-  );
-  const enableHistogramMode =
-    (params.dimensions.x?.aggType === BUCKET_TYPES.DATE_HISTOGRAM ||
-      params.dimensions.x?.aggType === BUCKET_TYPES.HISTOGRAM) &&
-    shouldEnableHistogramMode(params.seriesParams, aspects.y, yAxes);
-  const isTimeChart = (aspects.x.params as DateHistogramParams).date ?? false;
 
   return {
     // NOTE: downscale ratio to match current vislib implementation
@@ -94,11 +99,7 @@ const shouldEnableHistogramMode = (
   yAspects: Aspect[],
   yAxes: Array<AxisConfig<ScaleContinuousType>>
 ): boolean => {
-  const bars = seriesParams.filter(({ type, data: { id: paramId } }) => {
-    return (
-      type === ChartType.Histogram && yAspects.find(({ aggId }) => aggId === paramId) !== undefined
-    );
-  });
+  const bars = seriesParams.filter(({ type }) => type === ChartType.Histogram);
 
   const groupIds = [
     ...bars.reduce<Set<string>>((acc, { valueAxis: groupId, mode }) => {
@@ -114,6 +115,6 @@ const shouldEnableHistogramMode = (
   return bars.every(({ valueAxis: groupId, mode }) => {
     const yAxisScale = yAxes.find(({ groupId: axisGroupId }) => axisGroupId === groupId)?.scale;
 
-    return mode === 'stacked' || yAxisScale?.mode === 'percentage';
+    return mode === ChartMode.Stacked || yAxisScale?.mode === AxisMode.Percentage;
   });
 };
diff --git a/src/plugins/vis_types/xy/public/config/index.ts b/src/plugins/vis_types/xy/public/config/index.ts
@@ -7,4 +7,3 @@
  */
 
 export { getConfig } from './get_config';
-export { getAggId } from './get_agg_id';
diff --git a/src/plugins/vis_types/xy/public/expression_functions/xy_vis_fn.ts b/src/plugins/vis_types/xy/public/expression_functions/xy_vis_fn.ts
@@ -16,6 +16,7 @@ import type {
 import { prepareLogTable, Dimension } from '../../../../visualizations/public';
 import type { ChartType } from '../../common';
 import type { VisParams, XYVisConfig } from '../types';
+import { isValidSeriesForDimension } from '../utils/accessors';
 
 export const visName = 'xy_vis';
 export interface RenderValue {
@@ -153,6 +154,12 @@ export const visTypeXyVisFn = (): VisTypeXyExpressionFunctionDefinition => ({
           'Flag to indicate old vislib visualizations. Used for backwards compatibility including colors',
       }),
     },
+    enableHistogramMode: {
+      types: ['boolean'],
+      help: i18n.translate('visTypeXy.function.args.enableHistogramMode.help', {
+        defaultMessage: 'Flag to enable histogram mode',
+      }),
+    },
     detailedTooltip: {
       types: ['boolean'],
       help: i18n.translate('visTypeXy.function.args.detailedTooltip.help', {
@@ -255,10 +262,16 @@ export const visTypeXyVisFn = (): VisTypeXyExpressionFunctionDefinition => ({
         categoryLines: args.gridCategoryLines,
         valueAxis: args.gridValueAxis,
       },
-      seriesParams: args.seriesParams.map((seriesParam) => ({
-        ...seriesParam,
-        type: seriesParam.seriesParamType,
-      })),
+      seriesParams: args.seriesParams.map((seriesParam) => {
+        const matchedSeries = args.yDimension.filter(({ id, accessor }) =>
+          isValidSeriesForDimension(seriesParam.data.id)({ id, accessor })
+        );
+        return {
+          ...seriesParam,
+          show: matchedSeries.length > 0,
+          type: seriesParam.seriesParamType,
+        };
+      }),
       radiusRatio: args.radiusRatio,
       times: args.times,
       isVislibVis: args.isVislibVis,
@@ -269,6 +282,7 @@ export const visTypeXyVisFn = (): VisTypeXyExpressionFunctionDefinition => ({
       },
       fillOpacity: args.fillOpacity,
       fittingFunction: args.fittingFunction,
+      enableHistogramMode: args.enableHistogramMode,
       dimensions: {
         x: args.xDimension,
         y: args.yDimension,
Original file line number	Diff line number	Diff line change
Expand Up		@@ -7,4 +7,3 @@
		*/

		export { getConfig } from './get_config';
		export { getAggId } from './get_agg_id';