Skip to content

Commit

Permalink
[Enterprise Search] Move http out of React Context and to Kea Logic; …
Browse files Browse the repository at this point in the history
…prefer directly mounting (#78167) (#78204)

* Remove HttpProvider in favor of mounting HttpLogic directly w/ props

- removes need for initializeHttp call
- ensures http value is loaded into HttpLogic as soon as possible / should never load in as null, reducing # of rerenders/checks

see: https://kea.js.org/docs/guide/advanced#mounting-and-unmounting

* Update simplest components using http for sendTelemetry

* Update simplest tests for components using HttpLogic + default Kea mocks

- Kea mock import should now contain mock default values which can be overridden

* Update moderately complex tests using HttpLogic

send_telemetry:
- refactor to use shallow (w/ useEffect mocked) vs mount
- check mockHttpValues directly

engine_table:
- refactor to use mount w/ an I18nProvider rather than mountWithContext helper (which we'll likely need to overhaul in the future)
- assert mockHttpValues directly

* Update EngineOverview to HttpLogic + refactors

EngineOverview:
- Change use of FormattedMessage to i18n.translate (simpler, no provider required)

Tests:
- Create mock values/actions for FlashMessages, since EngineOverview calls it
- Create combined mockAllValues obj for easier overriding
- Create setMockValues helper for easier test overriding (credit to @scottybollinger for the idea!)
- Update engine_overview tests to setMockValues instead of passing context to mountWithAsyncContext
- Fix mountWithAsyncContext to accept an undefined obj

* Remove http from KibanaContext

- it should now only live in HttpLogic 🔥

* Remove FlashMessagesProvider in favor of mounting logic directly w/ props

- send history as prop
- refactor out now-unnecessary listenToHistory (we can just do it directly in afterMount without worrying about duplicate react rerenders)
- add mount helper

Tests:
- refactor history.listen mock to mockHistory (so that set_message_helpers can use it as well)
- use mountFlashMessagesLogic + create an even shorter mount() helper (credit to @JasonStoltz for the idea!)
- refactor out DEFAULT_VALUES since we're not really using it anywhere else in the file, and it's not super applicable to this store
- update history listener tests to account for logic occurring immediately on mount
  • Loading branch information
Constance authored Sep 22, 2020
1 parent 1542029 commit 52c37b8
Show file tree
Hide file tree
Showing 37 changed files with 268 additions and 332 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* 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.
*/

export const mockFlashMessagesValues = {
messages: [],
queuedMessages: [],
};

export const mockFlashMessagesActions = {
setFlashMessages: jest.fn(),
clearFlashMessages: jest.fn(),
setQueuedMessages: jest.fn(),
clearQueuedMessages: jest.fn(),
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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 { httpServiceMock } from 'src/core/public/mocks';

export const mockHttpValues = {
http: httpServiceMock.createSetupContract(),
errorConnecting: false,
readOnlyMode: false,
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
export { mockHistory, mockLocation } from './react_router_history.mock';
export { mockKibanaContext } from './kibana_context.mock';
export { mockLicenseContext } from './license_context.mock';
export { mockHttpValues } from './http_logic.mock';
export { mockFlashMessagesValues, mockFlashMessagesActions } from './flash_messages_logic.mock';
export { mockAllValues, mockAllActions, setMockValues } from './kea.mock';

export {
mountWithContext,
mountWithKibanaContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,46 @@
* you may not use this file except in compliance with the Elastic License.
*/

/**
* NOTE: These variable names MUST start with 'mock*' in order for
* Jest to accept its use within a jest.mock()
*/
import { mockHttpValues } from './http_logic.mock';
import { mockFlashMessagesValues, mockFlashMessagesActions } from './flash_messages_logic.mock';

export const mockAllValues = {
...mockHttpValues,
...mockFlashMessagesValues,
};
export const mockAllActions = {
...mockFlashMessagesActions,
};

/**
* Import this file directly to mock useValues with a set of default values for all shared logic files.
* Example usage:
*
* import '../../../__mocks__/kea'; // Must come before kea's import, adjust relative path as needed
*/
jest.mock('kea', () => ({
...(jest.requireActual('kea') as object),
useValues: jest.fn(() => ({})),
useActions: jest.fn(() => ({})),
useValues: jest.fn(() => ({ ...mockAllValues })),
useActions: jest.fn(() => ({ ...mockAllActions })),
}));

/**
* Call this function to override a specific set of Kea values while retaining all other defaults
* Example usage within a component test:
*
* import '../../../__mocks__/kea'; // Must come before kea's import, adjust relative path as needed
*
* import { useActions, useValues } from 'kea';
* import '../../../__mocks__/kea';
* import { setMockValues } from ''../../../__mocks__';
*
* it('some test', () => {
* (useValues as jest.Mock).mockImplementationOnce(() => ({ someValue: 'hello' }));
* (useActions as jest.Mock).mockImplementationOnce(() => ({ someAction: () => 'world' }));
* setMockValues({ someValue: 'hello' });
* });
*/
import { useValues } from 'kea';

export const setMockValues = (values: object) => {
(useValues as jest.Mock).mockImplementation(() => ({ ...mockAllValues, ...values }));
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { httpServiceMock } from 'src/core/public/mocks';
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
*/
export const mockKibanaContext = {
http: httpServiceMock.createSetupContract(),
navigateToUrl: jest.fn(),
setBreadcrumbs: jest.fn(),
setDocTitle: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const mountWithKibanaContext = (children: React.ReactNode, context?: obje
*/
export const mountWithAsyncContext = async (
children: React.ReactNode,
context: object
context?: object
): Promise<ReactWrapper> => {
let wrapper: ReactWrapper | undefined;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const mockHistory = {
location: {
pathname: '/current-path',
},
listen: jest.fn(() => jest.fn()),
};
export const mockLocation = {
key: 'someKey',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

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

import React from 'react';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
*/

import React, { useContext } 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 { SetAppSearchChrome as SetPageChrome } from '../../../../shared/kibana_chrome';
import { KibanaContext, IKibanaContext } from '../../../../index';
import { CREATE_ENGINES_PATH } from '../../../routes';
Expand All @@ -18,9 +20,9 @@ import { EngineOverviewHeader } from './header';
import './empty_state.scss';

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

const buttonProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

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

import React from 'react';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import React, { useContext } from 'react';
import { useValues } from 'kea';
import {
EuiPageHeader,
EuiPageHeaderSection,
Expand All @@ -16,12 +17,13 @@ import {
import { FormattedMessage } from '@kbn/i18n/react';

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

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

const buttonProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,21 @@
* you may not use this file except in compliance with the Elastic License.
*/

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

import React from 'react';
import { act } from 'react-dom/test-utils';
import { shallow, ReactWrapper } from 'enzyme';

import { mountWithAsyncContext, mockKibanaContext } from '../../../__mocks__';
import { mountWithAsyncContext, mockHttpValues, setMockValues } from '../../../__mocks__';

import { LoadingState, EmptyState } from './components';
import { EngineTable } from './engine_table';

import { EngineOverview } from './';

describe('EngineOverview', () => {
const mockHttp = mockKibanaContext.http;

describe('non-happy-path states', () => {
it('isLoading', () => {
const wrapper = shallow(<EngineOverview />);
Expand All @@ -28,15 +27,16 @@ describe('EngineOverview', () => {
});

it('isEmpty', async () => {
const wrapper = await mountWithAsyncContext(<EngineOverview />, {
setMockValues({
http: {
...mockHttp,
...mockHttpValues.http,
get: () => ({
results: [],
meta: { page: { total_results: 0 } },
}),
},
});
const wrapper = await mountWithAsyncContext(<EngineOverview />);

expect(wrapper.find(EmptyState)).toHaveLength(1);
});
Expand Down Expand Up @@ -65,12 +65,11 @@ describe('EngineOverview', () => {

beforeEach(() => {
jest.clearAllMocks();
setMockValues({ http: { ...mockHttpValues.http, get: mockApi } });
});

it('renders and calls the engines API', async () => {
const wrapper = await mountWithAsyncContext(<EngineOverview />, {
http: { ...mockHttp, get: mockApi },
});
const wrapper = await mountWithAsyncContext(<EngineOverview />);

expect(wrapper.find(EngineTable)).toHaveLength(1);
expect(mockApi).toHaveBeenNthCalledWith(1, '/api/app_search/engines', {
Expand All @@ -84,7 +83,6 @@ describe('EngineOverview', () => {
describe('when on a platinum license', () => {
it('renders a 2nd meta engines table & makes a 2nd meta engines API call', async () => {
const wrapper = await mountWithAsyncContext(<EngineOverview />, {
http: { ...mockHttp, get: mockApi },
license: { type: 'platinum', isActive: true },
});

Expand All @@ -103,19 +101,15 @@ describe('EngineOverview', () => {
wrapper.find(EngineTable).prop('pagination');

it('passes down page data from the API', async () => {
const wrapper = await mountWithAsyncContext(<EngineOverview />, {
http: { ...mockHttp, get: mockApi },
});
const wrapper = await mountWithAsyncContext(<EngineOverview />);
const pagination = getTablePagination(wrapper);

expect(pagination.totalEngines).toEqual(100);
expect(pagination.pageIndex).toEqual(0);
});

it('re-polls the API on page change', async () => {
const wrapper = await mountWithAsyncContext(<EngineOverview />, {
http: { ...mockHttp, get: mockApi },
});
const wrapper = await mountWithAsyncContext(<EngineOverview />);
await act(async () => getTablePagination(wrapper).onPaginate(5));
wrapper.update();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,21 @@
*/

import React, { useContext, useEffect, useState } from 'react';
import { useValues } from 'kea';
import {
EuiPageContent,
EuiPageContentHeader,
EuiPageContentBody,
EuiTitle,
EuiSpacer,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';

import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
import { SendAppSearchTelemetry as SendTelemetry } from '../../../shared/telemetry';
import { FlashMessages } from '../../../shared/flash_messages';
import { HttpLogic } from '../../../shared/http';
import { LicenseContext, ILicenseContext, hasPlatinumLicense } from '../../../shared/licensing';
import { KibanaContext, IKibanaContext } from '../../../index';

import { EngineIcon } from './assets/engine_icon';
import { MetaEngineIcon } from './assets/meta_engine_icon';
Expand All @@ -38,7 +39,7 @@ interface ISetEnginesCallbacks {
}

export const EngineOverview: React.FC = () => {
const { http } = useContext(KibanaContext) as IKibanaContext;
const { http } = useValues(HttpLogic);
const { license } = useContext(LicenseContext) as ILicenseContext;

const [isLoading, setIsLoading] = useState(true);
Expand Down Expand Up @@ -94,10 +95,9 @@ export const EngineOverview: React.FC = () => {
<EuiTitle size="s">
<h2>
<EngineIcon />
<FormattedMessage
id="xpack.enterpriseSearch.appSearch.enginesOverview.engines"
defaultMessage="Engines"
/>
{i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.engines', {
defaultMessage: 'Engines',
})}
</h2>
</EuiTitle>
</EuiPageContentHeader>
Expand All @@ -119,10 +119,9 @@ export const EngineOverview: React.FC = () => {
<EuiTitle size="s">
<h2>
<MetaEngineIcon />
<FormattedMessage
id="xpack.enterpriseSearch.appSearch.enginesOverview.metaEngines"
defaultMessage="Meta Engines"
/>
{i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.metaEngines', {
defaultMessage: 'Meta Engines',
})}
</h2>
</EuiTitle>
</EuiPageContentHeader>
Expand Down
Loading

0 comments on commit 52c37b8

Please sign in to comment.