From 632d77c6f232047f251ad8a1f1c6c38c25f305ae Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Wed, 20 Jul 2022 17:27:27 -0700 Subject: [PATCH] [D&D] Misc fixes (#1924) * fix: minor fixes Signed-off-by: Ashwin Pc * chore: nit syntax fixes Signed-off-by: Ashwin Pc * chore: simplify useSavedWizardVis Signed-off-by: Ashwin Pc --- .../wizard/type_selection/type_selection.tsx | 2 +- .../public/application/components/top_nav.tsx | 10 ++--- .../public/application/utils/use/index.ts | 1 + .../utils/use/use_index_pattern.tsx | 25 ++++++------ .../utils/use/use_saved_wizard_vis.ts | 7 ++-- src/plugins/wizard/public/plugin.test.ts | 3 +- src/plugins/wizard/public/plugin.ts | 5 +-- .../services/type_service/utils/index.ts | 6 --- .../type_service/utils/merge_array.test.ts | 24 ------------ .../type_service/utils/merge_arrays.ts | 39 ------------------- .../metric/to_expression.test.ts | 6 +++ .../visualizations/metric/to_expression.ts | 2 +- src/plugins/wizard/server/routes/index.ts | 27 ++++++------- 13 files changed, 46 insertions(+), 111 deletions(-) delete mode 100644 src/plugins/wizard/public/services/type_service/utils/index.ts delete mode 100644 src/plugins/wizard/public/services/type_service/utils/merge_array.test.ts delete mode 100644 src/plugins/wizard/public/services/type_service/utils/merge_arrays.ts create mode 100644 src/plugins/wizard/public/visualizations/metric/to_expression.test.ts diff --git a/src/plugins/visualizations/public/wizard/type_selection/type_selection.tsx b/src/plugins/visualizations/public/wizard/type_selection/type_selection.tsx index 797bc2e8cf93..27ea53f0f16f 100644 --- a/src/plugins/visualizations/public/wizard/type_selection/type_selection.tsx +++ b/src/plugins/visualizations/public/wizard/type_selection/type_selection.tsx @@ -222,7 +222,7 @@ class TypeSelection extends React.Component { let stage = {}; let highlightMsg; - if (!isVisTypeAlias(visType.type) && visType.type.stage === 'experimental') { + if (visType.type.stage === 'experimental') { stage = { betaBadgeLabel: i18n.translate('visualizations.newVisWizard.experimentalTitle', { defaultMessage: 'Experimental', diff --git a/src/plugins/wizard/public/application/components/top_nav.tsx b/src/plugins/wizard/public/application/components/top_nav.tsx index b1427e01523c..7a4bfa1010f4 100644 --- a/src/plugins/wizard/public/application/components/top_nav.tsx +++ b/src/plugins/wizard/public/application/components/top_nav.tsx @@ -11,9 +11,8 @@ import { getTopNavConfig } from '../utils/get_top_nav_config'; import { WizardServices } from '../../types'; import './top_nav.scss'; -import { useIndexPattern } from '../utils/use'; +import { useIndexPattern, useSavedWizardVis } from '../utils/use'; import { useTypedSelector } from '../utils/state_management'; -import { useSavedWizardVis } from '../utils/use/use_saved_wizard_vis'; export const TopNav = () => { // id will only be set for the edit route @@ -30,12 +29,11 @@ export const TopNav = () => { (state) => !!state.visualization.activeVisualization?.draftAgg ); - const savedWizardVis = useSavedWizardVis(services, visualizationIdFromUrl); + const savedWizardVis = useSavedWizardVis(visualizationIdFromUrl); const config = useMemo(() => { - if (savedWizardVis === undefined) { - return; - } + if (savedWizardVis === undefined) return; + const { visualization: visualizationState, style: styleState } = rootState; return getTopNavConfig( diff --git a/src/plugins/wizard/public/application/utils/use/index.ts b/src/plugins/wizard/public/application/utils/use/index.ts index 542ae073cde5..c9203242e63c 100644 --- a/src/plugins/wizard/public/application/utils/use/index.ts +++ b/src/plugins/wizard/public/application/utils/use/index.ts @@ -5,3 +5,4 @@ export { useVisualizationType } from './use_visualization_type'; export { useIndexPattern, useIndexPatterns } from './use_index_pattern'; +export { useSavedWizardVis } from './use_saved_wizard_vis'; diff --git a/src/plugins/wizard/public/application/utils/use/use_index_pattern.tsx b/src/plugins/wizard/public/application/utils/use/use_index_pattern.tsx index d5f255fcb10c..6ce448b9f055 100644 --- a/src/plugins/wizard/public/application/utils/use/use_index_pattern.tsx +++ b/src/plugins/wizard/public/application/utils/use/use_index_pattern.tsx @@ -2,7 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import { useCallback, useEffect, useState } from 'react'; +import { useEffect, useState } from 'react'; import { IndexPattern } from '../../../../../data/public'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { WizardServices } from '../../../types'; @@ -17,14 +17,14 @@ export const useIndexPattern = (): IndexPattern | undefined => { }, } = useOpenSearchDashboards(); - const handleIndexUpdate = useCallback(async () => { - const currentIndex = await indexPatterns.get(indexId); - setIndexPattern(currentIndex); - }, [indexId, indexPatterns]); - useEffect(() => { + const handleIndexUpdate = async () => { + const currentIndex = await indexPatterns.get(indexId); + setIndexPattern(currentIndex); + }; + handleIndexUpdate(); - }, [handleIndexUpdate]); + }, [indexId, indexPatterns]); return indexPattern; }; @@ -38,7 +38,7 @@ export const useIndexPatterns = () => { services: { data }, } = useOpenSearchDashboards(); - let foundSelected; + let foundSelected: IndexPattern; if (!loading && !error) { foundSelected = indexPatterns.filter((p) => p.id === indexId)[0]; if (foundSelected === undefined) { @@ -51,19 +51,18 @@ export const useIndexPatterns = () => { useEffect(() => { const handleUpdate = async () => { try { - const ids = await data.indexPatterns.getIds(indexId); + const ids = await data.indexPatterns.getIds(true); const patterns = await Promise.all(ids.map((id) => data.indexPatterns.get(id))); setIndexPatterns(patterns); } catch (e) { - setError(e); + setError(e as Error); } finally { setLoading(false); } }; + handleUpdate(); - // we want to run this hook exactly once, which you do by an empty dep array - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [data.indexPatterns]); return { indexPatterns, 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 30b72199f10d..108beaefdb14 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 @@ -16,11 +16,10 @@ import { MetricOptionsDefaults } from '../../../visualizations/metric/metric_viz 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'; -export const useSavedWizardVis = ( - services: WizardServices, - visualizationIdFromUrl: string | undefined -) => { +export const useSavedWizardVis = (visualizationIdFromUrl: string | undefined) => { + const { services } = useOpenSearchDashboards(); const [savedVisState, setSavedVisState] = useState(undefined); const dispatch = useTypedDispatch(); diff --git a/src/plugins/wizard/public/plugin.test.ts b/src/plugins/wizard/public/plugin.test.ts index 74936288a76f..6dafa46c86ff 100644 --- a/src/plugins/wizard/public/plugin.test.ts +++ b/src/plugins/wizard/public/plugin.test.ts @@ -33,11 +33,12 @@ describe('WizardPlugin', () => { const setup = plugin.setup(coreSetup, setupDeps); expect(setup).toHaveProperty('createVisualizationType'); expect(setupDeps.visualizations.registerAlias).toHaveBeenCalledWith( - // TODO: Update this once the properties are final expect.objectContaining({ name: PLUGIN_ID, title: PLUGIN_NAME, aliasPath: '#/', + aliasApp: PLUGIN_ID, + stage: 'experimental', }) ); }); diff --git a/src/plugins/wizard/public/plugin.ts b/src/plugins/wizard/public/plugin.ts index 8d9ebb406a0f..7a3fe5aa204e 100644 --- a/src/plugins/wizard/public/plugin.ts +++ b/src/plugins/wizard/public/plugin.ts @@ -94,11 +94,10 @@ export class WizardPlugin name: PLUGIN_ID, title: PLUGIN_NAME, description: i18n.translate('wizard.visPicker.description', { - defaultMessage: 'TODO...', + defaultMessage: 'Create visualizations using the new Drag & Drop experience', }), - // TODO: Replace with actual icon once available icon: wizardIcon, - stage: 'beta', + stage: 'experimental', aliasApp: PLUGIN_ID, aliasPath: '#/', }); diff --git a/src/plugins/wizard/public/services/type_service/utils/index.ts b/src/plugins/wizard/public/services/type_service/utils/index.ts deleted file mode 100644 index 4d0644b2d93e..000000000000 --- a/src/plugins/wizard/public/services/type_service/utils/index.ts +++ /dev/null @@ -1,6 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -export * from './merge_arrays'; diff --git a/src/plugins/wizard/public/services/type_service/utils/merge_array.test.ts b/src/plugins/wizard/public/services/type_service/utils/merge_array.test.ts deleted file mode 100644 index 01a95f240d5f..000000000000 --- a/src/plugins/wizard/public/services/type_service/utils/merge_array.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { mergeArrays } from './merge_arrays'; - -describe('mergeArrays', () => { - const arrayA = ['a', 'b', 'c'].map((x) => ({ id: x, value: `${x} in arrayA` })); - const arrayB = ['a', 'c', 'd'].map((x) => ({ id: x, value: `${x} in arrayB` })); - test('should merge two object arrays based on id in order without duplicates', () => { - const mergedArrays = mergeArrays(arrayA, arrayB, 'id'); - expect(mergedArrays.map((x) => x.id)).toEqual(['a', 'b', 'c', 'd']); - expect(mergedArrays[0].value).toEqual('a in arrayB'); - }); - - test('should throw an error if key is not a string or number', () => { - const arr = [{ id: {} }]; - - expect(() => mergeArrays(arrayA, arr, 'id')).toThrowErrorMatchingInlineSnapshot( - `"Can only merge arrays with keys of type number or string"` - ); - }); -}); diff --git a/src/plugins/wizard/public/services/type_service/utils/merge_arrays.ts b/src/plugins/wizard/public/services/type_service/utils/merge_arrays.ts deleted file mode 100644 index 3b5f9b0adfb8..000000000000 --- a/src/plugins/wizard/public/services/type_service/utils/merge_arrays.ts +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { uniq } from 'lodash'; - -export function mergeArrays(a: T[], b: T[], key: K): T[] { - const dict: Record = {}; - - const rawCombinedArrays = [...a, ...b]; - - // Create an ordered list of unique the array keys. Removing duplicates while keeping the order - const combinedOrder = uniq(rawCombinedArrays.map((entry) => entry[key])); - - // Create a map of all unique ID's and their values. - // If there is more than one entry with the same ID, the last entry is kept - rawCombinedArrays.forEach((entry) => { - if (!entry.hasOwnProperty(key)) { - throw new Error('Key not present in an object in one the arrays to merge'); - } - - const id = entry[key]; - - if (typeof id !== 'string' && typeof id !== 'number') { - throw new Error('Can only merge arrays with keys of type number or string'); - } - - dict[id] = entry; - }); - - // Return the combined array in order with unique keys keeping only the last entry for each unique id - return combinedOrder.map((entryId) => { - if (typeof entryId !== 'string' && typeof entryId !== 'number') { - throw new Error('Can only merge arrays with keys of type number or string'); - } - return dict[entryId]; - }); -} diff --git a/src/plugins/wizard/public/visualizations/metric/to_expression.test.ts b/src/plugins/wizard/public/visualizations/metric/to_expression.test.ts new file mode 100644 index 000000000000..9fd364ad256c --- /dev/null +++ b/src/plugins/wizard/public/visualizations/metric/to_expression.test.ts @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +// TODO: Cleanup the TODOs in './to_expression.ts' before writing tests for this function diff --git a/src/plugins/wizard/public/visualizations/metric/to_expression.ts b/src/plugins/wizard/public/visualizations/metric/to_expression.ts index 7b8d7b802e50..ce930d9b8e40 100644 --- a/src/plugins/wizard/public/visualizations/metric/to_expression.ts +++ b/src/plugins/wizard/public/visualizations/metric/to_expression.ts @@ -87,7 +87,7 @@ const getVisSchemas = (aggConfigs: AggConfigs): any => { return schemas; }; -interface MetricRootState extends RootState { +export interface MetricRootState extends RootState { style: MetricOptionsDefaults; } diff --git a/src/plugins/wizard/server/routes/index.ts b/src/plugins/wizard/server/routes/index.ts index f6268695e838..90b698ccb992 100644 --- a/src/plugins/wizard/server/routes/index.ts +++ b/src/plugins/wizard/server/routes/index.ts @@ -6,17 +6,18 @@ import { IRouter } from '../../../../core/server'; export function defineRoutes(router: IRouter) { - router.get( - { - path: '/api/wizard/example', - validate: false, - }, - async (context, request, response) => { - return response.ok({ - body: { - time: new Date().toISOString(), - }, - }); - } - ); + // Add server siude routes if needed like the example below + // router.get( + // { + // path: '/api/wizard/example', + // validate: false, + // }, + // async (context, request, response) => { + // return response.ok({ + // body: { + // time: new Date().toISOString(), + // }, + // }); + // } + // ); }