From c2e420d2c2b8780042d45f8051f131d857df0c92 Mon Sep 17 00:00:00 2001 From: "Qingyang(Abby) Hu" Date: Wed, 28 Sep 2022 17:39:30 -0700 Subject: [PATCH] [Viz Builder] State validation before dispatching and loading (#2351) * State validation before dispatching Validate style and visualization state schema before dispatching state updates when loading wizard. Show error via toast. Signed-off-by: abbyhu2000 * Fixes and add unit tests Signed-off-by: abbyhu2000 * Add PR to changelog.md Signed-off-by: abbyhu2000 Signed-off-by: abbyhu2000 (cherry picked from commit 0279588f7ac8e1e6ba6745bd03a84bb7cf294b4a) --- CHANGELOG.md | 1 + package.json | 1 + .../public/application/utils/schema.json | 28 ++++++++++++ .../utils/use/use_saved_wizard_vis.ts | 31 ++++++++++--- .../utils/wizard_state_validation.test.ts | 43 +++++++++++++++++++ .../utils/wizard_state_validation.ts | 19 ++++++++ yarn.lock | 2 +- 7 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 src/plugins/wizard/public/application/utils/schema.json create mode 100644 src/plugins/wizard/public/application/utils/wizard_state_validation.test.ts create mode 100644 src/plugins/wizard/public/application/utils/wizard_state_validation.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6509bd835027..8db80035714c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 📈 Features/Enhancements * Add updated_at column to objects' tables ([#1218](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/1218)) +* [Viz Builder] State validation before dispatching and loading ([#2351](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2351)) ### 🐛 Bug Fixes diff --git a/package.json b/package.json index b5086af4fe5e..7dd417779009 100644 --- a/package.json +++ b/package.json @@ -141,6 +141,7 @@ "@types/yauzl": "^2.9.1", "JSONStream": "1.3.5", "abortcontroller-polyfill": "^1.4.0", + "ajv": "^8.11.0", "angular": "^1.8.2", "angular-elastic": "^2.5.1", "angular-sanitize": "^1.8.0", diff --git a/src/plugins/wizard/public/application/utils/schema.json b/src/plugins/wizard/public/application/utils/schema.json new file mode 100644 index 000000000000..9effed97b2be --- /dev/null +++ b/src/plugins/wizard/public/application/utils/schema.json @@ -0,0 +1,28 @@ +{ + "type": "object", + "properties": { + "styleState": { + "type": "object" + }, + "visualizationState": { + "type": "object", + "properties": { + "activeVisualization": { + "type": "object", + "properties": { + "name": { "type": "string" }, + "aggConfigParams": { "type": "array" } + }, + "required": ["name", "aggConfigParams"], + "additionalProperties": false + }, + "indexPattern": { "type": "string" }, + "searchField": { "type": "string" } + }, + "required": ["searchField"], + "additionalProperties": false + } + }, + "required": ["styleState", "visualizationState"], + "additionalProperties": false +} \ No newline at end of file diff --git a/src/plugins/wizard/public/application/utils/use/use_saved_wizard_vis.ts b/src/plugins/wizard/public/application/utils/use/use_saved_wizard_vis.ts index db17e478a41c..36ed26b15d04 100644 --- a/src/plugins/wizard/public/application/utils/use/use_saved_wizard_vis.ts +++ b/src/plugins/wizard/public/application/utils/use/use_saved_wizard_vis.ts @@ -7,6 +7,7 @@ import { i18n } from '@osd/i18n'; import { useEffect, useState } from 'react'; import { SavedObject } from '../../../../../saved_objects/public'; import { + InvalidJSONProperty, redirectWhenMissing, SavedObjectNotFound, } from '../../../../../opensearch_dashboards_utils/public'; @@ -23,6 +24,7 @@ import { } from '../state_management'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { setEditorState } from '../state_management/metadata_slice'; +import { validateWizardState } from '../wizard_state_validation'; // This function can be used when instantiating a saved vis or creating a new one // using url parameters, embedding and destroying it in DOM @@ -39,6 +41,14 @@ export const useSavedWizardVis = (visualizationIdFromUrl: string | undefined) => http: { basePath }, toastNotifications, } = services; + const toastNotification = (message) => { + toastNotifications.addDanger({ + title: i18n.translate('visualize.createVisualization.failedToLoadErrorMessage', { + defaultMessage: 'Failed to load the visualization', + }), + text: message, + }); + }; const loadSavedWizardVis = async () => { try { const savedWizardVis = await getSavedWizardVis(services, visualizationIdFromUrl); @@ -58,7 +68,16 @@ export const useSavedWizardVis = (visualizationIdFromUrl: string | undefined) => activeVisualization: vizStateWithoutIndex.activeVisualization, indexPattern: savedWizardVis.searchSourceFields.index, }; - // TODO: Add validation and transformation, throw/handle errors + + const validateResult = validateWizardState({ styleState, visualizationState }); + if (!validateResult.valid) { + const err = validateResult.errors; + if (err) { + const errMsg = err[0].instancePath + ' ' + err[0].message; + throw new InvalidJSONProperty(errMsg); + } + } + dispatch(setStyleState(styleState)); dispatch(setVisualizationState(visualizationState)); } @@ -83,14 +102,12 @@ export const useSavedWizardVis = (visualizationIdFromUrl: string | undefined) => mapping: managementRedirectTarget, })(error); } + if (error instanceof InvalidJSONProperty) { + toastNotification(error.message); + } } catch (e) { const message = e instanceof Error ? e.message : ''; - toastNotifications.addWarning({ - title: i18n.translate('visualize.createVisualization.failedToLoadErrorMessage', { - defaultMessage: 'Failed to load the visualization', - }), - text: message, - }); + toastNotification(message); history.replace(EDIT_PATH); } } diff --git a/src/plugins/wizard/public/application/utils/wizard_state_validation.test.ts b/src/plugins/wizard/public/application/utils/wizard_state_validation.test.ts new file mode 100644 index 000000000000..6b02d9707438 --- /dev/null +++ b/src/plugins/wizard/public/application/utils/wizard_state_validation.test.ts @@ -0,0 +1,43 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { validateWizardState } from './wizard_state_validation'; + +describe('wizard state validation', () => { + const validStyleState = { + addLegend: true, + addTooltip: true, + legendPosition: '', + type: 'metric', + }; + const validVisualizationState = { + activeVisualization: { + name: 'metric', + aggConfigParams: [], + }, + indexPattern: '', + searchField: '', + }; + describe('correct return when validation suceeds', () => { + test('with correct wizard state', () => { + const validationResult = validateWizardState({ + styleState: validStyleState, + visualizationState: validVisualizationState, + }); + expect(validationResult.valid).toBeTruthy(); + expect(validationResult.errors).toBeNull(); + }); + }); + describe('correct return with errors when validation fails', () => { + test('with non object type styleStyle', () => { + const validationResult = validateWizardState({ + styleState: [], + visualizationState: validVisualizationState, + }); + expect(validationResult.valid).toBeFalsy(); + expect(validationResult.errors).toBeDefined(); + }); + }); +}); diff --git a/src/plugins/wizard/public/application/utils/wizard_state_validation.ts b/src/plugins/wizard/public/application/utils/wizard_state_validation.ts new file mode 100644 index 000000000000..6bcdf973e371 --- /dev/null +++ b/src/plugins/wizard/public/application/utils/wizard_state_validation.ts @@ -0,0 +1,19 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import Ajv from 'ajv'; +import wizardStateSchema from './schema.json'; + +const ajv = new Ajv(); +const validateState = ajv.compile(wizardStateSchema); + +export const validateWizardState = (wizardState) => { + const isWizardStateValid = validateState(wizardState); + + return { + valid: isWizardStateValid, + errors: validateState.errors, + }; +}; diff --git a/yarn.lock b/yarn.lock index 346e32b2b1f8..cccf92e88ff2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4221,7 +4221,7 @@ ajv@^6.1.0, ajv@^6.10.0, ajv@^6.10.2, ajv@^6.11.0, ajv@^6.12.3, ajv@^6.12.4, ajv json-schema-traverse "^0.4.1" uri-js "^4.2.2" -ajv@^8.0.1, ajv@^8.6.2: +ajv@^8.0.1, ajv@^8.11.0, ajv@^8.6.2: version "8.11.0" resolved "https://registry.yarnpkg.com/ajv/-/ajv-8.11.0.tgz#977e91dd96ca669f54a11e23e378e33b884a565f" integrity sha512-wGgprdCvMalC0BztXvitD2hC04YffAvtsUn93JbGXYLAtCUO4xd17mCCZQxUOItiBwZvJScWo8NIvQMQ71rdpg==