From 3d9725fb168d5419f007f4f5a90e87a43d93f631 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 4 Nov 2020 12:48:52 -0800 Subject: [PATCH 1/5] Add server-side route --- .../server/routes/app_search/engines.test.ts | 30 +++++++++++++++++-- .../server/routes/app_search/engines.ts | 19 +++++++++++- .../server/routes/app_search/index.ts | 4 +-- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts index 3bfe8abf8a2df..b7009c1b76fbc 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.test.ts @@ -6,7 +6,7 @@ import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; -import { registerEnginesRoute } from './engines'; +import { registerEnginesRoutes } from './engines'; describe('engine routes', () => { describe('GET /api/app_search/engines', () => { @@ -31,7 +31,7 @@ describe('engine routes', () => { payload: 'query', }); - registerEnginesRoute({ + registerEnginesRoutes({ ...mockDependencies, router: mockRouter.router, }); @@ -107,4 +107,30 @@ describe('engine routes', () => { }); }); }); + + describe('GET /api/app_search/engines/{name}', () => { + let mockRouter: MockRouter; + + beforeEach(() => { + jest.clearAllMocks(); + mockRouter = new MockRouter({ + method: 'get', + path: '/api/app_search/engines/{name}', + payload: 'params', + }); + + registerEnginesRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + }); + + it('creates a request to enterprise search', () => { + mockRouter.callRoute({ params: { name: 'some-engine' } }); + + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/some-engine/details', + }); + }); + }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts index 6cbd60e494fe3..2c4e235556ae3 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/engines.ts @@ -14,7 +14,7 @@ interface EnginesResponse { meta: { page: { total_results: number } }; } -export function registerEnginesRoute({ +export function registerEnginesRoutes({ router, enterpriseSearchRequestHandler, }: RouteDependencies) { @@ -43,4 +43,21 @@ export function registerEnginesRoute({ })(context, request, response); } ); + + // Single engine endpoints + router.get( + { + path: '/api/app_search/engines/{name}', + validate: { + params: schema.object({ + name: schema.string(), + }), + }, + }, + async (context, request, response) => { + return enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/${request.params.name}/details`, + })(context, request, response); + } + ); } diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/index.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/index.ts index 7ce3ee9654f42..faf74203cf17d 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/index.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/index.ts @@ -6,12 +6,12 @@ import { RouteDependencies } from '../../plugin'; -import { registerEnginesRoute } from './engines'; +import { registerEnginesRoutes } from './engines'; import { registerCredentialsRoutes } from './credentials'; import { registerSettingsRoutes } from './settings'; export const registerAppSearchRoutes = (dependencies: RouteDependencies) => { - registerEnginesRoute(dependencies); + registerEnginesRoutes(dependencies); registerCredentialsRoutes(dependencies); registerSettingsRoutes(dependencies); }; From 12d7805291a41b0ad41379edaa06c59374914e56 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 9 Nov 2020 14:58:37 -0800 Subject: [PATCH 2/5] Add EngineRouter logic - useEffect init - redirect/data loading - setQueuedErrorMessage helper --- .../components/engine/engine_nav.test.tsx | 56 ++++++++++++++++++- .../components/engine/engine_nav.tsx | 31 ++++++++-- .../shared/flash_messages/index.ts | 7 ++- .../set_message_helpers.test.ts | 12 ++++ .../flash_messages/set_message_helpers.ts | 7 +++ 5 files changed, 105 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx index 7bdc3c86a50d6..43dc592660564 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx @@ -5,18 +5,68 @@ */ import '../../../__mocks__/react_router_history.mock'; -import { setMockValues } from '../../../__mocks__/kea.mock'; +import { unmountHandler } from '../../../__mocks__/shallow_useeffect.mock'; +import { setMockValues, setMockActions } from '../../../__mocks__/kea.mock'; import React from 'react'; import { shallow, mount } from 'enzyme'; -import { Switch, useParams } from 'react-router-dom'; +import { Switch, Redirect, useParams } from 'react-router-dom'; import { EuiBadge } from '@elastic/eui'; +jest.mock('../../../shared/flash_messages', () => ({ + setQueuedErrorMessage: jest.fn(), +})); +import { setQueuedErrorMessage } from '../../../shared/flash_messages'; + import { EngineRouter, EngineNav } from './'; describe('EngineRouter', () => { + const values = { dataLoading: false, engineNotFound: false, myRole: {} }; + const actions = { setEngineName: jest.fn(), initializeEngine: jest.fn(), clearEngine: jest.fn() }; + + beforeEach(() => { + setMockValues(values); + setMockActions(actions); + }); + + describe('useEffect', () => { + beforeEach(() => { + (useParams as jest.Mock).mockReturnValue({ engineName: 'some-engine' }); + shallow(); + }); + + it('sets engineName based on the current route parameters', () => { + expect(actions.setEngineName).toHaveBeenCalledWith('some-engine'); + }); + + it('initializes/fetches engine API data', () => { + expect(actions.initializeEngine).toHaveBeenCalled(); + }); + + it('clears engine on unmount', () => { + unmountHandler(); + expect(actions.clearEngine).toHaveBeenCalled(); + }); + }); + + it('redirects to engines list and flashes an error if the engine param was not found', () => { + (useParams as jest.Mock).mockReturnValue({ engineName: '404-engine' }); + setMockValues({ ...values, engineNotFound: true }); + const wrapper = shallow(); + + expect(wrapper.find(Redirect).prop('to')).toEqual('/engines'); + expect(setQueuedErrorMessage).toHaveBeenCalledWith( + "No engine with name '404-engine' could be found." + ); + }); + + it('does not render if async data is still loading', () => { + setMockValues({ ...values, dataLoading: true }); + const wrapper = shallow(); + expect(wrapper.isEmptyRender()).toBe(true); + }); + it('renders a default engine overview', () => { - setMockValues({ myRole: {} }); const wrapper = shallow(); expect(wrapper.find(Switch)).toHaveLength(1); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx index f92fefc7124b8..8c03f59702639 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx @@ -4,18 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; -import { Route, Switch, useParams } from 'react-router-dom'; -import { useValues } from 'kea'; +import React, { useEffect } from 'react'; +import { Route, Switch, Redirect, useParams } from 'react-router-dom'; +import { useValues, useActions } from 'kea'; import { EuiText, EuiBadge } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { SideNavLink, SideNavItem } from '../../../shared/layout'; import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; +import { setQueuedErrorMessage } from '../../../shared/flash_messages'; import { AppLogic } from '../../app_logic'; import { getEngineRoute, + ENGINES_PATH, ENGINE_PATH, ENGINE_ANALYTICS_PATH, ENGINE_DOCUMENTS_PATH, @@ -45,6 +47,8 @@ import { API_LOGS_TITLE, } from './constants'; +import { EngineLogic } from './'; + import './engine_nav.scss'; export const EngineRouter: React.FC = () => { @@ -52,11 +56,30 @@ export const EngineRouter: React.FC = () => { myRole: { canViewEngineAnalytics }, } = useValues(AppLogic); - // TODO: EngineLogic + const { dataLoading, engineNotFound } = useValues(EngineLogic); + const { setEngineName, initializeEngine, clearEngine } = useActions(EngineLogic); const { engineName } = useParams() as { engineName: string }; const engineBreadcrumb = [ENGINES_TITLE, engineName]; + useEffect(() => { + setEngineName(engineName); + initializeEngine(); + return clearEngine; + }, [engineName]); + + if (engineNotFound) { + setQueuedErrorMessage( + i18n.translate('xpack.enterpriseSearch.appSearch.engine.notFound', { + defaultMessage: "No engine with name '{engineName}' could be found.", + values: { engineName }, + }) + ); + return ; + } + + if (dataLoading) return null; + return ( // TODO: Add more routes as we migrate them diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts index 8792c26f9bad4..a109640f09bbe 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts @@ -7,4 +7,9 @@ export { FlashMessages } from './flash_messages'; export { FlashMessagesLogic, IFlashMessage, mountFlashMessagesLogic } from './flash_messages_logic'; export { flashAPIErrors } from './handle_api_errors'; -export { setSuccessMessage, setErrorMessage, setQueuedSuccessMessage } from './set_message_helpers'; +export { + setSuccessMessage, + setErrorMessage, + setQueuedSuccessMessage, + setQueuedErrorMessage, +} from './set_message_helpers'; 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 index 46027fdfb22b1..c5ee8200c490d 100644 --- 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 @@ -15,6 +15,7 @@ import { setSuccessMessage, setErrorMessage, setQueuedSuccessMessage, + setQueuedErrorMessage, } from './'; describe('Flash Message Helpers', () => { @@ -56,4 +57,15 @@ describe('Flash Message Helpers', () => { }, ]); }); + + it('setQueuedErrorMessage()', () => { + setQueuedErrorMessage(message); + + expect(FlashMessagesLogic.values.queuedMessages).toEqual([ + { + message, + type: 'error', + }, + ]); + }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/set_message_helpers.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/set_message_helpers.ts index 6abb540b7c14b..cb73d54fd7b1e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/set_message_helpers.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/set_message_helpers.ts @@ -26,3 +26,10 @@ export const setQueuedSuccessMessage = (message: string) => { message, }); }; + +export const setQueuedErrorMessage = (message: string) => { + FlashMessagesLogic.actions.setQueuedMessages({ + type: 'error', + message, + }); +}; From 9fc3ef813f132136d7837bfae6a715366faee8f1 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 9 Nov 2020 15:44:34 -0800 Subject: [PATCH 3/5] Add EngineNav logic - logic values - icon alerts - update tests w/ real conditional testing + remove use of mount, mount combined with shallow_useeffect leads to weirdness --- .../components/engine/engine_nav.scss | 8 ++ .../components/engine/engine_nav.test.tsx | 109 +++++++++++++----- .../components/engine/engine_nav.tsx | 79 +++++++++++-- 3 files changed, 155 insertions(+), 41 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.scss b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.scss index d7740724204a7..529fb9c4bd63e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.scss +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.scss @@ -15,3 +15,11 @@ margin-top: $euiSizeXS; } } + +.appSearchNavIcons { + // EUI override + &.euiFlexItem { + flex-grow: 0; + flex-direction: row; + } +} diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx index 43dc592660564..4efbf88d1957f 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx @@ -9,7 +9,7 @@ import { unmountHandler } from '../../../__mocks__/shallow_useeffect.mock'; import { setMockValues, setMockActions } from '../../../__mocks__/kea.mock'; import React from 'react'; -import { shallow, mount } from 'enzyme'; +import { shallow } from 'enzyme'; import { Switch, Redirect, useParams } from 'react-router-dom'; import { EuiBadge } from '@elastic/eui'; @@ -82,106 +82,153 @@ describe('EngineRouter', () => { }); describe('EngineNav', () => { + const values = { myRole: {}, engineName: 'some-engine', dataLoading: false, engine: {} }; + beforeEach(() => { - (useParams as jest.Mock).mockReturnValue({ engineName: 'some-engine' }); + setMockValues(values); + }); + + it('does not render if async data is still loading', () => { + setMockValues({ ...values, dataLoading: true }); + const wrapper = shallow(); + expect(wrapper.isEmptyRender()).toBe(true); }); it('does not render without an engine name', () => { - setMockValues({ myRole: {} }); - (useParams as jest.Mock).mockReturnValue({ engineName: '' }); + setMockValues({ ...values, engineName: '' }); const wrapper = shallow(); expect(wrapper.isEmptyRender()).toBe(true); }); - it('renders an engine label', () => { - setMockValues({ myRole: {} }); - const wrapper = mount(); + it('renders an engine label and badges', () => { + setMockValues({ ...values, isSampleEngine: false, isMetaEngine: false }); + const wrapper = shallow(); + const label = wrapper.find('[data-test-subj="EngineLabel"]').find('.eui-textTruncate'); + + expect(label.text()).toEqual('SOME-ENGINE'); + expect(wrapper.find(EuiBadge)).toHaveLength(0); - const label = wrapper.find('[data-test-subj="EngineLabel"]').last(); - expect(label.text()).toEqual(expect.stringContaining('SOME-ENGINE')); + setMockValues({ ...values, isSampleEngine: true }); + wrapper.setProps({}); // Re-render + expect(wrapper.find(EuiBadge).prop('children')).toEqual('SAMPLE ENGINE'); - // TODO: Test sample & meta engine conditional rendering - expect(label.find(EuiBadge).text()).toEqual('SAMPLE ENGINE'); + setMockValues({ ...values, isMetaEngine: true }); + wrapper.setProps({}); // Re-render + expect(wrapper.find(EuiBadge).prop('children')).toEqual('META ENGINE'); }); it('renders a default engine overview link', () => { - setMockValues({ myRole: {} }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineOverviewLink"]')).toHaveLength(1); }); it('renders an analytics link', () => { - setMockValues({ myRole: { canViewEngineAnalytics: true } }); + setMockValues({ ...values, myRole: { canViewEngineAnalytics: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineAnalyticsLink"]')).toHaveLength(1); }); it('renders a documents link', () => { - setMockValues({ myRole: { canViewEngineDocuments: true } }); + setMockValues({ ...values, myRole: { canViewEngineDocuments: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineDocumentsLink"]')).toHaveLength(1); }); it('renders a schema link', () => { - setMockValues({ myRole: { canViewEngineSchema: true } }); + setMockValues({ ...values, myRole: { canViewEngineSchema: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineSchemaLink"]')).toHaveLength(1); + }); + + it('renders schema nav icons', () => { + const myRole = { canViewEngineSchema: true }; + const wrapper = shallow(); + + setMockValues({ ...values, myRole, hasUnconfirmedSchemaFields: true }); + wrapper.setProps({}); // Re-render + expect(wrapper.find('[data-test-subj="EngineNavSchemaUnconfirmedFields"]')).toHaveLength(1); - // TODO: Schema warning icon + setMockValues({ ...values, myRole, hasSchemaConflicts: true }); + wrapper.setProps({}); // Re-render + expect(wrapper.find('[data-test-subj="EngineNavSchemaConflicts"]')).toHaveLength(1); }); - // TODO: Unskip when EngineLogic is migrated - it.skip('renders a crawler link', () => { - setMockValues({ myRole: { canViewEngineCrawler: true } }); + it('renders a crawler link', () => { + const myRole = { canViewEngineCrawler: true }; + setMockValues({ ...values, myRole }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(1); - // TODO: Test that the crawler link does NOT show up for meta/sample engines + // Does not render for meta engines + setMockValues({ ...values, myRole, isMetaEngine: true }); + wrapper.setProps({}); // Re-render + expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(0); + + // Does not render for sample engine + setMockValues({ ...values, myRole, isSampleEngine: true }); + wrapper.setProps({}); // Re-render + expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(0); }); - // TODO: Unskip when EngineLogic is migrated - it.skip('renders a meta engine source engines link', () => { - setMockValues({ myRole: { canViewMetaEngineSourceEngines: true } }); + it('renders a meta engine source engines link', () => { + const myRole = { canViewMetaEngineSourceEngines: true }; + setMockValues({ ...values, myRole, isMetaEngine: true }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="MetaEngineEnginesLink"]')).toHaveLength(1); - // TODO: Test that the crawler link does NOT show up for non-meta engines + // Does not render if engine is not a meta-engine + setMockValues({ ...values, myRole, isMetaEngine: false }); + wrapper.setProps({}); // Re-render + expect(wrapper.find('[data-test-subj="MetaEngineEnginesLink"]')).toHaveLength(0); }); it('renders a relevance tuning link', () => { - setMockValues({ myRole: { canManageEngineRelevanceTuning: true } }); + setMockValues({ ...values, myRole: { canManageEngineRelevanceTuning: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineRelevanceTuningLink"]')).toHaveLength(1); + }); + + it('renders relevance tuning nav icons', () => { + setMockValues({ + ...values, + myRole: { canManageEngineRelevanceTuning: true }, + engine: { + unsearchedUnconfirmedFields: true, + invalidBoosts: true, + }, + }); + const wrapper = shallow(); - // TODO: Boost error icon + expect(wrapper.find('[data-test-subj="EngineNavRelevanceTuningInvalidBoosts"]')).toHaveLength(1); // prettier-ignore + expect(wrapper.find('[data-test-subj="EngineNavRelevanceTuningUnsearchedFields"]')).toHaveLength(1); // prettier-ignore }); it('renders a synonyms link', () => { - setMockValues({ myRole: { canManageEngineSynonyms: true } }); + setMockValues({ ...values, myRole: { canManageEngineSynonyms: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineSynonymsLink"]')).toHaveLength(1); }); it('renders a curations link', () => { - setMockValues({ myRole: { canManageEngineCurations: true } }); + setMockValues({ ...values, myRole: { canManageEngineCurations: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineCurationsLink"]')).toHaveLength(1); }); it('renders a results settings link', () => { - setMockValues({ myRole: { canManageEngineResultSettings: true } }); + setMockValues({ ...values, myRole: { canManageEngineResultSettings: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineResultSettingsLink"]')).toHaveLength(1); }); it('renders a Search UI link', () => { - setMockValues({ myRole: { canManageEngineSearchUi: true } }); + setMockValues({ ...values, myRole: { canManageEngineSearchUi: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineSearchUILink"]')).toHaveLength(1); }); it('renders an API logs link', () => { - setMockValues({ myRole: { canViewEngineApiLogs: true } }); + setMockValues({ ...values, myRole: { canViewEngineApiLogs: true } }); const wrapper = shallow(); expect(wrapper.find('[data-test-subj="EngineAPILogsLink"]')).toHaveLength(1); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx index 8c03f59702639..9efb0b7804b86 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx @@ -8,7 +8,7 @@ import React, { useEffect } from 'react'; import { Route, Switch, Redirect, useParams } from 'react-router-dom'; import { useValues, useActions } from 'kea'; -import { EuiText, EuiBadge } from '@elastic/eui'; +import { EuiText, EuiBadge, EuiIcon, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { SideNavLink, SideNavItem } from '../../../shared/layout'; @@ -48,6 +48,7 @@ import { } from './constants'; import { EngineLogic } from './'; +import { EngineDetails } from './types'; import './engine_nav.scss'; @@ -114,14 +115,22 @@ export const EngineNav: React.FC = () => { }, } = useValues(AppLogic); - // TODO: Use EngineLogic - const isSampleEngine = true; - const isMetaEngine = false; - const { engineName } = useParams() as { engineName: string }; - const engineRoute = engineName && getEngineRoute(engineName); + const { + engineName, + dataLoading, + isSampleEngine, + isMetaEngine, + hasSchemaConflicts, + hasUnconfirmedSchemaFields, + engine, + } = useValues(EngineLogic); + if (dataLoading) return null; if (!engineName) return null; + const engineRoute = getEngineRoute(engineName); + const { invalidBoosts, unsearchedUnconfirmedFields } = engine as Required; + return ( <> @@ -166,8 +175,33 @@ export const EngineNav: React.FC = () => { to={getAppSearchUrl(engineRoute + ENGINE_SCHEMA_PATH)} data-test-subj="EngineSchemaLink" > - {SCHEMA_TITLE} - {/* TODO: Engine schema warning icon */} + + {SCHEMA_TITLE} + + {hasUnconfirmedSchemaFields && ( + + )} + {hasSchemaConflicts && ( + + )} + + )} {canViewEngineCrawler && !isMetaEngine && !isSampleEngine && ( @@ -194,8 +228,33 @@ export const EngineNav: React.FC = () => { to={getAppSearchUrl(engineRoute + ENGINE_RELEVANCE_TUNING_PATH)} data-test-subj="EngineRelevanceTuningLink" > - {RELEVANCE_TUNING_TITLE} - {/* TODO: invalid boosts error icon */} + + {RELEVANCE_TUNING_TITLE} + + {invalidBoosts && ( + + )} + {unsearchedUnconfirmedFields && ( + + )} + + )} {canManageEngineSynonyms && ( From 2556da78070f9bf28448ddea57f7aa78fa7a2971 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 11 Nov 2020 08:43:57 -0800 Subject: [PATCH 4/5] Split out EngineRouter and EngineNav into separate files --- .../components/engine/engine_nav.test.tsx | 73 +----------- .../components/engine/engine_nav.tsx | 55 +-------- .../components/engine/engine_router.test.tsx | 81 ++++++++++++++ .../components/engine/engine_router.tsx | 105 ++++++++++++++++++ .../app_search/components/engine/index.ts | 3 +- 5 files changed, 192 insertions(+), 125 deletions(-) create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx index 4efbf88d1957f..bc0c61326a275 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx @@ -4,82 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import '../../../__mocks__/react_router_history.mock'; -import { unmountHandler } from '../../../__mocks__/shallow_useeffect.mock'; -import { setMockValues, setMockActions } from '../../../__mocks__/kea.mock'; +import { setMockValues } from '../../../__mocks__/kea.mock'; import React from 'react'; import { shallow } from 'enzyme'; -import { Switch, Redirect, useParams } from 'react-router-dom'; import { EuiBadge } from '@elastic/eui'; -jest.mock('../../../shared/flash_messages', () => ({ - setQueuedErrorMessage: jest.fn(), -})); -import { setQueuedErrorMessage } from '../../../shared/flash_messages'; - -import { EngineRouter, EngineNav } from './'; - -describe('EngineRouter', () => { - const values = { dataLoading: false, engineNotFound: false, myRole: {} }; - const actions = { setEngineName: jest.fn(), initializeEngine: jest.fn(), clearEngine: jest.fn() }; - - beforeEach(() => { - setMockValues(values); - setMockActions(actions); - }); - - describe('useEffect', () => { - beforeEach(() => { - (useParams as jest.Mock).mockReturnValue({ engineName: 'some-engine' }); - shallow(); - }); - - it('sets engineName based on the current route parameters', () => { - expect(actions.setEngineName).toHaveBeenCalledWith('some-engine'); - }); - - it('initializes/fetches engine API data', () => { - expect(actions.initializeEngine).toHaveBeenCalled(); - }); - - it('clears engine on unmount', () => { - unmountHandler(); - expect(actions.clearEngine).toHaveBeenCalled(); - }); - }); - - it('redirects to engines list and flashes an error if the engine param was not found', () => { - (useParams as jest.Mock).mockReturnValue({ engineName: '404-engine' }); - setMockValues({ ...values, engineNotFound: true }); - const wrapper = shallow(); - - expect(wrapper.find(Redirect).prop('to')).toEqual('/engines'); - expect(setQueuedErrorMessage).toHaveBeenCalledWith( - "No engine with name '404-engine' could be found." - ); - }); - - it('does not render if async data is still loading', () => { - setMockValues({ ...values, dataLoading: true }); - const wrapper = shallow(); - expect(wrapper.isEmptyRender()).toBe(true); - }); - - it('renders a default engine overview', () => { - const wrapper = shallow(); - - expect(wrapper.find(Switch)).toHaveLength(1); - expect(wrapper.find('[data-test-subj="EngineOverviewTODO"]')).toHaveLength(1); - }); - - it('renders an analytics view', () => { - setMockValues({ myRole: { canViewEngineAnalytics: true } }); - const wrapper = shallow(); - - expect(wrapper.find('[data-test-subj="AnalyticsTODO"]')).toHaveLength(1); - }); -}); +import { EngineNav } from './'; describe('EngineNav', () => { const values = { myRole: {}, engineName: 'some-engine', dataLoading: false, engine: {} }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx index 9efb0b7804b86..77aca8a71994d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.tsx @@ -4,21 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useEffect } from 'react'; -import { Route, Switch, Redirect, useParams } from 'react-router-dom'; -import { useValues, useActions } from 'kea'; +import React from 'react'; +import { useValues } from 'kea'; import { EuiText, EuiBadge, EuiIcon, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { SideNavLink, SideNavItem } from '../../../shared/layout'; -import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; -import { setQueuedErrorMessage } from '../../../shared/flash_messages'; import { AppLogic } from '../../app_logic'; import { getEngineRoute, - ENGINES_PATH, - ENGINE_PATH, ENGINE_ANALYTICS_PATH, ENGINE_DOCUMENTS_PATH, ENGINE_SCHEMA_PATH, @@ -52,52 +47,6 @@ import { EngineDetails } from './types'; import './engine_nav.scss'; -export const EngineRouter: React.FC = () => { - const { - myRole: { canViewEngineAnalytics }, - } = useValues(AppLogic); - - const { dataLoading, engineNotFound } = useValues(EngineLogic); - const { setEngineName, initializeEngine, clearEngine } = useActions(EngineLogic); - - const { engineName } = useParams() as { engineName: string }; - const engineBreadcrumb = [ENGINES_TITLE, engineName]; - - useEffect(() => { - setEngineName(engineName); - initializeEngine(); - return clearEngine; - }, [engineName]); - - if (engineNotFound) { - setQueuedErrorMessage( - i18n.translate('xpack.enterpriseSearch.appSearch.engine.notFound', { - defaultMessage: "No engine with name '{engineName}' could be found.", - values: { engineName }, - }) - ); - return ; - } - - if (dataLoading) return null; - - return ( - // TODO: Add more routes as we migrate them - - {canViewEngineAnalytics && ( - - -
Just testing right now
- - )} - - -
Overview
- - - ); -}; - export const EngineNav: React.FC = () => { const { myRole: { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.test.tsx new file mode 100644 index 0000000000000..e38381cad32ba --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.test.tsx @@ -0,0 +1,81 @@ +/* + * 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 '../../../__mocks__/react_router_history.mock'; +import { unmountHandler } from '../../../__mocks__/shallow_useeffect.mock'; +import { setMockValues, setMockActions } from '../../../__mocks__/kea.mock'; + +import React from 'react'; +import { shallow } from 'enzyme'; +import { Switch, Redirect, useParams } from 'react-router-dom'; + +jest.mock('../../../shared/flash_messages', () => ({ + setQueuedErrorMessage: jest.fn(), +})); +import { setQueuedErrorMessage } from '../../../shared/flash_messages'; + +import { EngineRouter } from './'; + +describe('EngineRouter', () => { + const values = { dataLoading: false, engineNotFound: false, myRole: {} }; + const actions = { setEngineName: jest.fn(), initializeEngine: jest.fn(), clearEngine: jest.fn() }; + + beforeEach(() => { + setMockValues(values); + setMockActions(actions); + }); + + describe('useEffect', () => { + beforeEach(() => { + (useParams as jest.Mock).mockReturnValue({ engineName: 'some-engine' }); + shallow(); + }); + + it('sets engineName based on the current route parameters', () => { + expect(actions.setEngineName).toHaveBeenCalledWith('some-engine'); + }); + + it('initializes/fetches engine API data', () => { + expect(actions.initializeEngine).toHaveBeenCalled(); + }); + + it('clears engine on unmount', () => { + unmountHandler(); + expect(actions.clearEngine).toHaveBeenCalled(); + }); + }); + + it('redirects to engines list and flashes an error if the engine param was not found', () => { + (useParams as jest.Mock).mockReturnValue({ engineName: '404-engine' }); + setMockValues({ ...values, engineNotFound: true }); + const wrapper = shallow(); + + expect(wrapper.find(Redirect).prop('to')).toEqual('/engines'); + expect(setQueuedErrorMessage).toHaveBeenCalledWith( + "No engine with name '404-engine' could be found." + ); + }); + + it('does not render if async data is still loading', () => { + setMockValues({ ...values, dataLoading: true }); + const wrapper = shallow(); + expect(wrapper.isEmptyRender()).toBe(true); + }); + + it('renders a default engine overview', () => { + const wrapper = shallow(); + + expect(wrapper.find(Switch)).toHaveLength(1); + expect(wrapper.find('[data-test-subj="EngineOverviewTODO"]')).toHaveLength(1); + }); + + it('renders an analytics view', () => { + setMockValues({ myRole: { canViewEngineAnalytics: true } }); + const wrapper = shallow(); + + expect(wrapper.find('[data-test-subj="AnalyticsTODO"]')).toHaveLength(1); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx new file mode 100644 index 0000000000000..3e6856771f7d9 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_router.tsx @@ -0,0 +1,105 @@ +/* + * 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, { useEffect } from 'react'; +import { Route, Switch, Redirect, useParams } from 'react-router-dom'; +import { useValues, useActions } from 'kea'; + +import { i18n } from '@kbn/i18n'; + +import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; +import { setQueuedErrorMessage } from '../../../shared/flash_messages'; +import { AppLogic } from '../../app_logic'; + +// TODO: Uncomment and add more routes as we migrate them +import { + ENGINES_PATH, + ENGINE_PATH, + ENGINE_ANALYTICS_PATH, + // ENGINE_DOCUMENTS_PATH, + // ENGINE_SCHEMA_PATH, + // ENGINE_CRAWLER_PATH, + // META_ENGINE_SOURCE_ENGINES_PATH, + // ENGINE_RELEVANCE_TUNING_PATH, + // ENGINE_SYNONYMS_PATH, + // ENGINE_CURATIONS_PATH, + // ENGINE_RESULT_SETTINGS_PATH, + // ENGINE_SEARCH_UI_PATH, + // ENGINE_API_LOGS_PATH, +} from '../../routes'; +import { ENGINES_TITLE } from '../engines'; +import { + OVERVIEW_TITLE, + ANALYTICS_TITLE, + // DOCUMENTS_TITLE, + // SCHEMA_TITLE, + // CRAWLER_TITLE, + // RELEVANCE_TUNING_TITLE, + // SYNONYMS_TITLE, + // CURATIONS_TITLE, + // RESULT_SETTINGS_TITLE, + // SEARCH_UI_TITLE, + // API_LOGS_TITLE, +} from './constants'; + +import { EngineLogic } from './'; + +export const EngineRouter: React.FC = () => { + const { + myRole: { + canViewEngineAnalytics, + // canViewEngineDocuments, + // canViewEngineSchema, + // canViewEngineCrawler, + // canViewMetaEngineSourceEngines, + // canManageEngineSynonyms, + // canManageEngineCurations, + // canManageEngineRelevanceTuning, + // canManageEngineResultSettings, + // canManageEngineSearchUi, + // canViewEngineApiLogs, + }, + } = useValues(AppLogic); + + const { dataLoading, engineNotFound } = useValues(EngineLogic); + const { setEngineName, initializeEngine, clearEngine } = useActions(EngineLogic); + + const { engineName } = useParams() as { engineName: string }; + const engineBreadcrumb = [ENGINES_TITLE, engineName]; + + useEffect(() => { + setEngineName(engineName); + initializeEngine(); + return clearEngine; + }, [engineName]); + + if (engineNotFound) { + setQueuedErrorMessage( + i18n.translate('xpack.enterpriseSearch.appSearch.engine.notFound', { + defaultMessage: "No engine with name '{engineName}' could be found.", + values: { engineName }, + }) + ); + return ; + } + + if (dataLoading) return null; + + return ( + + {canViewEngineAnalytics && ( + + +
Just testing right now
+ + )} + + +
Overview
+ + + ); +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/index.ts index 3d8f343312cc6..4e7d81f73fb8d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/index.ts @@ -4,5 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -export { EngineRouter, EngineNav } from './engine_nav'; +export { EngineRouter } from './engine_router'; +export { EngineNav } from './engine_nav'; export { EngineLogic } from './engine_logic'; From e10cc76df71ad4845d3f54e925a31bbb326f6ea9 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 11 Nov 2020 08:55:02 -0800 Subject: [PATCH 5/5] [PR feedback] More explicit test cases --- .../components/engine/engine_nav.test.tsx | 106 +++++++++++------- 1 file changed, 66 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx index bc0c61326a275..7e8228a89cb8d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine/engine_nav.test.tsx @@ -8,7 +8,7 @@ import { setMockValues } from '../../../__mocks__/kea.mock'; import React from 'react'; import { shallow } from 'enzyme'; -import { EuiBadge } from '@elastic/eui'; +import { EuiBadge, EuiIcon } from '@elastic/eui'; import { EngineNav } from './'; @@ -71,46 +71,58 @@ describe('EngineNav', () => { expect(wrapper.find('[data-test-subj="EngineSchemaLink"]')).toHaveLength(1); }); - it('renders schema nav icons', () => { + describe('schema nav icons', () => { const myRole = { canViewEngineSchema: true }; - const wrapper = shallow(); - setMockValues({ ...values, myRole, hasUnconfirmedSchemaFields: true }); - wrapper.setProps({}); // Re-render - expect(wrapper.find('[data-test-subj="EngineNavSchemaUnconfirmedFields"]')).toHaveLength(1); + it('renders unconfirmed schema fields info icon', () => { + setMockValues({ ...values, myRole, hasUnconfirmedSchemaFields: true }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="EngineNavSchemaUnconfirmedFields"]')).toHaveLength(1); + }); - setMockValues({ ...values, myRole, hasSchemaConflicts: true }); - wrapper.setProps({}); // Re-render - expect(wrapper.find('[data-test-subj="EngineNavSchemaConflicts"]')).toHaveLength(1); + it('renders schema conflicts alert icon', () => { + setMockValues({ ...values, myRole, hasSchemaConflicts: true }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="EngineNavSchemaConflicts"]')).toHaveLength(1); + }); }); - it('renders a crawler link', () => { + describe('crawler link', () => { const myRole = { canViewEngineCrawler: true }; - setMockValues({ ...values, myRole }); - const wrapper = shallow(); - expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(1); - // Does not render for meta engines - setMockValues({ ...values, myRole, isMetaEngine: true }); - wrapper.setProps({}); // Re-render - expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(0); + it('renders', () => { + setMockValues({ ...values, myRole }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(1); + }); - // Does not render for sample engine - setMockValues({ ...values, myRole, isSampleEngine: true }); - wrapper.setProps({}); // Re-render - expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(0); + it('does not render for meta engines', () => { + setMockValues({ ...values, myRole, isMetaEngine: true }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(0); + }); + + it('does not render for sample engine', () => { + setMockValues({ ...values, myRole, isSampleEngine: true }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="EngineCrawlerLink"]')).toHaveLength(0); + }); }); - it('renders a meta engine source engines link', () => { + describe('meta engine source engines link', () => { const myRole = { canViewMetaEngineSourceEngines: true }; - setMockValues({ ...values, myRole, isMetaEngine: true }); - const wrapper = shallow(); - expect(wrapper.find('[data-test-subj="MetaEngineEnginesLink"]')).toHaveLength(1); - // Does not render if engine is not a meta-engine - setMockValues({ ...values, myRole, isMetaEngine: false }); - wrapper.setProps({}); // Re-render - expect(wrapper.find('[data-test-subj="MetaEngineEnginesLink"]')).toHaveLength(0); + it('renders', () => { + setMockValues({ ...values, myRole, isMetaEngine: true }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="MetaEngineEnginesLink"]')).toHaveLength(1); + }); + + it('does not render for non meta engines', () => { + setMockValues({ ...values, myRole, isMetaEngine: false }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="MetaEngineEnginesLink"]')).toHaveLength(0); + }); }); it('renders a relevance tuning link', () => { @@ -119,19 +131,33 @@ describe('EngineNav', () => { expect(wrapper.find('[data-test-subj="EngineRelevanceTuningLink"]')).toHaveLength(1); }); - it('renders relevance tuning nav icons', () => { - setMockValues({ - ...values, - myRole: { canManageEngineRelevanceTuning: true }, - engine: { - unsearchedUnconfirmedFields: true, - invalidBoosts: true, - }, + describe('relevance tuning nav icons', () => { + const myRole = { canManageEngineRelevanceTuning: true }; + + it('renders unconfirmed schema fields info icon', () => { + const engine = { unsearchedUnconfirmedFields: true }; + setMockValues({ ...values, myRole, engine }); + const wrapper = shallow(); + expect( + wrapper.find('[data-test-subj="EngineNavRelevanceTuningUnsearchedFields"]') + ).toHaveLength(1); }); - const wrapper = shallow(); - expect(wrapper.find('[data-test-subj="EngineNavRelevanceTuningInvalidBoosts"]')).toHaveLength(1); // prettier-ignore - expect(wrapper.find('[data-test-subj="EngineNavRelevanceTuningUnsearchedFields"]')).toHaveLength(1); // prettier-ignore + it('renders schema conflicts alert icon', () => { + const engine = { invalidBoosts: true }; + setMockValues({ ...values, myRole, engine }); + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="EngineNavRelevanceTuningInvalidBoosts"]')).toHaveLength( + 1 + ); + }); + + it('can render multiple icons', () => { + const engine = { invalidBoosts: true, unsearchedUnconfirmedFields: true }; + setMockValues({ ...values, myRole, engine }); + const wrapper = shallow(); + expect(wrapper.find(EuiIcon)).toHaveLength(2); + }); }); it('renders a synonyms link', () => {