From 2dc48ff56ca1be56b5f7fa8e437ac143d0aabf0c Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Fri, 17 Jun 2022 11:20:22 +0200 Subject: [PATCH] [XY] Add xExtent validation within expression (#134546) * :sparkles: Add expression validation for xExtent * :white_check_mark: Add more tests --- .../expression_xy/common/__mocks__/index.ts | 4 - .../__snapshots__/xy_vis.test.ts.snap | 6 + .../common/expression_functions/validate.ts | 22 +++ .../expression_functions/xy_vis.test.ts | 125 ++++++++++++++++++ .../common/expression_functions/xy_vis_fn.ts | 2 + .../common/types/expression_functions.ts | 6 +- 6 files changed, 158 insertions(+), 7 deletions(-) diff --git a/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts b/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts index d5eaf3b086e65..e9810d2764025 100644 --- a/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts +++ b/src/plugins/chart_expressions/expression_xy/common/__mocks__/index.ts @@ -115,10 +115,6 @@ export const createArgsWithLayers = ( yLeft: false, yRight: false, }, - xExtent: { - mode: 'dataBounds', - type: 'axisExtentConfig', - }, yLeftExtent: { mode: 'full', type: 'axisExtentConfig', diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap b/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap index 80e1d140be28d..6ffe660fa99af 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/__snapshots__/xy_vis.test.ts.snap @@ -17,3 +17,9 @@ exports[`xyVis it should throw error if splitColumnAccessor is pointing to the a exports[`xyVis it should throw error if splitRowAccessor is pointing to the absent column 1`] = `"Provided column name or index is invalid: absent-accessor"`; exports[`xyVis throws the error if showLines is provided to the not line/area chart 1`] = `"Lines visibility can be controlled only at line charts"`; + +exports[`xyVis throws the error if the x axis extent is enabled for a date histogram 1`] = `"X axis extent is only supported for numeric histograms."`; + +exports[`xyVis throws the error if the x axis extent is enabled with the full mode 1`] = `"For x axis extent, the full mode is not supported."`; + +exports[`xyVis throws the error if the x axis extent is enabled without a histogram defined 1`] = `"X axis extent is only supported for numeric histograms."`; diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts index 2eec4ecaab950..6cf8f4c68d1db 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/validate.ts @@ -87,6 +87,14 @@ export const errors = { i18n.translate('expressionXY.reusable.function.xyVis.errors.dataBoundsForNotLineChartError', { defaultMessage: 'Only line charts can be fit to the data bounds', }), + extentFullModeIsInvalidError: () => + i18n.translate('expressionXY.reusable.function.xyVis.errors.extentFullModeIsInvalid', { + defaultMessage: 'For x axis extent, the full mode is not supported.', + }), + extentModeNotSupportedError: () => + i18n.translate('expressionXY.reusable.function.xyVis.errors.extentModeNotSupportedError', { + defaultMessage: 'X axis extent is only supported for numeric histograms.', + }), timeMarkerForNotTimeChartsError: () => i18n.translate('expressionXY.reusable.function.xyVis.errors.timeMarkerForNotTimeChartsError', { defaultMessage: 'Only time charts can have current time marker', @@ -136,6 +144,20 @@ export const validateExtentForDataBounds = ( } }; +export const validateXExtent = ( + extent: AxisExtentConfigResult | undefined, + dataLayers: Array +) => { + if (extent) { + if (extent.mode === AxisExtentModes.FULL) { + throw new Error(errors.extentFullModeIsInvalidError()); + } + if (isTimeChart(dataLayers) || dataLayers.every(({ isHistogram }) => !isHistogram)) { + throw new Error(errors.extentModeNotSupportedError()); + } + } +}; + export const validateExtent = ( extent: AxisExtentConfigResult, hasBarOrArea: boolean, diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts index ab8d243b72057..bbd906b724844 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis.test.ts @@ -215,4 +215,129 @@ describe('xyVis', () => { ) ).rejects.toThrowErrorMatchingSnapshot(); }); + + test('throws the error if the x axis extent is enabled for a date histogram', async () => { + const { + data, + args: { layers, ...rest }, + } = sampleArgs(); + const { layerId, layerType, table, type, ...restLayerArgs } = sampleLayer; + + expect( + xyVisFunction.fn( + data, + { + ...rest, + ...restLayerArgs, + referenceLines: [], + annotationLayers: [], + isHistogram: true, + xScaleType: 'time', + xExtent: { type: 'axisExtentConfig', mode: 'dataBounds' }, + }, + createMockExecutionContext() + ) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + test('throws the error if the x axis extent is enabled with the full mode', async () => { + const { + data, + args: { layers, ...rest }, + } = sampleArgs(); + const { layerId, layerType, table, type, ...restLayerArgs } = sampleLayer; + + expect( + xyVisFunction.fn( + data, + { + ...rest, + ...restLayerArgs, + referenceLines: [], + annotationLayers: [], + xExtent: { + type: 'axisExtentConfig', + mode: 'full', + lowerBound: undefined, + upperBound: undefined, + }, + }, + createMockExecutionContext() + ) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + test('throws the error if the x axis extent is enabled without a histogram defined', async () => { + const { + data, + args: { layers, ...rest }, + } = sampleArgs(); + const { layerId, layerType, table, type, ...restLayerArgs } = sampleLayer; + + expect( + xyVisFunction.fn( + data, + { + ...rest, + ...restLayerArgs, + referenceLines: [], + annotationLayers: [], + xExtent: { + type: 'axisExtentConfig', + mode: 'dataBounds', + }, + }, + createMockExecutionContext() + ) + ).rejects.toThrowErrorMatchingSnapshot(); + }); + + test('it renders with custom xExtent for a numeric histogram', async () => { + const { data, args } = sampleArgs(); + const { layers, ...rest } = args; + const { layerId, layerType, table, type, ...restLayerArgs } = sampleLayer; + const result = await xyVisFunction.fn( + data, + { + ...rest, + ...restLayerArgs, + referenceLines: [], + annotationLayers: [], + isHistogram: true, + xExtent: { + type: 'axisExtentConfig', + mode: 'custom', + lowerBound: 0, + upperBound: 10, + }, + }, + createMockExecutionContext() + ); + + expect(result).toEqual({ + type: 'render', + as: XY_VIS, + value: { + args: { + ...rest, + xExtent: { + type: 'axisExtentConfig', + mode: 'custom', + lowerBound: 0, + upperBound: 10, + }, + layers: [ + { + layerType, + table: data, + layerId: 'dataLayers-0', + type, + ...restLayerArgs, + isHistogram: true, + }, + ], + }, + }, + }); + }); }); diff --git a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts index 8e352a6f34b4b..c24f28f5ca173 100644 --- a/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts +++ b/src/plugins/chart_expressions/expression_xy/common/expression_functions/xy_vis_fn.ts @@ -33,6 +33,7 @@ import { validateLineWidthForChartType, validatePointsRadiusForChartType, validateLinesVisibilityForChartType, + validateXExtent, } from './validate'; const createDataLayer = (args: XYArgs, table: Datatable): DataLayerConfigResult => { @@ -120,6 +121,7 @@ export const xyVisFn: XyVisFn['fn'] = async (data, args, handlers) => { const hasBar = hasBarLayer(dataLayers); const hasArea = hasAreaLayer(dataLayers); + validateXExtent(args.xExtent, dataLayers); validateExtent(args.yLeftExtent, hasBar || hasArea, dataLayers); validateExtent(args.yRightExtent, hasBar || hasArea, dataLayers); validateFillOpacity(args.fillOpacity, hasArea); diff --git a/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts b/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts index e87704a919838..50e0439f757c0 100644 --- a/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts +++ b/src/plugins/chart_expressions/expression_xy/common/types/expression_functions.ts @@ -196,7 +196,7 @@ export interface XYArgs extends DataLayerArgs { xTitle: string; yTitle: string; yRightTitle: string; - xExtent: AxisExtentConfigResult; + xExtent?: AxisExtentConfigResult; yLeftExtent: AxisExtentConfigResult; yRightExtent: AxisExtentConfigResult; yLeftScale: YScaleType; @@ -231,7 +231,7 @@ export interface LayeredXYArgs { xTitle: string; yTitle: string; yRightTitle: string; - xExtent: AxisExtentConfigResult; + xExtent?: AxisExtentConfigResult; yLeftExtent: AxisExtentConfigResult; yRightExtent: AxisExtentConfigResult; yLeftScale: YScaleType; @@ -263,7 +263,7 @@ export interface XYProps { xTitle: string; yTitle: string; yRightTitle: string; - xExtent: AxisExtentConfigResult; + xExtent?: AxisExtentConfigResult; yLeftExtent: AxisExtentConfigResult; yRightExtent: AxisExtentConfigResult; yLeftScale: YScaleType;