From f275dad77fb7bd1c79be80a96bf81b2d217df5da Mon Sep 17 00:00:00 2001 From: Constance Chen <constance.chen.3@gmail.com> Date: Thu, 7 May 2020 15:38:40 -0700 Subject: [PATCH] [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: https://github.com/elastic/kibana/issues/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 --- .../applications/app_search/index.test.tsx | 4 +- .../public/applications/app_search/index.tsx | 6 +-- .../public/applications/index.test.ts | 26 ------------ .../public/applications/index.test.tsx | 40 +++++++++++++++++++ .../public/applications/index.tsx | 24 +++++------ .../enterprise_search/public/plugin.ts | 13 +++--- 6 files changed, 64 insertions(+), 49 deletions(-) delete mode 100644 x-pack/plugins/enterprise_search/public/applications/index.test.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/index.test.tsx 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 index 57cd70389e807..d11c47475089d 100644 --- 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 index 4c1a85358ea14..9afc3c9fd9761 100644 --- 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 deleted file mode 100644 index 7ea5b97feac6c..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/index.test.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* - * 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 { coreMock } from 'src/core/public/mocks'; -import { licensingMock } from '../../../licensing/public/mocks'; - -import { renderApp } from './'; - -describe('renderApp', () => { - it('mounts and unmounts UI', () => { - const params = coreMock.createAppMountParamters(); - const core = coreMock.createStart(); - const config = {}; - const plugins = { - licensing: licensingMock.createSetup(), - }; - - const unmount = renderApp(core, params, config, plugins); - expect(params.element.querySelector('.setup-guide')).not.toBeNull(); - unmount(); - expect(params.element.innerHTML).toEqual(''); - }); -}); diff --git a/x-pack/plugins/enterprise_search/public/applications/index.test.tsx b/x-pack/plugins/enterprise_search/public/applications/index.test.tsx new file mode 100644 index 0000000000000..fd88fc32ff4ae --- /dev/null +++ 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 index 0fb18a8b627bb..f8b1331e899cf 100644 --- a/x-pack/plugins/enterprise_search/public/applications/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/index.tsx @@ -6,7 +6,7 @@ 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'; @@ -14,8 +14,6 @@ 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/x-pack/plugins/enterprise_search/public/plugin.ts b/x-pack/plugins/enterprise_search/public/plugin.ts index ff1a78f1999cf..3f6493a81272f 100644 --- a/x-pack/plugins/enterprise_search/public/plugin.ts +++ b/x-pack/plugins/enterprise_search/public/plugin.ts @@ -40,17 +40,20 @@ export class EnterpriseSearchPlugin implements Plugin { const config = this.config; core.application.register({ - id: 'enterprise_search', - title: 'App Search', // TODO: This will eventually be 'Enterprise Search' once there's more than just App Search in here + id: 'app_search', + title: 'App Search', + // appRoute: '/app/enterprise_search/app_search', // TODO: Switch to this once https://github.com/elastic/kibana/issues/59190 is in category: DEFAULT_APP_CATEGORIES.enterpriseSearch, mount: async (params: AppMountParameters) => { const [coreStart] = await core.getStartServices(); const { renderApp } = await import('./applications'); + const { AppSearch } = await import('./applications/app_search'); - return renderApp(coreStart, params, config, plugins); + return renderApp(AppSearch, coreStart, params, config, plugins); }, }); + // TODO: Workplace Search will need to register its own plugin. plugins.home.featureCatalogue.register({ id: 'app_search', @@ -58,11 +61,11 @@ export class EnterpriseSearchPlugin implements Plugin { icon: AppSearchLogo, description: 'Leverage dashboards, analytics, and APIs for advanced application search made simple.', - path: '/app/enterprise_search/app_search', + path: '/app/app_search', // TODO: Switch to '/app/enterprise_search/app_search' once https://github.com/elastic/kibana/issues/59190 is in category: FeatureCatalogueCategory.DATA, showOnHomePage: true, }); - // TODO: Workplace Search will likely also register its own feature catalogue section/card. + // TODO: Workplace Search will need to register its own feature catalogue section/card. } public start(core: CoreStart) {}