Update plugin to use new Kibana nav + URL update (#12)

* Update new nav categories to add Enterprise Search + update plugin to use new category - per @johnbarrierwilson and Matt Riley, Enterprise Search should be under Kibana and above Observability - Run `node scripts/check_published_api_changes.js --accept` since this new category affects public API * [URL UPDATE] Change '/app/enterprise_search/app_search' to '/app/app_search' - This needs to be done because App Search and Workplace search *have* to be registered as separate plugins to have 2 distinct nav links - Currently Kibana doesn't support nested app names (see: elastic#59190) but potentially will in the future - To support this change, we need to update applications/index.tsx to NOT handle '/app/enterprise_search' level routing, but instead accept an async imported app component (e.g. AppSearch, WorkplaceSearch). - AppSearch should now treat its router as root '/' instead of '/app_search' - (Addl) Per Josh Dover's recommendation, switch to `<Router history={params.history}>` from `<BrowserRouter basename={params.appBasePath}>` since they're deprecating appBasePath * Update breadcrumbs helper to account for new URLs - Remove path for Enterprise Search breadcrumb, since '/app/enterprise_search' will not link anywhere meaningful for the foreseeable future, so the Enterprise Search root should not go anywhere - Update App Search helper to go to root path, per new React Router setup Test changes: - Mock custom basepath for App Search tests - Swap enterpriseSearchBreadcrumbs and appSearchBreadcrumbs test order (since the latter overrides the default mock)
cee-chen · Jun 16, 2020 · d7e58cd · d7e58cd
1 parent 5542dec
commit d7e58cd
Show file tree

Hide file tree

Showing 12 changed files with 149 additions and 114 deletions.
diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md
@@ -580,6 +580,12 @@ export const DEFAULT_APP_CATEGORIES: Readonly<{
         euiIconType: string;
         order: number;
     };
+    enterpriseSearch: {
+        id: string;
+        label: string;
+        order: number;
+        euiIconType: string;
+    };
     observability: {
         id: string;
         label: string;

diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md
@@ -705,6 +705,12 @@ export const DEFAULT_APP_CATEGORIES: Readonly<{
         euiIconType: string;
         order: number;
     };
+    enterpriseSearch: {
+        id: string;
+        label: string;
+        order: number;
+        euiIconType: string;
+    };
     observability: {
         id: string;
         label: string;

diff --git a/src/core/utils/default_app_categories.ts b/src/core/utils/default_app_categories.ts
@@ -29,20 +29,28 @@ export const DEFAULT_APP_CATEGORIES = Object.freeze({
     euiIconType: 'logoKibana',
     order: 1000,
   },
+  enterpriseSearch: {
+    id: 'enterpriseSearch',
+    label: i18n.translate('core.ui.enterpriseSearchNavList.label', {
+      defaultMessage: 'Enterprise Search',
+    }),
+    order: 2000,
+    euiIconType: 'logoEnterpriseSearch',
+  },
   observability: {
     id: 'observability',
     label: i18n.translate('core.ui.observabilityNavList.label', {
       defaultMessage: 'Observability',
     }),
     euiIconType: 'logoObservability',
-    order: 2000,
+    order: 3000,
   },
   security: {
     id: 'security',
     label: i18n.translate('core.ui.securityNavList.label', {
       defaultMessage: 'Security',
     }),
-    order: 3000,
+    order: 4000,
     euiIconType: 'logoSecurity',
   },
   management: {

diff --git a/.../enterprise_search/public/applications/app_search/components/empty_states/error_state.tsx b/.../enterprise_search/public/applications/app_search/components/empty_states/error_state.tsx
@@ -44,7 +44,7 @@ export const ErrorState: ReactFC<> = () => {
               </>
             }
             actions={
-              <EuiButton iconType="help" fill to="/app_search/setup_guide">
+              <EuiButton iconType="help" fill to="/setup_guide">
                 Review the setup guide
               </EuiButton>
             }

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
@@ -16,7 +16,7 @@ import { EngineOverview } from './components/engine_overview';
 import { AppSearch } from './';
 
 describe('App Search Routes', () => {
-  describe('/app_search', () => {
+  describe('/', () => {
     it('redirects to Setup Guide when enterpriseSearchUrl is not set', () => {
       useContext.mockImplementationOnce(() => ({ enterpriseSearchUrl: '' }));
       const wrapper = shallow(<AppSearch />);
@@ -34,7 +34,7 @@ describe('App Search Routes', () => {
     });
   });
 
-  describe('/app_search/setup_guide', () => {
+  describe('/setup_guide', () => {
     it('renders', () => {
       const wrapper = shallow(<AppSearch />);
 

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
@@ -17,10 +17,10 @@ export const AppSearch: React.FC<> = () => {
 
   return (
     <>
-      <Route exact path="/app_search">
-        {!enterpriseSearchUrl ? <Redirect to="/app_search/setup_guide" /> : <EngineOverview />}
+      <Route exact path="/">
+        {!enterpriseSearchUrl ? <Redirect to="/setup_guide" /> : <EngineOverview />}
       </Route>
-      <Route path="/app_search/setup_guide">
+      <Route path="/setup_guide">
         <SetupGuide />
       </Route>
     </>

diff --git a/x-pack/plugins/enterprise_search/public/applications/index.test.ts b/x-pack/plugins/enterprise_search/public/applications/index.test.ts
diff --git a/x-pack/plugins/enterprise_search/public/applications/index.test.tsx b/x-pack/plugins/enterprise_search/public/applications/index.test.tsx
@@ -0,0 +1,40 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+
+import { coreMock } from 'src/core/public/mocks';
+import { licensingMock } from '../../../licensing/public/mocks';
+
+import { renderApp } from './';
+import { AppSearch } from './app_search';
+
+describe('renderApp', () => {
+  const params = coreMock.createAppMountParamters();
+  const core = coreMock.createStart();
+  const config = {};
+  const plugins = {
+    licensing: licensingMock.createSetup(),
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('mounts and unmounts UI', () => {
+    const MockApp: React.FC = () => <div className="hello-world">Hello world!</div>;
+
+    const unmount = renderApp(MockApp, core, params, config, plugins);
+    expect(params.element.querySelector('.hello-world')).not.toBeNull();
+    unmount();
+    expect(params.element.innerHTML).toEqual('');
+  });
+
+  it('renders AppSearch', () => {
+    renderApp(AppSearch, core, params, config, plugins);
+    expect(params.element.querySelector('.setup-guide')).not.toBeNull();
+  });
+});
diff --git a/x-pack/plugins/enterprise_search/public/applications/index.tsx b/x-pack/plugins/enterprise_search/public/applications/index.tsx
@@ -6,16 +6,14 @@
 
 import React from 'react';
 import ReactDOM from 'react-dom';
-import { BrowserRouter, Route, Redirect } from 'react-router-dom';
+import { Router, Route, Redirect } from 'react-router-dom';
 
 import { CoreStart, AppMountParams, HttpHandler } from 'src/core/public';
 import { ClientConfigType, PluginsSetup } from '../plugin';
 import { TSetBreadcrumbs } from './shared/kibana_breadcrumbs';
 import { ILicense } from '../../../../licensing/public';
 import { LicenseProvider } from './shared/licensing';
 
-import { AppSearch } from './app_search';
-
 export interface IKibanaContext {
   enterpriseSearchUrl?: string;
   http(): HttpHandler;
@@ -25,7 +23,14 @@ export interface IKibanaContext {
 
 export const KibanaContext = React.createContext();
 
+/**
+ * This file serves as a reusable wrapper to share Kibana-level context and other helpers
+ * between various Enterprise Search plugins (e.g. AppSearch, WorkplaceSearch, ES landing page)
+ * which should be imported and passed in as the first param in plugin.ts.
+ */
+
 export const renderApp = (
+  App: React.Element,
   core: CoreStart,
   params: AppMountParams,
   config: ClientConfigType,
@@ -41,16 +46,9 @@ export const renderApp = (
       }}
     >
       <LicenseProvider>
-        <BrowserRouter basename={params.appBasePath}>
-          <Route exact path="/">
-            {/* This will eventually contain an Enterprise Search landing page,
-            and we'll also actually have a /workplace_search route */}
-            <Redirect to="/app_search" />
-          </Route>
-          <Route path="/app_search">
-            <AppSearch />
-          </Route>
-        </BrowserRouter>
+        <Router history={params.history}>
+          <App />
+        </Router>
       </LicenseProvider>
     </KibanaContext.Provider>,
     params.element

diff --git a/...erprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.test.ts b/...erprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.test.ts
@@ -47,9 +47,16 @@ describe('generateBreadcrumb', () => {
 
     expect(mockHistory.push).not.toHaveBeenCalled();
   });
+
+  it('does not generate link behavior if path is excluded', () => {
+    const breadcrumb = generateBreadcrumb({ text: 'Unclickable breadcrumb' });
+
+    expect(breadcrumb.href).toBeUndefined();
+    expect(breadcrumb.onClick).toBeUndefined();
+  });
 });
 
-describe('appSearchBreadcrumbs', () => {
+describe('enterpriseSearchBreadcrumbs', () => {
   const breadCrumbs = [
     {
       text: 'Page 1',
@@ -65,20 +72,13 @@ describe('appSearchBreadcrumbs', () => {
     jest.clearAllMocks();
   });
 
-  const subject = () => appSearchBreadcrumbs(mockHistory)(breadCrumbs);
+  const subject = () => enterpriseSearchBreadcrumbs(mockHistory)(breadCrumbs);
 
-  it('Builds a chain of breadcrumbs with Enterprise Search and App Search at the root', () => {
+  it('Builds a chain of breadcrumbs with Enterprise Search at the root', () => {
     expect(subject()).toEqual([
       {
-        href: '/enterprise_search/',
-        onClick: expect.any(Function),
         text: 'Enterprise Search',
       },
-      {
-        href: '/enterprise_search/app_search',
-        onClick: expect.any(Function),
-        text: 'App Search',
-      },
       {
         href: '/enterprise_search/page1',
         onClick: expect.any(Function),
@@ -93,17 +93,10 @@ describe('appSearchBreadcrumbs', () => {
   });
 
   it('shows just the root if breadcrumbs is empty', () => {
-    expect(appSearchBreadcrumbs(mockHistory)()).toEqual([
+    expect(enterpriseSearchBreadcrumbs(mockHistory)()).toEqual([
       {
-        href: '/enterprise_search/',
-        onClick: expect.any(Function),
         text: 'Enterprise Search',
       },
-      {
-        href: '/enterprise_search/app_search',
-        onClick: expect.any(Function),
-        text: 'App Search',
-      },
     ]);
   });
 
@@ -112,29 +105,23 @@ describe('appSearchBreadcrumbs', () => {
       preventDefault: jest.fn(),
     };
 
-    it('has a link to Enterprise Search Home page first', () => {
-      subject()[0].onClick(eventMock);
-      expect(mockHistory.push).toHaveBeenCalledWith('/');
+    it('has Enterprise Search text first', () => {
+      expect(subject()[0].onClick).toBeUndefined();
     });
 
-    it('has a link to App Search second', () => {
+    it('has a link to page 1 second', () => {
       subject()[1].onClick(eventMock);
-      expect(mockHistory.push).toHaveBeenCalledWith('/app_search');
-    });
-
-    it('has a link to page 1 third', () => {
-      subject()[2].onClick(eventMock);
       expect(mockHistory.push).toHaveBeenCalledWith('/page1');
     });
 
     it('has a link to page 2 last', () => {
-      subject()[3].onClick(eventMock);
+      subject()[2].onClick(eventMock);
       expect(mockHistory.push).toHaveBeenCalledWith('/page2');
     });
   });
 });
 
-describe('enterpriseSearchBreadcrumbs', () => {
+describe('appSearchBreadcrumbs', () => {
   const breadCrumbs = [
     {
       text: 'Page 1',
@@ -148,37 +135,46 @@ describe('enterpriseSearchBreadcrumbs', () => {
 
   beforeEach(() => {
     jest.clearAllMocks();
+    mockHistory.createHref.mockImplementation(
+      ({ pathname }) => `/enterprise_search/app_search${pathname}`
+    );
   });
 
-  const subject = () => enterpriseSearchBreadcrumbs(mockHistory)(breadCrumbs);
+  const subject = () => appSearchBreadcrumbs(mockHistory)(breadCrumbs);
 
-  it('Builds a chain of breadcrumbs with Enterprise Search at the root', () => {
+  it('Builds a chain of breadcrumbs with Enterprise Search and App Search at the root', () => {
     expect(subject()).toEqual([
       {
-        href: '/enterprise_search/',
-        onClick: expect.any(Function),
         text: 'Enterprise Search',
       },
       {
-        href: '/enterprise_search/page1',
+        href: '/enterprise_search/app_search/',
+        onClick: expect.any(Function),
+        text: 'App Search',
+      },
+      {
+        href: '/enterprise_search/app_search/page1',
         onClick: expect.any(Function),
         text: 'Page 1',
       },
       {
-        href: '/enterprise_search/page2',
+        href: '/enterprise_search/app_search/page2',
         onClick: expect.any(Function),
         text: 'Page 2',
       },
     ]);
   });
 
   it('shows just the root if breadcrumbs is empty', () => {
-    expect(enterpriseSearchBreadcrumbs(mockHistory)()).toEqual([
+    expect(appSearchBreadcrumbs(mockHistory)()).toEqual([
       {
-        href: '/enterprise_search/',
-        onClick: expect.any(Function),
         text: 'Enterprise Search',
       },
+      {
+        href: '/enterprise_search/app_search/',
+        onClick: expect.any(Function),
+        text: 'App Search',
+      },
     ]);
   });
 
@@ -187,18 +183,22 @@ describe('enterpriseSearchBreadcrumbs', () => {
       preventDefault: jest.fn(),
     };
 
-    it('has a link to Enterprise Search Home page first', () => {
-      subject()[0].onClick(eventMock);
-      expect(mockHistory.push).toHaveBeenCalledWith('/');
+    it('has Enterprise Search text first', () => {
+      expect(subject()[0].onClick).toBeUndefined();
     });
 
-    it('has a link to page 1 second', () => {
+    it('has a link to App Search second', () => {
       subject()[1].onClick(eventMock);
+      expect(mockHistory.push).toHaveBeenCalledWith('/');
+    });
+
+    it('has a link to page 1 third', () => {
+      subject()[2].onClick(eventMock);
       expect(mockHistory.push).toHaveBeenCalledWith('/page1');
     });
 
     it('has a link to page 2 last', () => {
-      subject()[2].onClick(eventMock);
+      subject()[3].onClick(eventMock);
       expect(mockHistory.push).toHaveBeenCalledWith('/page2');
     });
   });