Skip to content

Commit

Permalink
[Enterprise Search] Move externalUrl helper out of React Context (ela…
Browse files Browse the repository at this point in the history
…stic#78368) (elastic#78497)

* Add new/simpler externalUrl helper and initialize it on renderApp

- This uses a simple JS obj to store the enterpriseSearchUrl reference (once on plugin init)
  - This is vs. a class, which needs to be instantiated and passed around - the new obj can be imported flatly at any time
  - I also opted to not convert this into a Kea logic file - after some deliberation I decided against it because it felt really weird as one. It's not storing "state" per se that ever needs to be updated, it's simply a one-time set obj that contains helper functions.
  - There's also some hope that we might eventually not need this helper after the full migration, so the simpler it is to delete the better
- Uses a getter & setter to ensure that we don't accidentally mutate said obj after initialization

* Update all components using get*SearchUrl helpers

* Update tests for updated components

- Mostly just consists of mocking externalUrl and importing that mock

* Remove old ExternalUrl class/context

TODO in next commit: Address kibana_header_actions

* Update Workplace Search Header Actions to use new externalUrl helper

NOTE: this requires a temporary workaround of initializing externalUrl.enterpriseSearch in plugin.ts rather than in renderApp, because renderApp loads *after* the header app does.

I plan on fixing this in a future PR so that setHeaderActionMenu is called AFTER renderApp has done loading (to ensure all the state in headerActions we need is available).

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
Constance and elasticmachine authored Sep 28, 2020
1 parent 8dd6dcb commit 2e3e63d
Show file tree
Hide file tree
Showing 32 changed files with 147 additions and 169 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* 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 { externalUrl } from '../shared/enterprise_search_url';

externalUrl.enterpriseSearchUrl = 'http://localhost:3002';
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { ExternalUrl } from '../shared/enterprise_search_url';

/**
* A set of default Kibana context values to use across component tests.
* @see enterprise_search/public/index.tsx for the KibanaContext definition/import
Expand All @@ -15,5 +13,4 @@ export const mockKibanaContext = {
setBreadcrumbs: jest.fn(),
setDocTitle: jest.fn(),
config: { host: 'http://localhost:3002' },
externalUrl: new ExternalUrl('http://localhost:3002'),
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import '../../../../__mocks__/kea.mock';
import '../../../../__mocks__/shallow_usecontext.mock';

import React from 'react';
import { shallow } from 'enzyme';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useContext } from 'react';
import React from 'react';
import { useValues } from 'kea';
import { EuiPageContent, EuiEmptyPrompt, EuiButton } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';

import { sendTelemetry } from '../../../../shared/telemetry';
import { HttpLogic } from '../../../../shared/http';
import { getAppSearchUrl } from '../../../../shared/enterprise_search_url';
import { SetAppSearchChrome as SetPageChrome } from '../../../../shared/kibana_chrome';
import { KibanaContext, IKibanaContext } from '../../../../index';
import { CREATE_ENGINES_PATH } from '../../../routes';

import { EngineOverviewHeader } from './header';
Expand All @@ -21,9 +21,6 @@ import './empty_state.scss';

export const EmptyState: React.FC = () => {
const { http } = useValues(HttpLogic);
const {
externalUrl: { getAppSearchUrl },
} = useContext(KibanaContext) as IKibanaContext;

const buttonProps = {
href: getAppSearchUrl(CREATE_ENGINES_PATH),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import '../../../../__mocks__/kea.mock';
import '../../../../__mocks__/shallow_usecontext.mock';
import '../../../../__mocks__/enterprise_search_url.mock';

import React from 'react';
import { shallow } from 'enzyme';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useContext } from 'react';
import React from 'react';
import { useValues } from 'kea';
import {
EuiPageHeader,
Expand All @@ -18,13 +18,10 @@ import { FormattedMessage } from '@kbn/i18n/react';

import { sendTelemetry } from '../../../../shared/telemetry';
import { HttpLogic } from '../../../../shared/http';
import { KibanaContext, IKibanaContext } from '../../../../index';
import { getAppSearchUrl } from '../../../../shared/enterprise_search_url';

export const EngineOverviewHeader: React.FC = () => {
const { http } = useValues(HttpLogic);
const {
externalUrl: { getAppSearchUrl },
} = useContext(KibanaContext) as IKibanaContext;

const buttonProps = {
fill: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import '../../../__mocks__/kea.mock';
import '../../../__mocks__/shallow_usecontext.mock';
import '../../../__mocks__/enterprise_search_url.mock';
import { mockHttpValues } from '../../../__mocks__/';

import React from 'react';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useContext } from 'react';
import React from 'react';
import { useValues } from 'kea';
import { EuiBasicTable, EuiBasicTableColumn, EuiLink } from '@elastic/eui';
import { FormattedMessage, FormattedDate, FormattedNumber } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';

import { sendTelemetry } from '../../../shared/telemetry';
import { HttpLogic } from '../../../shared/http';
import { KibanaContext, IKibanaContext } from '../../../index';
import { getAppSearchUrl } from '../../../shared/enterprise_search_url';
import { getEngineRoute } from '../../routes';

import { ENGINES_PAGE_SIZE } from '../../../../../common/constants';
Expand Down Expand Up @@ -43,9 +43,6 @@ export const EngineTable: React.FC<IEngineTableProps> = ({
pagination: { totalEngines, pageIndex, onPaginate },
}) => {
const { http } = useValues(HttpLogic);
const {
externalUrl: { getAppSearchUrl },
} = useContext(KibanaContext) as IKibanaContext;

const engineLinkProps = (name: string) => ({
href: getAppSearchUrl(getEngineRoute(name)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import '../__mocks__/shallow_usecontext.mock';
import '../__mocks__/kea.mock';
import '../__mocks__/enterprise_search_url.mock';

import React, { useContext } from 'react';
import { Redirect } from 'react-router-dom';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { useActions, useValues } from 'kea';
import { i18n } from '@kbn/i18n';

import { KibanaContext, IKibanaContext } from '../index';
import { getAppSearchUrl } from '../shared/enterprise_search_url';
import { HttpLogic } from '../shared/http';
import { AppLogic } from './app_logic';
import { IInitialAppData } from '../../../common/types';
Expand Down Expand Up @@ -86,10 +87,6 @@ export const AppSearchConfigured: React.FC<IInitialAppData> = (props) => {
};

export const AppSearchNav: React.FC = () => {
const {
externalUrl: { getAppSearchUrl },
} = useContext(KibanaContext) as IKibanaContext;

const {
myRole: { canViewSettings, canViewAccountCredentials, canViewRoleMappings },
} = useValues(AppLogic);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('renderHeaderActions', () => {
const mockHeaderEl = document.createElement('header');
const MockHeaderActions = () => <button className="hello-world">Hello World</button>;

const unmount = renderHeaderActions(MockHeaderActions, mockHeaderEl, {} as any);
const unmount = renderHeaderActions(MockHeaderActions, mockHeaderEl);
expect(mockHeaderEl.querySelector('.hello-world')).not.toBeNull();

unmount();
Expand Down
18 changes: 5 additions & 13 deletions x-pack/plugins/enterprise_search/public/applications/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ import { PluginsStart, ClientConfigType, ClientData } from '../plugin';
import { mountLicensingLogic } from './shared/licensing';
import { mountHttpLogic } from './shared/http';
import { mountFlashMessagesLogic } from './shared/flash_messages';
import { IExternalUrl } from './shared/enterprise_search_url';
import { externalUrl } from './shared/enterprise_search_url';
import { IInitialAppData } from '../../common/types';

export interface IKibanaContext {
config: { host?: string };
externalUrl: IExternalUrl;
navigateToUrl: ApplicationStart['navigateToUrl'];
setBreadcrumbs(crumbs: ChromeBreadcrumb[]): void;
setDocTitle(title: string): void;
Expand All @@ -42,7 +41,8 @@ export const renderApp = (
{ params, core, plugins }: { params: AppMountParameters; core: CoreStart; plugins: PluginsStart },
{ config, data }: { config: ClientConfigType; data: ClientData }
) => {
const { externalUrl, errorConnecting, ...initialData } = data;
const { publicUrl, errorConnecting, ...initialData } = data;
externalUrl.enterpriseSearchUrl = publicUrl || config.host || '';

resetContext({ createStore: true });
const store = getContext().store as Store;
Expand All @@ -64,7 +64,6 @@ export const renderApp = (
<KibanaContext.Provider
value={{
config,
externalUrl,
navigateToUrl: core.application.navigateToUrl,
setBreadcrumbs: core.chrome.setBreadcrumbs,
setDocTitle: core.chrome.docTitle.change,
Expand Down Expand Up @@ -93,15 +92,8 @@ export const renderApp = (
* a custom HeaderActions component (e.g., WorkplaceSearchHeaderActions)
* @see https://github.com/elastic/kibana/blob/master/docs/development/core/public/kibana-plugin-core-public.appmountparameters.setheaderactionmenu.md
*/
interface IHeaderActionsProps {
externalUrl: IExternalUrl;
}

export const renderHeaderActions = (
HeaderActions: React.FC<IHeaderActionsProps>,
kibanaHeaderEl: HTMLElement,
externalUrl: IExternalUrl
) => {
ReactDOM.render(<HeaderActions externalUrl={externalUrl} />, kibanaHeaderEl);
export const renderHeaderActions = (HeaderActions: React.FC, kibanaHeaderEl: HTMLElement) => {
ReactDOM.render(<HeaderActions />, kibanaHeaderEl);
return () => ReactDOM.unmountComponentAtNode(kibanaHeaderEl);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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 { externalUrl, getEnterpriseSearchUrl, getAppSearchUrl, getWorkplaceSearchUrl } from './';

describe('Enterprise Search external URL helpers', () => {
describe('getter/setter tests', () => {
it('defaults to an empty string', () => {
expect(externalUrl.enterpriseSearchUrl).toEqual('');
});

it('sets the internal enterpriseSearchUrl value', () => {
externalUrl.enterpriseSearchUrl = 'http://localhost:3002';
expect(externalUrl.enterpriseSearchUrl).toEqual('http://localhost:3002');
});

it('does not allow mutating enterpriseSearchUrl once set', () => {
externalUrl.enterpriseSearchUrl = 'hello world';
expect(externalUrl.enterpriseSearchUrl).toEqual('http://localhost:3002');
});
});

describe('function helpers', () => {
it('generates a public Enterprise Search URL', () => {
expect(getEnterpriseSearchUrl()).toEqual('http://localhost:3002');
expect(getEnterpriseSearchUrl('/login')).toEqual('http://localhost:3002/login');
});

it('generates a public App Search URL', () => {
expect(getAppSearchUrl()).toEqual('http://localhost:3002/as');
expect(getAppSearchUrl('/path')).toEqual('http://localhost:3002/as/path');
});

it('generates a public Workplace Search URL', () => {
expect(getWorkplaceSearchUrl()).toEqual('http://localhost:3002/ws');
expect(getWorkplaceSearchUrl('/path')).toEqual('http://localhost:3002/ws/path');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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.
*/

/**
* NOTE: The externalUrl obj holds the reference to externalUrl, which should
* only ever be updated once on plugin init. We're using a getter and setter
* here to ensure it isn't accidentally mutated.
*
* Someday (8.x+), when our UI is entirely on Kibana and no longer on
* Enterprise Search's standalone UI, we can potentially deprecate this helper.
*/
export const externalUrl = {
_enterpriseSearchUrl: '',
get enterpriseSearchUrl() {
return this._enterpriseSearchUrl;
},
set enterpriseSearchUrl(value) {
if (this._enterpriseSearchUrl) {
// enterpriseSearchUrl is set once on plugin init - we should not mutate it
return;
}
this._enterpriseSearchUrl = value;
},
};

export const getEnterpriseSearchUrl = (path: string = ''): string => {
return externalUrl.enterpriseSearchUrl + path;
};
export const getAppSearchUrl = (path: string = ''): string => {
return getEnterpriseSearchUrl('/as' + path);
};
export const getWorkplaceSearchUrl = (path: string = ''): string => {
return getEnterpriseSearchUrl('/ws' + path);
};

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { ExternalUrl, IExternalUrl } from './generate_external_url';
export {
externalUrl,
getEnterpriseSearchUrl,
getAppSearchUrl,
getWorkplaceSearchUrl,
} from './external_url';
Loading

0 comments on commit 2e3e63d

Please sign in to comment.