[Data Explorer] Persisted state and syncing, shared context, linking …

…state to component + smaller changes (#4678)

* Merge branch 'routing'

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* fix initial state of discover

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* fixes layout

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

* fix tests

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

---------

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

.github/workflows/build_and_test_workflow.yml

@@ -3,10 +3,10 @@
 
 name: Build and test
 
-# trigger on every commit push and PR for all branches except pushes for backport branches
+# trigger on every commit push and PR for all branches except pushes for backport branches and feature branches
 on:
   push:
-    branches: ['**', '!backport/**']
+    branches: ['**', '!backport/**', '!feature/**']
     paths-ignore:
       - '**/*.md'
       - 'docs/**'

.github/workflows/cypress_workflow.yml

@@ -1,9 +1,9 @@
 name: Run cypress tests
 
-# trigger on every PR for all branches
+# trigger on every PR for all branches except feature branches
 on:
   pull_request:
-    branches: [ '**' ]
+    branches: [ '**', '!feature/**' ]
     paths-ignore:
       - '**/*.md'
 

src/plugins/data_explorer/public/components/app.tsx

@@ -3,13 +3,34 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import React from 'react';
+import React, { useEffect } from 'react';
+import { useLocation } from 'react-router-dom';
 import { AppMountParameters } from '../../../../core/public';
 import { useView } from '../utils/use';
 import { AppContainer } from './app_container';
+import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public';
+import { DataExplorerServices } from '../types';
+import { syncQueryStateWithUrl } from '../../../data/public';
 
 export const DataExplorerApp = ({ params }: { params: AppMountParameters }) => {
   const { view } = useView();
+  const {
+    services: {
+      data: { query },
+      osdUrlStateStorage,
+    },
+  } = useOpenSearchDashboards<DataExplorerServices>();
+  const { pathname } = useLocation();
+
+  useEffect(() => {
+    // syncs `_g` portion of url with query services
+    const { stop } = syncQueryStateWithUrl(query, osdUrlStateStorage);
+
+    return () => stop();
+
+    // this effect should re-run when pathname is changed to preserve querystring part,
+    // so the global state is always preserved
+  }, [query, osdUrlStateStorage, pathname]);
 
   return <AppContainer view={view} params={params} />;
 };

src/plugins/data_explorer/public/components/app_container.scss

@@ -0,0 +1,18 @@
+$osdHeaderOffset: $euiHeaderHeightCompensation;
+
+.deSidebar {
+  max-width: 462px;
+  min-width: 400px;
+}
+
+.deLayout {
+  height: calc(100vh - #{$osdHeaderOffset});
+
+  &__canvas {
+    height: 100%;
+  }
+}
+
+.headerIsExpanded .deLayout {
+  height: calc(100vh - #{$osdHeaderOffset * 2});
+}

src/plugins/data_explorer/public/components/app_container.tsx

@@ -4,41 +4,36 @@
  */
 
 import React from 'react';
-import { EuiPageTemplate } from '@elastic/eui';
+import { EuiPage, EuiPageBody } from '@elastic/eui';
 import { Suspense } from 'react';
 import { AppMountParameters } from '../../../../core/public';
 import { Sidebar } from './sidebar';
 import { NoView } from './no_view';
 import { View } from '../services/view_service/view';
+import './app_container.scss';
 
 export const AppContainer = ({ view, params }: { view?: View; params: AppMountParameters }) => {
   // TODO: Make this more robust.
   if (!view) {
     return <NoView />;
   }
 
-  const { Canvas, Panel } = view;
+  const { Canvas, Panel, Context } = view;
 
   // Render the application DOM.
-  // Note that `navigation.ui.TopNavMenu` is a stateful component exported on the `navigation` plugin's start contract.
   return (
-    <EuiPageTemplate
-      pageSideBar={
-        <Sidebar>
-          <Suspense fallback={<div>Loading...</div>}>
-            <Panel {...params} />
-          </Suspense>
-        </Sidebar>
-      }
-      className="dePageTemplate"
-      template="default"
-      restrictWidth={false}
-      paddingSize="none"
-    >
-      {/* TODO: improve loading state */}
+    <EuiPage className="deLayout" paddingSize="none">
+      {/* TODO: improve fallback state */}
       <Suspense fallback={<div>Loading...</div>}>
-        <Canvas {...params} />
+        <Context {...params}>
+          <Sidebar>
+            <Panel {...params} />
+          </Sidebar>
+          <EuiPageBody className="deLayout__canvas">
+            <Canvas {...params} />
+          </EuiPageBody>
+        </Context>
       </Suspense>
-    </EuiPageTemplate>
+    </EuiPage>
   );
 };

src/plugins/data_explorer/public/components/sidebar/index.tsx

@@ -5,7 +5,14 @@
 
 import React, { useMemo, FC, useEffect, useState } from 'react';
 import { i18n } from '@osd/i18n';
-import { EuiPanel, EuiComboBox, EuiSelect, EuiComboBoxOptionOption } from '@elastic/eui';
+import {
+  EuiComboBox,
+  EuiSelect,
+  EuiComboBoxOptionOption,
+  EuiSpacer,
+  EuiSplitPanel,
+  EuiPageSideBar,
+} from '@elastic/eui';
 import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public';
 import { useView } from '../../utils/use';
 import { DataExplorerServices } from '../../types';
@@ -57,43 +64,50 @@ export const Sidebar: FC = ({ children }) => {
   }, [indexPatternId, options]);
 
   return (
-    <>
-      <EuiPanel borderRadius="none" hasShadow={false}>
-        <EuiComboBox
-          placeholder="Select a datasource"
-          singleSelection={{ asPlainText: true }}
-          options={options}
-          selectedOptions={selectedOption ? [selectedOption] : []}
-          onChange={(selected) => {
-            // TODO: There are many issues with this approach, but it's a start
-            // 1. Combo box can delete a selected index pattern. This should not be possible
-            // 2. Combo box is severely truncated. This should be fixed in the EUI component
-            // 3. The onchange can fire with a option that is not valid. discuss where to handle this.
-            // 4. value is optional. If the combobox needs to act as a slecet, this should be required.
-            const { value } = selected[0] || {};
+    <EuiPageSideBar className="deSidebar" sticky>
+      <EuiSplitPanel.Outer className="eui-yScroll" hasBorder={true} borderRadius="none">
+        <EuiSplitPanel.Inner paddingSize="s" grow={false}>
+          <EuiComboBox
+            placeholder="Select a datasource"
+            singleSelection={{ asPlainText: true }}
+            options={options}
+            selectedOptions={selectedOption ? [selectedOption] : []}
+            fullWidth
+            onChange={(selected) => {
+              // TODO: There are many issues with this approach, but it's a start
+              // 1. Combo box can delete a selected index pattern. This should not be possible
+              // 2. Combo box is severely truncated. This should be fixed in the EUI component
+              // 3. The onchange can fire with a option that is not valid. discuss where to handle this.
+              // 4. value is optional. If the combobox needs to act as a slecet, this should be required.
+              const { value } = selected[0] || {};
 
-            if (!value) {
-              toasts.addWarning({
-                id: 'index-pattern-not-found',
-                title: i18n.translate('dataExplorer.indexPatternError', {
-                  defaultMessage: 'Index pattern not found',
-                }),
-              });
-              return;
-            }
+              if (!value) {
+                toasts.addWarning({
+                  id: 'index-pattern-not-found',
+                  title: i18n.translate('dataExplorer.indexPatternError', {
+                    defaultMessage: 'Index pattern not found',
+                  }),
+                });
+                return;
+              }
 
-            dispatch(setIndexPattern(value));
-          }}
-        />
-        <EuiSelect
-          options={viewOptions}
-          value={view?.id}
-          onChange={(e) => {
-            dispatch(setView(e.target.value));
-          }}
-        />
-      </EuiPanel>
-      {children}
-    </>
+              dispatch(setIndexPattern(value));
+            }}
+          />
+          <EuiSpacer size="s" />
+          <EuiSelect
+            options={viewOptions}
+            value={view?.id}
+            onChange={(e) => {
+              dispatch(setView(e.target.value));
+            }}
+            fullWidth
+          />
+        </EuiSplitPanel.Inner>
+        <EuiSplitPanel.Inner paddingSize="none" color="subdued" className="eui-yScroll">
+          {children}
+        </EuiSplitPanel.Inner>
+      </EuiSplitPanel.Outer>
+    </EuiPageSideBar>
   );
 };

src/plugins/data_explorer/public/index.ts

@@ -12,6 +12,6 @@ import { DataExplorerPlugin } from './plugin';
 export function plugin() {
   return new DataExplorerPlugin();
 }
-export { DataExplorerPluginSetup, DataExplorerPluginStart, ViewRedirectParams } from './types';
+export { DataExplorerPluginSetup, DataExplorerPluginStart, DataExplorerServices } from './types';
 export { ViewProps, ViewDefinition } from './services/view_service';
 export { RootState, useTypedSelector, useTypedDispatch } from './utils/state_management';

src/plugins/data_explorer/public/services/view_service/types.ts

@@ -7,8 +7,6 @@ import { Slice } from '@reduxjs/toolkit';
 import { LazyExoticComponent } from 'react';
 import { AppMountParameters } from '../../../../../core/public';
 
-// TODO: State management props
-
 interface ViewListItem {
   id: string;
   label: string;
@@ -25,6 +23,9 @@ export interface ViewDefinition<T = any> {
   };
   readonly Canvas: LazyExoticComponent<(props: ViewProps) => React.ReactElement>;
   readonly Panel: LazyExoticComponent<(props: ViewProps) => React.ReactElement>;
+  readonly Context: LazyExoticComponent<
+    (props: React.PropsWithChildren<ViewProps>) => React.ReactElement
+  >;
   readonly defaultPath: string;
   readonly appExtentions: {
     savedObject: {

src/plugins/data_explorer/public/services/view_service/view.ts

@@ -15,6 +15,7 @@ export class View implements IView {
   readonly shouldShow?: (state: any) => boolean;
   readonly Canvas: IView['Canvas'];
   readonly Panel: IView['Panel'];
+  readonly Context: IView['Context'];
 
   constructor(options: ViewDefinition) {
     this.id = options.id;
@@ -25,5 +26,6 @@ export class View implements IView {
     this.shouldShow = options.shouldShow;
     this.Canvas = options.Canvas;
     this.Panel = options.Panel;
+    this.Context = options.Context;
   }
 }

src/plugins/data_explorer/public/types.ts

@@ -26,11 +26,6 @@ export interface DataExplorerPluginStartDependencies {
   data: DataPublicPluginStart;
 }
 
-export interface ViewRedirectParams {
-  view: string;
-  path?: string;
-}
-
 export interface DataExplorerServices extends CoreStart {
   store?: Store;
   viewRegistry: ViewServiceStart;

src/plugins/data_explorer/public/utils/mocks.ts

@@ -27,7 +27,7 @@ export const createDataExplorerServicesMock = () => {
     scopedHistory: (scopedHistoryMock.create() as unknown) as ScopedHistory,
     viewRegistry: {
       get: jest.fn(),
-      all: jest.fn(),
+      all: jest.fn(() => []),
     },
   };
 

src/plugins/data_explorer/public/utils/state_management/redux_persistence.test.tsx

@@ -6,7 +6,6 @@
 import { DataExplorerServices } from '../../types';
 import { createDataExplorerServicesMock } from '../mocks';
 import { loadReduxState, persistReduxState } from './redux_persistence';
-import { RootState } from './store';
 
 describe('test redux state persistence', () => {
   let mockServices: jest.Mocked<DataExplorerServices>;
@@ -15,20 +14,21 @@ describe('test redux state persistence', () => {
   beforeEach(() => {
     mockServices = createDataExplorerServicesMock();
     reduxStateParams = {
-      style: 'style',
-      visualization: 'visualization',
+      discover: 'visualization',
       metadata: 'metadata',
-      ui: 'ui',
     };
   });
 
-  test('test load redux state when url is empty', async () => {
-    const defaultStates: RootState = {
-      metadata: {},
-    };
-
+  test('test load default redux state when url is empty', async () => {
     const returnStates = await loadReduxState(mockServices);
-    expect(returnStates).toStrictEqual(defaultStates);
+    expect(returnStates).toMatchInlineSnapshot(`
+      Object {
+        "metadata": Object {
+          "indexPattern": "id",
+          "originatingApp": undefined,
+        },
+      }
+    `);
   });
 
   test('test load redux state', async () => {

src/plugins/data_explorer/public/utils/state_management/store.ts

@@ -3,19 +3,21 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import React from 'react';
 import { combineReducers, configureStore, PreloadedState, Reducer, Slice } from '@reduxjs/toolkit';
 import { isEqual } from 'lodash';
-import { Provider } from 'react-redux';
-import { reducer as metadataReducer, MetadataState } from './metadata_slice';
+import { reducer as metadataReducer } from './metadata_slice';
 import { loadReduxState, persistReduxState } from './redux_persistence';
 import { DataExplorerServices } from '../../types';
 
-const dynamicReducers: {
-  metadata: Reducer<MetadataState>;
+const commonReducers = {
+  metadata: metadataReducer,
+};
+
+let dynamicReducers: {
+  metadata: typeof metadataReducer;
   [key: string]: Reducer;
 } = {
-  metadata: metadataReducer,
+  ...commonReducers,
 };
 
 const rootReducer = combineReducers(dynamicReducers);
@@ -60,7 +62,15 @@ export const getPreloadedStore = async (services: DataExplorerServices) => {
   // the store subscriber will automatically detect changes and call handleChange function
   const unsubscribe = store.subscribe(handleChange);
 
-  return { store, unsubscribe };
+  const onUnsubscribe = () => {
+    dynamicReducers = {
+      ...commonReducers,
+    };
+
+    unsubscribe();
+  };
+
+  return { store, unsubscribe: onUnsubscribe };
 };
 
 export const registerSlice = (slice: Slice) => {