From 618d7ce2d008eedd245a8f61422fd157c8672fcb Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Fri, 4 Mar 2022 08:13:16 -0600 Subject: [PATCH] [Lens] allow user to disable auto apply (#125158) --- .../lens/public/app_plugin/app.test.tsx | 12 + x-pack/plugins/lens/public/app_plugin/app.tsx | 3 + .../config_panel/dimension_container.tsx | 24 +- .../editor_frame/data_panel_wrapper.test.tsx | 87 ++++ .../editor_frame/data_panel_wrapper.tsx | 8 +- .../editor_frame/editor_frame.tsx | 2 +- .../editor_frame/frame_layout.scss | 1 - .../editor_frame/suggestion_helpers.ts | 11 +- .../editor_frame/suggestion_panel.scss | 4 + .../editor_frame/suggestion_panel.test.tsx | 73 ++- .../editor_frame/suggestion_panel.tsx | 135 ++++-- .../workspace_panel/chart_switch.test.tsx | 2 + .../workspace_panel/chart_switch.tsx | 2 +- .../workspace_panel/workspace_panel.test.tsx | 436 ++++++++++++++---- .../workspace_panel/workspace_panel.tsx | 57 ++- .../workspace_panel_wrapper.scss | 9 + .../workspace_panel_wrapper.test.tsx | 148 ++++++ .../workspace_panel_wrapper.tsx | 136 ++++-- .../indexpattern_datasource/datapanel.tsx | 2 +- .../indexpattern_datasource/indexpattern.tsx | 2 +- .../indexpattern_datasource/loader.test.ts | 13 +- .../public/indexpattern_datasource/loader.ts | 89 ++-- .../plugins/lens/public/settings_storage.tsx | 2 +- .../shared_components/axis_title_settings.tsx | 11 +- .../context_middleware/index.test.ts | 50 +- .../context_middleware/index.ts | 17 +- .../lens/public/state_management/index.ts | 3 + .../state_management/init_middleware/index.ts | 9 +- .../init_middleware/load_initial.ts | 12 +- .../state_management/lens_slice.test.ts | 60 ++- .../public/state_management/lens_slice.ts | 24 + .../public/state_management/selectors.test.ts | 49 ++ .../lens/public/state_management/selectors.ts | 10 + .../lens/public/state_management/types.ts | 3 + x-pack/plugins/lens/public/types.ts | 26 +- .../xy_config_panel/axis_settings_popover.tsx | 3 +- x-pack/plugins/lens/server/usage/schema.ts | 6 + .../schema/xpack_plugins.json | 12 + .../apps/lens/disable_auto_apply.ts | 99 ++++ x-pack/test/functional/apps/lens/index.ts | 1 + .../test/functional/page_objects/lens_page.ts | 28 ++ 41 files changed, 1430 insertions(+), 251 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.test.tsx create mode 100644 x-pack/plugins/lens/public/state_management/selectors.test.ts create mode 100644 x-pack/test/functional/apps/lens/disable_auto_apply.ts diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index b16afbfc56a4abe..c4e82aca9ad4543 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -635,6 +635,18 @@ describe('Lens App', () => { ); }); + it('applies all changes on-save', async () => { + const { lensStore } = await save({ + initialSavedObjectId: undefined, + newCopyOnSave: false, + newTitle: 'hello there', + preloadedState: { + applyChangesCounter: 0, + }, + }); + expect(lensStore.getState().lens.applyChangesCounter).toBe(1); + }); + it('adds to the recently accessed list on save', async () => { const { services } = await save({ initialSavedObjectId: undefined, diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 5ef9e05cf590b03..6312225af579b63 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -24,6 +24,7 @@ import { Document } from '../persistence/saved_object_store'; import { setState, + applyChanges, useLensSelector, useLensDispatch, LensAppState, @@ -276,6 +277,7 @@ export function App({ const runSave = useCallback( (saveProps: SaveProps, options: { saveToLibrary: boolean }) => { + dispatch(applyChanges()); return runSaveLensVisualization( { lastKnownDoc, @@ -316,6 +318,7 @@ export function App({ redirectTo, lensAppServices, dispatchSetState, + dispatch, setIsSaveModalVisible, ] ); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx index f7402e78ebd96f1..5bc6a69b2efaf1e 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx @@ -24,6 +24,26 @@ import { import { i18n } from '@kbn/i18n'; +/** + * The dimension container is set up to close when it detects a click outside it. + * Use this CSS class to exclude particular elements from this behavior. + */ +export const DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS = + 'lensDontCloseDimensionContainerOnClick'; + +function fromExcludedClickTarget(event: Event) { + for ( + let node: HTMLElement | null = event.target as HTMLElement; + node !== null; + node = node!.parentElement + ) { + if (node.classList!.contains(DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS)) { + return true; + } + } + return false; +} + export function DimensionContainer({ isOpen, groupLabel, @@ -77,8 +97,8 @@ export function DimensionContainer({ { - if (isFullscreen) { + onOutsideClick={(event) => { + if (isFullscreen || fromExcludedClickTarget(event)) { return; } closeFlyout(); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.test.tsx new file mode 100644 index 000000000000000..64656a2eedf63c0 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.test.tsx @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { DataPanelWrapper } from './data_panel_wrapper'; +import { Datasource, DatasourceDataPanelProps } from '../../types'; +import { DragDropIdentifier } from '../../drag_drop'; +import { UiActionsStart } from 'src/plugins/ui_actions/public'; +import { mockStoreDeps, mountWithProvider } from '../../mocks'; +import { disableAutoApply } from '../../state_management/lens_slice'; +import { selectTriggerApplyChanges } from '../../state_management'; + +describe('Data Panel Wrapper', () => { + describe('Datasource data panel properties', () => { + let datasourceDataPanelProps: DatasourceDataPanelProps; + let lensStore: Awaited>['lensStore']; + beforeEach(async () => { + const renderDataPanel = jest.fn(); + + const datasourceMap = { + activeDatasource: { + renderDataPanel, + } as unknown as Datasource, + }; + + const mountResult = await mountWithProvider( + {}} + core={{} as DatasourceDataPanelProps['core']} + dropOntoWorkspace={(field: DragDropIdentifier) => {}} + hasSuggestionForField={(field: DragDropIdentifier) => true} + plugins={{ uiActions: {} as UiActionsStart }} + />, + { + preloadedState: { + activeDatasourceId: 'activeDatasource', + datasourceStates: { + activeDatasource: { + isLoading: false, + state: { + age: 'old', + }, + }, + }, + }, + storeDeps: mockStoreDeps({ datasourceMap }), + } + ); + + lensStore = mountResult.lensStore; + + datasourceDataPanelProps = renderDataPanel.mock.calls[0][1] as DatasourceDataPanelProps; + }); + + describe('setState', () => { + it('applies state immediately when option true', async () => { + lensStore.dispatch(disableAutoApply()); + selectTriggerApplyChanges(lensStore.getState()); + + const newDatasourceState = { age: 'new' }; + datasourceDataPanelProps.setState(newDatasourceState, { applyImmediately: true }); + + expect(lensStore.getState().lens.datasourceStates.activeDatasource.state).toEqual( + newDatasourceState + ); + expect(selectTriggerApplyChanges(lensStore.getState())).toBeTruthy(); + }); + + it('does not apply state immediately when option false', async () => { + lensStore.dispatch(disableAutoApply()); + selectTriggerApplyChanges(lensStore.getState()); + + const newDatasourceState = { age: 'new' }; + datasourceDataPanelProps.setState(newDatasourceState, { applyImmediately: false }); + + const lensState = lensStore.getState().lens; + expect(lensState.datasourceStates.activeDatasource.state).toEqual(newDatasourceState); + expect(selectTriggerApplyChanges(lensStore.getState())).toBeFalsy(); + }); + }); + }); +}); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.tsx index b77d313973432cc..17f3d385123c200 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/data_panel_wrapper.tsx @@ -20,6 +20,7 @@ import { updateDatasourceState, useLensSelector, setState, + applyChanges, selectExecutionContext, selectActiveDatasourceId, selectDatasourceStates, @@ -45,8 +46,8 @@ export const DataPanelWrapper = memo((props: DataPanelWrapperProps) => { : true; const dispatchLens = useLensDispatch(); - const setDatasourceState: StateSetter = useMemo(() => { - return (updater) => { + const setDatasourceState: StateSetter = useMemo(() => { + return (updater: unknown | ((prevState: unknown) => unknown), options) => { dispatchLens( updateDatasourceState({ updater, @@ -54,6 +55,9 @@ export const DataPanelWrapper = memo((props: DataPanelWrapperProps) => { clearStagedPreview: true, }) ); + if (options?.applyImmediately) { + dispatchLens(applyChanges()); + } }; }, [activeDatasourceId, dispatchLens]); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx index f2e4af61ddbdb6c..7f1c673d0d1ddc6 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx @@ -79,7 +79,7 @@ export function EditorFrame(props: EditorFrameProps) { const suggestion = getSuggestionForField.current!(field); if (suggestion) { trackUiEvent('drop_onto_workspace'); - switchToSuggestion(dispatchLens, suggestion, true); + switchToSuggestion(dispatchLens, suggestion, { clearStagedPreview: true }); } }, [getSuggestionForField, dispatchLens] diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/frame_layout.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/frame_layout.scss index c8c0a6e2ebbd22c..b49c77bb8b41981 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/frame_layout.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/frame_layout.scss @@ -73,7 +73,6 @@ a tilemap in an iframe: https://github.com/elastic/kibana/issues/16457 */ } &.lnsFrameLayout__pageBody-isFullscreen { - background: $euiColorEmptyShade; flex: 1; padding: 0; } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts index b8ce851f25349cb..c9d237961b4752f 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts @@ -27,6 +27,7 @@ import { switchVisualization, DatasourceStates, VisualizationState, + applyChanges, } from '../../state_management'; /** @@ -232,7 +233,10 @@ export function switchToSuggestion( Suggestion, 'visualizationId' | 'visualizationState' | 'datasourceState' | 'datasourceId' >, - clearStagedPreview?: boolean + options?: { + clearStagedPreview?: boolean; + applyImmediately?: boolean; + } ) { dispatchLens( switchVisualization({ @@ -242,9 +246,12 @@ export function switchToSuggestion( datasourceState: suggestion.datasourceState, datasourceId: suggestion.datasourceId!, }, - clearStagedPreview, + clearStagedPreview: options?.clearStagedPreview, }) ); + if (options?.applyImmediately) { + dispatchLens(applyChanges()); + } } export function getTopSuggestionForField( diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.scss index 37a4a88c32f22e5..804bfbf11d74060 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.scss @@ -88,3 +88,7 @@ text-align: center; flex-grow: 0; } + +.lnsSuggestionPanel__applyChangesPrompt { + height: $lnsSuggestionHeight; +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.test.tsx index c9ddc0ea6551c78..8d9ea9b3c70b714 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.test.tsx @@ -21,7 +21,21 @@ import { getSuggestions } from './suggestion_helpers'; import { EuiIcon, EuiPanel, EuiToolTip, EuiAccordion } from '@elastic/eui'; import { LensIconChartDatatable } from '../../assets/chart_datatable'; import { mountWithProvider } from '../../mocks'; -import { LensAppState, PreviewState, setState, setToggleFullscreen } from '../../state_management'; +import { + applyChanges, + LensAppState, + PreviewState, + setState, + setToggleFullscreen, + VisualizationState, +} from '../../state_management'; +import { setChangesApplied } from '../../state_management/lens_slice'; + +const SELECTORS = { + APPLY_CHANGES_BUTTON: 'button[data-test-subj="lnsSuggestionApplyChanges"]', + SUGGESTIONS_PANEL: '[data-test-subj="lnsSuggestionsPanel"]', + SUGGESTION_TILE_BUTTON: 'button[data-test-subj="lnsSuggestion"]', +}; jest.mock('./suggestion_helpers'); @@ -108,6 +122,38 @@ describe('suggestion_panel', () => { expect(instance.find(SuggestionPanel).exists()).toBe(true); }); + it('should display apply-changes prompt when changes not applied', async () => { + const { instance, lensStore } = await mountWithProvider(, { + preloadedState: { + ...preloadedState, + visualization: { + ...preloadedState.visualization, + state: { + something: 'changed', + }, + } as VisualizationState, + changesApplied: false, + autoApplyDisabled: true, + }, + }); + + expect(instance.exists(SELECTORS.APPLY_CHANGES_BUTTON)).toBeTruthy(); + expect(instance.exists(SELECTORS.SUGGESTION_TILE_BUTTON)).toBeFalsy(); + + instance.find(SELECTORS.APPLY_CHANGES_BUTTON).simulate('click'); + + // check changes applied + expect(lensStore.dispatch).toHaveBeenCalledWith(applyChanges()); + + // simulate workspace panel behavior + lensStore.dispatch(setChangesApplied(true)); + instance.update(); + + // check UI updated + expect(instance.exists(SELECTORS.APPLY_CHANGES_BUTTON)).toBeFalsy(); + expect(instance.exists(SELECTORS.SUGGESTION_TILE_BUTTON)).toBeTruthy(); + }); + it('should list passed in suggestions', async () => { const { instance } = await mountWithProvider(, { preloadedState, @@ -173,12 +219,12 @@ describe('suggestion_panel', () => { preloadedState, }); act(() => { - instance.find('[data-test-subj="lnsSuggestion"]').at(2).simulate('click'); + instance.find(SELECTORS.SUGGESTION_TILE_BUTTON).at(2).simulate('click'); }); instance.update(); - expect(instance.find('[data-test-subj="lnsSuggestion"]').at(2).prop('className')).toContain( + expect(instance.find(SELECTORS.SUGGESTION_TILE_BUTTON).at(2).prop('className')).toContain( 'lnsSuggestionPanel__button-isSelected' ); }); @@ -189,13 +235,13 @@ describe('suggestion_panel', () => { ); act(() => { - instance.find('[data-test-subj="lnsSuggestion"]').at(2).simulate('click'); + instance.find(SELECTORS.SUGGESTION_TILE_BUTTON).at(2).simulate('click'); }); instance.update(); act(() => { - instance.find('[data-test-subj="lnsSuggestion"]').at(0).simulate('click'); + instance.find(SELECTORS.SUGGESTION_TILE_BUTTON).at(0).simulate('click'); }); instance.update(); @@ -203,6 +249,10 @@ describe('suggestion_panel', () => { expect(lensStore.dispatch).toHaveBeenCalledWith({ type: 'lens/rollbackSuggestion', }); + // check that it immediately applied any state changes in case auto-apply disabled + expect(lensStore.dispatch).toHaveBeenLastCalledWith({ + type: applyChanges.type, + }); }); }); @@ -212,7 +262,7 @@ describe('suggestion_panel', () => { }); act(() => { - instance.find('button[data-test-subj="lnsSuggestion"]').at(1).simulate('click'); + instance.find(SELECTORS.SUGGESTION_TILE_BUTTON).at(1).simulate('click'); }); expect(lensStore.dispatch).toHaveBeenCalledWith( @@ -228,6 +278,7 @@ describe('suggestion_panel', () => { }, }) ); + expect(lensStore.dispatch).toHaveBeenLastCalledWith({ type: applyChanges.type }); }); it('should render render icon if there is no preview expression', async () => { @@ -264,10 +315,10 @@ describe('suggestion_panel', () => { preloadedState, }); - expect(instance.find('[data-test-subj="lnsSuggestionsPanel"]').find(EuiIcon)).toHaveLength(1); - expect( - instance.find('[data-test-subj="lnsSuggestionsPanel"]').find(EuiIcon).prop('type') - ).toEqual(LensIconChartDatatable); + expect(instance.find(SELECTORS.SUGGESTIONS_PANEL).find(EuiIcon)).toHaveLength(1); + expect(instance.find(SELECTORS.SUGGESTIONS_PANEL).find(EuiIcon).prop('type')).toEqual( + LensIconChartDatatable + ); }); it('should return no suggestion if visualization has missing index-patterns', async () => { @@ -301,7 +352,7 @@ describe('suggestion_panel', () => { instance.find(EuiAccordion).at(0).simulate('change'); }); - expect(instance.find('[data-test-subj="lnsSuggestionsPanel"]')).toEqual({}); + expect(instance.find(SELECTORS.SUGGESTIONS_PANEL)).toEqual({}); }); it('should render preview expression if there is one', () => { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx index f6fccbb831eae2e..e6a9831e0aae595 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx @@ -19,6 +19,10 @@ import { EuiToolTip, EuiButtonEmpty, EuiAccordion, + EuiFlexGroup, + EuiFlexItem, + EuiButton, + EuiSpacer, } from '@elastic/eui'; import { IconType } from '@elastic/eui/src/components/icon/icon'; import { Ast, toExpression } from '@kbn/interpreter'; @@ -55,7 +59,10 @@ import { selectActiveDatasourceId, selectActiveData, selectDatasourceStates, + selectChangesApplied, + applyChanges, } from '../../state_management'; +import { DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS } from './config_panel/dimension_container'; const MAX_SUGGESTIONS_DISPLAYED = 5; const LOCAL_STORAGE_SUGGESTIONS_PANEL = 'LENS_SUGGESTIONS_PANEL_HIDDEN'; @@ -190,6 +197,7 @@ export function SuggestionPanel({ const existsStagedPreview = useLensSelector((state) => Boolean(state.lens.stagedPreview)); const currentVisualization = useLensSelector(selectCurrentVisualization); const currentDatasourceStates = useLensSelector(selectCurrentDatasourceStates); + const changesApplied = useLensSelector(selectChangesApplied); // get user's selection from localStorage, this key defines if the suggestions panel will be hidden or not const [hideSuggestions, setHideSuggestions] = useLocalStorage( LOCAL_STORAGE_SUGGESTIONS_PANEL, @@ -327,9 +335,92 @@ export function SuggestionPanel({ trackSuggestionEvent('back_to_current'); setLastSelectedSuggestion(-1); dispatchLens(rollbackSuggestion()); + dispatchLens(applyChanges()); } } + const applyChangesPrompt = ( + + + +

+ +

+ + dispatchLens(applyChanges())} + data-test-subj="lnsSuggestionApplyChanges" + > + + + + + + ); + + const suggestionsUI = ( + <> + {currentVisualization.activeId && !hideSuggestions && ( + + )} + {!hideSuggestions && + suggestions.map((suggestion, index) => { + return ( + { + trackUiEvent('suggestion_clicked'); + if (lastSelectedSuggestion === index) { + rollbackToCurrentVisualization(); + } else { + setLastSelectedSuggestion(index); + switchToSuggestion(dispatchLens, suggestion, { applyImmediately: true }); + } + }} + selected={index === lastSelectedSuggestion} + /> + ); + })} + + ); + return (
- {currentVisualization.activeId && !hideSuggestions && ( - - )} - {!hideSuggestions && - suggestions.map((suggestion, index) => { - return ( - { - trackUiEvent('suggestion_clicked'); - if (lastSelectedSuggestion === index) { - rollbackToCurrentVisualization(); - } else { - setLastSelectedSuggestion(index); - switchToSuggestion(dispatchLens, suggestion); - } - }} - selected={index === lastSelectedSuggestion} - /> - ); - })} + {changesApplied ? suggestionsUI : applyChangesPrompt}
diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx index c325e6d516c8b7b..a486b6315c3f46e 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx @@ -31,6 +31,7 @@ jest.mock('react-virtualized-auto-sizer', () => { import { Visualization, FramePublicAPI, DatasourcePublicAPI } from '../../../types'; import { ChartSwitch } from './chart_switch'; import { PaletteOutput } from 'src/plugins/charts/public'; +import { applyChanges } from '../../../state_management'; describe('chart_switch', () => { function generateVisualization(id: string): jest.Mocked { @@ -189,6 +190,7 @@ describe('chart_switch', () => { clearStagedPreview: true, }, }); + expect(lensStore.dispatch).not.toHaveBeenCalledWith({ type: applyChanges.type }); // should not apply changes automatically }); it('should use initial state if there is no suggestion from the target visualization', async () => { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx index d24ed0a736ae241..5c528832ac5b28d 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx @@ -132,7 +132,7 @@ export const ChartSwitch = memo(function ChartSwitch(props: Props) { ...selection, visualizationState: selection.getVisualizationState(), }, - true + { clearStagedPreview: true } ); if ( diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx index ccd9e8aace2ab23..7359f7cdc185b29 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.test.tsx @@ -35,9 +35,15 @@ import { UiActionsStart } from '../../../../../../../src/plugins/ui_actions/publ import { uiActionsPluginMock } from '../../../../../../../src/plugins/ui_actions/public/mocks'; import { TriggerContract } from '../../../../../../../src/plugins/ui_actions/public/triggers'; import { VIS_EVENT_TO_TRIGGER } from '../../../../../../../src/plugins/visualizations/public/embeddable'; -import { LensRootStore, setState } from '../../../state_management'; +import { + applyChanges, + setState, + updateDatasourceState, + updateVisualizationState, +} from '../../../state_management'; import { getLensInspectorService } from '../../../lens_inspector_service'; import { inspectorPluginMock } from '../../../../../../../src/plugins/inspector/public/mocks'; +import { disableAutoApply, enableAutoApply } from '../../../state_management/lens_slice'; const defaultPermissions: Record>> = { navLinks: { management: true }, @@ -102,12 +108,13 @@ describe('workspace_panel', () => { }} ExpressionRenderer={expressionRendererMock} />, - { preloadedState: { visualization: { activeId: null, state: {} }, datasourceStates: {} }, } ); instance = mounted.instance; + instance.update(); + expect(instance.find('[data-test-subj="empty-workspace"]')).toHaveLength(2); expect(instance.find(expressionRendererMock)).toHaveLength(0); }); @@ -124,6 +131,7 @@ describe('workspace_panel', () => { { preloadedState: { datasourceStates: {} } } ); instance = mounted.instance; + instance.update(); expect(instance.find('[data-test-subj="empty-workspace"]')).toHaveLength(2); expect(instance.find(expressionRendererMock)).toHaveLength(0); @@ -141,6 +149,7 @@ describe('workspace_panel', () => { { preloadedState: { datasourceStates: {} } } ); instance = mounted.instance; + instance.update(); expect(instance.find('[data-test-subj="empty-workspace"]')).toHaveLength(2); expect(instance.find(expressionRendererMock)).toHaveLength(0); @@ -170,6 +179,8 @@ describe('workspace_panel', () => { instance = mounted.instance; + instance.update(); + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` "kibana | lens_merge_tables layerIds=\\"first\\" tables={datasource} @@ -177,6 +188,188 @@ describe('workspace_panel', () => { `); }); + it('should give user control when auto-apply disabled', async () => { + const framePublicAPI = createMockFramePublicAPI(); + framePublicAPI.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + mockDatasource.toExpression.mockReturnValue('datasource'); + mockDatasource.getLayers.mockReturnValue(['first']); + + const mounted = await mountWithProvider( + 'testVis' }, + }} + ExpressionRenderer={expressionRendererMock} + />, + { + preloadedState: { + autoApplyDisabled: true, + }, + } + ); + + instance = mounted.instance; + instance.update(); + + // allows initial render + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` + "kibana + | lens_merge_tables layerIds=\\"first\\" tables={datasource} + | testVis" + `); + + mockDatasource.toExpression.mockReturnValue('new-datasource'); + act(() => { + instance.setProps({ + visualizationMap: { + testVis: { ...mockVisualization, toExpression: () => 'new-vis' }, + }, + }); + }); + + // nothing should change + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` + "kibana + | lens_merge_tables layerIds=\\"first\\" tables={datasource} + | testVis" + `); + + act(() => { + mounted.lensStore.dispatch(applyChanges()); + }); + instance.update(); + + // should update + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` + "kibana + | lens_merge_tables layerIds=\\"first\\" tables={new-datasource} + | new-vis" + `); + + mockDatasource.toExpression.mockReturnValue('other-new-datasource'); + act(() => { + instance.setProps({ + visualizationMap: { + testVis: { ...mockVisualization, toExpression: () => 'other-new-vis' }, + }, + }); + }); + + // should not update + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` + "kibana + | lens_merge_tables layerIds=\\"first\\" tables={new-datasource} + | new-vis" + `); + + act(() => { + mounted.lensStore.dispatch(enableAutoApply()); + }); + instance.update(); + + // reenabling auto-apply triggers an update as well + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` + "kibana + | lens_merge_tables layerIds=\\"first\\" tables={other-new-datasource} + | other-new-vis" + `); + }); + + it('should base saveability on working changes when auto-apply disabled', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockVisualization.getErrorMessages.mockImplementation((currentVisualizationState: any) => { + if (currentVisualizationState.hasProblem) { + return [{ shortMessage: 'An error occurred', longMessage: 'An long description here' }]; + } else { + return []; + } + }); + + const framePublicAPI = createMockFramePublicAPI(); + framePublicAPI.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + mockDatasource.toExpression.mockReturnValue('datasource'); + mockDatasource.getLayers.mockReturnValue(['first']); + + const mounted = await mountWithProvider( + 'testVis' }, + }} + ExpressionRenderer={expressionRendererMock} + /> + ); + + instance = mounted.instance; + const isSaveable = () => mounted.lensStore.getState().lens.isSaveable; + + instance.update(); + + // allows initial render + expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(` + "kibana + | lens_merge_tables layerIds=\\"first\\" tables={datasource} + | testVis" + `); + expect(isSaveable()).toBe(true); + + act(() => { + mounted.lensStore.dispatch( + updateVisualizationState({ + visualizationId: 'testVis', + newState: { activeId: 'testVis', hasProblem: true }, + }) + ); + }); + instance.update(); + + expect(isSaveable()).toBe(false); + }); + + it('should allow empty workspace as initial render when auto-apply disabled', async () => { + mockVisualization.toExpression.mockReturnValue('testVis'); + const framePublicAPI = createMockFramePublicAPI(); + framePublicAPI.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + + const mounted = await mountWithProvider( + , + { + preloadedState: { + autoApplyDisabled: true, + }, + } + ); + + instance = mounted.instance; + instance.update(); + + expect(instance.exists('[data-test-subj="empty-workspace"]')).toBeTruthy(); + }); + it('should execute a trigger on expression event', async () => { const framePublicAPI = createMockFramePublicAPI(); framePublicAPI.datasourceLayers = { @@ -289,6 +482,7 @@ describe('workspace_panel', () => { } ); instance = mounted.instance; + instance.update(); const ast = fromExpression(instance.find(expressionRendererMock).prop('expression') as string); @@ -342,27 +536,25 @@ describe('workspace_panel', () => { expressionRendererMock = jest.fn((_arg) => ); - await act(async () => { - const mounted = await mountWithProvider( - 'testVis' }, - }} - ExpressionRenderer={expressionRendererMock} - /> - ); - instance = mounted.instance; - }); + const mounted = await mountWithProvider( + 'testVis' }, + }} + ExpressionRenderer={expressionRendererMock} + /> + ); + instance = mounted.instance; instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(1); + expect(expressionRendererMock).toHaveBeenCalledTimes(2); - await act(async () => { + act(() => { instance.setProps({ framePublicAPI: { ...framePublicAPI, @@ -373,7 +565,7 @@ describe('workspace_panel', () => { instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(2); + expect(expressionRendererMock).toHaveBeenCalledTimes(3); }); it('should run the expression again if the filters change', async () => { @@ -388,31 +580,29 @@ describe('workspace_panel', () => { .mockReturnValueOnce('datasource second'); expressionRendererMock = jest.fn((_arg) => ); - await act(async () => { - const mounted = await mountWithProvider( - 'testVis' }, - }} - ExpressionRenderer={expressionRendererMock} - /> - ); - instance = mounted.instance; - }); + const mounted = await mountWithProvider( + 'testVis' }, + }} + ExpressionRenderer={expressionRendererMock} + /> + ); + instance = mounted.instance; instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(1); + expect(expressionRendererMock).toHaveBeenCalledTimes(2); const indexPattern = { id: 'index1' } as unknown as IndexPattern; const field = { name: 'myfield' } as unknown as FieldSpec; - await act(async () => { + act(() => { instance.setProps({ framePublicAPI: { ...framePublicAPI, @@ -423,7 +613,7 @@ describe('workspace_panel', () => { instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(2); + expect(expressionRendererMock).toHaveBeenCalledTimes(3); }); it('should show an error message if there are missing indexpatterns in the visualization', async () => { @@ -572,6 +762,9 @@ describe('workspace_panel', () => { /> ); instance = mounted.instance; + act(() => { + instance.update(); + }); expect(instance.find('[data-test-subj="configuration-failure"]').exists()).toBeTruthy(); expect(instance.find(expressionRendererMock)).toHaveLength(0); @@ -642,6 +835,97 @@ describe('workspace_panel', () => { expect(instance.find(expressionRendererMock)).toHaveLength(0); }); + it('should NOT display errors for unapplied changes', async () => { + // this test is important since we don't want the workspace panel to + // display errors if the user has disabled auto-apply, messed something up, + // but not yet applied their changes + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockDatasource.getErrorMessages.mockImplementation((currentDatasourceState: any) => { + if (currentDatasourceState.hasProblem) { + return [{ shortMessage: 'An error occurred', longMessage: 'An long description here' }]; + } else { + return []; + } + }); + mockDatasource.getLayers.mockReturnValue(['first']); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockVisualization.getErrorMessages.mockImplementation((currentVisualizationState: any) => { + if (currentVisualizationState.hasProblem) { + return [{ shortMessage: 'An error occurred', longMessage: 'An long description here' }]; + } else { + return []; + } + }); + mockVisualization.toExpression.mockReturnValue('testVis'); + const framePublicAPI = createMockFramePublicAPI(); + framePublicAPI.datasourceLayers = { + first: mockDatasource.publicAPIMock, + }; + + const mounted = await mountWithProvider( + + ); + + instance = mounted.instance; + const lensStore = mounted.lensStore; + + const showingErrors = () => + instance.exists('[data-test-subj="configuration-failure-error"]') || + instance.exists('[data-test-subj="configuration-failure-more-errors"]'); + + expect(showingErrors()).toBeFalsy(); + + act(() => { + lensStore.dispatch(disableAutoApply()); + }); + instance.update(); + + expect(showingErrors()).toBeFalsy(); + + // introduce some issues + act(() => { + lensStore.dispatch( + updateDatasourceState({ + datasourceId: 'testDatasource', + updater: { hasProblem: true }, + }) + ); + }); + instance.update(); + + expect(showingErrors()).toBeFalsy(); + + act(() => { + lensStore.dispatch( + updateVisualizationState({ + visualizationId: 'testVis', + newState: { activeId: 'testVis', hasProblem: true }, + }) + ); + }); + instance.update(); + + expect(showingErrors()).toBeFalsy(); + + // errors should appear when problem changes are applied + act(() => { + lensStore.dispatch(applyChanges()); + }); + instance.update(); + + expect(showingErrors()).toBeTruthy(); + }); + it('should show an error message if the expression fails to parse', async () => { mockDatasource.toExpression.mockReturnValue('|||'); mockDatasource.getLayers.mockReturnValue(['first']); @@ -676,30 +960,27 @@ describe('workspace_panel', () => { first: mockDatasource.publicAPIMock, }; - await act(async () => { - const mounted = await mountWithProvider( - 'testVis' }, - }} - ExpressionRenderer={expressionRendererMock} - /> - ); - instance = mounted.instance; - }); - + const mounted = await mountWithProvider( + 'testVis' }, + }} + ExpressionRenderer={expressionRendererMock} + /> + ); + instance = mounted.instance; instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(1); + expect(expressionRendererMock).toHaveBeenCalledTimes(2); instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(1); + expect(expressionRendererMock).toHaveBeenCalledTimes(2); }); it('should attempt to run the expression again if it changes', async () => { @@ -709,28 +990,25 @@ describe('workspace_panel', () => { framePublicAPI.datasourceLayers = { first: mockDatasource.publicAPIMock, }; - let lensStore: LensRootStore; - await act(async () => { - const mounted = await mountWithProvider( - 'testVis' }, - }} - ExpressionRenderer={expressionRendererMock} - /> - ); - instance = mounted.instance; - lensStore = mounted.lensStore; - }); + const mounted = await mountWithProvider( + 'testVis' }, + }} + ExpressionRenderer={expressionRendererMock} + /> + ); + instance = mounted.instance; + const lensStore = mounted.lensStore; instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(1); + expect(expressionRendererMock).toHaveBeenCalledTimes(2); expressionRendererMock.mockImplementation((_) => { return ; @@ -746,7 +1024,7 @@ describe('workspace_panel', () => { ); instance.update(); - expect(expressionRendererMock).toHaveBeenCalledTimes(2); + expect(expressionRendererMock).toHaveBeenCalledTimes(3); expect(instance.find(expressionRendererMock)).toHaveLength(1); }); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index a26d72f1b4fc2d9..acebc640e3a09b4 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useState, useEffect, useMemo, useContext, useCallback } from 'react'; +import React, { useState, useEffect, useMemo, useContext, useCallback, useRef } from 'react'; import classNames from 'classnames'; import { FormattedMessage } from '@kbn/i18n-react'; import { toExpression } from '@kbn/interpreter'; @@ -66,9 +66,12 @@ import { selectDatasourceStates, selectActiveDatasourceId, selectSearchSessionId, + selectAutoApplyEnabled, + selectTriggerApplyChanges, } from '../../../state_management'; import type { LensInspector } from '../../../lens_inspector_service'; import { inferTimeField } from '../../../utils'; +import { setChangesApplied } from '../../../state_management/lens_slice'; export interface WorkspacePanelProps { visualizationMap: VisualizationMap; @@ -88,6 +91,7 @@ interface WorkspaceState { fixAction?: DatasourceFixAction; }>; expandError: boolean; + expressionToRender: string | null | undefined; } const dropProps = { @@ -136,13 +140,22 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ const visualization = useLensSelector(selectVisualization); const activeDatasourceId = useLensSelector(selectActiveDatasourceId); const datasourceStates = useLensSelector(selectDatasourceStates); + const autoApplyEnabled = useLensSelector(selectAutoApplyEnabled); + const triggerApply = useLensSelector(selectTriggerApplyChanges); - const { datasourceLayers } = framePublicAPI; const [localState, setLocalState] = useState({ expressionBuildError: undefined, expandError: false, + expressionToRender: undefined, }); + // const expressionToRender = useRef(); + const initialRenderComplete = useRef(); + + const shouldApplyExpression = autoApplyEnabled || !initialRenderComplete.current || triggerApply; + + const { datasourceLayers } = framePublicAPI; + const activeVisualization = visualization.activeId ? visualizationMap[visualization.activeId] : null; @@ -186,7 +199,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ [activeVisualization, visualization.state, activeDatasourceId, datasourceMap, datasourceStates] ); - const expression = useMemo(() => { + const _expression = useMemo(() => { if (!configurationValidationError?.length && !missingRefsErrors.length && !unknownVisError) { try { const ast = buildExpression({ @@ -238,10 +251,32 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ visualization.activeId, ]); - const expressionExists = Boolean(expression); useEffect(() => { - dispatchLens(setSaveable(expressionExists)); - }, [expressionExists, dispatchLens]); + dispatchLens(setSaveable(Boolean(_expression))); + }, [_expression, dispatchLens]); + + useEffect(() => { + if (!autoApplyEnabled) { + dispatchLens(setChangesApplied(_expression === localState.expressionToRender)); + } + }); + + useEffect(() => { + if (shouldApplyExpression) { + setLocalState((s) => ({ ...s, expressionToRender: _expression })); + } + }, [_expression, shouldApplyExpression]); + + const expressionExists = Boolean(localState.expressionToRender); + useEffect(() => { + // null signals an empty workspace which should count as an initial render + if ( + (expressionExists || localState.expressionToRender === null) && + !initialRenderComplete.current + ) { + initialRenderComplete.current = true; + } + }, [expressionExists, localState.expressionToRender]); const onEvent = useCallback( (event: ExpressionRendererEvent) => { @@ -291,7 +326,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ if (suggestionForDraggedField) { trackUiEvent('drop_onto_workspace'); trackUiEvent(expressionExists ? 'drop_non_empty' : 'drop_empty'); - switchToSuggestion(dispatchLens, suggestionForDraggedField, true); + switchToSuggestion(dispatchLens, suggestionForDraggedField, { clearStagedPreview: true }); } }, [suggestionForDraggedField, expressionExists, dispatchLens]); @@ -343,12 +378,12 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ }; const renderVisualization = () => { - if (expression === null) { + if (localState.expressionToRender === null) { return renderEmptyWorkspace(); } return ( { @@ -392,7 +425,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ order={dropProps.order} > - {element} + {renderVisualization()} ); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss index 9a87f1ba46e94aa..9b4502ea8194454 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss @@ -32,6 +32,7 @@ &.lnsWorkspacePanelWrapper--fullscreen { margin-bottom: 0; } + } .lnsWorkspacePanel__dragDrop { @@ -80,6 +81,14 @@ .lnsWorkspacePanelWrapper__toolbar { margin-bottom: 0; + + &.lnsWorkspacePanelWrapper__toolbar--fullscreen { + padding: $euiSizeS $euiSizeS 0 $euiSizeS; + } + + & > .euiFlexItem { + min-height: $euiButtonHeightSmall; + } } .lnsDropIllustration__adjustFill { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.test.tsx index fb77ff75324f09e..3aab4d6e7d85c44 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.test.tsx @@ -10,6 +10,14 @@ import { Visualization } from '../../../types'; import { createMockVisualization, createMockFramePublicAPI, FrameMock } from '../../../mocks'; import { WorkspacePanelWrapper } from './workspace_panel_wrapper'; import { mountWithProvider } from '../../../mocks'; +import { ReactWrapper } from 'enzyme'; +import { + selectAutoApplyEnabled, + updateVisualizationState, + disableAutoApply, + selectTriggerApplyChanges, +} from '../../../state_management'; +import { setChangesApplied } from '../../../state_management/lens_slice'; describe('workspace_panel_wrapper', () => { let mockVisualization: jest.Mocked; @@ -61,4 +69,144 @@ describe('workspace_panel_wrapper', () => { setState: expect.anything(), }); }); + + describe('auto-apply controls', () => { + class Harness { + private _instance: ReactWrapper; + + constructor(instance: ReactWrapper) { + this._instance = instance; + } + + update() { + this._instance.update(); + } + + private get applyChangesButton() { + return this._instance.find('button[data-test-subj="lensApplyChanges"]'); + } + + private get autoApplyToggleSwitch() { + return this._instance.find('button[data-test-subj="lensToggleAutoApply"]'); + } + + toggleAutoApply() { + this.autoApplyToggleSwitch.simulate('click'); + } + + public get autoApplySwitchOn() { + return this.autoApplyToggleSwitch.prop('aria-checked'); + } + + applyChanges() { + this.applyChangesButton.simulate('click'); + } + + public get applyChangesExists() { + return this.applyChangesButton.exists(); + } + + public get applyChangesDisabled() { + if (!this.applyChangesExists) { + throw Error('apply changes button doesnt exist'); + } + return this.applyChangesButton.prop('disabled'); + } + } + + let store: Awaited>['lensStore']; + let harness: Harness; + beforeEach(async () => { + const { instance, lensStore } = await mountWithProvider( + +
+ + ); + + store = lensStore; + harness = new Harness(instance); + }); + + it('toggles auto-apply', async () => { + store.dispatch(disableAutoApply()); + harness.update(); + + expect(selectAutoApplyEnabled(store.getState())).toBeFalsy(); + expect(harness.autoApplySwitchOn).toBeFalsy(); + expect(harness.applyChangesExists).toBeTruthy(); + + harness.toggleAutoApply(); + + expect(selectAutoApplyEnabled(store.getState())).toBeTruthy(); + expect(harness.autoApplySwitchOn).toBeTruthy(); + expect(harness.applyChangesExists).toBeFalsy(); + + harness.toggleAutoApply(); + + expect(selectAutoApplyEnabled(store.getState())).toBeFalsy(); + expect(harness.autoApplySwitchOn).toBeFalsy(); + expect(harness.applyChangesExists).toBeTruthy(); + }); + + it('apply-changes button works', () => { + store.dispatch(disableAutoApply()); + harness.update(); + + expect(selectAutoApplyEnabled(store.getState())).toBeFalsy(); + expect(harness.applyChangesDisabled).toBeTruthy(); + + // make a change + store.dispatch( + updateVisualizationState({ + visualizationId: store.getState().lens.visualization.activeId as string, + newState: { something: 'changed' }, + }) + ); + // simulate workspace panel behavior + store.dispatch(setChangesApplied(false)); + harness.update(); + + expect(harness.applyChangesDisabled).toBeFalsy(); + + harness.applyChanges(); + + expect(selectTriggerApplyChanges(store.getState())).toBeTruthy(); + // simulate workspace panel behavior + store.dispatch(setChangesApplied(true)); + harness.update(); + + expect(harness.applyChangesDisabled).toBeTruthy(); + }); + + it('enabling auto apply while having unapplied changes works', () => { + // setup + store.dispatch(disableAutoApply()); + store.dispatch( + updateVisualizationState({ + visualizationId: store.getState().lens.visualization.activeId as string, + newState: { something: 'changed' }, + }) + ); + store.dispatch(setChangesApplied(false)); // simulate workspace panel behavior + harness.update(); + + expect(harness.applyChangesDisabled).toBeFalsy(); + expect(harness.autoApplySwitchOn).toBeFalsy(); + expect(harness.applyChangesExists).toBeTruthy(); + + // enable auto apply + harness.toggleAutoApply(); + + expect(harness.autoApplySwitchOn).toBeTruthy(); + expect(harness.applyChangesExists).toBeFalsy(); + }); + }); }); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx index be2306348861058..274992c5f5e6d9d 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx @@ -8,8 +8,12 @@ import './workspace_panel_wrapper.scss'; import React, { useCallback } from 'react'; -import { EuiPageContent, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { EuiPageContent, EuiFlexGroup, EuiFlexItem, EuiSwitch, EuiButton } from '@elastic/eui'; import classNames from 'classnames'; +import { FormattedMessage } from '@kbn/i18n-react'; +import { i18n } from '@kbn/i18n'; +import { trackUiEvent } from '../../../lens_ui_telemetry'; +import { Storage } from '../../../../../../../src/plugins/kibana_utils/public'; import { DatasourceMap, FramePublicAPI, VisualizationMap } from '../../../types'; import { NativeRenderer } from '../../../native_renderer'; import { ChartSwitch } from './chart_switch'; @@ -20,8 +24,18 @@ import { DatasourceStates, VisualizationState, updateDatasourceState, + useLensSelector, + selectChangesApplied, + applyChanges, + enableAutoApply, + disableAutoApply, + selectAutoApplyEnabled, } from '../../../state_management'; import { WorkspaceTitle } from './title'; +import { DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS } from '../config_panel/dimension_container'; +import { writeToStorage } from '../../../settings_storage'; + +export const AUTO_APPLY_DISABLED_STORAGE_KEY = 'autoApplyDisabled'; export interface WorkspacePanelWrapperProps { children: React.ReactNode | React.ReactNode[]; @@ -46,6 +60,9 @@ export function WorkspacePanelWrapper({ }: WorkspacePanelWrapperProps) { const dispatchLens = useLensDispatch(); + const changesApplied = useLensSelector(selectChangesApplied); + const autoApplyEnabled = useLensSelector(selectAutoApplyEnabled); + const activeVisualization = visualizationId ? visualizationMap[visualizationId] : null; const setVisualizationState = useCallback( (newState: unknown) => { @@ -72,6 +89,18 @@ export function WorkspacePanelWrapper({ }, [dispatchLens] ); + + const toggleAutoApply = useCallback(() => { + trackUiEvent('toggle_autoapply'); + + writeToStorage( + new Storage(localStorage), + AUTO_APPLY_DISABLED_STORAGE_KEY, + String(autoApplyEnabled) + ); + dispatchLens(autoApplyEnabled ? disableAutoApply() : enableAutoApply()); + }, [dispatchLens, autoApplyEnabled]); + const warningMessages: React.ReactNode[] = []; if (activeVisualization?.getWarningMessages) { warningMessages.push( @@ -93,44 +122,93 @@ export function WorkspacePanelWrapper({
- {!isFullscreen ? ( - - + + + {!isFullscreen && ( - + + + + + {activeVisualization && activeVisualization.renderToolbar && ( + + + + )} + - {activeVisualization && activeVisualization.renderToolbar && ( + )} + + - - )} - - - ) : null} + {!autoApplyEnabled && ( + +
+ dispatchLens(applyChanges())} + size="s" + data-test-subj="lensApplyChanges" + > + + +
+ + )} + + + + {warningMessages && warningMessages.length ? ( {warningMessages} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx index 199131564f7c433..d8b5874050b2a8e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/datapanel.tsx @@ -52,7 +52,7 @@ export type Props = Omit, 'co changeIndexPattern: ( id: string, state: IndexPatternPrivateState, - setState: StateSetter + setState: StateSetter ) => void; charts: ChartsPluginSetup; core: CoreStart; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 0ac77696d5987c7..f40f3b9623ca857 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -138,7 +138,7 @@ export function getIndexPatternDatasource({ const handleChangeIndexPattern = ( id: string, state: IndexPatternPrivateState, - setState: StateSetter + setState: StateSetter ) => { changeIndexPattern({ id, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts index 9099b68cdaf0e8c..d992e36a0c6d80c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts @@ -854,7 +854,9 @@ describe('loader', () => { }); expect(setState).toHaveBeenCalledTimes(1); - expect(setState.mock.calls[0][0](state)).toMatchObject({ + const [fn, options] = setState.mock.calls[0]; + expect(options).toEqual({ applyImmediately: true }); + expect(fn(state)).toMatchObject({ currentIndexPatternId: '1', indexPatterns: { '1': { @@ -1071,7 +1073,8 @@ describe('loader', () => { expect(fetchJson).toHaveBeenCalledTimes(3); expect(setState).toHaveBeenCalledTimes(1); - const [fn] = setState.mock.calls[0]; + const [fn, options] = setState.mock.calls[0]; + expect(options).toEqual({ applyImmediately: true }); const newState = fn({ foo: 'bar', existingFields: {}, @@ -1155,7 +1158,8 @@ describe('loader', () => { await syncExistingFields(args); - const [fn] = setState.mock.calls[0]; + const [fn, options] = setState.mock.calls[0]; + expect(options).toEqual({ applyImmediately: true }); const newState = fn({ foo: 'bar', existingFields: {}, @@ -1204,7 +1208,8 @@ describe('loader', () => { await syncExistingFields(args); - const [fn] = setState.mock.calls[0]; + const [fn, options] = setState.mock.calls[0]; + expect(options).toEqual({ applyImmediately: true }); const newState = fn({ foo: 'bar', existingFields: {}, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts index 8b3a0556b032024..9495276f15960ea 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts @@ -9,7 +9,11 @@ import { uniq, mapValues, difference } from 'lodash'; import type { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import type { DataView } from 'src/plugins/data_views/public'; import type { HttpSetup, SavedObjectReference } from 'kibana/public'; -import type { InitializationOptions, StateSetter, VisualizeEditorContext } from '../types'; +import type { + DatasourceDataPanelProps, + InitializationOptions, + VisualizeEditorContext, +} from '../types'; import { IndexPattern, IndexPatternRef, @@ -33,7 +37,7 @@ import { readFromStorage, writeToStorage } from '../settings_storage'; import { getFieldByNameFactory } from './pure_helpers'; import { memoizedGetAvailableOperationsByMetadata } from './operations'; -type SetState = StateSetter; +type SetState = DatasourceDataPanelProps['setState']; type IndexPatternsService = Pick; type ErrorHandler = (err: Error) => void; @@ -326,17 +330,20 @@ export async function changeIndexPattern({ } try { - setState((s) => ({ - ...s, - layers: isSingleEmptyLayer(state.layers) - ? mapValues(state.layers, (layer) => updateLayerIndexPattern(layer, indexPatterns[id])) - : state.layers, - indexPatterns: { - ...s.indexPatterns, - [id]: indexPatterns[id], - }, - currentIndexPatternId: id, - })); + setState( + (s) => ({ + ...s, + layers: isSingleEmptyLayer(state.layers) + ? mapValues(state.layers, (layer) => updateLayerIndexPattern(layer, indexPatterns[id])) + : state.layers, + indexPatterns: { + ...s.indexPatterns, + [id]: indexPatterns[id], + }, + currentIndexPatternId: id, + }), + { applyImmediately: true } + ); setLastUsedIndexPatternId(storage, id); } catch (err) { onError(err); @@ -458,33 +465,39 @@ export async function syncExistingFields({ } } - setState((state) => ({ - ...state, - isFirstExistenceFetch: false, - existenceFetchFailed: false, - existenceFetchTimeout: false, - existingFields: emptinessInfo.reduce( - (acc, info) => { - acc[info.indexPatternTitle] = booleanMap(info.existingFieldNames); - return acc; - }, - { ...state.existingFields } - ), - })); + setState( + (state) => ({ + ...state, + isFirstExistenceFetch: false, + existenceFetchFailed: false, + existenceFetchTimeout: false, + existingFields: emptinessInfo.reduce( + (acc, info) => { + acc[info.indexPatternTitle] = booleanMap(info.existingFieldNames); + return acc; + }, + { ...state.existingFields } + ), + }), + { applyImmediately: true } + ); } catch (e) { // show all fields as available if fetch failed or timed out - setState((state) => ({ - ...state, - existenceFetchFailed: e.res?.status !== 408, - existenceFetchTimeout: e.res?.status === 408, - existingFields: indexPatterns.reduce( - (acc, pattern) => { - acc[pattern.title] = booleanMap(pattern.fields.map((field) => field.name)); - return acc; - }, - { ...state.existingFields } - ), - })); + setState( + (state) => ({ + ...state, + existenceFetchFailed: e.res?.status !== 408, + existenceFetchTimeout: e.res?.status === 408, + existingFields: indexPatterns.reduce( + (acc, pattern) => { + acc[pattern.title] = booleanMap(pattern.fields.map((field) => field.name)); + return acc; + }, + { ...state.existingFields } + ), + }), + { applyImmediately: true } + ); } } diff --git a/x-pack/plugins/lens/public/settings_storage.tsx b/x-pack/plugins/lens/public/settings_storage.tsx index fa59bff166c3097..ebe812915242ede 100644 --- a/x-pack/plugins/lens/public/settings_storage.tsx +++ b/x-pack/plugins/lens/public/settings_storage.tsx @@ -14,5 +14,5 @@ export const readFromStorage = (storage: IStorageWrapper, key: string) => { return data && data[key]; }; export const writeToStorage = (storage: IStorageWrapper, key: string, value: string) => { - storage.set(STORAGE_KEY, { [key]: value }); + storage.set(STORAGE_KEY, { ...storage.get(STORAGE_KEY), [key]: value }); }; diff --git a/x-pack/plugins/lens/public/shared_components/axis_title_settings.tsx b/x-pack/plugins/lens/public/shared_components/axis_title_settings.tsx index edecee61d7709ce..f54b07905b94cfa 100644 --- a/x-pack/plugins/lens/public/shared_components/axis_title_settings.tsx +++ b/x-pack/plugins/lens/public/shared_components/axis_title_settings.tsx @@ -49,10 +49,13 @@ export const AxisTitleSettings: React.FunctionComponent isAxisTitleVisible, toggleAxisTitleVisibility, }) => { - const { inputValue: title, handleInputChange: onTitleChange } = useDebouncedValue({ - value: axisTitle || '', - onChange: updateTitleState, - }); + const { inputValue: title, handleInputChange: onTitleChange } = useDebouncedValue( + { + value: axisTitle || '', + onChange: updateTitleState, + }, + { allowFalsyValue: true } + ); return ( <> diff --git a/x-pack/plugins/lens/public/state_management/context_middleware/index.test.ts b/x-pack/plugins/lens/public/state_management/context_middleware/index.test.ts index f115cb59e6121d5..d256fcf9b11e53f 100644 --- a/x-pack/plugins/lens/public/state_management/context_middleware/index.test.ts +++ b/x-pack/plugins/lens/public/state_management/context_middleware/index.test.ts @@ -10,12 +10,12 @@ import moment from 'moment'; import { contextMiddleware } from '.'; import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public'; -import { initialState } from '../lens_slice'; +import { applyChanges, initialState } from '../lens_slice'; import { LensAppState } from '../types'; import { mockDataPlugin, mockStoreDeps } from '../../mocks'; const storeDeps = mockStoreDeps(); -const createMiddleware = (data: DataPublicPluginStart) => { +const createMiddleware = (data: DataPublicPluginStart, state?: Partial) => { const middleware = contextMiddleware({ ...storeDeps, lensServices: { @@ -24,12 +24,13 @@ const createMiddleware = (data: DataPublicPluginStart) => { }, }); const store = { - getState: jest.fn(() => ({ lens: initialState })), + getState: jest.fn(() => ({ lens: state || initialState })), dispatch: jest.fn(), }; const next = jest.fn(); - const invoke = (action: PayloadAction>) => middleware(store)(next)(action); + const invoke = (action: PayloadAction | void>) => + middleware(store)(next)(action); return { store, next, invoke }; }; @@ -70,6 +71,47 @@ describe('contextMiddleware', () => { }); expect(next).toHaveBeenCalledWith(action); }); + describe('when auto-apply is disabled', () => { + it('only updates searchSessionId when user applies changes', () => { + // setup + const data = mockDataPlugin(); + (data.nowProvider.get as jest.Mock).mockReturnValue(new Date(Date.now() - 30000)); + (data.query.timefilter.timefilter.getTime as jest.Mock).mockReturnValue({ + from: 'now-2m', + to: 'now', + }); + (data.query.timefilter.timefilter.getBounds as jest.Mock).mockReturnValue({ + min: moment(Date.now() - 100000), + max: moment(Date.now() - 30000), + }); + const { invoke, store } = createMiddleware(data, { + ...initialState, + autoApplyDisabled: true, + }); + + // setState shouldn't trigger + const setStateAction = { + type: 'lens/setState', + payload: { + visualization: { + state: {}, + activeId: 'id2', + }, + }, + }; + invoke(setStateAction); + expect(store.dispatch).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'lens/setState' }) + ); + + // applyChanges should trigger + const applyChangesAction = applyChanges(); + invoke(applyChangesAction); + expect(store.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: 'lens/setState' }) + ); + }); + }); it('does not update the searchSessionId when the state changes and too little time has passed', () => { const data = mockDataPlugin(); // time range is 100,000ms ago to 300ms ago (that's a lag of .3 percent, not enough to trigger a session update) diff --git a/x-pack/plugins/lens/public/state_management/context_middleware/index.ts b/x-pack/plugins/lens/public/state_management/context_middleware/index.ts index 25dea5527d06127..3ca806d17dcb782 100644 --- a/x-pack/plugins/lens/public/state_management/context_middleware/index.ts +++ b/x-pack/plugins/lens/public/state_management/context_middleware/index.ts @@ -8,7 +8,14 @@ import { Dispatch, MiddlewareAPI, PayloadAction } from '@reduxjs/toolkit'; import moment from 'moment'; import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public'; -import { setState, LensDispatch, LensStoreDeps, navigateAway } from '..'; +import { + setState, + LensDispatch, + LensStoreDeps, + navigateAway, + applyChanges, + selectAutoApplyEnabled, +} from '..'; import { LensAppState } from '../types'; import { getResolvedDateRange, containsDynamicMath } from '../../utils'; import { subscribeToExternalContext } from './subscribe_to_external_context'; @@ -20,8 +27,12 @@ export const contextMiddleware = (storeDeps: LensStoreDeps) => (store: Middlewar store.getState, store.dispatch ); - return (next: Dispatch) => (action: PayloadAction>) => { - if (!action.payload?.searchSessionId && !onActiveDataChange.match(action)) { + return (next: Dispatch) => (action: PayloadAction) => { + if ( + !(action.payload as Partial)?.searchSessionId && + !onActiveDataChange.match(action) && + (selectAutoApplyEnabled(store.getState()) || applyChanges.match(action)) + ) { updateTimeRange(storeDeps.lensServices.data, store.dispatch); } if (navigateAway.match(action)) { diff --git a/x-pack/plugins/lens/public/state_management/index.ts b/x-pack/plugins/lens/public/state_management/index.ts index bdd1bd8f39cc035..7b9c345ff89f630 100644 --- a/x-pack/plugins/lens/public/state_management/index.ts +++ b/x-pack/plugins/lens/public/state_management/index.ts @@ -20,6 +20,9 @@ export const { loadInitial, navigateAway, setState, + enableAutoApply, + disableAutoApply, + applyChanges, setSaveable, onActiveDataChange, updateState, diff --git a/x-pack/plugins/lens/public/state_management/init_middleware/index.ts b/x-pack/plugins/lens/public/state_management/init_middleware/index.ts index 88a045ed0b506d7..164941d5d5f89fd 100644 --- a/x-pack/plugins/lens/public/state_management/init_middleware/index.ts +++ b/x-pack/plugins/lens/public/state_management/init_middleware/index.ts @@ -9,11 +9,18 @@ import { Dispatch, MiddlewareAPI, PayloadAction } from '@reduxjs/toolkit'; import { LensStoreDeps } from '..'; import { loadInitial as loadInitialAction } from '..'; import { loadInitial } from './load_initial'; +import { readFromStorage } from '../../settings_storage'; +import { Storage } from '../../../../../../src/plugins/kibana_utils/public'; +import { AUTO_APPLY_DISABLED_STORAGE_KEY } from '../../editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper'; + +const autoApplyDisabled = () => { + return readFromStorage(new Storage(localStorage), AUTO_APPLY_DISABLED_STORAGE_KEY) === 'true'; +}; export const initMiddleware = (storeDeps: LensStoreDeps) => (store: MiddlewareAPI) => { return (next: Dispatch) => (action: PayloadAction) => { if (loadInitialAction.match(action)) { - return loadInitial(store, storeDeps, action.payload); + return loadInitial(store, storeDeps, action.payload, autoApplyDisabled()); } next(action); }; diff --git a/x-pack/plugins/lens/public/state_management/init_middleware/load_initial.ts b/x-pack/plugins/lens/public/state_management/init_middleware/load_initial.ts index 372d08017ee2a03..709577594ceae95 100644 --- a/x-pack/plugins/lens/public/state_management/init_middleware/load_initial.ts +++ b/x-pack/plugins/lens/public/state_management/init_middleware/load_initial.ts @@ -9,7 +9,7 @@ import { MiddlewareAPI } from '@reduxjs/toolkit'; import { i18n } from '@kbn/i18n'; import { History } from 'history'; import { setState, initEmpty, LensStoreDeps } from '..'; -import { getPreloadedState } from '../lens_slice'; +import { disableAutoApply, getPreloadedState } from '../lens_slice'; import { SharingSavedObjectProps } from '../../types'; import { LensEmbeddableInput, LensByReferenceInput } from '../../embeddable/embeddable'; import { getInitialDatasourceId } from '../../utils'; @@ -93,7 +93,8 @@ export function loadInitial( redirectCallback: (savedObjectId?: string) => void; initialInput?: LensEmbeddableInput; history?: History; - } + }, + autoApplyDisabled: boolean ) { const { lensServices, datasourceMap, embeddableEditorIncomingState, initialContext } = storeDeps; const { resolvedDateRange, searchSessionId, isLinkedToOriginatingApp, ...emptyState } = @@ -129,6 +130,9 @@ export function loadInitial( initialContext, }) ); + if (autoApplyDisabled) { + store.dispatch(disableAutoApply()); + } }) .catch((e: { message: string }) => { notifications.toasts.addDanger({ @@ -209,6 +213,10 @@ export function loadInitial( isLoading: false, }) ); + + if (autoApplyDisabled) { + store.dispatch(disableAutoApply()); + } }) .catch((e: { message: string }) => notifications.toasts.addDanger({ diff --git a/x-pack/plugins/lens/public/state_management/lens_slice.test.ts b/x-pack/plugins/lens/public/state_management/lens_slice.test.ts index 85061f36ce35e74..4a183c11d896bbb 100644 --- a/x-pack/plugins/lens/public/state_management/lens_slice.test.ts +++ b/x-pack/plugins/lens/public/state_management/lens_slice.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { EnhancedStore } from '@reduxjs/toolkit'; import { Query } from 'src/plugins/data/public'; import { switchDatasource, @@ -16,13 +17,20 @@ import { removeOrClearLayer, addLayer, LensRootStore, + selectTriggerApplyChanges, + selectChangesApplied, } from '.'; import { layerTypes } from '../../common'; import { makeLensStore, defaultState, mockStoreDeps } from '../mocks'; import { DatasourceMap, VisualizationMap } from '../types'; +import { applyChanges, disableAutoApply, enableAutoApply, setChangesApplied } from './lens_slice'; +import { LensAppState } from './types'; describe('lensSlice', () => { - const { store } = makeLensStore({}); + let store: EnhancedStore<{ lens: LensAppState }>; + beforeEach(() => { + store = makeLensStore({}).store; + }); const customQuery = { query: 'custom' } as Query; describe('state update', () => { @@ -34,6 +42,56 @@ describe('lensSlice', () => { expect(changedState).toEqual({ ...defaultState, query: customQuery }); }); + describe('auto-apply-related actions', () => { + it('should disable auto apply', () => { + expect(store.getState().lens.autoApplyDisabled).toBeUndefined(); + expect(store.getState().lens.changesApplied).toBeUndefined(); + + store.dispatch(disableAutoApply()); + + expect(store.getState().lens.autoApplyDisabled).toBe(true); + expect(store.getState().lens.changesApplied).toBe(true); + }); + + it('should enable auto-apply', () => { + store.dispatch(disableAutoApply()); + + expect(store.getState().lens.autoApplyDisabled).toBe(true); + + store.dispatch(enableAutoApply()); + + expect(store.getState().lens.autoApplyDisabled).toBe(false); + }); + + it('applies changes when auto-apply disabled', () => { + store.dispatch(disableAutoApply()); + + store.dispatch(applyChanges()); + + expect(selectTriggerApplyChanges(store.getState())).toBe(true); + }); + + it('does not apply changes if auto-apply enabled', () => { + expect(store.getState().lens.autoApplyDisabled).toBeUndefined(); + + store.dispatch(applyChanges()); + + expect(selectTriggerApplyChanges(store.getState())).toBe(false); + }); + + it('sets changes-applied flag', () => { + expect(store.getState().lens.changesApplied).toBeUndefined(); + + store.dispatch(setChangesApplied(true)); + + expect(selectChangesApplied(store.getState())).toBe(true); + + store.dispatch(setChangesApplied(false)); + + expect(selectChangesApplied(store.getState())).toBe(true); + }); + }); + it('updateState: updates state with updater', () => { const customUpdater = jest.fn((state) => ({ ...state, query: customQuery })); store.dispatch(updateState({ updater: customUpdater })); diff --git a/x-pack/plugins/lens/public/state_management/lens_slice.ts b/x-pack/plugins/lens/public/state_management/lens_slice.ts index 099929cdf47962a..56ff89f506c8581 100644 --- a/x-pack/plugins/lens/public/state_management/lens_slice.ts +++ b/x-pack/plugins/lens/public/state_management/lens_slice.ts @@ -83,6 +83,10 @@ export const getPreloadedState = ({ export const setState = createAction>('lens/setState'); export const onActiveDataChange = createAction('lens/onActiveDataChange'); export const setSaveable = createAction('lens/setSaveable'); +export const enableAutoApply = createAction('lens/enableAutoApply'); +export const disableAutoApply = createAction('lens/disableAutoApply'); +export const applyChanges = createAction('lens/applyChanges'); +export const setChangesApplied = createAction('lens/setChangesApplied'); export const updateState = createAction<{ updater: (prevState: LensAppState) => LensAppState; }>('lens/updateState'); @@ -162,6 +166,10 @@ export const lensActions = { setState, onActiveDataChange, setSaveable, + enableAutoApply, + disableAutoApply, + applyChanges, + setChangesApplied, updateState, updateDatasourceState, updateVisualizationState, @@ -202,6 +210,22 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { isSaveable: payload, }; }, + [enableAutoApply.type]: (state) => { + state.autoApplyDisabled = false; + }, + [disableAutoApply.type]: (state) => { + state.autoApplyDisabled = true; + state.changesApplied = true; + }, + [applyChanges.type]: (state) => { + if (typeof state.applyChangesCounter === 'undefined') { + state.applyChangesCounter = 0; + } + state.applyChangesCounter!++; + }, + [setChangesApplied.type]: (state, { payload: applied }) => { + state.changesApplied = applied; + }, [updateState.type]: ( state, { diff --git a/x-pack/plugins/lens/public/state_management/selectors.test.ts b/x-pack/plugins/lens/public/state_management/selectors.test.ts new file mode 100644 index 000000000000000..2313d341b7e03ee --- /dev/null +++ b/x-pack/plugins/lens/public/state_management/selectors.test.ts @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { LensAppState, selectTriggerApplyChanges, selectChangesApplied } from '.'; + +describe('lens selectors', () => { + describe('selecting changes applied', () => { + it('should be true when auto-apply disabled and flag is set', () => { + const lensState = { + changesApplied: true, + autoApplyDisabled: true, + } as Partial; + + expect(selectChangesApplied({ lens: lensState as LensAppState })).toBeTruthy(); + }); + + it('should be false when auto-apply disabled and flag is false', () => { + const lensState = { + changesApplied: false, + autoApplyDisabled: true, + } as Partial; + + expect(selectChangesApplied({ lens: lensState as LensAppState })).toBeFalsy(); + }); + + it('should be true when auto-apply enabled no matter what', () => { + const lensState = { + changesApplied: false, + autoApplyDisabled: false, + } as Partial; + + expect(selectChangesApplied({ lens: lensState as LensAppState })).toBeTruthy(); + }); + }); + it('should select apply changes trigger', () => { + selectTriggerApplyChanges({ lens: { applyChangesCounter: 1 } as LensAppState }); // get the counters in sync + + expect( + selectTriggerApplyChanges({ lens: { applyChangesCounter: 2 } as LensAppState }) + ).toBeTruthy(); + expect( + selectTriggerApplyChanges({ lens: { applyChangesCounter: 2 } as LensAppState }) + ).toBeFalsy(); + }); +}); diff --git a/x-pack/plugins/lens/public/state_management/selectors.ts b/x-pack/plugins/lens/public/state_management/selectors.ts index 250e9dde31373dc..26a0d70d068f5c8 100644 --- a/x-pack/plugins/lens/public/state_management/selectors.ts +++ b/x-pack/plugins/lens/public/state_management/selectors.ts @@ -19,12 +19,22 @@ export const selectFilters = (state: LensState) => state.lens.filters; export const selectResolvedDateRange = (state: LensState) => state.lens.resolvedDateRange; export const selectVisualization = (state: LensState) => state.lens.visualization; export const selectStagedPreview = (state: LensState) => state.lens.stagedPreview; +export const selectAutoApplyEnabled = (state: LensState) => !state.lens.autoApplyDisabled; +export const selectChangesApplied = (state: LensState) => + !state.lens.autoApplyDisabled || Boolean(state.lens.changesApplied); export const selectDatasourceStates = (state: LensState) => state.lens.datasourceStates; export const selectActiveDatasourceId = (state: LensState) => state.lens.activeDatasourceId; export const selectActiveData = (state: LensState) => state.lens.activeData; export const selectIsFullscreenDatasource = (state: LensState) => Boolean(state.lens.isFullscreenDatasource); +let applyChangesCounter: number | undefined; +export const selectTriggerApplyChanges = (state: LensState) => { + const shouldApply = state.lens.applyChangesCounter !== applyChangesCounter; + applyChangesCounter = state.lens.applyChangesCounter; + return shouldApply; +}; + export const selectExecutionContext = createSelector( [selectQuery, selectFilters, selectResolvedDateRange], (query, filters, dateRange) => ({ diff --git a/x-pack/plugins/lens/public/state_management/types.ts b/x-pack/plugins/lens/public/state_management/types.ts index b0ff49862d9b838..0c902f944072d04 100644 --- a/x-pack/plugins/lens/public/state_management/types.ts +++ b/x-pack/plugins/lens/public/state_management/types.ts @@ -33,6 +33,9 @@ export interface PreviewState { export interface EditorFrameState extends PreviewState { activeDatasourceId: string | null; stagedPreview?: PreviewState; + autoApplyDisabled?: boolean; + applyChangesCounter?: number; + changesApplied?: boolean; isFullscreenDatasource?: boolean; } export interface LensAppState extends EditorFrameState { diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 276c31328bb0507..7047201c5dba389 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -160,7 +160,12 @@ export interface DatasourceSuggestion { keptLayerIds: string[]; } -export type StateSetter = (newState: T | ((prevState: T) => T)) => void; +type StateSetterArg = T | ((prevState: T) => T); + +export type StateSetter = ( + newState: StateSetterArg, + options?: OptionsShape +) => void; export interface InitializationOptions { isFullEditor?: boolean; @@ -361,7 +366,7 @@ export interface DatasourcePublicAPI { export interface DatasourceDataPanelProps { state: T; dragDropContext: DragContextState; - setState: StateSetter; + setState: StateSetter; showNoDataPopover: () => void; core: Pick; query: Query; @@ -400,13 +405,13 @@ export type ParamEditorCustomProps = Record & { label?: string // The only way a visualization has to restrict the query building export type DatasourceDimensionEditorProps = DatasourceDimensionProps & { // Not a StateSetter because we have this unique use case of determining valid columns - setState: ( - newState: Parameters>[0], - publishToVisualization?: { + setState: StateSetter< + T, + { isDimensionComplete?: boolean; forceRender?: boolean; } - ) => void; + >; core: Pick; dateRange: DateRange; dimensionGroups: VisualizationDimensionGroupConfig[]; @@ -449,7 +454,13 @@ export type DatasourceDimensionDropProps = SharedDimensionProps & { groupId: string; columnId: string; state: T; - setState: StateSetter; + setState: StateSetter< + T, + { + isDimensionComplete?: boolean; + forceRender?: boolean; + } + >; dimensionGroups: VisualizationDimensionGroupConfig[]; }; @@ -651,6 +662,7 @@ export interface VisualizationSuggestion { export interface FramePublicAPI { datasourceLayers: Record; + appliedDatasourceLayers?: Record; // this is only set when auto-apply is turned off /** * Data of the chart currently rendered in the preview. * This data might be not available (e.g. if the chart can't be rendered) or outdated and belonging to another chart. diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx index 20d2bd31c7c6489..3766e1f022c8828 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/axis_settings_popover.tsx @@ -18,6 +18,7 @@ import { EuiFieldNumber, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import { isEqual } from 'lodash'; import { XYLayerConfig, AxesSettingsConfig, AxisExtentConfig } from '../../../common/expressions'; import { ToolbarPopover, @@ -241,7 +242,7 @@ export const AxisSettingsPopover: React.FunctionComponent = { type: 'long', _meta: { description: 'Number of times the user opened the in-product formula help popover.' }, }, + toggle_autoapply: { + type: 'long', + _meta: { + description: 'Number of times the user toggled auto-apply.', + }, + }, toggle_fullscreen_formula: { type: 'long', _meta: { diff --git a/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json b/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json index e0dd709a54e5743..f7a82f69ae817ce 100644 --- a/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json +++ b/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json @@ -3720,6 +3720,12 @@ "description": "Number of times the user toggled fullscreen mode on formula." } }, + "toggle_autoapply": { + "type": "long", + "_meta": { + "description": "Number of times the user toggled auto-apply." + } + }, "indexpattern_field_info_click": { "type": "long" }, @@ -3967,6 +3973,12 @@ "description": "Number of times the user toggled fullscreen mode on formula." } }, + "toggle_autoapply": { + "type": "long", + "_meta": { + "description": "Number of times the user toggled auto-apply." + } + }, "indexpattern_field_info_click": { "type": "long" }, diff --git a/x-pack/test/functional/apps/lens/disable_auto_apply.ts b/x-pack/test/functional/apps/lens/disable_auto_apply.ts new file mode 100644 index 000000000000000..3660de10ecd477e --- /dev/null +++ b/x-pack/test/functional/apps/lens/disable_auto_apply.ts @@ -0,0 +1,99 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const PageObjects = getPageObjects(['lens', 'visualize']); + const browser = getService('browser'); + const testSubjects = getService('testSubjects'); + const retry = getService('retry'); + + describe('lens disable auto-apply tests', () => { + it('should persist auto-apply setting across page refresh', async () => { + await PageObjects.visualize.navigateToNewVisualization(); + await PageObjects.visualize.clickVisType('lens'); + + expect(await PageObjects.lens.getAutoApplyEnabled()).to.be.ok(); + + await PageObjects.lens.disableAutoApply(); + + expect(await PageObjects.lens.getAutoApplyEnabled()).not.to.be.ok(); + + await browser.refresh(); + PageObjects.lens.waitForEmptyWorkspace(); + + expect(await PageObjects.lens.getAutoApplyEnabled()).not.to.be.ok(); + + await PageObjects.lens.enableAutoApply(); + + expect(await PageObjects.lens.getAutoApplyEnabled()).to.be.ok(); + + await browser.refresh(); + PageObjects.lens.waitForEmptyWorkspace(); + + expect(await PageObjects.lens.getAutoApplyEnabled()).to.be.ok(); + + await PageObjects.lens.disableAutoApply(); + + expect(await PageObjects.lens.getAutoApplyEnabled()).not.to.be.ok(); + }); + + it('should preserve auto-apply controls with full-screen datasource', async () => { + await PageObjects.lens.goToTimeRange(); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'formula', + formula: `count()`, + keepOpen: true, + }); + + PageObjects.lens.toggleFullscreen(); + + expect(await PageObjects.lens.getAutoApplyToggleExists()).to.be.ok(); + + PageObjects.lens.toggleFullscreen(); + + PageObjects.lens.closeDimensionEditor(); + }); + + it('should apply changes when "Apply" is clicked', async () => { + await retry.waitForWithTimeout('x dimension to be available', 1000, () => + testSubjects + .existOrFail('lnsXY_xDimensionPanel > lns-empty-dimension') + .then(() => true) + .catch(() => false) + ); + + // configureDimension + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension', + operation: 'date_histogram', + field: '@timestamp', + }); + + // assert that changes haven't been applied + await PageObjects.lens.waitForEmptyWorkspace(); + + await PageObjects.lens.applyChanges(); + + await PageObjects.lens.waitForVisualization(); + }); + + it('should hide suggestions when a change is made', async () => { + await PageObjects.lens.switchToVisualization('lnsDatatable'); + + expect(await PageObjects.lens.getAreSuggestionsPromptingToApply()).to.be.ok(); + + await PageObjects.lens.applyChanges(true); + + expect(await PageObjects.lens.getAreSuggestionsPromptingToApply()).not.to.be.ok(); + }); + }); +} diff --git a/x-pack/test/functional/apps/lens/index.ts b/x-pack/test/functional/apps/lens/index.ts index 45c53ea18a60168..3687aab7bfb69bd 100644 --- a/x-pack/test/functional/apps/lens/index.ts +++ b/x-pack/test/functional/apps/lens/index.ts @@ -66,6 +66,7 @@ export default function ({ getService, loadTestFile, getPageObjects }: FtrProvid loadTestFile(require.resolve('./chart_data')); loadTestFile(require.resolve('./time_shift')); loadTestFile(require.resolve('./drag_and_drop')); + loadTestFile(require.resolve('./disable_auto_apply')); loadTestFile(require.resolve('./geo_field')); loadTestFile(require.resolve('./formula')); loadTestFile(require.resolve('./heatmap')); diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index 61b0cd10750b239..1898ef10345e6a7 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -1330,5 +1330,33 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont hasEmptySizeRatioButtonGroup() { return testSubjects.exists('lnsEmptySizeRatioButtonGroup'); }, + + getAutoApplyToggleExists() { + return testSubjects.exists('lensToggleAutoApply'); + }, + + enableAutoApply() { + return testSubjects.setEuiSwitch('lensToggleAutoApply', 'check'); + }, + + disableAutoApply() { + return testSubjects.setEuiSwitch('lensToggleAutoApply', 'uncheck'); + }, + + getAutoApplyEnabled() { + return testSubjects.isEuiSwitchChecked('lensToggleAutoApply'); + }, + + async applyChanges(throughSuggestions = false) { + const applyButtonSelector = throughSuggestions + ? 'lnsSuggestionApplyChanges' + : 'lensApplyChanges'; + await testSubjects.waitForEnabled(applyButtonSelector); + await testSubjects.click(applyButtonSelector); + }, + + async getAreSuggestionsPromptingToApply() { + return testSubjects.exists('lnsSuggestionApplyChanges'); + }, }); }