Skip to content

Commit

Permalink
[Viz Builder] State validation before dispatching and loading (#2351)
Browse files Browse the repository at this point in the history
* 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 <abigailhu2000@gmail.com>

* Fixes and add unit tests

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

* Add PR to changelog.md

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
(cherry picked from commit 0279588)
  • Loading branch information
abbyhu2000 authored and github-actions[bot] committed Sep 29, 2022
1 parent dd3ade1 commit c2e420d
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
28 changes: 28 additions & 0 deletions src/plugins/wizard/public/application/utils/schema.json
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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<MetricOptionsDefaults>(styleState));
dispatch(setVisualizationState(visualizationState));
}
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
Original file line number Diff line number Diff line change
@@ -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,
};
};
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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==
Expand Down

0 comments on commit c2e420d

Please sign in to comment.