Skip to content

Commit

Permalink
[D&D] metadata slice 1879 (#2193)
Browse files Browse the repository at this point in the history
* Use metadata slice to save if the visualization is savable or not. Disable the save button if not and show error messages.

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

* Add some comments about store subscriber and editor state

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

* create a seperate use_can_save hook

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

Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>
  • Loading branch information
abbyhu2000 and ashwin-pc authored Aug 30, 2022
1 parent 730a75a commit 65005be
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ const EDITOR_KEY = 'CONFIG_PANEL';

export function SecondaryPanel() {
const draftAgg = useTypedSelector((state) => state.visualization.activeVisualization!.draftAgg);
const isEditorValid = useTypedSelector(
(state) => state.metadata.editorState.validity[EDITOR_KEY]
);
const isEditorValid = useTypedSelector((state) => state.metadata.editor.validity[EDITOR_KEY]);
const [touched, setTouched] = useState(false);
const dispatch = useTypedDispatch();
const vizType = useVisualizationType();
Expand Down
20 changes: 14 additions & 6 deletions src/plugins/wizard/public/application/components/top_nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@

import React, { useMemo } from 'react';
import { useParams } from 'react-router-dom';
import { useUnmount } from 'react-use';
import { PLUGIN_ID } from '../../../common';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { getTopNavConfig } from '../utils/get_top_nav_config';
import { WizardServices } from '../../types';

import './top_nav.scss';
import { useIndexPatterns, useSavedWizardVis } from '../utils/use';
import { useTypedSelector } from '../utils/state_management';
import { useTypedSelector, useTypedDispatch } from '../utils/state_management';
import { setEditorState } from '../utils/state_management/metadata_slice';
import { useCanSave } from '../utils/use/use_can_save';

export const TopNav = () => {
// id will only be set for the edit route
Expand All @@ -25,10 +28,9 @@ export const TopNav = () => {
},
} = services;
const rootState = useTypedSelector((state) => state);
const hasUnappliedChanges = useTypedSelector(
(state) => !!state.visualization.activeVisualization?.draftAgg
);
const dispatch = useTypedDispatch();

const saveDisabledReason = useCanSave();
const savedWizardVis = useSavedWizardVis(visualizationIdFromUrl);

const config = useMemo(() => {
Expand All @@ -42,14 +44,20 @@ export const TopNav = () => {
savedWizardVis,
visualizationState,
styleState,
hasUnappliedChanges,
saveDisabledReason,
dispatch,
},
services
);
}, [hasUnappliedChanges, rootState, savedWizardVis, services, visualizationIdFromUrl]);
}, [rootState, savedWizardVis, services, visualizationIdFromUrl, saveDisabledReason, dispatch]);

const indexPattern = useIndexPatterns().selected;

// reset validity before component destroyed
useUnmount(() => {
dispatch(setEditorState({ state: 'loading' }));
});

return (
<div className="wizTopNav">
<TopNavMenu
Expand Down
17 changes: 11 additions & 6 deletions src/plugins/wizard/public/application/utils/get_top_nav_config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ import {
} from '../../../../saved_objects/public';
import { WizardServices } from '../..';
import { WizardVisSavedObject } from '../../types';
import { StyleState, VisualizationState } from './state_management';
import { StyleState, VisualizationState, AppDispatch } from './state_management';
import { EDIT_PATH } from '../../../common';
import { setEditorState } from './state_management/metadata_slice';

interface TopNavConfigParams {
visualizationIdFromUrl: string;
savedWizardVis: WizardVisSavedObject;
visualizationState: VisualizationState;
styleState: StyleState;
hasUnappliedChanges: boolean;
saveDisabledReason?: string;
dispatch: AppDispatch;
}

export const getTopNavConfig = (
Expand All @@ -54,7 +57,8 @@ export const getTopNavConfig = (
savedWizardVis,
visualizationState,
styleState,
hasUnappliedChanges,
saveDisabledReason,
dispatch,
}: TopNavConfigParams,
{ history, toastNotifications, i18n: { Context: I18nContext } }: WizardServices
) => {
Expand All @@ -71,11 +75,11 @@ export const getTopNavConfig = (
defaultMessage: 'save',
}),
testId: 'wizardSaveButton',
disableButton: hasUnappliedChanges,
disableButton: !!saveDisabledReason,
tooltip() {
if (hasUnappliedChanges) {
if (saveDisabledReason) {
return i18n.translate('wizard.topNavMenu.saveVisualizationDisabledButtonTooltip', {
defaultMessage: 'Apply aggregation configuration changes before saving', // TODO: Update text to match agg save flow
defaultMessage: saveDisabledReason,
});
}
},
Expand Down Expand Up @@ -127,6 +131,7 @@ export const getTopNavConfig = (
pathname: `${EDIT_PATH}/${id}`,
});
}
dispatch(setEditorState({ state: 'clean' }));
} else {
// reset title if save not successful
savedWizardVis.title = currentTitle;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,27 @@
import { createSlice, PayloadAction } from '@reduxjs/toolkit';
import { WizardServices } from '../../../types';

/*
* Initial state: default state when opening wizard plugin
* Clean state: when viz finished loading and ready to be edited
* Dirty state: when there are changes applied to the viz after it finished loading
*/
type EditorState = 'loading' | 'clean' | 'dirty';

export interface MetadataState {
editorState: {
editor: {
validity: {
// Validity for each section in the editor
[key: string]: boolean;
};
state: EditorState;
};
}

const initialState: MetadataState = {
editorState: {
editor: {
validity: {},
state: 'loading',
},
};

Expand All @@ -36,7 +45,10 @@ export const slice = createSlice({
reducers: {
setValidity: (state, action: PayloadAction<{ key: string; valid: boolean }>) => {
const { key, valid } = action.payload;
state.editorState.validity[key] = valid;
state.editor.validity[key] = valid;
},
setEditorState: (state, action: PayloadAction<{ state: EditorState }>) => {
state.editor.state = action.payload.state;
},
setState: (_state, action: PayloadAction<MetadataState>) => {
return action.payload;
Expand All @@ -45,4 +57,4 @@ export const slice = createSlice({
});

export const { reducer } = slice;
export const { setValidity, setState } = slice.actions;
export const { setValidity, setEditorState, setState } = slice.actions;
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { reducer as visualizationReducer } from './visualization_slice';
import { reducer as metadataReducer } from './metadata_slice';
import { WizardServices } from '../../..';
import { getPreloadedState } from './preload';
import { setEditorState } from './metadata_slice';

const rootReducer = combineReducers({
style: styleReducer,
Expand All @@ -25,7 +26,48 @@ export const configurePreloadedStore = (preloadedState: PreloadedState<RootState

export const getPreloadedStore = async (services: WizardServices) => {
const preloadedState = await getPreloadedState(services);
return configurePreloadedStore(preloadedState);
const store = configurePreloadedStore(preloadedState);

const { metadata: metadataState, style: styleState, visualization: vizState } = store.getState();
let previousStore = {
viz: vizState,
style: styleState,
};
let previousMetadata = metadataState;

// Listen to changes
const handleChange = () => {
const {
metadata: currentMetadataState,
style: currentStyleState,
visualization: currentVizState,
} = store.getState();
const currentStore = {
viz: currentVizState,
style: currentStyleState,
};
const currentMetadata = currentMetadataState;

// Need to make sure the editorStates are in the clean states(not the initial states) to indicate the viz finished loading
// Because when loading a saved viz from saved object, the previousStore will differ from
// the currentStore even tho there is no changes applied ( aggParams will
// first be empty, and it then will change to not empty once the viz finished loading)
if (
previousMetadata.editor.state === 'clean' &&
currentMetadata.editor.state === 'clean' &&
JSON.stringify(currentStore) !== JSON.stringify(previousStore)
) {
store.dispatch(setEditorState({ state: 'dirty' }));
}

previousStore = currentStore;
previousMetadata = currentMetadata;
};

// the store subscriber will automatically detect changes and call handleChange function
store.subscribe(handleChange);

return store;
};

// Infer the `RootState` and `AppDispatch` types from the store itself
Expand Down
32 changes: 32 additions & 0 deletions src/plugins/wizard/public/application/utils/use/use_can_save.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { useTypedSelector } from '../state_management';

export const useCanSave = () => {
const isEmpty = useTypedSelector(
(state) => state.visualization.activeVisualization?.aggConfigParams?.length === 0
);
const hasNoChange = useTypedSelector((state) => state.metadata.editor.state !== 'dirty');
const hasDraftAgg = useTypedSelector(
(state) => !!state.visualization.activeVisualization?.draftAgg
);
const errorMsg = getErrorMsg(isEmpty, hasNoChange, hasDraftAgg);

return errorMsg;
};

// TODO: Need to finalize the error messages
const getErrorMsg = (isEmpty, hasNoChange, hasDraftAgg) => {
if (isEmpty) {
return 'The canvas is empty. Add some aggregations before saving.';
} else if (hasNoChange) {
return 'Add some changes before saving.';
} else if (hasDraftAgg) {
return 'Has unapplied aggregations changes, update them before saving.';
} else {
return undefined;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { getCreateBreadcrumbs, getEditBreadcrumbs } from '../breadcrumbs';
import { getSavedWizardVis } from '../get_saved_wizard_vis';
import { useTypedDispatch, setStyleState, setVisualizationState } from '../state_management';
import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
import { setEditorState } from '../state_management/metadata_slice';

export const useSavedWizardVis = (visualizationIdFromUrl: string | undefined) => {
const { services } = useOpenSearchDashboards<WizardServices>();
Expand Down Expand Up @@ -51,6 +52,7 @@ export const useSavedWizardVis = (visualizationIdFromUrl: string | undefined) =>
}

setSavedVisState(savedWizardVis);
dispatch(setEditorState({ state: 'clean' }));
} catch (error) {
const managementRedirectTarget = {
[PLUGIN_ID]: {
Expand Down
16 changes: 8 additions & 8 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9035,7 +9035,7 @@ glob@7.1.3:
once "^1.3.0"
path-is-absolute "^1.0.0"

glob@7.1.7, glob@~7.1.1, glob@~7.1.6:
glob@7.1.7, glob@~7.1.6:
version "7.1.7"
resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.7.tgz#3b193e9233f01d42d0b3f78294bbeeb418f94a90"
integrity sha512-OvD9ENzPLbegENnYP5UUfJIirTg4+XwMWGaQfQTY0JenxNvvIKP3U3/tAQSPIu/lHxXYSZmpXlUHeqAIdKzBLQ==
Expand Down Expand Up @@ -12052,7 +12052,7 @@ lodash.uniq@^4.5.0:
resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773"
integrity sha1-0CJTc662Uq3BvILklFM5qEJ1R3M=

lodash@4.17.21, lodash@^4.0.0, lodash@^4.0.1, lodash@^4.10.0, lodash@^4.17.10, lodash@^4.17.11, lodash@^4.17.13, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.7.0, lodash@~4.17.10, lodash@~4.17.15, lodash@~4.17.19, lodash@~4.17.21:
lodash@4.17.21, lodash@^4.0.1, lodash@^4.10.0, lodash@^4.17.10, lodash@^4.17.11, lodash@^4.17.13, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.7.0, lodash@~4.17.15, lodash@~4.17.19, lodash@~4.17.21:
version "4.17.21"
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
Expand Down Expand Up @@ -12550,7 +12550,7 @@ minimatch@3.0.4:
dependencies:
brace-expansion "^1.1.7"

minimatch@~3.0.2, minimatch@~3.0.4:
minimatch@~3.0.4:
version "3.0.8"
resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.8.tgz#5e6a59bd11e2ab0de1cfb843eb2d82e546c321c1"
integrity sha512-6FsRAQsxQ61mw+qP1ZzbL9Bc78x2p5OqNgNpnoAFLTrX8n5Kxph0CsnhmKKNXTWjXqU5L0pGPR7hYk+XWZr60Q==
Expand Down Expand Up @@ -12827,7 +12827,7 @@ mute-stream@0.0.8:
resolved "https://registry.yarnpkg.com/mute-stream/-/mute-stream-0.0.8.tgz#1630c42b2251ff81e2a283de96a5497ea92e5e0d"
integrity sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==

nan@^2.12.1, nan@^2.13.2, nan@^2.14.1, nan@^2.14.2:
nan@^2.12.1, nan@^2.14.1, nan@^2.14.2:
version "2.15.0"
resolved "https://registry.yarnpkg.com/nan/-/nan-2.15.0.tgz#3f34a473ff18e15c1b5626b62903b5ad6e665fee"
integrity sha512-8ZtvEnA2c5aYCZYd1cvgdnU6cqwixRoYg70xPLWUws5ORTa/lnw+u4amixRS/Ac5U5mQVgp9pnlSUnbNWFaWZQ==
Expand Down Expand Up @@ -12995,7 +12995,7 @@ node-gyp-build@^4.2.3:
resolved "https://registry.yarnpkg.com/node-gyp-build/-/node-gyp-build-4.3.0.tgz#9f256b03e5826150be39c764bf51e993946d71a3"
integrity sha512-iWjXZvmboq0ja1pUGULQBexmxq8CV4xBhX7VDOTbL7ZR4FOowwY/VOtRxBN/yKxmdGoIp4j5ysNT4u3S2pDQ3Q==

node-gyp@^7.0.0, node-gyp@^7.1.0:
node-gyp@^7.0.0:
version "7.1.2"
resolved "https://registry.yarnpkg.com/node-gyp/-/node-gyp-7.1.2.tgz#21a810aebb187120251c3bcec979af1587b188ae"
integrity sha512-CbpcIo7C3eMu3dL1c3d0xw449fHIGALIJsRP4DDPHpyiW8vcriNY7ubh9TE4zEKfSxscY7PjeFnshE7h75ynjQ==
Expand Down Expand Up @@ -13174,7 +13174,7 @@ npm-run-path@^4.0.0, npm-run-path@^4.0.1:
dependencies:
path-key "^3.0.0"

npmlog@^4.0.0, npmlog@^4.1.2:
npmlog@^4.1.2:
version "4.1.2"
resolved "https://registry.yarnpkg.com/npmlog/-/npmlog-4.1.2.tgz#08a7f2a8bf734604779a9efa4ad5cc717abb954b"
integrity sha512-2uUqazuKlTaSI/dC8AzicUck7+IrEaOnN/e0jd3Xtt1KcGpwx30v50mL7oPyr/h9bL3E4aZccVwpwP+5W9Vjkg==
Expand Down Expand Up @@ -15212,7 +15212,7 @@ replace-ext@^1.0.0:
resolved "https://registry.yarnpkg.com/replace-ext/-/replace-ext-1.0.1.tgz#2d6d996d04a15855d967443631dd5f77825b016a"
integrity sha512-yD5BHCe7quCgBph4rMQ+0KkIRKwWCrHDOX1p1Gp6HwjPM5kVoCdKGNhN7ydqqsX6lJEnQDKZ/tFMiEdQ1dvPEw==

request@^2.88.0, request@^2.88.2:
request@^2.88.2:
version "2.88.2"
resolved "https://registry.yarnpkg.com/request/-/request-2.88.2.tgz#d73c918731cb5a87da047e207234146f664d12b3"
integrity sha512-MsvtOrfG9ZcrOwAW+Qi+F6HbD0CWXEh9ou77uOb7FM2WPhwT7smM833PzanhJLsgXjN89Ir6V2PczXNnMpwKhw==
Expand Down Expand Up @@ -18892,7 +18892,7 @@ yargs-unparser@1.6.0:
lodash "^4.17.15"
yargs "^13.3.0"

yargs@13.3.2, yargs@^13.3.0, yargs@^13.3.2:
yargs@13.3.2, yargs@^13.3.0:
version "13.3.2"
resolved "https://registry.yarnpkg.com/yargs/-/yargs-13.3.2.tgz#ad7ffefec1aa59565ac915f82dccb38a9c31a2dd"
integrity sha512-AX3Zw5iPruN5ie6xGRIDgqkT+ZhnRlZMLMHAs8tg7nRruy2Nb+i5o9bwghAogtM08q1dpr2LVoS8KSTMYpWXUw==
Expand Down

0 comments on commit 65005be

Please sign in to comment.