[Enterprise Search] DRY out createHref helper for navigateToUrl calls (#78717) (#78800)

Constance · web-flow · commit 57d76ed57299 · 2020-09-29T10:14:34.000-07:00
* DRY out createHref helper
- that navigateToUrl will shortly also use

+ fix mockHistory type complaint

* KibanaLogic: Update navigateToUrl to use createHref helper

+ add param.history value, since we're technically supposed to be using Kibana's passed history over anything from useHistory

* Update breadcrumbs &amp; eui link helpers w/ new KibanaLogic changes

- navigateToUrl is now double-dipping createHref, so we update it to not do so
- remove useHistory in favor of grabbing history from KibanaLogic

* [Optional?] Update FlashMessages to grab history from KibanaLogic

- since we're now storing it there, we might as well grab it as well instead of passing in props?

* [Misc] Fix failing tests due to react router mock
diff --git a/x-pack/plugins/enterprise_search/public/applications/__mocks__/kibana_logic.mock.ts b/x-pack/plugins/enterprise_search/public/applications/__mocks__/kibana_logic.mock.ts
@@ -4,8 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { mockHistory } from './';
+
 export const mockKibanaValues = {
   config: { host: 'http://localhost:3002' },
+  history: mockHistory,
   navigateToUrl: jest.fn(),
   setBreadcrumbs: jest.fn(),
   setDocTitle: jest.fn(),
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
@@ -15,7 +15,7 @@ export const mockHistory = {
     pathname: '/current-path',
   },
   listen: jest.fn(() => jest.fn()),
-};
+} as any;
 export const mockLocation = {
   key: 'someKey',
   pathname: '/current-path',
@@ -25,6 +25,7 @@ export const mockLocation = {
 };
 
 jest.mock('react-router-dom', () => ({
+  ...(jest.requireActual('react-router-dom') as object),
   useHistory: jest.fn(() => mockHistory),
   useLocation: jest.fn(() => mockLocation),
 }));
diff --git a/x-pack/plugins/enterprise_search/public/applications/index.tsx b/x-pack/plugins/enterprise_search/public/applications/index.tsx
@@ -41,6 +41,7 @@ export const renderApp = (
 
   const unmountKibanaLogic = mountKibanaLogic({
     config,
+    history: params.history,
     navigateToUrl: core.application.navigateToUrl,
     setBreadcrumbs: core.chrome.setBreadcrumbs,
     setDocTitle: core.chrome.docTitle.change,
@@ -53,9 +54,7 @@ export const renderApp = (
     errorConnecting,
     readOnlyMode: initialData.readOnlyMode,
   });
-  const unmountFlashMessagesLogic = mountFlashMessagesLogic({
-    history: params.history,
-  });
+  const unmountFlashMessagesLogic = mountFlashMessagesLogic();
 
   ReactDOM.render(
     <I18nProvider>
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.test.ts
@@ -7,11 +7,14 @@
 import { resetContext } from 'kea';
 
 import { mockHistory } from '../../__mocks__';
+jest.mock('../kibana', () => ({
+  KibanaLogic: { values: { history: mockHistory } },
+}));
 
 import { FlashMessagesLogic, mountFlashMessagesLogic, IFlashMessage } from './';
 
 describe('FlashMessagesLogic', () => {
-  const mount = () => mountFlashMessagesLogic({ history: mockHistory as any });
+  const mount = () => mountFlashMessagesLogic();
 
   beforeEach(() => {
     jest.clearAllMocks();
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.ts
@@ -6,7 +6,8 @@
 
 import { kea, MakeLogicType } from 'kea';
 import { ReactNode } from 'react';
-import { History } from 'history';
+
+import { KibanaLogic } from '../kibana';
 
 export interface IFlashMessage {
   type: 'success' | 'info' | 'warning' | 'error';
@@ -61,10 +62,10 @@ export const FlashMessagesLogic = kea<MakeLogicType<IFlashMessagesValues, IFlash
       },
     ],
   },
-  events: ({ props, values, actions }) => ({
+  events: ({ values, actions }) => ({
     afterMount: () => {
       // On React Router navigation, clear previous flash messages and load any queued messages
-      const unlisten = props.history.listen(() => {
+      const unlisten = KibanaLogic.values.history.listen(() => {
         actions.clearFlashMessages();
         actions.setFlashMessages(values.queuedMessages);
         actions.clearQueuedMessages();
@@ -81,11 +82,7 @@ export const FlashMessagesLogic = kea<MakeLogicType<IFlashMessagesValues, IFlash
 /**
  * Mount/props helper
  */
-interface IFlashMessagesLogicProps {
-  history: History;
-}
-export const mountFlashMessagesLogic = (props: IFlashMessagesLogicProps) => {
-  FlashMessagesLogic(props);
+export const mountFlashMessagesLogic = () => {
   const unmount = FlashMessagesLogic.mount();
   return unmount;
 };
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/set_message_helpers.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/set_message_helpers.test.ts
@@ -5,6 +5,9 @@
  */
 
 import { mockHistory } from '../../__mocks__';
+jest.mock('../kibana', () => ({
+  KibanaLogic: { values: { history: mockHistory } },
+}));
 
 import {
   FlashMessagesLogic,
@@ -18,7 +21,7 @@ describe('Flash Message Helpers', () => {
   const message = 'I am a message';
 
   beforeEach(() => {
-    mountFlashMessagesLogic({ history: mockHistory as any });
+    mountFlashMessagesLogic();
   });
 
   it('setSuccessMessage()', () => {
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.test.ts
@@ -20,7 +20,10 @@ describe('KibanaLogic', () => {
     it('sets values from props', () => {
       mountKibanaLogic(mockKibanaValues);
 
-      expect(KibanaLogic.values).toEqual(mockKibanaValues);
+      expect(KibanaLogic.values).toEqual({
+        ...mockKibanaValues,
+        navigateToUrl: expect.any(Function),
+      });
     });
 
     it('gracefully handles missing configs', () => {
@@ -29,4 +32,20 @@ describe('KibanaLogic', () => {
       expect(KibanaLogic.values.config).toEqual({});
     });
   });
+
+  describe('navigateToUrl()', () => {
+    beforeEach(() => mountKibanaLogic(mockKibanaValues));
+
+    it('runs paths through createHref before calling navigateToUrl', () => {
+      KibanaLogic.values.navigateToUrl('/test');
+
+      expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/app/enterprise_search/test');
+    });
+
+    it('does not run paths through createHref if the shouldNotCreateHref option is passed', () => {
+      KibanaLogic.values.navigateToUrl('/test', { shouldNotCreateHref: true });
+
+      expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/test');
+    });
+  });
 });
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.ts
@@ -6,26 +6,40 @@
 
 import { kea, MakeLogicType } from 'kea';
 
+import { History } from 'history';
 import { ApplicationStart, ChromeBreadcrumb } from 'src/core/public';
 
-export interface IKibanaValues {
+import { createHref, ICreateHrefOptions } from '../react_router_helpers';
+
+interface IKibanaLogicProps {
   config: { host?: string };
+  history: History;
   navigateToUrl: ApplicationStart['navigateToUrl'];
   setBreadcrumbs(crumbs: ChromeBreadcrumb[]): void;
   setDocTitle(title: string): void;
 }
+export interface IKibanaValues extends IKibanaLogicProps {
+  navigateToUrl(path: string, options?: ICreateHrefOptions): Promise<void>;
+}
 
 export const KibanaLogic = kea<MakeLogicType<IKibanaValues>>({
   path: ['enterprise_search', 'kibana_logic'],
   reducers: ({ props }) => ({
     config: [props.config || {}, {}],
-    navigateToUrl: [props.navigateToUrl, {}],
+    history: [props.history, {}],
+    navigateToUrl: [
+      (url: string, options?: ICreateHrefOptions) => {
+        const href = createHref(url, props.history, options);
+        return props.navigateToUrl(href);
+      },
+      {},
+    ],
     setBreadcrumbs: [props.setBreadcrumbs, {}],
     setDocTitle: [props.setDocTitle, {}],
   }),
 });
 
-export const mountKibanaLogic = (props: IKibanaValues) => {
+export const mountKibanaLogic = (props: IKibanaLogicProps) => {
   KibanaLogic(props);
   const unmount = KibanaLogic.mount();
   return unmount;
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.test.ts
@@ -5,10 +5,12 @@
  */
 
 import '../../__mocks__/kea.mock';
-import '../../__mocks__/react_router_history.mock';
 import { mockKibanaValues, mockHistory } from '../../__mocks__';
 
-jest.mock('../react_router_helpers', () => ({ letBrowserHandleEvent: jest.fn(() => false) }));
+jest.mock('../react_router_helpers', () => ({
+  letBrowserHandleEvent: jest.fn(() => false),
+  createHref: jest.requireActual('../react_router_helpers').createHref,
+}));
 import { letBrowserHandleEvent } from '../react_router_helpers';
 
 import {
@@ -50,21 +52,23 @@ describe('useBreadcrumbs', () => {
 
   it('prevents default navigation and uses React Router history on click', () => {
     const breadcrumb = useBreadcrumbs([{ text: '', path: '/test' }])[0] as any;
+
+    expect(breadcrumb.href).toEqual('/app/enterprise_search/test');
+    expect(mockHistory.createHref).toHaveBeenCalled();
+
     const event = { preventDefault: jest.fn() };
     breadcrumb.onClick(event);
 
-    expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/app/enterprise_search/test');
-    expect(mockHistory.createHref).toHaveBeenCalled();
     expect(event.preventDefault).toHaveBeenCalled();
+    expect(mockKibanaValues.navigateToUrl).toHaveBeenCalled();
   });
 
   it('does not call createHref if shouldNotCreateHref is passed', () => {
     const breadcrumb = useBreadcrumbs([
       { text: '', path: '/test', shouldNotCreateHref: true },
     ])[0] as any;
-    breadcrumb.onClick({ preventDefault: () => null });
 
-    expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/test');
+    expect(breadcrumb.href).toEqual('/test');
     expect(mockHistory.createHref).not.toHaveBeenCalled();
   });
 
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.ts
@@ -5,7 +5,6 @@
  */
 
 import { useValues } from 'kea';
-import { useHistory } from 'react-router-dom';
 import { EuiBreadcrumb } from '@elastic/eui';
 
 import { KibanaLogic } from '../../shared/kibana';
@@ -16,7 +15,7 @@ import {
   WORKPLACE_SEARCH_PLUGIN,
 } from '../../../../common/constants';
 
-import { letBrowserHandleEvent } from '../react_router_helpers';
+import { letBrowserHandleEvent, createHref } from '../react_router_helpers';
 
 /**
  * Generate React-Router-friendly EUI breadcrumb objects
@@ -33,20 +32,17 @@ interface IBreadcrumb {
 export type TBreadcrumbs = IBreadcrumb[];
 
 export const useBreadcrumbs = (breadcrumbs: TBreadcrumbs) => {
-  const history = useHistory();
-  const { navigateToUrl } = useValues(KibanaLogic);
+  const { navigateToUrl, history } = useValues(KibanaLogic);
 
   return breadcrumbs.map(({ text, path, shouldNotCreateHref }) => {
     const breadcrumb = { text } as EuiBreadcrumb;
 
     if (path) {
-      const href = shouldNotCreateHref ? path : (history.createHref({ pathname: path }) as string);
-
-      breadcrumb.href = href;
+      breadcrumb.href = createHref(path, history, { shouldNotCreateHref });
       breadcrumb.onClick = (event) => {
         if (letBrowserHandleEvent(event)) return;
         event.preventDefault();
-        navigateToUrl(href);
+        navigateToUrl(path, { shouldNotCreateHref });
       };
     }
 
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/create_href.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/create_href.test.ts
@@ -0,0 +1,19 @@
+/*
+ * 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 { mockHistory } from '../../__mocks__';
+
+import { createHref } from './';
+
+describe('createHref', () => {
+  it('generates a path with the React Router basename included', () => {
+    expect(createHref('/test', mockHistory)).toEqual('/app/enterprise_search/test');
+  });
+
+  it('does not include the basename if shouldNotCreateHref is passed', () => {
+    expect(createHref('/test', mockHistory, { shouldNotCreateHref: true })).toEqual('/test');
+  });
+});
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/create_href.ts b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/create_href.ts
@@ -0,0 +1,27 @@
+/*
+ * 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 { History } from 'history';
+
+/**
+ * This helper uses React Router's createHref function to generate links with router basenames accounted for.
+ * For example, if we perform navigateToUrl('/engines') within App Search, we expect the app basename
+ * to be taken into account to be intelligently routed to '/app/enterprise_search/app_search/engines'.
+ *
+ * This helper accomplishes that, while still giving us an escape hatch for navigation *between* apps.
+ * For example, if we want to navigate the user from App Search to Enterprise Search we could
+ * navigateToUrl('/app/enterprise_search', { shouldNotCreateHref: true })
+ */
+export interface ICreateHrefOptions {
+  shouldNotCreateHref?: boolean;
+}
+export const createHref = (
+  path: string,
+  history: History,
+  options?: ICreateHrefOptions
+): string => {
+  return options?.shouldNotCreateHref ? path : history.createHref({ pathname: path });
+};
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.test.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.test.tsx
@@ -5,7 +5,6 @@
  */
 
 import '../../__mocks__/kea.mock';
-import '../../__mocks__/react_router_history.mock';
 
 import React from 'react';
 import { shallow, mount } from 'enzyme';
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.tsx
@@ -6,11 +6,10 @@
 
 import React from 'react';
 import { useValues } from 'kea';
-import { useHistory } from 'react-router-dom';
 import { EuiLink, EuiButton, EuiButtonProps, EuiLinkAnchorProps } from '@elastic/eui';
 
 import { KibanaLogic } from '../../shared/kibana';
-import { letBrowserHandleEvent } from './link_events';
+import { letBrowserHandleEvent, createHref } from './';
 
 /**
  * Generates either an EuiLink or EuiButton with a React-Router-ified link
@@ -33,11 +32,10 @@ export const EuiReactRouterHelper: React.FC<IEuiReactRouterProps> = ({
   shouldNotCreateHref,
   children,
 }) => {
-  const history = useHistory();
-  const { navigateToUrl } = useValues(KibanaLogic);
+  const { navigateToUrl, history } = useValues(KibanaLogic);
 
   // Generate the correct link href (with basename etc. accounted for)
-  const href = shouldNotCreateHref ? to : history.createHref({ pathname: to });
+  const href = createHref(to, history, { shouldNotCreateHref });
 
   const reactRouterLinkClick = (event: React.MouseEvent) => {
     if (onClick) onClick(); // Run any passed click events (e.g. telemetry)
@@ -47,7 +45,7 @@ export const EuiReactRouterHelper: React.FC<IEuiReactRouterProps> = ({
     event.preventDefault();
 
     // Perform SPA navigation.
-    navigateToUrl(href);
+    navigateToUrl(to, { shouldNotCreateHref });
   };
 
   const reactRouterProps = { href, onClick: reactRouterLinkClick };
diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/index.ts
@@ -5,5 +5,6 @@
  */
 
 export { letBrowserHandleEvent } from './link_events';
+export { createHref, ICreateHrefOptions } from './create_href';
 export { EuiReactRouterLink as EuiLink } from './eui_link';
 export { EuiReactRouterButton as EuiButton } from './eui_link';
