Refactor app state and cleanup unused imports (#4504)

Clean up app state for Dashboards plugin. * Removes the dashboard container hook in place of a single dashboard app state container * Still recovers some follow-ups and clean up * Skips test for rendering of a legacy test. Signed-off-by: Kawika Avilla <kavilla414@gmail.com> --------- Signed-off-by: Kawika Avilla <kavilla414@gmail.com> Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> Co-authored-by: Qingyang(Abby) Hu <abigailhu2000@gmail.com> Co-authored-by: Miki <amoo_miki@yahoo.com>
opensearch-project · Jul 11, 2023 · bd72786 · bd72786
1 parent 98a6b4b
commit bd72786
Show file tree

Hide file tree

Showing 29 changed files with 1,685 additions and 792 deletions.
diff --git a/src/plugins/dashboard/public/application/components/dashboard_editor.tsx b/src/plugins/dashboard/public/application/components/dashboard_editor.tsx
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import React, { useEffect, useState } from 'react';
+import React, { useState } from 'react';
 import { useParams } from 'react-router-dom';
 import { EventEmitter } from 'events';
 import { DashboardTopNav } from '../components/dashboard_top_nav';
@@ -12,95 +12,54 @@ import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react
 import { useSavedDashboardInstance } from '../utils/use/use_saved_dashboard_instance';
 import { DashboardServices } from '../../types';
 import { useDashboardAppAndGlobalState } from '../utils/use/use_dashboard_app_state';
-import { useDashboardContainer } from '../utils/use/use_dashboard_container';
 import { useEditorUpdates } from '../utils/use/use_editor_updates';
-import {
-  setBreadcrumbsForExistingDashboard,
-  setBreadcrumbsForNewDashboard,
-} from '../utils/breadcrumbs';
 
 export const DashboardEditor = () => {
   const { id: dashboardIdFromUrl } = useParams<{ id: string }>();
   const { services } = useOpenSearchDashboards<DashboardServices>();
   const { chrome } = services;
-  const isChromeVisible = useChromeVisibility(chrome);
+  const isChromeVisible = useChromeVisibility({ chrome });
   const [eventEmitter] = useState(new EventEmitter());
 
-  const { savedDashboard: savedDashboardInstance, dashboard } = useSavedDashboardInstance(
+  const { savedDashboard: savedDashboardInstance, dashboard } = useSavedDashboardInstance({
     services,
     eventEmitter,
     isChromeVisible,
-    dashboardIdFromUrl
-  );
+    dashboardIdFromUrl,
+  });
 
-  const { appState } = useDashboardAppAndGlobalState(
+  const { appState, currentContainer, indexPatterns } = useDashboardAppAndGlobalState({
     services,
     eventEmitter,
-    savedDashboardInstance
-  );
-
-  const { dashboardContainer, indexPatterns } = useDashboardContainer(
-    services,
-    dashboard,
     savedDashboardInstance,
-    appState
-  );
+    dashboard,
+  });
 
-  const { isEmbeddableRendered, currentAppState } = useEditorUpdates(
+  const { isEmbeddableRendered, currentAppState } = useEditorUpdates({
     services,
     eventEmitter,
-    dashboard,
     savedDashboardInstance,
-    dashboardContainer,
-    appState
-  );
-
-  useEffect(() => {
-    if (currentAppState && dashboard) {
-      if (savedDashboardInstance?.id) {
-        chrome.setBreadcrumbs(
-          setBreadcrumbsForExistingDashboard(
-            savedDashboardInstance.title,
-            currentAppState.viewMode,
-            dashboard.isDirty
-          )
-        );
-        chrome.docTitle.change(savedDashboardInstance.title);
-      } else {
-        chrome.setBreadcrumbs(
-          setBreadcrumbsForNewDashboard(currentAppState.viewMode, dashboard.isDirty)
-        );
-      }
-    }
-  }, [currentAppState, savedDashboardInstance, chrome, dashboard]);
-
-  useEffect(() => {
-    // clean up all registered listeners if any is left
-    return () => {
-      eventEmitter.removeAllListeners();
-    };
-  }, [eventEmitter]);
+    dashboard,
+    dashboardContainer: currentContainer,
+    appState,
+  });
 
   return (
     <div>
       <div>
-        {savedDashboardInstance &&
-          appState &&
-          dashboardContainer &&
-          currentAppState &&
-          dashboard && (
-            <DashboardTopNav
-              isChromeVisible={isChromeVisible}
-              savedDashboardInstance={savedDashboardInstance}
-              stateContainer={appState}
-              dashboard={dashboard}
-              currentAppState={currentAppState}
-              isEmbeddableRendered={isEmbeddableRendered}
-              indexPatterns={indexPatterns}
-              dashboardContainer={dashboardContainer}
-              dashboardIdFromUrl={dashboardIdFromUrl}
-            />
-          )}
+        {savedDashboardInstance && appState && currentAppState && currentContainer && dashboard && (
+          <DashboardTopNav
+            isChromeVisible={isChromeVisible}
+            savedDashboardInstance={savedDashboardInstance}
+            appState={appState!}
+            dashboard={dashboard}
+            currentAppState={currentAppState}
+            isEmbeddableRendered={isEmbeddableRendered}
+            indexPatterns={indexPatterns}
+            currentContainer={currentContainer}
+            dashboardIdFromUrl={dashboardIdFromUrl}
+          />
+        )}
       </div>
     </div>
   );

diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav.tsx b/src/plugins/dashboard/public/application/components/dashboard_top_nav.tsx
@@ -17,12 +17,12 @@ import { Dashboard } from '../../dashboard';
 interface DashboardTopNavProps {
   isChromeVisible: boolean;
   savedDashboardInstance: any;
-  stateContainer: DashboardAppStateContainer;
+  appState: DashboardAppStateContainer;
   dashboard: Dashboard;
   currentAppState: DashboardAppState;
   isEmbeddableRendered: boolean;
   indexPatterns: IndexPattern[];
-  dashboardContainer?: DashboardContainer;
+  currentContainer?: DashboardContainer;
   dashboardIdFromUrl?: string;
 }
 
@@ -37,11 +37,11 @@ export enum UrlParams {
 const TopNav = ({
   isChromeVisible,
   savedDashboardInstance,
-  stateContainer,
+  appState,
   dashboard,
   currentAppState,
   isEmbeddableRendered,
-  dashboardContainer,
+  currentContainer,
   indexPatterns,
   dashboardIdFromUrl,
 }: DashboardTopNavProps) => {
@@ -57,11 +57,11 @@ const TopNav = ({
 
   const handleRefresh = useCallback(
     (_payload: any, isUpdate?: boolean) => {
-      if (!isUpdate && dashboardContainer) {
-        dashboardContainer.reload();
+      if (!isUpdate && currentContainer) {
+        currentContainer.reload();
       }
     },
-    [dashboardContainer]
+    [currentContainer]
   );
 
   const isEmbeddedExternally = Boolean(queryParameters.get('embed'));
@@ -78,12 +78,12 @@ const TopNav = ({
   useEffect(() => {
     if (isEmbeddableRendered) {
       const navActions = getNavActions(
-        stateContainer,
+        appState,
         savedDashboardInstance,
         services,
         dashboard,
         dashboardIdFromUrl,
-        dashboardContainer
+        currentContainer
       );
       setTopNavMenu(
         getTopNavConfig(
@@ -97,9 +97,9 @@ const TopNav = ({
     currentAppState,
     services,
     dashboardConfig,
-    dashboardContainer,
+    currentContainer,
     savedDashboardInstance,
-    stateContainer,
+    appState,
     isEmbeddableRendered,
     dashboard,
     dashboardIdFromUrl,
@@ -139,7 +139,7 @@ const TopNav = ({
       showSaveQuery={services.dashboardCapabilities.saveQuery as boolean}
       savedQuery={undefined}
       onSavedQueryIdChange={(savedQueryId?: string) => {
-        stateContainer.transitions.set('savedQuery', savedQueryId);
+        appState.transitions.set('savedQuery', savedQueryId);
       }}
       savedQueryId={currentAppState?.savedQuery}
       onQuerySubmit={handleRefresh}

diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx
@@ -113,8 +113,9 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
   public readonly type = DASHBOARD_CONTAINER_TYPE;
 
   public renderEmpty?: undefined | (() => React.ReactNode);
-  public getChangesFromAppStateForContainerState?: (containerInput: any) => any;
-  public updateAppStateUrl?: undefined | ((pathname: string, replace: boolean) => void);
+  public updateAppStateUrl?:
+    | undefined
+    | (({ replace, pathname }: { replace: boolean; pathname?: string }) => void);
 
   private embeddablePanel: EmbeddableStart['EmbeddablePanel'];
 

diff --git a/src/plugins/dashboard/public/application/embeddable/grid/dashboard_grid.tsx b/src/plugins/dashboard/public/application/embeddable/grid/dashboard_grid.tsx
@@ -28,7 +28,6 @@
  * under the License.
  */
 
-import 'react-grid-layout/css/styles.css';
 import 'react-resizable/css/styles.css';
 
 // @ts-ignore
@@ -39,7 +38,7 @@ import classNames from 'classnames';
 import _ from 'lodash';
 import React from 'react';
 import { Subscription } from 'rxjs';
-import ReactGridLayout, { Layout } from 'react-grid-layout';
+import ReactGridLayout, { Layout, ReactGridLayoutProps } from 'react-grid-layout';
 import { GridData } from '../../../../common';
 import { ViewMode, EmbeddableChildPanel, EmbeddableStart } from '../../../embeddable_plugin';
 import { DASHBOARD_GRID_COLUMN_COUNT, DASHBOARD_GRID_HEIGHT } from '../dashboard_constants';
@@ -76,9 +75,9 @@ function ResponsiveGrid({
   size: { width: number };
   isViewMode: boolean;
   layout: Layout[];
-  onLayoutChange: () => void;
+  onLayoutChange: ReactGridLayoutProps['onLayoutChange'];
   children: JSX.Element[];
-  maximizedPanelId: string;
+  maximizedPanelId?: string;
   useMargins: boolean;
 }) {
   // This is to prevent a bug where view mode changes when the panel is expanded.  View mode changes will trigger
@@ -171,7 +170,7 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
     let layout;
     try {
       layout = this.buildLayoutFromPanels();
-    } catch (error) {
+    } catch (error: any) {
       console.error(error); // eslint-disable-line no-console
 
       isLayoutInvalid = true;
@@ -283,6 +282,7 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
           }}
         >
           <EmbeddableChildPanel
+            key={panel.type}
             embeddableId={panel.explicitInput.id}
             container={this.props.container}
             PanelComponent={this.props.PanelComponent}
@@ -304,7 +304,7 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
         isViewMode={isViewMode}
         layout={this.buildLayoutFromPanels()}
         onLayoutChange={this.onLayoutChange}
-        maximizedPanelId={this.state.expandedPanelId}
+        maximizedPanelId={this.state.expandedPanelId!}
         useMargins={this.state.useMargins}
       >
         {this.renderPanels()}

diff --git a/src/plugins/dashboard/public/application/index.tsx b/src/plugins/dashboard/public/application/index.tsx
@@ -20,17 +20,14 @@ import { DashboardServices } from '../types';
 export * from './embeddable';
 export * from './actions';
 
-export const renderApp = (
-  { element, appBasePath, onAppLeave }: AppMountParameters,
-  services: DashboardServices
-) => {
+export const renderApp = ({ element }: AppMountParameters, services: DashboardServices) => {
   addHelpMenuToAppChrome(services.chrome, services.docLinks);
 
   const app = (
     <Router history={services.history}>
       <OpenSearchDashboardsContextProvider services={services}>
         <services.i18n.Context>
-          <DashboardApp onAppLeave={onAppLeave} />
+          <DashboardApp />
         </services.i18n.Context>
       </OpenSearchDashboardsContextProvider>
     </Router>

diff --git a/src/plugins/dashboard/public/application/lib/migrate_app_state.ts b/src/plugins/dashboard/public/application/lib/migrate_app_state.ts
@@ -106,9 +106,9 @@ export function migrateAppState(
     delete appState.uiState;
   }
 
-  appState.panels.forEach((panel) => {
-    panel.version = opensearchDashboardsVersion;
-  });
+  // appState.panels.forEach((panel) => {
+  //   panel.version = opensearchDashboardsVersion;
+  // });
 
   return appState;
 }
diff --git a/src/plugins/dashboard/public/application/lib/save_dashboard.ts b/src/plugins/dashboard/public/application/lib/save_dashboard.ts
@@ -34,6 +34,7 @@ import { updateSavedDashboard } from './update_saved_dashboard';
 
 import { DashboardAppStateContainer } from '../../types';
 import { Dashboard } from '../../dashboard';
+import { SavedObjectDashboard } from '../../saved_dashboards';
 
 /**
  * Saves the dashboard.
@@ -43,7 +44,7 @@ import { Dashboard } from '../../dashboard';
 export function saveDashboard(
   timeFilter: TimefilterContract,
   stateContainer: DashboardAppStateContainer,
-  savedDashboard: any,
+  savedDashboard: SavedObjectDashboard,
   saveOptions: SavedObjectSaveOpts,
   dashboard: Dashboard
 ): Promise<string> {
@@ -54,7 +55,9 @@ export function saveDashboard(
   // TODO: should update Dashboard class in the if(id) block
   return savedDashboard.save(saveOptions).then((id: string) => {
     if (id) {
+      dashboard.id = id;
       return id;
     }
+    return id;
   });
 }