From 75186b576da0f21054cf94d5f87f2ca5d4a3b4db Mon Sep 17 00:00:00 2001 From: "Mr.Dr.Professor Patrick" Date: Wed, 20 Dec 2023 12:17:46 +0100 Subject: [PATCH] feat(D3 plugin): add basic data validation (#366) * feat(D3 plugin): add basic data validation * fix: review fixes --- src/i18n/keysets/en.json | 7 +- src/i18n/keysets/ru.json | 7 +- src/libs/chartkit-error/chartkit-error.ts | 1 + src/plugins/d3/examples/bar-x/Basic.tsx | 21 ++- src/plugins/d3/renderer/D3Widget.tsx | 55 +++--- .../d3/renderer/constants/defaults/axis.ts | 4 + .../d3/renderer/hooks/useAxisScales/index.ts | 5 +- .../d3/renderer/validation/__mocks__/index.ts | 47 +++++ .../validation/__tests__/validation.test.ts | 73 ++++++++ src/plugins/d3/renderer/validation/index.ts | 177 ++++++++++++++++++ 10 files changed, 362 insertions(+), 35 deletions(-) create mode 100644 src/plugins/d3/renderer/validation/__mocks__/index.ts create mode 100644 src/plugins/d3/renderer/validation/__tests__/validation.test.ts create mode 100644 src/plugins/d3/renderer/validation/index.ts diff --git a/src/i18n/keysets/en.json b/src/i18n/keysets/en.json index cdf3527e..1b6eadf5 100644 --- a/src/i18n/keysets/en.json +++ b/src/i18n/keysets/en.json @@ -28,7 +28,12 @@ "error": { "label_no-data": "No data", "label_unknown-plugin": "Unknown plugin type \"{{type}}\"", - "label_unknown-error": "Unknown error" + "label_unknown-error": "Unknown error", + "label_invalid-axis-category-data-point": "It seems you are trying to use inappropriate data type for \"{{key}}\" value in series \"{{seriesName}}\" for axis with type \"category\". Strings and numbers are allowed.", + "label_invalid-axis-datetime-data-point": "It seems you are trying to use inappropriate data type for \"{{key}}\" value in series \"{{seriesName}}\" for axis with type \"datetime\". Only numbers are allowed.", + "label_invalid-axis-linear-data-point": "It seems you are trying to use inappropriate data type for \"{{key}}\" value in series \"{{seriesName}}\" for axis with type \"linear\". Numbers and nulls are allowed.", + "label_invalid-pie-data-value": "It seems you are trying to use inappropriate data type for \"value\" value. Only numbers are allowed.", + "label_invalid-series-type": "It seems you haven't defined \"series.type\" property, or defined it incorrectly. Available values: [{{types}}]." }, "highcharts": { "reset-zoom-title": "Reset zoom", diff --git a/src/i18n/keysets/ru.json b/src/i18n/keysets/ru.json index 9632e34a..59693f8c 100644 --- a/src/i18n/keysets/ru.json +++ b/src/i18n/keysets/ru.json @@ -30,7 +30,12 @@ "error": { "label_no-data": "Нет данных", "label_unknown-plugin": "Неизвестный тип плагина \"{{type}}\"", - "label_unknown-error": "Неизвестная ошибка" + "label_unknown-error": "Неизвестная ошибка", + "label_invalid-axis-category-data-point": "Похоже, что вы пытаетесь использовать недопустимый тип данных для значения \"{{key}}\" в серии \"{{seriesName}}\" для оси с типом \"category\". Допускается использование строк и чисел.", + "label_invalid-axis-datetime-data-point": "Похоже, что вы пытаетесь использовать недопустимый тип данных для значения \"{{key}}\" в серии \"{{seriesName}}\" для оси с типом \"datetime\". Допускается только использование чисел.", + "label_invalid-axis-linear-data-point": "Похоже, что вы пытаетесь использовать недопустимый тип данных для значения \"{{key}}\" в серии \"{{seriesName}}\" для оси с типом \"linear\". Допускается использование чисел и значений null.", + "label_invalid-pie-data-value": "Похоже, что вы пытаетесь использовать недопустимый тип данных для значения \"value\". Допускается только использование чисел.", + "label_invalid-series-type": "Похоже, что вы не указали значение \"series.type\" или указали его неверно. Доступные значения: [{{types}}]." }, "highcharts": { "reset-zoom-title": "Сбросить увеличение", diff --git a/src/libs/chartkit-error/chartkit-error.ts b/src/libs/chartkit-error/chartkit-error.ts index 887a7afe..4369d2b7 100644 --- a/src/libs/chartkit-error/chartkit-error.ts +++ b/src/libs/chartkit-error/chartkit-error.ts @@ -6,6 +6,7 @@ export type ChartKitErrorArgs = { export const CHARTKIT_ERROR_CODE = { NO_DATA: 'ERR.CK.NO_DATA', + INVALID_DATA: 'ERR.CK.INVALID_DATA', UNKNOWN: 'ERR.CK.UNKNOWN_ERROR', UNKNOWN_PLUGIN: 'ERR.CK.UNKNOWN_PLUGIN', TOO_MANY_LINES: 'ERR.CK.TOO_MANY_LINES', diff --git a/src/plugins/d3/examples/bar-x/Basic.tsx b/src/plugins/d3/examples/bar-x/Basic.tsx index 65c4d973..1d26f642 100644 --- a/src/plugins/d3/examples/bar-x/Basic.tsx +++ b/src/plugins/d3/examples/bar-x/Basic.tsx @@ -4,15 +4,25 @@ import type {ChartKitWidgetData, BarXSeries, BarXSeriesData} from '../../../../t import nintendoGames from '../nintendoGames'; import {groups} from 'd3'; -function prepareData(field: 'platform' | 'meta_score' | 'date' = 'platform') { +function prepareData( + {field, filterNulls}: {field: 'platform' | 'meta_score' | 'date'; filterNulls?: boolean} = { + field: 'platform', + }, +) { const gamesByPlatform = groups(nintendoGames, (item) => item[field]); - const data = gamesByPlatform.map(([value, games]) => ({ + let resultData = gamesByPlatform; + + if (filterNulls) { + resultData = gamesByPlatform.filter(([value]) => typeof value === 'number'); + } + + const data = resultData.map(([value, games]) => ({ x: value, y: games.length, })); return { - categories: gamesByPlatform.map(([key]) => key), + categories: resultData.map(([key]) => key), series: [ { data, @@ -24,7 +34,6 @@ function prepareData(field: 'platform' | 'meta_score' | 'date' = 'platform') { export const BasicBarXChart = () => { const {categories, series} = prepareData(); - const widgetData: ChartKitWidgetData = { series: { data: series.map((s) => ({ @@ -47,7 +56,7 @@ export const BasicBarXChart = () => { }; export const BasicLinearBarXChart = () => { - const {series} = prepareData('meta_score'); + const {series} = prepareData({field: 'meta_score'}); const widgetData: ChartKitWidgetData = { series: { @@ -68,7 +77,7 @@ export const BasicLinearBarXChart = () => { }; export const BasicDateTimeBarXChart = () => { - const {series} = prepareData('date'); + const {series} = prepareData({field: 'date', filterNulls: true}); const widgetData: ChartKitWidgetData = { series: { diff --git a/src/plugins/d3/renderer/D3Widget.tsx b/src/plugins/d3/renderer/D3Widget.tsx index 9277f7ce..56e30b71 100644 --- a/src/plugins/d3/renderer/D3Widget.tsx +++ b/src/plugins/d3/renderer/D3Widget.tsx @@ -7,6 +7,7 @@ import afterFrame from 'afterframe'; import type {ChartKitProps, ChartKitWidgetRef} from '../../../types'; import {getRandomCKId, measurePerformance} from '../../../utils'; import {Chart} from './components'; +import {validateData} from './validation'; type ChartDimensions = { width: number; @@ -23,31 +24,6 @@ const D3Widget = React.forwardRef { - if (onChartLoad) { - onChartLoad({}); - } - }, [onChartLoad]); - - React.useLayoutEffect(() => { - if (dimensions?.width) { - if (!performanceMeasure.current) { - performanceMeasure.current = measurePerformance(); - } - - afterFrame(() => { - const renderTime = performanceMeasure.current?.end(); - onRender?.({ - renderTime, - }); - onLoad?.({ - widgetRendering: renderTime, - }); - performanceMeasure.current = null; - }); - } - }, [data, onRender, onLoad, dimensions]); - const handleResize = React.useCallback(() => { const parentElement = ref.current?.parentElement; @@ -90,6 +66,35 @@ const D3Widget = React.forwardRef { + validateData(data); + }, [data]); + + React.useLayoutEffect(() => { + if (onChartLoad) { + onChartLoad({}); + } + }, [onChartLoad]); + + React.useLayoutEffect(() => { + if (dimensions?.width) { + if (!performanceMeasure.current) { + performanceMeasure.current = measurePerformance(); + } + + afterFrame(() => { + const renderTime = performanceMeasure.current?.end(); + onRender?.({ + renderTime, + }); + onLoad?.({ + widgetRendering: renderTime, + }); + performanceMeasure.current = null; + }); + } + }, [data, onRender, onLoad, dimensions]); + return (
@@ -58,7 +59,7 @@ const filterCategoriesByVisibleSeries = (args: { }; export function createYScale(axis: PreparedAxis, series: PreparedSeries[], boundsHeight: number) { - const yType = get(axis, 'type', 'linear'); + const yType = get(axis, 'type', DEFAULT_AXIS_TYPE); const yMin = get(axis, 'min'); const yCategories = get(axis, 'categories'); const yTimestamps = get(axis, 'timestamps'); @@ -133,7 +134,7 @@ export function createXScale( boundsWidth: number, ) { const xMin = get(axis, 'min'); - const xType = get(axis, 'type', 'linear'); + const xType = get(axis, 'type', DEFAULT_AXIS_TYPE); const xCategories = get(axis, 'categories'); const xTimestamps = get(axis, 'timestamps'); const maxPadding = get(axis, 'maxPadding', 0); diff --git a/src/plugins/d3/renderer/validation/__mocks__/index.ts b/src/plugins/d3/renderer/validation/__mocks__/index.ts new file mode 100644 index 00000000..a7f5638c --- /dev/null +++ b/src/plugins/d3/renderer/validation/__mocks__/index.ts @@ -0,0 +1,47 @@ +import {ChartKitWidgetData} from '../../../../../types'; + +export const XY_SERIES: Record = { + INVALID_CATEGORY_X: { + series: { + data: [{type: 'scatter', data: [{x: undefined, y: 1}], name: 'Series'}], + }, + xAxis: {type: 'category'}, + }, + INVALID_CATEGORY_Y: { + series: { + data: [{type: 'scatter', data: [{x: 1, y: undefined}], name: 'Series'}], + }, + yAxis: [{type: 'category'}], + }, + INVALID_DATETIME_X: { + series: { + data: [{type: 'scatter', data: [{x: undefined, y: 1}], name: 'Series'}], + }, + xAxis: {type: 'datetime'}, + }, + INVALID_DATETIME_Y: { + series: { + data: [{type: 'scatter', data: [{x: undefined, y: 1}], name: 'Series'}], + }, + yAxis: [{type: 'datetime'}], + }, + INVALID_LINEAR_X: { + series: { + data: [{type: 'scatter', data: [{x: 'str', y: 1}], name: 'Series'}], + }, + }, + INVALID_LINEAR_Y: { + series: { + data: [{type: 'scatter', data: [{x: 1, y: 'str'}], name: 'Series'}], + }, + }, +}; + +export const PIE_SERIES: Record = { + INVALID_VALUE: { + series: { + // @ts-expect-error + data: [{type: 'pie', data: [{value: undefined, name: 'Series'}]}], + }, + }, +}; diff --git a/src/plugins/d3/renderer/validation/__tests__/validation.test.ts b/src/plugins/d3/renderer/validation/__tests__/validation.test.ts new file mode 100644 index 00000000..1db9c2b2 --- /dev/null +++ b/src/plugins/d3/renderer/validation/__tests__/validation.test.ts @@ -0,0 +1,73 @@ +import {ChartKitError, CHARTKIT_ERROR_CODE} from '../../../../../libs'; +import {ChartKitWidgetData} from '../../../../../types'; +import {validateData} from '../'; +import {PIE_SERIES, XY_SERIES} from '../__mocks__'; + +describe('plugins/d3/validation', () => { + test.each([undefined, null, {}, {series: {}}, {series: {data: []}}])( + 'validateData should throw an error in case of empty data (data: %j)', + (data) => { + let error: ChartKitError | null = null; + + try { + validateData(data); + } catch (e) { + error = e as ChartKitError; + } + + expect(error?.code).toEqual(CHARTKIT_ERROR_CODE.NO_DATA); + }, + ); + + test.each([ + {series: {data: [{data: [{x: 1, y: 1}]}]}}, + {series: {data: [{type: 'invalid-type', data: [{x: 1, y: 1}]}]}}, + ])('validateData should throw an error in case of incorrect series type (data: %j)', (data) => { + let error: ChartKitError | null = null; + + try { + validateData(data); + } catch (e) { + error = e as ChartKitError; + } + + expect(error?.code).toEqual(CHARTKIT_ERROR_CODE.INVALID_DATA); + }); + + test.each([ + XY_SERIES.INVALID_CATEGORY_X, + XY_SERIES.INVALID_CATEGORY_Y, + XY_SERIES.INVALID_DATETIME_X, + XY_SERIES.INVALID_DATETIME_Y, + XY_SERIES.INVALID_LINEAR_X, + XY_SERIES.INVALID_LINEAR_Y, + ])( + '[XY Series] validateData should throw an error in case of invalid data (data: %j)', + (data) => { + let error: ChartKitError | null = null; + + try { + validateData(data); + } catch (e) { + error = e as ChartKitError; + } + + expect(error?.code).toEqual(CHARTKIT_ERROR_CODE.INVALID_DATA); + }, + ); + + test.each([PIE_SERIES.INVALID_VALUE])( + '[Pie Series] validateData should throw an error in case of invalid data (data: %j)', + (data) => { + let error: ChartKitError | null = null; + + try { + validateData(data); + } catch (e) { + error = e as ChartKitError; + } + + expect(error?.code).toEqual(CHARTKIT_ERROR_CODE.INVALID_DATA); + }, + ); +}); diff --git a/src/plugins/d3/renderer/validation/index.ts b/src/plugins/d3/renderer/validation/index.ts new file mode 100644 index 00000000..1fef611f --- /dev/null +++ b/src/plugins/d3/renderer/validation/index.ts @@ -0,0 +1,177 @@ +import get from 'lodash/get'; +import isEmpty from 'lodash/isEmpty'; +import {ChartKitError, CHARTKIT_ERROR_CODE} from '../../../../libs'; +import { + BarXSeries, + BarYSeries, + ChartKitWidgetAxis, + ChartKitWidgetData, + ChartKitWidgetSeries, + LineSeries, + PieSeries, + ScatterSeries, +} from '../../../../types'; +import {i18n} from '../../../../i18n'; +import {DEFAULT_AXIS_TYPE} from '../constants'; + +type XYSeries = ScatterSeries | BarXSeries | BarYSeries | LineSeries; + +const AVAILABLE_SERIES_TYPES: ChartKitWidgetSeries['type'][] = [ + 'area', + 'bar-x', + 'bar-y', + 'line', + 'pie', + 'scatter', +]; + +const validateXYSeries = (args: { + series: XYSeries; + xAxis?: ChartKitWidgetAxis; + yAxis?: ChartKitWidgetAxis; +}) => { + const {series, xAxis, yAxis} = args; + const xType = get(xAxis, 'type', DEFAULT_AXIS_TYPE); + const yType = get(yAxis, 'type', DEFAULT_AXIS_TYPE); + series.data.forEach(({x, y}) => { + switch (xType) { + case 'category': { + if (typeof x !== 'string' && typeof x !== 'number') { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-axis-category-data-point', { + key: 'x', + seriesName: series.name, + }), + }); + } + + break; + } + case 'datetime': { + if (typeof x !== 'number') { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-axis-datetime-data-point', { + key: 'x', + seriesName: series.name, + }), + }); + } + + break; + } + case 'linear': { + if (typeof x !== 'number' && x !== null) { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-axis-linear-data-point', { + key: 'x', + seriesName: series.name, + }), + }); + } + } + } + switch (yType) { + case 'category': { + if (typeof y !== 'string' && typeof y !== 'number') { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-axis-category-data-point', { + key: 'y', + seriesName: series.name, + }), + }); + } + + break; + } + case 'datetime': { + if (typeof y !== 'number') { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-axis-datetime-data-point', { + key: 'y', + seriesName: series.name, + }), + }); + } + + break; + } + case 'linear': { + if (typeof y !== 'number' && y !== null) { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-axis-linear-data-point', { + key: 'y', + seriesName: series.name, + }), + }); + } + } + } + }); +}; + +const validatePieSeries = ({series}: {series: PieSeries}) => { + series.data.forEach(({value}) => { + if (typeof value !== 'number') { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-pie-data-value'), + }); + } + }); +}; + +const validateSeries = (args: { + series: ChartKitWidgetSeries; + xAxis?: ChartKitWidgetAxis; + yAxis?: ChartKitWidgetAxis; +}) => { + const {series, xAxis, yAxis} = args; + + if (!AVAILABLE_SERIES_TYPES.includes(series.type)) { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: i18n('error', 'label_invalid-series-type', { + types: AVAILABLE_SERIES_TYPES.join(', '), + }), + }); + } + + switch (series.type) { + case 'bar-x': + case 'bar-y': + case 'line': + case 'scatter': { + validateXYSeries({series, xAxis, yAxis}); + break; + } + case 'pie': { + validatePieSeries({series}); + } + } +}; + +export const validateData = (data?: ChartKitWidgetData) => { + if (isEmpty(data) || isEmpty(data.series) || isEmpty(data.series.data)) { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.NO_DATA, + message: i18n('error', 'label_no-data'), + }); + } + + if (data.series.data.some((s) => isEmpty(s.data))) { + throw new ChartKitError({ + code: CHARTKIT_ERROR_CODE.INVALID_DATA, + message: 'You should specify data for all series', + }); + } + + data.series.data.forEach((series) => { + validateSeries({series, yAxis: data.yAxis?.[0], xAxis: data.xAxis}); + }); +};