[Lens] Fix filters reappearing in the saved object when saving (#110460)

* bugs fixed * test for loading * fix workspace panel * Update x-pack/plugins/lens/public/xy_visualization/visualization.tsx * revert useEffect for external embeddables
2025-04-23 09:19:04 -04:00 · 2021-08-31 11:51:54 +02:00 · 2021-08-31 11:51:54 +02:00 · 03469515cf
commit 03469515cf
parent 7ebed9321a
9 changed files with 177 additions and 135 deletions
--- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx
+++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx
@ -37,6 +37,7 @@ import {
  navigateAway,
  LensRootStore,
  loadInitial,
+  LensAppState,
  LensState,
 } from '../state_management';
 import { getPreloadedState } from '../state_management/lens_slice';
@ -186,8 +187,9 @@ export async function mountApp(
    embeddableEditorIncomingState,
    initialContext,
  };
+  const emptyState = getPreloadedState(storeDeps) as LensAppState;
  const lensStore: LensRootStore = makeConfigureStore(storeDeps, {
-    lens: getPreloadedState(storeDeps),
+    lens: emptyState,
  } as DeepPartial<LensState>);

  const EditorRenderer = React.memo(
@ -200,7 +202,8 @@ export async function mountApp(
      );
      trackUiEvent('loaded');
      const initialInput = getInitialInput(props.id, props.editByValue);
-      lensStore.dispatch(loadInitial({ redirectCallback, initialInput }));
+
+      lensStore.dispatch(loadInitial({ redirectCallback, initialInput, emptyState }));

      return (
        <Provider store={lensStore}>
--- a/x-pack/plugins/lens/public/app_plugin/save_modal_container.tsx
+++ b/x-pack/plugins/lens/public/app_plugin/save_modal_container.tsx
@ -10,18 +10,18 @@ import { i18n } from '@kbn/i18n';
 import { METRIC_TYPE } from '@kbn/analytics';
 import { partition } from 'lodash';

-import type { ChromeStart, NotificationsStart, SavedObjectReference } from 'kibana/public';
+import type { SavedObjectReference } from 'kibana/public';
 import { SaveModal } from './save_modal';
 import type { LensAppProps, LensAppServices } from './types';
 import type { SaveProps } from './app';
 import { Document, injectFilterReferences } from '../persistence';
 import type { LensByReferenceInput, LensEmbeddableInput } from '../embeddable';
-import type { LensAttributeService } from '../lens_attribute_service';
-import { DataPublicPluginStart, esFilters } from '../../../../../src/plugins/data/public';
+import { esFilters } from '../../../../../src/plugins/data/public';
 import { APP_ID, getFullPath, LENS_EMBEDDABLE_TYPE } from '../../common';
 import { trackUiEvent } from '../lens_ui_telemetry';
 import { checkForDuplicateTitle } from '../../../../../src/plugins/saved_objects/public';
 import type { LensAppState } from '../state_management';
+import { getPersisted } from '../state_management/init_middleware/load_initial';

 type ExtraProps = Pick<LensAppProps, 'initialInput'> &
  Partial<Pick<LensAppProps, 'redirectToOrigin' | 'redirectTo' | 'onAppLeave'>>;
@ -51,54 +51,41 @@ export function SaveModalContainer({
  redirectToOrigin,
  getAppNameFromId = () => undefined,
  isSaveable = true,
-  lastKnownDoc: initLastKnowDoc,
+  lastKnownDoc: initLastKnownDoc,
  lensServices,
 }: SaveModalContainerProps) {
-  const [lastKnownDoc, setLastKnownDoc] = useState<Document | undefined>(initLastKnowDoc);
  let title = '';
  let description;
  let savedObjectId;
+  const [lastKnownDoc, setLastKnownDoc] = useState<Document | undefined>(initLastKnownDoc);
  if (lastKnownDoc) {
    title = lastKnownDoc.title;
    description = lastKnownDoc.description;
    savedObjectId = lastKnownDoc.savedObjectId;
  }

-  const {
-    attributeService,
-    notifications,
-    data,
-    chrome,
-    savedObjectsTagging,
-    application,
-    dashboardFeatureFlag,
-  } = lensServices;
+  const { attributeService, savedObjectsTagging, application, dashboardFeatureFlag } = lensServices;

  useEffect(() => {
-    setLastKnownDoc(initLastKnowDoc);
-  }, [initLastKnowDoc]);
+    setLastKnownDoc(initLastKnownDoc);
+  }, [initLastKnownDoc]);

  useEffect(() => {
    let isMounted = true;
-    async function loadPersistedDoc() {
-      if (initialInput) {
-        getPersistedDoc({
-          data,
-          initialInput,
-          chrome,
-          notifications,
-          attributeService,
-        }).then((doc) => {
-          if (doc && isMounted) setLastKnownDoc(doc);
-        });
-      }
+
+    if (initialInput) {
+      getPersisted({
+        initialInput,
+        lensServices,
+      }).then((persisted) => {
+        if (persisted?.doc && isMounted) setLastKnownDoc(persisted.doc);
+      });
    }

-    loadPersistedDoc();
    return () => {
      isMounted = false;
    };
-  }, [chrome, data, initialInput, notifications, attributeService]);
+  }, [initialInput, lensServices]);

  const tagsIds =
    persistedDoc && savedObjectsTagging
@ -109,27 +96,25 @@ export function SaveModalContainer({
    if (runSave) {
      // inside lens, we use the function that's passed to it
      runSave(saveProps, options);
-    } else {
-      if (attributeService && lastKnownDoc) {
-        runSaveLensVisualization(
-          {
-            ...lensServices,
-            lastKnownDoc,
-            initialInput,
-            attributeService,
-            redirectTo,
-            redirectToOrigin,
-            originatingApp,
-            getIsByValueMode: () => false,
-            onAppLeave: () => {},
-          },
-          saveProps,
-          options
-        ).then(() => {
-          onSave?.();
-          onClose();
-        });
-      }
+    } else if (attributeService && lastKnownDoc) {
+      runSaveLensVisualization(
+        {
+          ...lensServices,
+          lastKnownDoc,
+          initialInput,
+          attributeService,
+          redirectTo,
+          redirectToOrigin,
+          originatingApp,
+          getIsByValueMode: () => false,
+          onAppLeave: () => {},
+        },
+        saveProps,
+        options
+      ).then(() => {
+        onSave?.();
+        onClose();
+      });
    }
  };

@ -384,51 +369,5 @@ export function getLastKnownDocWithoutPinnedFilters(doc?: Document) {
    : doc;
 }

-export const getPersistedDoc = async ({
-  initialInput,
-  attributeService,
-  data,
-  notifications,
-  chrome,
-}: {
-  initialInput: LensEmbeddableInput;
-  attributeService: LensAttributeService;
-  data: DataPublicPluginStart;
-  notifications: NotificationsStart;
-  chrome: ChromeStart;
-}): Promise<Document | undefined> => {
-  let doc: Document;
-
-  try {
-    const attributes = await attributeService.unwrapAttributes(initialInput);
-
-    doc = {
-      ...initialInput,
-      ...attributes,
-      type: LENS_EMBEDDABLE_TYPE,
-    };
-
-    if (attributeService.inputIsRefType(initialInput)) {
-      chrome.recentlyAccessed.add(
-        getFullPath(initialInput.savedObjectId),
-        attributes.title,
-        initialInput.savedObjectId
-      );
-    }
-
-    // Don't overwrite any pinned filters
-    data.query.filterManager.setAppFilters(
-      injectFilterReferences(doc.state.filters, doc.references)
-    );
-    return doc;
-  } catch (e) {
-    notifications.toasts.addDanger(
-      i18n.translate('xpack.lens.app.docLoadingError', {
-        defaultMessage: 'Error loading saved document',
-      })
-    );
-  }
-};
-
 // eslint-disable-next-line import/no-default-export
 export default SaveModalContainer;
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx
+++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx
@ -230,7 +230,7 @@ describe('editor_frame', () => {
        await mountWithProvider(<EditorFrame {...props} />, {
          data: props.plugins.data,
          preloadedState: {
-            visualization: { activeId: 'testVis', state: null },
+            visualization: { activeId: 'testVis', state: {} },
            datasourceStates: {
              testDatasource: {
                isLoading: false,
@ -285,7 +285,7 @@ describe('editor_frame', () => {
        await mountWithProvider(<EditorFrame {...props} />, {
          data: props.plugins.data,
          preloadedState: {
-            visualization: { activeId: 'testVis', state: null },
+            visualization: { activeId: 'testVis', state: {} },
            datasourceStates: {
              testDatasource: {
                isLoading: false,
--- 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
@ -45,7 +45,8 @@ export function EditorFrame(props: EditorFrameProps) {
  const activeDatasourceId = useLensSelector(selectActiveDatasourceId);
  const datasourceStates = useLensSelector(selectDatasourceStates);
  const visualization = useLensSelector(selectVisualization);
-  const allLoaded = useLensSelector(selectAreDatasourcesLoaded);
+  const areDatasourcesLoaded = useLensSelector(selectAreDatasourcesLoaded);
+  const isVisualizationLoaded = !!visualization.state;
  const framePublicAPI: FramePublicAPI = useLensSelector((state) =>
    selectFramePublicAPI(state, datasourceMap)
  );
@ -95,7 +96,7 @@ export function EditorFrame(props: EditorFrameProps) {
          />
        }
        configPanel={
-          allLoaded && (
+          areDatasourcesLoaded && (
            <ConfigPanelWrapper
              core={props.core}
              datasourceMap={datasourceMap}
@ -105,7 +106,8 @@ export function EditorFrame(props: EditorFrameProps) {
          )
        }
        workspacePanel={
-          allLoaded && (
+          areDatasourcesLoaded &&
+          isVisualizationLoaded && (
            <WorkspacePanel
              core={props.core}
              plugins={props.plugins}
@ -119,7 +121,7 @@ export function EditorFrame(props: EditorFrameProps) {
          )
        }
        suggestionsPanel={
-          allLoaded && (
+          areDatasourcesLoaded && (
            <SuggestionPanelWrapper
              ExpressionRenderer={props.ExpressionRenderer}
              datasourceMap={datasourceMap}
--- a/x-pack/plugins/lens/public/mocks.tsx
+++ b/x-pack/plugins/lens/public/mocks.tsx
@ -208,12 +208,14 @@ export const defaultDoc = ({
  savedObjectId: '1234',
  title: 'An extremely cool default document!',
  expression: 'definitely a valid expression',
+  visualizationType: 'testVis',
  state: {
    query: 'kuery',
    filters: [{ query: { match_phrase: { src: 'test' } } }],
    datasourceStates: {
      testDatasource: 'datasource',
    },
+    visualization: {},
  },
  references: [{ type: 'index-pattern', id: '1', name: 'index-pattern-0' }],
 } as unknown) as Document;
--- a/x-pack/plugins/lens/public/state_management/init_middleware/index.ts
+++ b/x-pack/plugins/lens/public/state_management/init_middleware/index.ts
@ -23,7 +23,8 @@ export const initMiddleware = (storeDeps: LensStoreDeps) => (store: MiddlewareAP
        store,
        storeDeps,
        action.payload.redirectCallback,
-        action.payload.initialInput
+        action.payload.initialInput,
+        action.payload.emptyState
      );
    } else if (lensSlice.actions.navigateAway.match(action)) {
      return unsubscribeFromExternalContext();
--- a/x-pack/plugins/lens/public/state_management/init_middleware/load_initial.test.tsx
+++ b/x-pack/plugins/lens/public/state_management/init_middleware/load_initial.test.tsx
@ -15,6 +15,8 @@ import {
 import { act } from 'react-dom/test-utils';
 import { loadInitial } from './load_initial';
 import { LensEmbeddableInput } from '../../embeddable';
+import { getPreloadedState } from '../lens_slice';
+import { LensAppState } from '..';

 const defaultSavedObjectId = '1234';
 const preloadedState = {
@ -63,7 +65,6 @@ describe('Mounter', () => {

  it('should initialize initial datasource', async () => {
    const services = makeDefaultServices();
-    const redirectCallback = jest.fn();
    services.attributeService.unwrapAttributes = jest.fn().mockResolvedValue(defaultDoc);

    const lensStore = await makeLensStore({
@ -78,7 +79,7 @@ describe('Mounter', () => {
          datasourceMap,
          visualizationMap,
        },
-        redirectCallback,
+        jest.fn(),
        ({ savedObjectId: defaultSavedObjectId } as unknown) as LensEmbeddableInput
      );
    });
@ -87,7 +88,6 @@ describe('Mounter', () => {

  it('should have initialized only the initial datasource and visualization', async () => {
    const services = makeDefaultServices();
-    const redirectCallback = jest.fn();
    services.attributeService.unwrapAttributes = jest.fn().mockResolvedValue(defaultDoc);

    const lensStore = await makeLensStore({ data: services.data, preloadedState });
@ -99,7 +99,7 @@ describe('Mounter', () => {
          datasourceMap,
          visualizationMap,
        },
-        redirectCallback
+        jest.fn()
      );
    });
    expect(mockDatasource.initialize).toHaveBeenCalled();
@ -121,7 +121,6 @@ describe('Mounter', () => {
  describe('loadInitial', () => {
    it('does not load a document if there is no initial input', async () => {
      const services = makeDefaultServices();
-      const redirectCallback = jest.fn();
      const lensStore = makeLensStore({ data: services.data, preloadedState });
      await loadInitial(
        lensStore,
@ -130,14 +129,66 @@ describe('Mounter', () => {
          datasourceMap,
          visualizationMap,
        },
-        redirectCallback
+        jest.fn()
      );
      expect(services.attributeService.unwrapAttributes).not.toHaveBeenCalled();
    });

+    it('cleans datasource and visualization state properly when reloading', async () => {
+      const services = makeDefaultServices();
+      const storeDeps = {
+        lensServices: services,
+        datasourceMap,
+        visualizationMap,
+      };
+      services.attributeService.unwrapAttributes = jest.fn().mockResolvedValue(defaultDoc);
+      const lensStore = await makeLensStore({
+        data: services.data,
+        preloadedState: {
+          ...preloadedState,
+          visualization: {
+            activeId: 'testVis',
+            state: {},
+          },
+          datasourceStates: { testDatasource: { isLoading: false, state: {} } },
+        },
+      });
+
+      expect(lensStore.getState()).toEqual({
+        lens: expect.objectContaining({
+          visualization: {
+            activeId: 'testVis',
+            state: {},
+          },
+          activeDatasourceId: 'testDatasource',
+          datasourceStates: {
+            testDatasource: { isLoading: false, state: {} },
+          },
+        }),
+      });
+
+      const emptyState = getPreloadedState(storeDeps) as LensAppState;
+      services.attributeService.unwrapAttributes = jest.fn();
+      await act(async () => {
+        await loadInitial(lensStore, storeDeps, jest.fn(), undefined, emptyState);
+      });
+
+      expect(lensStore.getState()).toEqual({
+        lens: expect.objectContaining({
+          visualization: {
+            activeId: 'testVis',
+            state: null, // resets to null
+          },
+          activeDatasourceId: 'testDatasource2', // resets to first on the list
+          datasourceStates: {
+            testDatasource: { isLoading: false, state: undefined }, // state resets to undefined
+          },
+        }),
+      });
+    });
+
    it('loads a document and uses query and filters if initial input is provided', async () => {
      const services = makeDefaultServices();
-      const redirectCallback = jest.fn();
      services.attributeService.unwrapAttributes = jest.fn().mockResolvedValue(defaultDoc);

      const lensStore = await makeLensStore({ data: services.data, preloadedState });
@ -149,7 +200,7 @@ describe('Mounter', () => {
            datasourceMap,
            visualizationMap,
          },
-          redirectCallback,
+          jest.fn(),
          ({ savedObjectId: defaultSavedObjectId } as unknown) as LensEmbeddableInput
        );
      });
@ -173,7 +224,6 @@ describe('Mounter', () => {
    });

    it('does not load documents on sequential renders unless the id changes', async () => {
-      const redirectCallback = jest.fn();
      const services = makeDefaultServices();
      const lensStore = makeLensStore({ data: services.data, preloadedState });

@ -185,7 +235,7 @@ describe('Mounter', () => {
            datasourceMap,
            visualizationMap,
          },
-          redirectCallback,
+          jest.fn(),
          ({ savedObjectId: defaultSavedObjectId } as unknown) as LensEmbeddableInput
        );
      });
@ -198,7 +248,7 @@ describe('Mounter', () => {
            datasourceMap,
            visualizationMap,
          },
-          redirectCallback,
+          jest.fn(),
          ({ savedObjectId: defaultSavedObjectId } as unknown) as LensEmbeddableInput
        );
      });
@ -213,7 +263,7 @@ describe('Mounter', () => {
            datasourceMap,
            visualizationMap,
          },
-          redirectCallback,
+          jest.fn(),
          ({ savedObjectId: '5678' } as unknown) as LensEmbeddableInput
        );
      });
@ -249,8 +299,6 @@ describe('Mounter', () => {
    });

    it('adds to the recently accessed list on load', async () => {
-      const redirectCallback = jest.fn();
-
      const services = makeDefaultServices();
      const lensStore = makeLensStore({ data: services.data, preloadedState });
      await act(async () => {
@ -261,7 +309,7 @@ describe('Mounter', () => {
            datasourceMap,
            visualizationMap,
          },
-          redirectCallback,
+          jest.fn(),
          ({ savedObjectId: defaultSavedObjectId } as unknown) as LensEmbeddableInput
        );
      });
--- 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
@ -7,7 +7,8 @@

 import { MiddlewareAPI } from '@reduxjs/toolkit';
 import { isEqual } from 'lodash';
-import { setState } from '..';
+import { i18n } from '@kbn/i18n';
+import { LensAppState, setState } from '..';
 import { updateLayer, updateVisualizationState, LensStoreDeps } from '..';
 import { LensEmbeddableInput, LensByReferenceInput } from '../../embeddable/embeddable';
 import { getInitialDatasourceId } from '../../utils';
@ -17,7 +18,40 @@ import {
  getVisualizeFieldSuggestions,
  switchToSuggestion,
 } from '../../editor_frame_service/editor_frame/suggestion_helpers';
-import { getPersistedDoc } from '../../app_plugin/save_modal_container';
+import { LensAppServices } from '../../app_plugin/types';
+import { getFullPath, LENS_EMBEDDABLE_TYPE } from '../../../common/constants';
+import { Document, injectFilterReferences } from '../../persistence';
+
+export const getPersisted = async ({
+  initialInput,
+  lensServices,
+}: {
+  initialInput: LensEmbeddableInput;
+  lensServices: LensAppServices;
+}): Promise<{ doc: Document } | undefined> => {
+  const { notifications, attributeService } = lensServices;
+  let doc: Document;
+
+  try {
+    const attributes = await attributeService.unwrapAttributes(initialInput);
+
+    doc = {
+      ...initialInput,
+      ...attributes,
+      type: LENS_EMBEDDABLE_TYPE,
+    };
+
+    return {
+      doc,
+    };
+  } catch (e) {
+    notifications.toasts.addDanger(
+      i18n.translate('xpack.lens.app.docLoadingError', {
+        defaultMessage: 'Error loading saved document',
+      })
+    );
+  }
+};

 export function loadInitial(
  store: MiddlewareAPI,
@ -29,11 +63,13 @@ export function loadInitial(
    initialContext,
  }: LensStoreDeps,
  redirectCallback: (savedObjectId?: string) => void,
-  initialInput?: LensEmbeddableInput
+  initialInput?: LensEmbeddableInput,
+  emptyState?: LensAppState
 ) {
  const { getState, dispatch } = store;
-  const { attributeService, chrome, notifications, data, dashboardFeatureFlag } = lensServices;
+  const { attributeService, notifications, data, dashboardFeatureFlag } = lensServices;
  const { persistedDoc } = getState().lens;
+
  if (
    !initialInput ||
    (attributeService.inputIsRefType(initialInput) &&
@ -61,6 +97,7 @@ export function loadInitial(
        );
        dispatch(
          setState({
+            ...emptyState,
            datasourceStates,
            isLoading: false,
          })
@ -109,17 +146,23 @@ export function loadInitial(
        redirectCallback();
      });
  }
-  getPersistedDoc({
-    initialInput,
-    attributeService,
-    data,
-    chrome,
-    notifications,
-  })
+  getPersisted({ initialInput, lensServices })
    .then(
-      (doc) => {
-        if (doc) {
-          const currentSessionId = data.search.session.getSessionId();
+      (persisted) => {
+        if (persisted) {
+          const { doc } = persisted;
+          if (attributeService.inputIsRefType(initialInput)) {
+            lensServices.chrome.recentlyAccessed.add(
+              getFullPath(initialInput.savedObjectId),
+              doc.title,
+              initialInput.savedObjectId
+            );
+          }
+          // Don't overwrite any pinned filters
+          data.query.filterManager.setAppFilters(
+            injectFilterReferences(doc.state.filters, doc.references)
+          );
+
          const docDatasourceStates = Object.entries(doc.state.datasourceStates).reduce(
            (stateMap, [datasourceId, datasourceState]) => ({
              ...stateMap,
@ -143,6 +186,8 @@ export function loadInitial(
            .then((result) => {
              const activeDatasourceId = getInitialDatasourceId(datasourceMap, doc);

+              const currentSessionId = data.search.session.getSessionId();
+
              dispatch(
                setState({
                  query: doc.state.query,
--- a/x-pack/plugins/lens/public/state_management/lens_slice.ts
+++ b/x-pack/plugins/lens/public/state_management/lens_slice.ts
@ -12,6 +12,7 @@ import { getInitialDatasourceId, getResolvedDateRange } from '../utils';
 import { LensAppState, LensStoreDeps } from './types';

 export const initialState: LensAppState = {
+  persistedDoc: undefined,
  searchSessionId: '',
  filters: [],
  query: { language: 'kuery', query: '' },
@ -299,6 +300,7 @@ export const lensSlice = createSlice({
      payload: PayloadAction<{
        initialInput?: LensEmbeddableInput;
        redirectCallback: (savedObjectId?: string) => void;
+        emptyState: LensAppState;
      }>
    ) => state,
  },