From 73ad80b49312baad1debf130b890f44683f37746 Mon Sep 17 00:00:00 2001 From: Constance Date: Fri, 8 May 2020 08:48:56 -0700 Subject: [PATCH] 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: 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 `` from `` 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) --- src/core/public/public.api.md | 6 ++ src/core/server/server.api.md | 6 ++ src/core/utils/default_app_categories.ts | 12 ++- .../components/empty_states/error_state.tsx | 2 +- .../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 +++--- .../generate_breadcrumbs.test.ts | 86 +++++++++---------- .../generate_breadcrumbs.ts | 34 ++++---- .../enterprise_search/public/plugin.ts | 17 ++-- 12 files changed, 149 insertions(+), 114 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/src/core/public/public.api.md b/src/core/public/public.api.md index 74c41d010ca8d..7edc2ab9ce14e 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -459,6 +459,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 index eef071e9488bf..76d73b0741a3d 100644 --- 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 index 5708bcfeac31a..cc9bfb1db04d5 100644 --- 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/x-pack/plugins/enterprise_search/public/applications/app_search/components/empty_states/error_state.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/empty_states/error_state.tsx index 7459800d4a893..9067f8dfc9c81 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/empty_states/error_state.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/empty_states/error_state.tsx @@ -44,7 +44,7 @@ export const ErrorState: ReactFC<> = () => { } actions={ - + Review the setup guide } 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(); @@ -34,7 +34,7 @@ describe('App Search Routes', () => { }); }); - describe('/app_search/setup_guide', () => { + describe('/setup_guide', () => { it('renders', () => { const wrapper = shallow(); 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 ( <> - - {!enterpriseSearchUrl ? : } + + {!enterpriseSearchUrl ? : } - + 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 = () =>
Hello world!
; + + 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 = ( }} > - - - {/* This will eventually contain an Enterprise Search landing page, - and we'll also actually have a /workplace_search route */} - - - - - - + + + , params.element diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.test.ts index cc6353fd3d3d0..b07aacf443abb 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.test.ts +++ b/x-pack/plugins/enterprise_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,24 +135,30 @@ 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', }, @@ -173,12 +166,15 @@ describe('enterpriseSearchBreadcrumbs', () => { }); 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'); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.ts index 9aab47e51433a..fb59af54ca84c 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_breadcrumbs/generate_breadcrumbs.ts @@ -16,19 +16,24 @@ import { letBrowserHandleEvent } from '../react_router_helpers'; interface IGenerateBreadcrumbProps { text: string; - path: string; - history: History; + path?: string; + history?: History; } -export const generateBreadcrumb = ({ text, path, history }: IGenerateBreadcrumbProps) => ({ - text, - href: history.createHref({ pathname: path }), - onClick: event => { - if (letBrowserHandleEvent(event)) return; - event.preventDefault(); - history.push(path); - }, -}); +export const generateBreadcrumb = ({ text, path, history }: IGenerateBreadcrumbProps) => { + const breadcrumb = { text }; + + if (path && history) { + breadcrumb.href = history.createHref({ pathname: path }); + breadcrumb.onClick = event => { + if (letBrowserHandleEvent(event)) return; + event.preventDefault(); + history.push(path); + }; + } + + return breadcrumb; +}; /** * Product-specific breadcrumb helpers @@ -39,12 +44,9 @@ type TBreadcrumbs = EuiBreadcrumb[] | []; export const enterpriseSearchBreadcrumbs = (history: History) => ( breadcrumbs: TBreadcrumbs = [] ) => [ - generateBreadcrumb({ text: 'Enterprise Search', path: '/', history }), + generateBreadcrumb({ text: 'Enterprise Search' }), ...breadcrumbs.map(({ text, path }) => generateBreadcrumb({ text, path, history })), ]; export const appSearchBreadcrumbs = (history: History) => (breadcrumbs: TBreadcrumbs = []) => - enterpriseSearchBreadcrumbs(history)([ - { text: 'App Search', path: '/app_search' }, - ...breadcrumbs, - ]); + enterpriseSearchBreadcrumbs(history)([{ text: 'App Search', path: '/' }, ...breadcrumbs]); diff --git a/x-pack/plugins/enterprise_search/public/plugin.ts b/x-pack/plugins/enterprise_search/public/plugin.ts index cf495b6a6f9de..3f6493a81272f 100644 --- a/x-pack/plugins/enterprise_search/public/plugin.ts +++ b/x-pack/plugins/enterprise_search/public/plugin.ts @@ -40,19 +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 - euiIconType: AppSearchLogo, // TODO: Temporary - App Search will likely no longer need an icon once the nav structure changes. - category: DEFAULT_APP_CATEGORIES.management, // TODO - This is likely not final/correct - order: 10, // TODO - This will also likely not be needed once new nav structure changes land + 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', @@ -60,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) {}