Skip to content

Commit

Permalink
[VisBuilder] 2 way communication using UI actions and bug fixes (#3732)
Browse files Browse the repository at this point in the history
* adds uiActions to visBuilder
* prevents multiple errors on load
* fixes visbuilder type errors
* fixes save
* updates changelog

---------

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
  • Loading branch information
ashwin-pc and joshuarrrr authored Apr 17, 2023
1 parent 8f890e9 commit 1edb195
Show file tree
Hide file tree
Showing 18 changed files with 359 additions and 138 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Use mirrors to download Node.js binaries to escape sporadic 404 errors ([#3619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3619))
- [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544))
- [Data] Add geo shape filter field ([#3605](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3605))
- [VisBuilder] Add UI actions handler ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [Dashboard] Indicate that IE is no longer supported ([#3641](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3641))
- [UI] Add support for comma delimiters in the global filter bar ([#3686](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3686))
- [Multiple DataSource] Allow create and distinguish index pattern with same name but from different datasources ([#3571](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3571))
Expand Down Expand Up @@ -118,6 +119,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Clean up and rebuild `@osd/pm` ([#3570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3570))
- [Vega] Add Filter custom label for opensearchDashboardsAddFilter ([#3640](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3640))
- [Timeline] Fix y-axis label color in dark mode ([#3698](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3698))
- [VisBuilder] Fix multiple warnings thrown on page load ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [VisBuilder] Fix Firefox legend selection issue ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [VisBuilder] Fix type errors ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [Table Visualization] Fix table rendering empty unused space ([#3797](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3797))
- [Table Visualization] Fix data table not adjusting height on the initial load ([#3816](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3816))
- Cleanup unused url ([#3847](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3847))
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/vis_builder/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"expressions",
"navigation",
"savedObjects",
"visualizations"
"visualizations",
"uiActions"
],
"requiredBundles": [
"charts",
Expand All @@ -20,4 +21,4 @@
"visDefaultEditor",
"visTypeVislib"
]
}
}
3 changes: 2 additions & 1 deletion src/plugins/vis_builder/public/application/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import { DragDropProvider } from './utils/drag_drop/drag_drop_context';
import { LeftNav } from './components/left_nav';
import { TopNav } from './components/top_nav';
import { Workspace } from './components/workspace';
import './app.scss';
import { RightNav } from './components/right_nav';
import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public';
import { VisBuilderServices } from '../types';
import { syncQueryStateWithUrl } from '../../../data/public';

import './app.scss';

export const VisBuilderApp = () => {
const {
services: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
} from '../utils/state_management';
import { getPersistedAggParams } from '../utils/get_persisted_agg_params';

export const RightNav = () => {
export const RightNavUI = () => {
const { ui, name: activeVisName } = useVisualizationType();
const [confirmAggs, setConfirmAggs] = useState<ActiveVisPayload | undefined>();
const {
Expand Down Expand Up @@ -121,3 +121,7 @@ const OptionItem = ({ icon, title }: { icon: IconType; title: string }) => (
<span>{title}</span>
</>
);

// The app uses EuiResizableContainer that triggers a rerender for ever mouseover action.
// To prevent this child component from unnecessarily rerendering in that instance, it needs to be memoized
export const RightNav = React.memo(RightNavUI);
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { i18n } from '@osd/i18n';
import { EuiEmptyPrompt, EuiFlexGroup, EuiFlexItem, EuiIcon, EuiPanel } from '@elastic/eui';
import React, { FC, useState, useMemo, useEffect, useLayoutEffect } from 'react';
import React, { useState, useMemo, useEffect, useLayoutEffect } from 'react';
import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
import { IExpressionLoaderParams } from '../../../../expressions/public';
import { VisBuilderServices } from '../../types';
Expand All @@ -19,17 +19,19 @@ import fields_bg from '../../assets/fields_bg.svg';

import './workspace.scss';
import { ExperimentalInfo } from './experimental_info';
import { handleVisEvent } from '../utils/handle_vis_event';

export const Workspace: FC = ({ children }) => {
export const WorkspaceUI = () => {
const {
services: {
expressions: { ReactExpressionRenderer },
notifications: { toasts },
data,
uiActions,
},
} = useOpenSearchDashboards<VisBuilderServices>();
const { toExpression, ui } = useVisualizationType();
const { aggConfigs } = useAggs();
const { aggConfigs, indexPattern } = useAggs();
const [expression, setExpression] = useState<string>();
const [searchContext, setSearchContext] = useState<IExpressionLoaderParams['searchContext']>({
query: data.query.queryString.getQuery(),
Expand All @@ -44,15 +46,17 @@ export const Workspace: FC = ({ children }) => {
async function loadExpression() {
const schemas = ui.containerConfig.data.schemas;

const noAggs = aggConfigs?.aggs?.length === 0;
const noAggs = (aggConfigs?.aggs?.length ?? 0) === 0;
const schemaValidation = validateSchemaState(schemas, rootState.visualization);
const aggValidation = validateAggregations(aggConfigs?.aggs || []);

if (noAggs || !aggValidation.valid || !schemaValidation.valid) {
if (!aggValidation.valid || !schemaValidation.valid) {
setExpression(undefined);
if (noAggs) return; // don't show error when there are no active aggregations

const err = schemaValidation.errorMsg || aggValidation.errorMsg;

if (err) toasts.addWarning(err);
setExpression(undefined);

return;
}
Expand Down Expand Up @@ -91,6 +95,7 @@ export const Workspace: FC = ({ children }) => {
expression={expression}
searchContext={searchContext}
uiState={uiState}
onEvent={(event) => handleVisEvent(event, uiActions, indexPattern?.timeFieldName)}
/>
) : (
<EuiFlexItem className="vbWorkspace__empty" data-test-subj="emptyWorkspace">
Expand Down Expand Up @@ -127,3 +132,7 @@ export const Workspace: FC = ({ children }) => {
</section>
);
};

// The app uses EuiResizableContainer that triggers a rerender for ever mouseover action.
// To prevent this child component from unnecessarily rerendering in that instance, it needs to be memoized
export const Workspace = React.memo(WorkspaceUI);

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ import {
showSaveModal,
} from '../../../../saved_objects/public';
import { VisBuilderServices } from '../..';
import { VisBuilderVisSavedObject } from '../../types';
import { VisBuilderSavedObject } from '../../types';
import { AppDispatch } from './state_management';
import { EDIT_PATH, VISBUILDER_SAVED_OBJECT } from '../../../common';
import { setEditorState } from './state_management/metadata_slice';
export interface TopNavConfigParams {
visualizationIdFromUrl: string;
savedVisBuilderVis: VisBuilderVisSavedObject;
savedVisBuilderVis: VisBuilderSavedObject;
saveDisabledReason?: string;
dispatch: AppDispatch;
originatingApp?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { ExpressionRendererEvent } from '../../../../expressions/public';
import { VIS_EVENT_TO_TRIGGER } from '../../../../visualizations/public';
import { handleVisEvent } from './handle_vis_event';
import { uiActionsPluginMock } from '../../../../ui_actions/public/mocks';
import { Action, ActionType, createAction } from '../../../../ui_actions/public';

const executeFn = jest.fn();

function createTestAction<C extends object>(
type: string,
checkCompatibility: (context: C) => boolean,
autoExecutable = true
): Action<object> {
return createAction({
type: type as ActionType,
id: type,
isCompatible: (context: C) => Promise.resolve(checkCompatibility(context)),
execute: (context) => {
return executeFn(context);
},
shouldAutoExecute: () => Promise.resolve(autoExecutable),
});
}

let uiActions: ReturnType<typeof uiActionsPluginMock.createPlugin>;

describe('handleVisEvent', () => {
beforeEach(() => {
uiActions = uiActionsPluginMock.createPlugin();

executeFn.mockClear();
jest.useFakeTimers();
});

test('should trigger the correct event', async () => {
const event: ExpressionRendererEvent = {
name: 'filter',
data: {},
};
const action = createTestAction('test1', () => true);
const timeFieldName = 'test-timefeild-name';
uiActions.setup.addTriggerAction(VIS_EVENT_TO_TRIGGER.filter, action);

await handleVisEvent(event, uiActions.doStart(), timeFieldName);

jest.runAllTimers();

expect(executeFn).toBeCalledTimes(1);
expect(executeFn).toBeCalledWith(
expect.objectContaining({
data: { timeFieldName },
})
);
});

test('should trigger the default trigger when not found', async () => {
const event: ExpressionRendererEvent = {
name: 'test',
data: {},
};
const action = createTestAction('test2', () => true);
const timeFieldName = 'test-timefeild-name';
uiActions.setup.addTriggerAction(VIS_EVENT_TO_TRIGGER.filter, action);

await handleVisEvent(event, uiActions.doStart(), timeFieldName);

jest.runAllTimers();

expect(executeFn).toBeCalledTimes(1);
expect(executeFn).toBeCalledWith(
expect.objectContaining({
data: { timeFieldName },
})
);
});

test('should have the correct context for `applyfilter`', async () => {
const event: ExpressionRendererEvent = {
name: 'applyFilter',
data: {},
};
const action = createTestAction('test3', () => true);
const timeFieldName = 'test-timefeild-name';
uiActions.setup.addTriggerAction(VIS_EVENT_TO_TRIGGER.applyFilter, action);

await handleVisEvent(event, uiActions.doStart(), timeFieldName);

jest.runAllTimers();

expect(executeFn).toBeCalledTimes(1);
expect(executeFn).toBeCalledWith(
expect.objectContaining({
timeFieldName,
})
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { ExpressionRendererEvent } from '../../../../expressions/public';
import { VIS_EVENT_TO_TRIGGER } from '../../../../visualizations/public';
import { UiActionsStart } from '../../../../ui_actions/public';

export const handleVisEvent = async (
event: ExpressionRendererEvent,
uiActions: UiActionsStart,
timeFieldName?: string
) => {
const triggerId = VIS_EVENT_TO_TRIGGER[event.name] ?? VIS_EVENT_TO_TRIGGER.filter;
const isApplyFilter = triggerId === VIS_EVENT_TO_TRIGGER.applyFilter;
const dataContext = {
timeFieldName,
...event.data,
};
const context = isApplyFilter ? dataContext : { data: dataContext };

await uiActions.getTrigger(triggerId).exec(context);
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,10 @@ import {
import { EDIT_PATH, PLUGIN_ID } from '../../../../common';
import { VisBuilderServices } from '../../../types';
import { getCreateBreadcrumbs, getEditBreadcrumbs } from '../breadcrumbs';
import { getSavedVisBuilderVis } from '../get_saved_vis_builder_vis';
import {
useTypedDispatch,
setStyleState,
setVisualizationState,
VisualizationState,
} from '../state_management';
import { useTypedDispatch, setStyleState, setVisualizationState } from '../state_management';
import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
import { setEditorState } from '../state_management/metadata_slice';
import { validateVisBuilderState } from '../validations/vis_builder_state_validation';
import { getStateFromSavedObject } from '../../../saved_visualizations/transforms';

// 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 +33,7 @@ export const useSavedVisBuilderVis = (visualizationIdFromUrl: string | undefined
history,
http: { basePath },
toastNotifications,
savedVisBuilderLoader,
} = services;
const toastNotification = (message: string) => {
toastNotifications.addDanger({
Expand All @@ -51,42 +46,22 @@ export const useSavedVisBuilderVis = (visualizationIdFromUrl: string | undefined

const loadSavedVisBuilderVis = async () => {
try {
const savedVisBuilderVis = await getSavedVisBuilderVis(services, visualizationIdFromUrl);
const savedVisBuilderVis = await getSavedVisBuilderVis(
savedVisBuilderLoader,
visualizationIdFromUrl
);

if (savedVisBuilderVis.id) {
chrome.setBreadcrumbs(getEditBreadcrumbs(savedVisBuilderVis.title, navigateToApp));
chrome.docTitle.change(savedVisBuilderVis.title);
const { title, state } = getStateFromSavedObject(savedVisBuilderVis);
chrome.setBreadcrumbs(getEditBreadcrumbs(title, navigateToApp));
chrome.docTitle.change(title);

dispatch(setStyleState(state.style));
dispatch(setVisualizationState(state.visualization));
} else {
chrome.setBreadcrumbs(getCreateBreadcrumbs(navigateToApp));
}

if (
savedVisBuilderVis.styleState !== '{}' &&
savedVisBuilderVis.visualizationState !== '{}'
) {
const styleState = JSON.parse(savedVisBuilderVis.styleState);
const vizStateWithoutIndex = JSON.parse(savedVisBuilderVis.visualizationState);
const visualizationState: VisualizationState = {
searchField: vizStateWithoutIndex.searchField,
activeVisualization: vizStateWithoutIndex.activeVisualization,
indexPattern: savedVisBuilderVis.searchSourceFields.index,
};

const validateResult = validateVisBuilderState({ styleState, visualizationState });
if (!validateResult.valid) {
throw new InvalidJSONProperty(
validateResult.errorMsg ||
i18n.translate('visBuilder.useSavedVisBuilderVis.genericJSONError', {
defaultMessage:
'Something went wrong while loading your saved object. The object may be corrupted or does not match the latest schema',
})
);
}

dispatch(setStyleState(styleState));
dispatch(setVisualizationState(visualizationState));
}

setSavedVisState(savedVisBuilderVis);
dispatch(setEditorState({ state: 'clean' }));
} catch (error) {
Expand Down Expand Up @@ -123,3 +98,12 @@ export const useSavedVisBuilderVis = (visualizationIdFromUrl: string | undefined

return savedVisState;
};

async function getSavedVisBuilderVis(
savedVisBuilderLoader: VisBuilderServices['savedVisBuilderLoader'],
visBuilderVisId?: string
) {
const savedVisBuilderVis = await savedVisBuilderLoader.get(visBuilderVisId);

return savedVisBuilderVis;
}
Loading

0 comments on commit 1edb195

Please sign in to comment.