Skip to content

Commit

Permalink
Update plugin to use new Kibana nav + URL update (#12)
Browse files Browse the repository at this point in the history
* 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)
  • Loading branch information
Constance authored and cee-chen committed Jun 4, 2020
1 parent ee2d6a6 commit 73ad80b
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 114 deletions.
6 changes: 6 additions & 0 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 10 additions & 2 deletions src/core/utils/default_app_categories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 />);
Expand All @@ -34,7 +34,7 @@ describe('App Search Routes', () => {
});
});

describe('/app_search/setup_guide', () => {
describe('/setup_guide', () => {
it('renders', () => {
const wrapper = shallow(<AppSearch />);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
Expand Down
26 changes: 0 additions & 26 deletions x-pack/plugins/enterprise_search/public/applications/index.test.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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();
});
});
24 changes: 11 additions & 13 deletions x-pack/plugins/enterprise_search/public/applications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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),
Expand All @@ -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',
},
]);
});

Expand All @@ -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',
Expand All @@ -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',
},
]);
});

Expand All @@ -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');
});
});
Expand Down
Loading

0 comments on commit 73ad80b

Please sign in to comment.