Skip to content

Commit

Permalink
[Enterprise Search] Fix cross-application links & breadcrumbs (#79530) (
Browse files Browse the repository at this point in the history
#79555)

* Update our cross-app links to correctly include Kibana basePaths

- see https://www.elastic.co/guide/en/kibana/master/kibana-navigation.html
- for this change, we need to pass in the `http` lib as another dependency alongside `history` to createHelper

* Update helpers using createHref to pass new expected dependency

- Note that a lot of their createHref tests did not functionally change in output because Kibana mocks an empty basePath ('/') in tests
  • Loading branch information
Constance authored Oct 5, 2020
1 parent 23150c8 commit 6686044
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@

import { resetContext } from 'kea';

import { mockKibanaValues } from '../../__mocks__';
import { mockKibanaValues, mockHttpValues } from '../../__mocks__';
jest.mock('../http', () => ({
HttpLogic: { values: { http: mockHttpValues.http } },
}));

import { KibanaLogic, mountKibanaLogic } from './kibana_logic';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { FC } from 'react';
import { History } from 'history';
import { ApplicationStart, ChromeBreadcrumb } from 'src/core/public';

import { HttpLogic } from '../http';
import { createHref, ICreateHrefOptions } from '../react_router_helpers';

interface IKibanaLogicProps {
Expand All @@ -31,7 +32,8 @@ export const KibanaLogic = kea<MakeLogicType<IKibanaValues>>({
history: [props.history, {}],
navigateToUrl: [
(url: string, options?: ICreateHrefOptions) => {
const href = createHref(url, props.history, options);
const deps = { history: props.history, http: HttpLogic.values.http };
const href = createHref(url, deps, options);
return props.navigateToUrl(href);
},
{},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
import { useValues } from 'kea';
import { EuiBreadcrumb } from '@elastic/eui';

import { KibanaLogic } from '../../shared/kibana';
import { KibanaLogic } from '../kibana';
import { HttpLogic } from '../http';

import {
ENTERPRISE_SEARCH_PLUGIN,
Expand Down Expand Up @@ -66,12 +67,13 @@ export const useGenerateBreadcrumbs = (trail: TBreadcrumbTrail): TBreadcrumbs =>

export const useEuiBreadcrumbs = (breadcrumbs: TBreadcrumbs): EuiBreadcrumb[] => {
const { navigateToUrl, history } = useValues(KibanaLogic);
const { http } = useValues(HttpLogic);

return breadcrumbs.map(({ text, path, shouldNotCreateHref }) => {
const breadcrumb: EuiBreadcrumb = { text };

if (path) {
breadcrumb.href = createHref(path, history, { shouldNotCreateHref });
breadcrumb.href = createHref(path, { history, http }, { shouldNotCreateHref });
breadcrumb.onClick = (event) => {
if (letBrowserHandleEvent(event)) return;
event.preventDefault();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,33 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { httpServiceMock } from 'src/core/public/mocks';
import { mockHistory } from '../../__mocks__';

import { createHref } from './';

describe('createHref', () => {
const dependencies = {
history: mockHistory,
http: httpServiceMock.createSetupContract(),
};

it('generates a path with the React Router basename included', () => {
expect(createHref('/test', mockHistory)).toEqual('/app/enterprise_search/test');
expect(createHref('/test', dependencies)).toEqual('/app/enterprise_search/test');
});

it('does not include the basename if shouldNotCreateHref is passed', () => {
expect(createHref('/test', mockHistory, { shouldNotCreateHref: true })).toEqual('/test');
describe('shouldNotCreateHref', () => {
const options = { shouldNotCreateHref: true };

it('does not include the router basename,', () => {
expect(createHref('/test', dependencies, options)).toEqual('/test');
});

it('does include the Kibana basepath,', () => {
const http = httpServiceMock.createSetupContract({ basePath: '/xyz/s/custom-space' });
const basePathDeps = { ...dependencies, http };

expect(createHref('/test', basePathDeps, options)).toEqual('/xyz/s/custom-space/test');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,35 @@
*/

import { History } from 'history';
import { HttpSetup } from 'src/core/public';

/**
* This helper uses React Router's createHref function to generate links with router basenames accounted for.
* This helper uses React Router's createHref function to generate links with router basenames included.
* 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'.
* to be taken into account & 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 })
*
* Said escape hatch should still contain all of Kibana's basepaths - for example,
* 'localhost:5601/xyz' when developing locally, or '/s/some-custom-space/' for space basepaths.
* See: https://www.elastic.co/guide/en/kibana/master/kibana-navigation.html
*
* Links completely outside of Kibana should not use our React Router helpers or navigateToUrl.
*/
interface ICreateHrefDeps {
history: History;
http: HttpSetup;
}
export interface ICreateHrefOptions {
shouldNotCreateHref?: boolean;
}

export const createHref = (
path: string,
history: History,
options?: ICreateHrefOptions
{ history, http }: ICreateHrefDeps,
{ shouldNotCreateHref }: ICreateHrefOptions = {}
): string => {
return options?.shouldNotCreateHref ? path : history.createHref({ pathname: path });
return shouldNotCreateHref ? http.basePath.prepend(path) : history.createHref({ pathname: path });
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import React from 'react';
import { useValues } from 'kea';
import { EuiLink, EuiButton, EuiButtonProps, EuiLinkAnchorProps } from '@elastic/eui';

import { KibanaLogic } from '../../shared/kibana';
import { KibanaLogic } from '../kibana';
import { HttpLogic } from '../http';
import { letBrowserHandleEvent, createHref } from './';

/**
Expand All @@ -33,9 +34,10 @@ export const EuiReactRouterHelper: React.FC<IEuiReactRouterProps> = ({
children,
}) => {
const { navigateToUrl, history } = useValues(KibanaLogic);
const { http } = useValues(HttpLogic);

// Generate the correct link href (with basename etc. accounted for)
const href = createHref(to, history, { shouldNotCreateHref });
const href = createHref(to, { history, http }, { shouldNotCreateHref });

const reactRouterLinkClick = (event: React.MouseEvent) => {
if (onClick) onClick(); // Run any passed click events (e.g. telemetry)
Expand Down

0 comments on commit 6686044

Please sign in to comment.