[App Search] Fix error connecting state (#98234)

* Fix ErrorConnecting state to be caught/returned earlier - This fixes AppSearch crashing when in an error connecting state and missing props required for AppSearchConfigured - This also fixes the error connecting page not showing up for engine routes - The SetupGuide route was moved to a top-level route, so that all views always have access to it no matter what * [Extra] Simplify engine route/path - Move engine/route path under the main <Layout> - Change <AppSearchNav> behavior to pass subNavs based on route matches, rather than requiring a prop passed to it + add useRouteMatch mock Co-authored-by: Kibana Machine <[email protected]>
elastic · Apr 26, 2021 · 9b2d0c3 · 9b2d0c3
1 parent f07ebc8
commit 9b2d0c3
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 80 deletions.
diff --git a/x-pack/plugins/enterprise_search/public/applications/__mocks__/react_router_history.mock.ts b/x-pack/plugins/enterprise_search/public/applications/__mocks__/react_router_history.mock.ts
@@ -32,6 +32,7 @@ jest.mock('react-router-dom', () => {
     useHistory: jest.fn(() => mockHistory),
     useLocation: jest.fn(() => mockLocation),
     useParams: jest.fn(() => ({})),
+    useRouteMatch: jest.fn(() => null),
     // Note: RR's generatePath() opinionatedly encodeURI()s paths (although this doesn't actually
     // show up/affect the final browser URL). Since we already have a generateEncodedPath helper &
     // RR is removing this behavior in history 5.0+, I'm mocking tests to remove the extra encoding

diff --git a/...se_search/public/applications/app_search/components/error_connecting/error_connecting.tsx b/...se_search/public/applications/app_search/components/error_connecting/error_connecting.tsx
@@ -7,7 +7,7 @@
 
 import React from 'react';
 
-import { EuiPageContent } from '@elastic/eui';
+import { EuiPage, EuiPageContent } from '@elastic/eui';
 
 import { ErrorStatePrompt } from '../../../shared/error_state';
 import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
@@ -19,9 +19,11 @@ export const ErrorConnecting: React.FC = () => {
       <SetPageChrome />
       <SendTelemetry action="error" metric="cannot_connect" />
 
-      <EuiPageContent hasBorder>
-        <ErrorStatePrompt />
-      </EuiPageContent>
+      <EuiPage restrictWidth>
+        <EuiPageContent hasBorder>
+          <ErrorStatePrompt />
+        </EuiPageContent>
+      </EuiPage>
     </>
   );
 };
diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/index.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/index.test.tsx
@@ -6,12 +6,13 @@
  */
 
 import { DEFAULT_INITIAL_APP_DATA } from '../../../common/__mocks__';
-import '../__mocks__/enterprise_search_url.mock';
 import { setMockValues, rerender } from '../__mocks__';
+import '../__mocks__/enterprise_search_url.mock';
+import '../__mocks__/react_router_history.mock';
 
 import React from 'react';
 
-import { Redirect } from 'react-router-dom';
+import { Redirect, useRouteMatch } from 'react-router-dom';
 
 import { shallow, ShallowWrapper } from 'enzyme';
 
@@ -20,7 +21,7 @@ import { Layout, SideNav, SideNavLink } from '../shared/layout';
 jest.mock('./app_logic', () => ({ AppLogic: jest.fn() }));
 import { AppLogic } from './app_logic';
 
-import { EngineRouter } from './components/engine';
+import { EngineRouter, EngineNav } from './components/engine';
 import { EngineCreation } from './components/engine_creation';
 import { EnginesOverview } from './components/engines';
 import { ErrorConnecting } from './components/error_connecting';
@@ -31,26 +32,38 @@ import { SetupGuide } from './components/setup_guide';
 import { AppSearch, AppSearchUnconfigured, AppSearchConfigured, AppSearchNav } from './';
 
 describe('AppSearch', () => {
+  it('always renders the Setup Guide', () => {
+    const wrapper = shallow(<AppSearch />);
+
+    expect(wrapper.find(SetupGuide)).toHaveLength(1);
+  });
+
   it('renders AppSearchUnconfigured when config.host is not set', () => {
     setMockValues({ config: { host: '' } });
     const wrapper = shallow(<AppSearch />);
 
     expect(wrapper.find(AppSearchUnconfigured)).toHaveLength(1);
   });
 
-  it('renders AppSearchConfigured when config.host set', () => {
-    setMockValues({ config: { host: 'some.url' } });
+  it('renders ErrorConnecting when Enterprise Search is unavailable', () => {
+    setMockValues({ errorConnecting: true });
+    const wrapper = shallow(<AppSearch />);
+
+    expect(wrapper.find(ErrorConnecting)).toHaveLength(1);
+  });
+
+  it('renders AppSearchConfigured when config.host is set & available', () => {
+    setMockValues({ errorConnecting: false, config: { host: 'some.url' } });
     const wrapper = shallow(<AppSearch />);
 
     expect(wrapper.find(AppSearchConfigured)).toHaveLength(1);
   });
 });
 
 describe('AppSearchUnconfigured', () => {
-  it('renders the Setup Guide and redirects to the Setup Guide', () => {
+  it('redirects to the Setup Guide', () => {
     const wrapper = shallow(<AppSearchUnconfigured />);
 
-    expect(wrapper.find(SetupGuide)).toHaveLength(1);
     expect(wrapper.find(Redirect)).toHaveLength(1);
   });
 });
@@ -64,8 +77,8 @@ describe('AppSearchConfigured', () => {
   });
 
   it('renders with layout', () => {
-    expect(wrapper.find(Layout)).toHaveLength(2);
-    expect(wrapper.find(Layout).last().prop('readOnlyMode')).toBeFalsy();
+    expect(wrapper.find(Layout)).toHaveLength(1);
+    expect(wrapper.find(Layout).prop('readOnlyMode')).toBeFalsy();
     expect(wrapper.find(EnginesOverview)).toHaveLength(1);
     expect(wrapper.find(EngineRouter)).toHaveLength(1);
   });
@@ -74,13 +87,6 @@ describe('AppSearchConfigured', () => {
     expect(AppLogic).toHaveBeenCalledWith(DEFAULT_INITIAL_APP_DATA);
   });
 
-  it('renders ErrorConnecting', () => {
-    setMockValues({ myRole: {}, errorConnecting: true });
-    rerender(wrapper);
-
-    expect(wrapper.find(ErrorConnecting)).toHaveLength(1);
-  });
-
   it('passes readOnlyMode state', () => {
     setMockValues({ myRole: {}, readOnlyMode: true });
     rerender(wrapper);
@@ -145,11 +151,22 @@ describe('AppSearchNav', () => {
     expect(wrapper.find(SideNavLink).prop('to')).toEqual('/engines');
   });
 
-  it('renders an Engine subnav if passed', () => {
-    const wrapper = shallow(<AppSearchNav subNav={<div data-test-subj="subnav">Testing</div>} />);
-    const link = wrapper.find(SideNavLink).dive();
+  describe('engine subnavigation', () => {
+    const getEnginesLink = (wrapper: ShallowWrapper) => wrapper.find(SideNavLink).dive();
 
-    expect(link.find('[data-test-subj="subnav"]')).toHaveLength(1);
+    it('does not render the engine subnav on top-level routes', () => {
+      (useRouteMatch as jest.Mock).mockReturnValueOnce(false);
+      const wrapper = shallow(<AppSearchNav />);
+
+      expect(getEnginesLink(wrapper).find(EngineNav)).toHaveLength(0);
+    });
+
+    it('renders the engine subnav if currently on an engine route', () => {
+      (useRouteMatch as jest.Mock).mockReturnValueOnce(true);
+      const wrapper = shallow(<AppSearchNav />);
+
+      expect(getEnginesLink(wrapper).find(EngineNav)).toHaveLength(1);
+    });
   });
 
   it('renders the Settings link', () => {

diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx
@@ -6,7 +6,7 @@
  */
 
 import React from 'react';
-import { Route, Redirect, Switch } from 'react-router-dom';
+import { Route, Redirect, Switch, useRouteMatch } from 'react-router-dom';
 
 import { useValues } from 'kea';
 
@@ -45,18 +45,28 @@ import {
 
 export const AppSearch: React.FC<InitialAppData> = (props) => {
   const { config } = useValues(KibanaLogic);
-  return !config.host ? (
-    <AppSearchUnconfigured />
-  ) : (
-    <AppSearchConfigured {...(props as Required<InitialAppData>)} />
+  const { errorConnecting } = useValues(HttpLogic);
+
+  return (
+    <Switch>
+      <Route exact path={SETUP_GUIDE_PATH}>
+        <SetupGuide />
+      </Route>
+      <Route>
+        {!config.host ? (
+          <AppSearchUnconfigured />
+        ) : errorConnecting ? (
+          <ErrorConnecting />
+        ) : (
+          <AppSearchConfigured {...(props as Required<InitialAppData>)} />
+        )}
+      </Route>
+    </Switch>
   );
 };
 
 export const AppSearchUnconfigured: React.FC = () => (
   <Switch>
-    <Route exact path={SETUP_GUIDE_PATH}>
-      <SetupGuide />
-    </Route>
     <Route>
       <Redirect to={SETUP_GUIDE_PATH} />
     </Route>
@@ -67,79 +77,68 @@ export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) =
   const {
     myRole: { canManageEngines, canManageMetaEngines, canViewRoleMappings },
   } = useValues(AppLogic(props));
-  const { errorConnecting, readOnlyMode } = useValues(HttpLogic);
+  const { readOnlyMode } = useValues(HttpLogic);
 
   return (
     <Switch>
-      <Route exact path={SETUP_GUIDE_PATH}>
-        <SetupGuide />
-      </Route>
       {process.env.NODE_ENV === 'development' && (
         <Route path={LIBRARY_PATH}>
           <Library />
         </Route>
       )}
-      <Route path={ENGINE_PATH}>
-        <Layout navigation={<AppSearchNav subNav={<EngineNav />} />} readOnlyMode={readOnlyMode}>
-          <EngineRouter />
-        </Layout>
-      </Route>
       <Route>
         <Layout navigation={<AppSearchNav />} readOnlyMode={readOnlyMode}>
-          {errorConnecting ? (
-            <ErrorConnecting />
-          ) : (
-            <Switch>
-              <Route exact path={ROOT_PATH}>
-                <Redirect to={ENGINES_PATH} />
+          <Switch>
+            <Route exact path={ROOT_PATH}>
+              <Redirect to={ENGINES_PATH} />
+            </Route>
+            <Route exact path={ENGINES_PATH}>
+              <EnginesOverview />
+            </Route>
+            <Route path={ENGINE_PATH}>
+              <EngineRouter />
+            </Route>
+            <Route exact path={SETTINGS_PATH}>
+              <Settings />
+            </Route>
+            <Route exact path={CREDENTIALS_PATH}>
+              <Credentials />
+            </Route>
+            {canViewRoleMappings && (
+              <Route path={ROLE_MAPPINGS_PATH}>
+                <RoleMappingsRouter />
               </Route>
-              <Route exact path={ENGINES_PATH}>
-                <EnginesOverview />
+            )}
+            {canManageEngines && (
+              <Route exact path={ENGINE_CREATION_PATH}>
+                <EngineCreation />
               </Route>
-              <Route exact path={SETTINGS_PATH}>
-                <Settings />
+            )}
+            {canManageMetaEngines && (
+              <Route exact path={META_ENGINE_CREATION_PATH}>
+                <MetaEngineCreation />
               </Route>
-              <Route exact path={CREDENTIALS_PATH}>
-                <Credentials />
-              </Route>
-              {canViewRoleMappings && (
-                <Route path={ROLE_MAPPINGS_PATH}>
-                  <RoleMappingsRouter />
-                </Route>
-              )}
-              {canManageEngines && (
-                <Route exact path={ENGINE_CREATION_PATH}>
-                  <EngineCreation />
-                </Route>
-              )}
-              {canManageMetaEngines && (
-                <Route exact path={META_ENGINE_CREATION_PATH}>
-                  <MetaEngineCreation />
-                </Route>
-              )}
-              <Route>
-                <NotFound product={APP_SEARCH_PLUGIN} />
-              </Route>
-            </Switch>
-          )}
+            )}
+            <Route>
+              <NotFound product={APP_SEARCH_PLUGIN} />
+            </Route>
+          </Switch>
         </Layout>
       </Route>
     </Switch>
   );
 };
 
-interface AppSearchNavProps {
-  subNav?: React.ReactNode;
-}
-
-export const AppSearchNav: React.FC<AppSearchNavProps> = ({ subNav }) => {
+export const AppSearchNav: React.FC = () => {
   const {
     myRole: { canViewSettings, canViewAccountCredentials, canViewRoleMappings },
   } = useValues(AppLogic);
 
+  const isEngineRoute = !!useRouteMatch(ENGINE_PATH);
+
   return (
     <SideNav product={APP_SEARCH_PLUGIN}>
-      <SideNavLink to={ENGINES_PATH} subNav={subNav} isRoot>
+      <SideNavLink to={ENGINES_PATH} subNav={isEngineRoute ? <EngineNav /> : null} isRoot>
         {ENGINES_TITLE}
       </SideNavLink>
       {canViewSettings && <SideNavLink to={SETTINGS_PATH}>{SETTINGS_TITLE}</SideNavLink>}