[Textbased] Refactor flyout state management (#161256)

## Summary This PR doesn't introduce a new feature to the users. It is mostly a refactoring for the edit flyout. This change will make it easier to make the flyout work with the formBased charts as it uses the exact same functions as the mounter (Lens editor) ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
2025-04-24 01:38:56 -04:00 · 2023-07-13 11:57:13 +03:00 · 2023-07-13 11:57:13 +03:00 · 99aa6ab6ab
commit 99aa6ab6ab
parent 88d45238f9
14 changed files with 108 additions and 163 deletions
--- a/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts
+++ b/src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts
@ -69,7 +69,7 @@ export const useLensSuggestions = ({
  return {
    allSuggestions,
    currentSuggestion,
-    suggestionUnsupported: !dataView.isTimeBased(),
+    suggestionUnsupported: !currentSuggestion && !dataView.isTimeBased(),
  };
 };

--- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/get_edit_lens_configuration.tsx
+++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/get_edit_lens_configuration.tsx
@ -18,9 +18,10 @@ import {
  LensRootStore,
  LensAppState,
  LensState,
+  loadInitial,
 } from '../../../state_management';
 import { getPreloadedState } from '../../../state_management/lens_slice';
-
+import { generateId } from '../../../id_generator';
 import type { DatasourceMap, VisualizationMap } from '../../../types';
 import {
  LensEditConfigurationFlyout,
@ -55,6 +56,7 @@ export function getEditLensConfiguration(
    wrapInFlyout,
    datasourceId,
    adaptersTables,
+    panelId,
  }: EditLensConfigurationProps) => {
    const [lensServices, setLensServices] = useState<LensAppServices>();
    useEffect(() => {
@ -89,6 +91,14 @@ export function getEditLensConfiguration(
    const lensStore: LensRootStore = makeConfigureStore(storeDeps, {
      lens: getPreloadedState(storeDeps) as LensAppState,
    } as unknown as PreloadedState<LensState>);
+    lensStore.dispatch(
+      loadInitial({
+        initialInput: {
+          attributes,
+          id: panelId ?? generateId(),
+        },
+      })
+    );

    const getWrapper = (children: JSX.Element) => {
      if (wrapInFlyout) {
--- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.test.tsx
+++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.test.tsx
@ -148,12 +148,7 @@ describe('LensEditConfigurationFlyout', () => {
        "activeData": Object {},
        "dataViews": Object {
          "indexPatternRefs": Array [],
-          "indexPatterns": Object {
-            "index1": Object {
-              "id": "index1",
-              "isPersisted": [Function],
-            },
-          },
+          "indexPatterns": Object {},
        },
        "datasourceLayers": Object {
          "a": Object {
@ -207,12 +202,7 @@ describe('LensEditConfigurationFlyout', () => {
                  "activeData": Object {},
                  "dataViews": Object {
                    "indexPatternRefs": Array [],
-                    "indexPatterns": Object {
-                      "index1": Object {
-                        "id": "index1",
-                        "isPersisted": [Function],
-                      },
-                    },
+                    "indexPatterns": Object {},
                  },
                  "datasourceLayers": Object {
                    "a": Object {
@ -257,12 +247,7 @@ describe('LensEditConfigurationFlyout', () => {
                  "activeData": Object {},
                  "dataViews": Object {
                    "indexPatternRefs": Array [],
-                    "indexPatterns": Object {
-                      "index1": Object {
-                        "id": "index1",
-                        "isPersisted": [Function],
-                      },
-                    },
+                    "indexPatterns": Object {},
                  },
                  "datasourceLayers": Object {
                    "a": Object {
--- a/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx
+++ b/x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx
@ -22,16 +22,11 @@ import { css } from '@emotion/react';
 import type { CoreStart } from '@kbn/core/public';
 import type { Datatable } from '@kbn/expressions-plugin/public';
 import type { DataView } from '@kbn/data-views-plugin/public';
-import { getResolvedDateRange } from '../../../utils';
 import type { LensPluginStartDependencies } from '../../../plugin';
-import {
-  DataViewsState,
-  useLensDispatch,
-  updateStateFromSuggestion,
-} from '../../../state_management';
+import { useLensSelector, selectFramePublicAPI } from '../../../state_management';
 import { VisualizationToolbar } from '../../../editor_frame_service/editor_frame/workspace_panel';

-import type { DatasourceMap, VisualizationMap, DatasourceLayers } from '../../../types';
+import type { DatasourceMap, VisualizationMap } from '../../../types';
 import type { TypedLensByValueInput } from '../../../embeddable/embeddable_component';
 import { ConfigPanelWrapper } from '../../../editor_frame_service/editor_frame/config_panel/config_panel';

@ -45,6 +40,7 @@ export interface EditConfigPanelProps {
  datasourceMap: DatasourceMap;
  closeFlyout?: () => void;
  wrapInFlyout?: boolean;
+  panelId?: string;
  datasourceId: 'formBased' | 'textBased';
  adaptersTables?: Record<string, Datatable>;
 }
@ -61,57 +57,33 @@ export function LensEditConfigurationFlyout({
  closeFlyout,
  adaptersTables,
 }: EditConfigPanelProps) {
-  const currentDataViewId = dataView.id ?? '';
  const datasourceState = attributes.state.datasourceStates[datasourceId];
  const activeVisualization = visualizationMap[attributes.visualizationType];
  const activeDatasource = datasourceMap[datasourceId];
-  const dispatchLens = useLensDispatch();
  const { euiTheme } = useEuiTheme();
-  const dataViews = useMemo(() => {
-    return {
-      indexPatterns: {
-        [currentDataViewId]: dataView,
-      },
-      indexPatternRefs: [],
-    } as unknown as DataViewsState;
-  }, [currentDataViewId, dataView]);
-  dispatchLens(
-    updateStateFromSuggestion({
-      newDatasourceId: datasourceId,
-      visualizationId: activeVisualization.id,
-      visualizationState: attributes.state.visualization,
-      datasourceState,
-      dataViews,
-    })
-  );

-  const datasourceLayers: DatasourceLayers = useMemo(() => {
-    return {};
-  }, []);
  const activeData: Record<string, Datatable> = useMemo(() => {
    return {};
  }, []);
  const layers = activeDatasource.getLayers(datasourceState);
  layers.forEach((layer) => {
-    datasourceLayers[layer] = datasourceMap[datasourceId].getPublicAPI({
-      state: datasourceState,
-      layerId: layer,
-      indexPatterns: dataViews.indexPatterns,
-    });
    if (adaptersTables) {
      activeData[layer] = Object.values(adaptersTables)[0];
    }
  });

-  const dateRange = getResolvedDateRange(startDependencies.data.query.timefilter.timefilter);
-  const framePublicAPI = useMemo(() => {
-    return {
-      activeData,
-      dataViews,
-      datasourceLayers,
-      dateRange,
+  const framePublicAPI = useLensSelector((state) => {
+    const newState = {
+      ...state,
+      lens: {
+        ...state.lens,
+        activeData,
+      },
    };
-  }, [activeData, dataViews, datasourceLayers, dateRange]);
+    return selectFramePublicAPI(newState, datasourceMap);
+  });
+  const { isLoading } = useLensSelector((state) => state.lens);
+  if (isLoading) return null;

  const layerPanelsProps = {
    framePublicAPI,
@ -120,7 +92,7 @@ export function LensEditConfigurationFlyout({
    core: coreStart,
    dataViews: startDependencies.dataViews,
    uiActions: startDependencies.uiActions,
-    hideLayerHeader: true,
+    hideLayerHeader: datasourceId === 'textBased',
    onUpdateStateCb: updateAll,
  };
  return (
@ -135,13 +107,15 @@ export function LensEditConfigurationFlyout({
      >
        <EuiFlexGroup gutterSize="s">
          <EuiFlexItem>
-            <EuiCallOut
-              size="s"
-              title={i18n.translate('xpack.lens.config.configFlyoutCallout', {
-                defaultMessage: 'SQL currently offers limited configuration options',
-              })}
-              iconType="iInCircle"
-            />
+            {datasourceId === 'textBased' && (
+              <EuiCallOut
+                size="s"
+                title={i18n.translate('xpack.lens.config.configFlyoutCallout', {
+                  defaultMessage: 'SQL currently offers limited configuration options',
+                })}
+                iconType="iInCircle"
+              />
+            )}
            <EuiSpacer size="m" />
            <VisualizationToolbar
              activeVisualization={activeVisualization}
--- a/x-pack/plugins/lens/public/datasources/text_based/datapanel.tsx
+++ b/x-pack/plugins/lens/public/datasources/text_based/datapanel.tsx
@ -97,7 +97,7 @@ export function TextBasedDataPanel({

  const { fieldListFiltersProps, fieldListGroupedProps } = useGroupedFields<DatatableColumn>({
    dataViewId: null,
-    allFields: dataHasLoaded ? fieldList : null,
+    allFields: dataHasLoaded ? fieldList ?? [] : null,
    services: {
      dataViews,
      core,
--- a/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx
+++ b/x-pack/plugins/lens/public/datasources/text_based/text_based_languages.tsx
@ -225,7 +225,6 @@ export function getTextBasedDatasource({
      const initState = state || { layers: {} };
      return {
        ...initState,
-        fieldList: [],
        indexPatternRefs: refs,
        initialContext: context,
      };
@ -407,7 +406,7 @@ export function getTextBasedDatasource({
        (column) => column.columnId === props.columnId
      );

-      const updatedFields = fields.map((f) => {
+      const updatedFields = fields?.map((f) => {
        return {
          ...f,
          compatible: props.isMetricDimension
@ -430,10 +429,10 @@ export function getTextBasedDatasource({
            className="lnsIndexPatternDimensionEditor--padded"
          >
            <FieldSelect
-              existingFields={updatedFields}
+              existingFields={updatedFields ?? []}
              selectedField={selectedField}
              onChoose={(choice) => {
-                const meta = fields.find((f) => f.name === choice.field)?.meta;
+                const meta = fields?.find((f) => f.name === choice.field)?.meta;
                const newColumn = {
                  columnId: props.columnId,
                  fieldName: choice.field,
@ -621,7 +620,7 @@ export function getTextBasedDatasource({
      };
    },
    getDatasourceSuggestionsForField(state, draggedField) {
-      const field = state.fieldList.find((f) => f.id === (draggedField as TextBasedField).id);
+      const field = state.fieldList?.find((f) => f.id === (draggedField as TextBasedField).id);
      if (!field) return [];
      return Object.entries(state.layers)?.map(([id, layer]) => {
        const newId = generateId();
--- a/x-pack/plugins/lens/public/datasources/text_based/types.ts
+++ b/x-pack/plugins/lens/public/datasources/text_based/types.ts
@ -32,11 +32,11 @@ export interface TextBasedLayer {
 export interface TextBasedPersistedState {
  layers: Record<string, TextBasedLayer>;
  initialContext?: VisualizeFieldContext | VisualizeEditorContext;
+  fieldList?: DatatableColumn[];
 }

 export type TextBasedPrivateState = TextBasedPersistedState & {
  indexPatternRefs: IndexPatternRef[];
-  fieldList: DatatableColumn[];
 };

 export interface IndexPatternRef {
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx
+++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx
@ -89,14 +89,18 @@ export function LayerPanels(
        if (datasourceId) {
          dispatchLens(
            updateDatasourceState({
-              updater: (prevState: unknown) =>
-                typeof newState === 'function' ? newState(prevState) : newState,
+              updater: (prevState: unknown) => {
+                onUpdateStateCb?.(
+                  typeof newState === 'function' ? newState(prevState) : newState,
+                  visualization.state
+                );
+                return typeof newState === 'function' ? newState(prevState) : newState;
+              },
              datasourceId,
              clearStagedPreview: false,
              dontSyncLinkedDimensions,
            })
          );
-          onUpdateStateCb?.(newState, visualization.state);
        }
      },
    [dispatchLens, onUpdateStateCb, visualization.state]
@ -136,6 +140,7 @@ export function LayerPanels(
                  typeof newVisualizationState === 'function'
                    ? newVisualizationState(prevState.visualization.state)
                    : newVisualizationState;
+                onUpdateStateCb?.(updatedDatasourceState, updatedVisualizationState);

                return {
                  ...prevState,
@ -154,7 +159,6 @@ export function LayerPanels(
              },
            })
          );
-          onUpdateStateCb?.(newDatasourceState, newVisualizationState);
        }, 0);
      },
    [dispatchLens, onUpdateStateCb]
@ -195,14 +199,24 @@ export function LayerPanels(
          layerIds,
        })
      );
+      if (activeDatasourceId && onUpdateStateCb) {
+        const newState = lensStore.getState().lens;
+        onUpdateStateCb(
+          newState.datasourceStates[activeDatasourceId].state,
+          newState.visualization.state
+        );
+      }
      removeLayerRef(layerToRemoveId);
    },
    [
+      activeDatasourceId,
      activeVisualization.id,
      datasourceMap,
      datasourceStates,
      dispatchLens,
      layerIds,
+      lensStore,
+      onUpdateStateCb,
      props.framePublicAPI.datasourceLayers,
      props.uiActions,
      removeLayerRef,
@ -242,7 +256,16 @@ export function LayerPanels(

  const addLayer: AddLayerFunction = (layerType, extraArg, ignoreInitialValues) => {
    const layerId = generateId();
+
    dispatchLens(addLayerAction({ layerId, layerType, extraArg, ignoreInitialValues }));
+
+    if (activeDatasourceId && onUpdateStateCb) {
+      const newState = lensStore.getState().lens;
+      onUpdateStateCb(
+        newState.datasourceStates[activeDatasourceId].state,
+        newState.visualization.state
+      );
+    }
    setNextFocusedLayerId(layerId);
  };

--- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx
+++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx
@ -120,6 +120,7 @@ import {
  getIndexPatternsObjects,
  getSearchWarningMessages,
  inferTimeField,
+  extractReferencesFromState,
 } from '../utils';
 import { getLayerMetaInfo, combineQueryAndFilters } from '../app_plugin/show_underlying_data';
 import {
@ -741,19 +742,37 @@ export class Embeddable

  async updateVisualization(datasourceState: unknown, visualizationState: unknown) {
    const viz = this.savedVis;
-    const datasourceId = (this.activeDatasourceId ??
+    const activeDatasourceId = (this.activeDatasourceId ??
      'formBased') as EditLensConfigurationProps['datasourceId'];
    if (viz?.state) {
+      const datasourceStates = {
+        ...viz.state.datasourceStates,
+        [activeDatasourceId]: datasourceState,
+      };
+      const references = extractReferencesFromState({
+        activeDatasources: Object.keys(datasourceStates).reduce(
+          (acc, datasourceId) => ({
+            ...acc,
+            [datasourceId]: this.deps.datasourceMap[datasourceId],
+          }),
+          {}
+        ),
+        datasourceStates: Object.fromEntries(
+          Object.entries(datasourceStates).map(([id, state]) => [id, { isLoading: false, state }])
+        ),
+        visualizationState,
+        activeVisualization: this.activeVisualizationId
+          ? this.deps.visualizationMap[this.activeVisualizationId]
+          : undefined,
+      });
      const attrs = {
        ...viz,
        state: {
          ...viz.state,
          visualization: visualizationState,
-          datasourceStates: {
-            ...viz.state.datasourceStates,
-            [datasourceId]: datasourceState,
-          },
+          datasourceStates,
        },
+        references,
      };
      this.updateInput({ attributes: attrs });
    }
@ -770,6 +789,7 @@ export class Embeddable

    const datasourceId = (this.activeDatasourceId ??
      'formBased') as EditLensConfigurationProps['datasourceId'];
+
    const attributes = this.savedVis as TypedLensByValueInput['attributes'];
    const dataView = this.dataViews[0];
    if (attributes) {
@ -780,6 +800,7 @@ export class Embeddable
          updateAll={this.updateVisualization.bind(this)}
          datasourceId={datasourceId}
          adaptersTables={this.lensInspector.adapters.tables?.tables}
+          panelId={this.id}
        />
      );
    }
--- a/x-pack/plugins/lens/public/state_management/index.ts
+++ b/x-pack/plugins/lens/public/state_management/index.ts
@ -35,7 +35,6 @@ export const {
  submitSuggestion,
  switchDatasource,
  switchAndCleanDatasource,
-  updateStateFromSuggestion,
  updateIndexPatterns,
  setToggleFullscreen,
  initEmpty,
--- 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
@ -92,7 +92,7 @@ export function loadInitial(
    initialInput,
    history,
  }: {
-    redirectCallback: (savedObjectId?: string) => void;
+    redirectCallback?: (savedObjectId?: string) => void;
    initialInput?: LensEmbeddableInput;
    history?: History<unknown>;
  },
@ -269,7 +269,7 @@ export function loadInitial(
        notifications.toasts.addDanger({
          title: e.message,
        });
-        redirectCallback();
+        redirectCallback?.();
      });
  }

@ -376,7 +376,7 @@ export function loadInitial(
              })
            );
        } else {
-          redirectCallback();
+          redirectCallback?.();
        }
      },
      () => {
@ -385,13 +385,13 @@ export function loadInitial(
            isLoading: false,
          })
        );
-        redirectCallback();
+        redirectCallback?.();
      }
    )
    .catch((e: { message: string }) => {
      notifications.toasts.addDanger({
        title: e.message,
      });
-      redirectCallback();
+      redirectCallback?.();
    });
 }
--- a/x-pack/plugins/lens/public/state_management/lens_slice.test.ts
+++ b/x-pack/plugins/lens/public/state_management/lens_slice.test.ts
@ -10,7 +10,6 @@ import type { Query } from '@kbn/es-query';
 import {
  switchDatasource,
  switchAndCleanDatasource,
-  updateStateFromSuggestion,
  switchVisualization,
  setState,
  updateState,
@ -272,28 +271,6 @@ describe('lensSlice', () => {
      });
    });

-    describe('update the state from the suggestion', () => {
-      it('should switch active datasource and initialize new state', () => {
-        store.dispatch(
-          updateStateFromSuggestion({
-            newDatasourceId: 'testDatasource2',
-            visualizationId: 'testVis',
-            visualizationState: ['col1', 'col2'],
-            datasourceState: {},
-            dataViews: { indexPatterns: {} } as DataViewsState,
-          })
-        );
-        expect(store.getState().lens.activeDatasourceId).toEqual('testDatasource2');
-        expect(store.getState().lens.datasourceStates.testDatasource2.isLoading).toEqual(false);
-        expect(store.getState().lens.datasourceStates.testDatasource2.state).toStrictEqual({});
-        expect(store.getState().lens.visualization).toStrictEqual({
-          activeId: 'testVis',
-          state: ['col1', 'col2'],
-        });
-        expect(store.getState().lens.dataViews).toEqual({ indexPatterns: {} });
-      });
-    });
-
    describe('adding or removing layer', () => {
      const testDatasource = (datasourceId: string) => {
        return {
--- a/x-pack/plugins/lens/public/state_management/lens_slice.ts
+++ b/x-pack/plugins/lens/public/state_management/lens_slice.ts
@ -187,18 +187,11 @@ export const switchAndCleanDatasource = createAction<{
  visualizationId: string | null;
  currentIndexPatternId?: string;
 }>('lens/switchAndCleanDatasource');
-export const updateStateFromSuggestion = createAction<{
-  newDatasourceId: string;
-  visualizationId: string | null;
-  visualizationState: unknown;
-  datasourceState: unknown;
-  dataViews: DataViewsState;
-}>('lens/updateStateFromSuggestion');
 export const navigateAway = createAction<void>('lens/navigateAway');
 export const loadInitial = createAction<{
  initialInput?: LensEmbeddableInput;
-  redirectCallback: (savedObjectId?: string) => void;
-  history: History<unknown>;
+  redirectCallback?: (savedObjectId?: string) => void;
+  history?: History<unknown>;
 }>('lens/loadInitial');
 export const initEmpty = createAction(
  'initEmpty',
@ -288,7 +281,6 @@ export const lensActions = {
  submitSuggestion,
  switchDatasource,
  switchAndCleanDatasource,
-  updateStateFromSuggestion,
  navigateAway,
  loadInitial,
  initEmpty,
@ -870,49 +862,13 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => {
        },
      };
    },
-    [updateStateFromSuggestion.type]: (
-      state,
-      {
-        payload,
-      }: {
-        payload: {
-          newDatasourceId: string;
-          visualizationId: string;
-          visualizationState: unknown;
-          datasourceState: unknown;
-          dataViews: DataViewsState;
-        };
-      }
-    ) => {
-      const visualization = {
-        activeId: payload.visualizationId,
-        state: payload.visualizationState,
-      };
-
-      const datasourceState = payload.datasourceState;
-
-      return {
-        ...state,
-        datasourceStates: {
-          [payload.newDatasourceId]: {
-            state: datasourceState,
-            isLoading: false,
-          },
-        },
-        activeDatasourceId: payload.newDatasourceId,
-        visualization: {
-          ...visualization,
-        },
-        dataViews: payload.dataViews,
-      };
-    },
    [navigateAway.type]: (state) => state,
    [loadInitial.type]: (
      state,
      payload: PayloadAction<{
        initialInput?: LensEmbeddableInput;
-        redirectCallback: (savedObjectId?: string) => void;
-        history: History<unknown>;
+        redirectCallback?: (savedObjectId?: string) => void;
+        history?: History<unknown>;
      }>
    ) => state,
    [initEmpty.type]: (
--- a/x-pack/test/functional/apps/discover/visualize_field.ts
+++ b/x-pack/test/functional/apps/discover/visualize_field.ts
@ -171,6 +171,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {

      await PageObjects.discover.chooseLensChart('Bar vertical stacked');
      await PageObjects.header.waitUntilLoadingHasFinished();
+      await testSubjects.click('TextBasedLangEditor-expand');
      await testSubjects.click('unifiedHistogramEditFlyoutVisualization');
      expect(await testSubjects.exists('xyVisChart')).to.be(true);
      expect(await PageObjects.lens.canRemoveDimension('lnsXY_xDimensionPanel')).to.equal(true);