From 9d22cbd88ce90ddfd9500e8d4b75aa8185d56c93 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Thu, 21 Jan 2021 10:10:25 -0500 Subject: [PATCH 01/10] Added server routes --- .../server/__mocks__/router.mock.ts | 9 +- .../server/routes/app_search/index.ts | 2 + .../routes/app_search/search_settings.test.ts | 202 ++++++++++++++++++ .../routes/app_search/search_settings.ts | 101 +++++++++ 4 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts create mode 100644 x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts index c02d5cf0ff1309..cfb33b701ebdb4 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts @@ -68,7 +68,14 @@ export class MockRouter { public validateRoute = (request: MockRouterRequest) => { if (!this.payload) throw new Error('Cannot validate wihout a payload type specified.'); - const [config] = this.router[this.method].mock.calls[0]; + // @ts-ignore + const configForMethodAndPath = this.router[this.method].mock.calls.find( + // @ts-ignore + ([config]) => config.path === this.path + ); + if (!configForMethodAndPath) + throw new Error(`No route registered for ${this.method} & ${this.path}`); + const [config] = configForMethodAndPath; const validate = config.validate as RouteValidatorConfig<{}, {}, {}>; const payloadValidation = validate[this.payload] as { validate(request: KibanaRequest): void }; 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 a20e7854db171b..c384826f469f8b 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 @@ -11,6 +11,7 @@ import { registerCredentialsRoutes } from './credentials'; import { registerSettingsRoutes } from './settings'; import { registerAnalyticsRoutes } from './analytics'; import { registerDocumentsRoutes, registerDocumentRoutes } from './documents'; +import { registerSearchSettingsRoutes } from './search_settings'; export const registerAppSearchRoutes = (dependencies: RouteDependencies) => { registerEnginesRoutes(dependencies); @@ -19,4 +20,5 @@ export const registerAppSearchRoutes = (dependencies: RouteDependencies) => { registerAnalyticsRoutes(dependencies); registerDocumentsRoutes(dependencies); registerDocumentRoutes(dependencies); + registerSearchSettingsRoutes(dependencies); }; diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts new file mode 100644 index 00000000000000..2d714d5ef9a5fa --- /dev/null +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts @@ -0,0 +1,202 @@ +/* + * 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 { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; + +import { registerSearchSettingsRoutes } from './search_settings'; + +describe('search settings routes', () => { + const boosts = { foo: [{}] }; + const resultFields = { foo: {} }; + const searchFields = { foo: {} }; + const searchSettings = { + boosts, + result_fields: resultFields, + search_fields: searchFields, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('GET /api/app_search/engines/{name}/search_settings/details', () => { + let mockRouter: MockRouter; + + beforeEach(() => { + jest.clearAllMocks(); + mockRouter = new MockRouter({ + method: 'get', + path: '/api/app_search/engines/{engineName}/search_settings/details', + }); + + registerSearchSettingsRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + }); + + it('creates a request to enterprise search', () => { + mockRouter.callRoute({ + params: { engineName: 'some-engine' }, + }); + + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/some-engine/search_settings/details', + }); + }); + }); + + describe('PUT /api/app_search/engines/{name}/search_settings', () => { + let mockRouter: MockRouter; + + beforeEach(() => { + jest.clearAllMocks(); + mockRouter = new MockRouter({ + method: 'put', + path: '/api/app_search/engines/{engineName}/search_settings', + payload: 'body', + }); + + registerSearchSettingsRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + }); + + it('creates a request to enterprise search', () => { + mockRouter.callRoute({ + params: { engineName: 'some-engine' }, + body: searchSettings, + }); + + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/some-engine/search_settings', + }); + }); + + describe('validates', () => { + it('correctly', () => { + const request = { body: searchSettings }; + mockRouter.shouldValidate(request); + }); + + it('missing required fields', () => { + const request = { body: {} }; + mockRouter.shouldThrow(request); + }); + }); + }); + + describe('POST /api/app_search/engines/{name}/search_settings/reset', () => { + let mockRouter: MockRouter; + + beforeEach(() => { + jest.clearAllMocks(); + mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings/reset', + }); + + registerSearchSettingsRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + }); + + it('creates a request to enterprise search', () => { + mockRouter.callRoute({ + params: { engineName: 'some-engine' }, + }); + + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/some-engine/search_settings/reset', + }); + }); + }); + + describe('POST /api/app_search/engines/{name}/search_settings_search', () => { + let mockRouter: MockRouter; + + it('creates a request to enterprise search', () => { + mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings_search', + payload: 'body', + }); + + registerSearchSettingsRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + + mockRouter.callRoute({ + params: { engineName: 'some-engine' }, + body: searchSettings, + }); + + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/some-engine/search_settings_search', + }); + }); + + describe('validates body', () => { + beforeEach(() => { + mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings_search', + payload: 'body', + }); + + registerSearchSettingsRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + }); + + it('correctly', () => { + const request = { + body: { + boosts, + search_fields: searchFields, + }, + }; + mockRouter.shouldValidate(request); + }); + + it('missing required fields', () => { + const request = { body: {} }; + mockRouter.shouldThrow(request); + }); + }); + + describe('validates query', () => { + it('correctly', () => { + mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings_search', + payload: 'query', + }); + + registerSearchSettingsRoutes({ + ...mockDependencies, + router: mockRouter.router, + }); + + const request = { + query: { + query: 'foo', + }, + }; + mockRouter.shouldValidate(request); + }); + + it('missing required fields', () => { + const request = { query: {} }; + mockRouter.shouldThrow(request); + }); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts new file mode 100644 index 00000000000000..062c44e56dd66c --- /dev/null +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts @@ -0,0 +1,101 @@ +/* + * 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 { schema } from '@kbn/config-schema'; + +import { RouteDependencies } from '../../plugin'; + +// We only do a very light type check here, and allow unknowns, because the request is validated +// on the ent-search server, so it would be redundant to check it here as well. +const boosts = schema.recordOf( + schema.string(), + schema.arrayOf(schema.object({}, { unknowns: 'allow' })) +); +const resultFields = schema.recordOf(schema.string(), schema.object({}, { unknowns: 'allow' })); +const searchFields = schema.recordOf(schema.string(), schema.object({}, { unknowns: 'allow' })); + +const searchSettingsSchema = schema.object({ + boosts, + result_fields: resultFields, + search_fields: searchFields, +}); + +export function registerSearchSettingsRoutes({ + router, + enterpriseSearchRequestHandler, +}: RouteDependencies) { + router.get( + { + path: '/api/app_search/engines/{engineName}/search_settings/details', + validate: { + params: schema.object({ + engineName: schema.string(), + }), + }, + }, + async (context, request, response) => { + return enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/${request.params.engineName}/search_settings/details`, + })(context, request, response); + } + ); + + router.post( + { + path: '/api/app_search/engines/{engineName}/search_settings/reset', + validate: { + params: schema.object({ + engineName: schema.string(), + }), + }, + }, + async (context, request, response) => { + return enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/${request.params.engineName}/search_settings/reset`, + })(context, request, response); + } + ); + + router.put( + { + path: '/api/app_search/engines/{engineName}/search_settings', + validate: { + params: schema.object({ + engineName: schema.string(), + }), + body: searchSettingsSchema, + }, + }, + async (context, request, response) => { + return enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/${request.params.engineName}/search_settings`, + })(context, request, response); + } + ); + + router.post( + { + path: '/api/app_search/engines/{engineName}/search_settings_search', + validate: { + params: schema.object({ + engineName: schema.string(), + }), + body: schema.object({ + boosts, + search_fields: searchFields, + }), + query: schema.object({ + query: schema.string(), + }), + }, + }, + async (context, request, response) => { + return enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/${request.params.engineName}/search_settings_search`, + })(context, request, response); + } + ); +} From 643c874a6bff3888e510db2c782141f9e5e8dc09 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Thu, 21 Jan 2021 10:53:43 -0500 Subject: [PATCH 02/10] Added a stub Relevance Tuning page --- .../components/engine/engine_nav.tsx | 3 +- .../components/engine/engine_router.tsx | 6 ++- .../components/relevance_tuning/index.ts | 1 + .../relevance_tuning.test.tsx | 22 ++++++++++ .../relevance_tuning/relevance_tuning.tsx | 43 +++++++++++++++++++ .../public/applications/app_search/routes.ts | 2 +- 6 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx 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 0e5a7d56e90653..6f0d8571ab281f 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 @@ -174,8 +174,7 @@ export const EngineNav: React.FC = () => { )} {canManageEngineRelevanceTuning && ( 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 index fd21507a427d5c..688e8d458929ca 100644 --- 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 @@ -23,7 +23,7 @@ import { // ENGINE_SCHEMA_PATH, // ENGINE_CRAWLER_PATH, // META_ENGINE_SOURCE_ENGINES_PATH, - // ENGINE_RELEVANCE_TUNING_PATH, + ENGINE_RELEVANCE_TUNING_PATH, // ENGINE_SYNONYMS_PATH, // ENGINE_CURATIONS_PATH, // ENGINE_RESULT_SETTINGS_PATH, @@ -37,6 +37,7 @@ import { Loading } from '../../../shared/loading'; import { EngineOverview } from '../engine_overview'; import { AnalyticsRouter } from '../analytics'; import { DocumentDetail, Documents } from '../documents'; +import { RelevanceTuning } from '../relevance_tuning'; import { EngineLogic } from './'; @@ -95,6 +96,9 @@ export const EngineRouter: React.FC = () => { + + + diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/index.ts index 40f3ddbf2899be..55070255ac81b1 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/index.ts @@ -5,3 +5,4 @@ */ export { RELEVANCE_TUNING_TITLE } from './constants'; +export { RelevanceTuning } from './relevance_tuning'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx new file mode 100644 index 00000000000000..5934aca6be5f2a --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.test.tsx @@ -0,0 +1,22 @@ +/* + * 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 from 'react'; + +import { shallow } from 'enzyme'; + +import { RelevanceTuning } from './relevance_tuning'; + +describe('RelevanceTuning', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders', () => { + const wrapper = shallow(); + expect(wrapper.isEmptyRender()).toBe(false); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx new file mode 100644 index 00000000000000..cca352904930b1 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning.tsx @@ -0,0 +1,43 @@ +/* + * 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 from 'react'; +import { + EuiPageHeader, + EuiPageHeaderSection, + EuiTitle, + EuiPageContentBody, + EuiPageContent, +} from '@elastic/eui'; + +import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; +import { FlashMessages } from '../../../shared/flash_messages'; + +import { RELEVANCE_TUNING_TITLE } from './constants'; + +interface Props { + engineBreadcrumb: string[]; +} + +export const RelevanceTuning: React.FC = ({ engineBreadcrumb }) => { + return ( + <> + + + + +

{RELEVANCE_TUNING_TITLE}

+ + + + + + + + + + ); +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts index 41e9bfa19e0f0f..91ddca46de62b3 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts @@ -41,7 +41,7 @@ export const ENGINE_CRAWLER_PATH = `${ENGINE_PATH}/crawler`; export const META_ENGINE_SOURCE_ENGINES_PATH = `${ENGINE_PATH}/engines`; -export const ENGINE_RELEVANCE_TUNING_PATH = `${ENGINE_PATH}/search-settings`; +export const ENGINE_RELEVANCE_TUNING_PATH = `${ENGINE_PATH}/relevance-tuning`; export const ENGINE_SYNONYMS_PATH = `${ENGINE_PATH}/synonyms`; export const ENGINE_CURATIONS_PATH = `${ENGINE_PATH}/curations`; // TODO: Curations sub-pages From dcc92e3861ca266189421b438d361b9498296679 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Fri, 29 Jan 2021 12:44:00 -0500 Subject: [PATCH 03/10] PR feedback about beforeEach --- .../routes/app_search/search_settings.test.ts | 82 +++++++------------ 1 file changed, 30 insertions(+), 52 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts index 2d714d5ef9a5fa..4f2ab39c1928b7 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts @@ -23,15 +23,12 @@ describe('search settings routes', () => { }); describe('GET /api/app_search/engines/{name}/search_settings/details', () => { - let mockRouter: MockRouter; + const mockRouter = new MockRouter({ + method: 'get', + path: '/api/app_search/engines/{engineName}/search_settings/details', + }); beforeEach(() => { - jest.clearAllMocks(); - mockRouter = new MockRouter({ - method: 'get', - path: '/api/app_search/engines/{engineName}/search_settings/details', - }); - registerSearchSettingsRoutes({ ...mockDependencies, router: mockRouter.router, @@ -50,16 +47,13 @@ describe('search settings routes', () => { }); describe('PUT /api/app_search/engines/{name}/search_settings', () => { - let mockRouter: MockRouter; + const mockRouter = new MockRouter({ + method: 'put', + path: '/api/app_search/engines/{engineName}/search_settings', + payload: 'body', + }); beforeEach(() => { - jest.clearAllMocks(); - mockRouter = new MockRouter({ - method: 'put', - path: '/api/app_search/engines/{engineName}/search_settings', - payload: 'body', - }); - registerSearchSettingsRoutes({ ...mockDependencies, router: mockRouter.router, @@ -91,15 +85,12 @@ describe('search settings routes', () => { }); describe('POST /api/app_search/engines/{name}/search_settings/reset', () => { - let mockRouter: MockRouter; + const mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings/reset', + }); beforeEach(() => { - jest.clearAllMocks(); - mockRouter = new MockRouter({ - method: 'post', - path: '/api/app_search/engines/{engineName}/search_settings/reset', - }); - registerSearchSettingsRoutes({ ...mockDependencies, router: mockRouter.router, @@ -118,20 +109,20 @@ describe('search settings routes', () => { }); describe('POST /api/app_search/engines/{name}/search_settings_search', () => { - let mockRouter: MockRouter; - - it('creates a request to enterprise search', () => { - mockRouter = new MockRouter({ - method: 'post', - path: '/api/app_search/engines/{engineName}/search_settings_search', - payload: 'body', - }); + const mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings_search', + payload: 'body', + }); + beforeEach(() => { registerSearchSettingsRoutes({ ...mockDependencies, router: mockRouter.router, }); + }); + it('creates a request to enterprise search', () => { mockRouter.callRoute({ params: { engineName: 'some-engine' }, body: searchSettings, @@ -143,19 +134,6 @@ describe('search settings routes', () => { }); describe('validates body', () => { - beforeEach(() => { - mockRouter = new MockRouter({ - method: 'post', - path: '/api/app_search/engines/{engineName}/search_settings_search', - payload: 'body', - }); - - registerSearchSettingsRoutes({ - ...mockDependencies, - router: mockRouter.router, - }); - }); - it('correctly', () => { const request = { body: { @@ -173,16 +151,16 @@ describe('search settings routes', () => { }); describe('validates query', () => { - it('correctly', () => { - mockRouter = new MockRouter({ - method: 'post', - path: '/api/app_search/engines/{engineName}/search_settings_search', - payload: 'query', - }); + const queryRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/search_settings_search', + payload: 'query', + }); + it('correctly', () => { registerSearchSettingsRoutes({ ...mockDependencies, - router: mockRouter.router, + router: queryRouter.router, }); const request = { @@ -190,12 +168,12 @@ describe('search settings routes', () => { query: 'foo', }, }; - mockRouter.shouldValidate(request); + queryRouter.shouldValidate(request); }); it('missing required fields', () => { const request = { query: {} }; - mockRouter.shouldThrow(request); + queryRouter.shouldThrow(request); }); }); }); From 1f9879d08d78793d273437013ba38ca382f56ea0 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Fri, 29 Jan 2021 12:56:18 -0500 Subject: [PATCH 04/10] Added a comment about ts-ingore --- .../plugins/enterprise_search/server/__mocks__/router.mock.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts index cfb33b701ebdb4..5ffb83bf132e14 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts @@ -68,10 +68,10 @@ export class MockRouter { public validateRoute = (request: MockRouterRequest) => { if (!this.payload) throw new Error('Cannot validate wihout a payload type specified.'); + // Ignoring a TS error here because it's tricky to resolve and unimportant to fix // @ts-ignore const configForMethodAndPath = this.router[this.method].mock.calls.find( - // @ts-ignore - ([config]) => config.path === this.path + ([config]: Array<{ path: string }>) => config.path === this.path ); if (!configForMethodAndPath) throw new Error(`No route registered for ${this.method} & ${this.path}`); From 755e26beab0c650491c978f41ba368fd6872ca2b Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Mon, 1 Feb 2021 11:09:35 -0500 Subject: [PATCH 05/10] relevance-tuning to relevance_tuning --- .../enterprise_search/public/applications/app_search/routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts index bdc129558c23cc..080f6efcc71fb8 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts @@ -41,7 +41,7 @@ export const ENGINE_CRAWLER_PATH = `${ENGINE_PATH}/crawler`; export const META_ENGINE_SOURCE_ENGINES_PATH = `${ENGINE_PATH}/engines`; -export const ENGINE_RELEVANCE_TUNING_PATH = `${ENGINE_PATH}/relevance-tuning`; +export const ENGINE_RELEVANCE_TUNING_PATH = `${ENGINE_PATH}/relevance_tuning`; export const ENGINE_SYNONYMS_PATH = `${ENGINE_PATH}/synonyms`; export const ENGINE_CURATIONS_PATH = `${ENGINE_PATH}/curations`; // TODO: Curations sub-pages From c779f6a55497be9b5d20ad3d42710952ac42c752 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Mon, 1 Feb 2021 11:20:44 -0500 Subject: [PATCH 06/10] Switch to simplified route handlers --- .../routes/app_search/search_settings.test.ts | 8 ++--- .../routes/app_search/search_settings.ts | 32 +++++++------------ 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts index 4f2ab39c1928b7..6af9a0503cfed1 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts @@ -41,7 +41,7 @@ describe('search settings routes', () => { }); expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ - path: '/as/engines/some-engine/search_settings/details', + path: '/as/engines/:engineName/search_settings/details', }); }); }); @@ -67,7 +67,7 @@ describe('search settings routes', () => { }); expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ - path: '/as/engines/some-engine/search_settings', + path: '/as/engines/:engineName/search_settings', }); }); @@ -103,7 +103,7 @@ describe('search settings routes', () => { }); expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ - path: '/as/engines/some-engine/search_settings/reset', + path: '/as/engines/:engineName/search_settings/reset', }); }); }); @@ -129,7 +129,7 @@ describe('search settings routes', () => { }); expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ - path: '/as/engines/some-engine/search_settings_search', + path: '/as/engines/:engineName/search_settings_search', }); }); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts index 062c44e56dd66c..eb50d736ac9759 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.ts @@ -36,11 +36,9 @@ export function registerSearchSettingsRoutes({ }), }, }, - async (context, request, response) => { - return enterpriseSearchRequestHandler.createRequest({ - path: `/as/engines/${request.params.engineName}/search_settings/details`, - })(context, request, response); - } + enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/:engineName/search_settings/details`, + }) ); router.post( @@ -52,11 +50,9 @@ export function registerSearchSettingsRoutes({ }), }, }, - async (context, request, response) => { - return enterpriseSearchRequestHandler.createRequest({ - path: `/as/engines/${request.params.engineName}/search_settings/reset`, - })(context, request, response); - } + enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/:engineName/search_settings/reset`, + }) ); router.put( @@ -69,11 +65,9 @@ export function registerSearchSettingsRoutes({ body: searchSettingsSchema, }, }, - async (context, request, response) => { - return enterpriseSearchRequestHandler.createRequest({ - path: `/as/engines/${request.params.engineName}/search_settings`, - })(context, request, response); - } + enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/:engineName/search_settings`, + }) ); router.post( @@ -92,10 +86,8 @@ export function registerSearchSettingsRoutes({ }), }, }, - async (context, request, response) => { - return enterpriseSearchRequestHandler.createRequest({ - path: `/as/engines/${request.params.engineName}/search_settings_search`, - })(context, request, response); - } + enterpriseSearchRequestHandler.createRequest({ + path: `/as/engines/:engineName/search_settings_search`, + }) ); } From 9b245e43c2d8b23bafa307a3b84b30bc7ee55a13 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Mon, 1 Feb 2021 12:08:12 -0500 Subject: [PATCH 07/10] Added ability check --- .../components/engine/engine_router.test.tsx | 8 ++++++++ .../app_search/components/engine/engine_router.tsx | 10 ++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) 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 index aa8b406cf7774e..f4fabc29a6b592 100644 --- 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 @@ -16,6 +16,7 @@ import { Switch, Redirect, useParams } from 'react-router-dom'; import { Loading } from '../../../shared/loading'; import { EngineOverview } from '../engine_overview'; import { AnalyticsRouter } from '../analytics'; +import { RelevanceTuning } from '../relevance_tuning'; import { EngineRouter } from './engine_router'; @@ -93,4 +94,11 @@ describe('EngineRouter', () => { expect(wrapper.find(AnalyticsRouter)).toHaveLength(1); }); + + it('renders an relevance tuning view', () => { + setMockValues({ ...values, myRole: { canManageEngineRelevanceTuning: true } }); + const wrapper = shallow(); + + expect(wrapper.find(RelevanceTuning)).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 index 688e8d458929ca..ba00792237971f 100644 --- 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 @@ -45,13 +45,13 @@ export const EngineRouter: React.FC = () => { const { myRole: { canViewEngineAnalytics, + canManageEngineRelevanceTuning, // canViewEngineDocuments, // canViewEngineSchema, // canViewEngineCrawler, // canViewMetaEngineSourceEngines, // canManageEngineSynonyms, // canManageEngineCurations, - // canManageEngineRelevanceTuning, // canManageEngineResultSettings, // canManageEngineSearchUi, // canViewEngineApiLogs, @@ -96,9 +96,11 @@ export const EngineRouter: React.FC = () => { - - - + {canManageEngineRelevanceTuning && ( + + + + )} From fd0809ba481d503be220c280f5ccdbedc687d6ff Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Mon, 1 Feb 2021 12:14:26 -0500 Subject: [PATCH 08/10] Added realistic server data --- .../routes/app_search/search_settings.test.ts | 42 +++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts index 6af9a0503cfed1..56a6e6297d1f83 100644 --- a/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/search_settings.test.ts @@ -9,9 +9,45 @@ import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks_ import { registerSearchSettingsRoutes } from './search_settings'; describe('search settings routes', () => { - const boosts = { foo: [{}] }; - const resultFields = { foo: {} }; - const searchFields = { foo: {} }; + const boosts = { + types: [ + { + type: 'value', + factor: 6.2, + value: ['1313'], + }, + ], + hp: [ + { + function: 'exponential', + type: 'functional', + factor: 1, + operation: 'add', + }, + ], + }; + const resultFields = { + id: { + raw: {}, + }, + hp: { + raw: {}, + }, + name: { + raw: {}, + }, + }; + const searchFields = { + hp: { + weight: 1, + }, + name: { + weight: 1, + }, + id: { + weight: 1, + }, + }; const searchSettings = { boosts, result_fields: resultFields, From db7e587577029baf45e67d6e1358e40ee87f9ac9 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Mon, 1 Feb 2021 12:40:49 -0500 Subject: [PATCH 09/10] Refactor MockRouter --- .../server/__mocks__/router.mock.ts | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts index 5ffb83bf132e14..98a6c5b98e16e4 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts @@ -50,12 +50,7 @@ export class MockRouter { }; public callRoute = async (request: MockRouterRequest) => { - const routerCalls = this.router[this.method].mock.calls as any[]; - if (!routerCalls.length) throw new Error('No routes registered.'); - - const route = routerCalls.find(([router]: any) => router.path === this.path); - if (!route) throw new Error('No matching registered routes found - check method/path keys'); - + const route = this.findRouteRegistration(); const [, handler] = route; const context = {} as jest.Mocked; await handler(context, httpServerMock.createKibanaRequest(request as any), this.response); @@ -68,14 +63,8 @@ export class MockRouter { public validateRoute = (request: MockRouterRequest) => { if (!this.payload) throw new Error('Cannot validate wihout a payload type specified.'); - // Ignoring a TS error here because it's tricky to resolve and unimportant to fix - // @ts-ignore - const configForMethodAndPath = this.router[this.method].mock.calls.find( - ([config]: Array<{ path: string }>) => config.path === this.path - ); - if (!configForMethodAndPath) - throw new Error(`No route registered for ${this.method} & ${this.path}`); - const [config] = configForMethodAndPath; + const route = this.findRouteRegistration(); + const [config] = route; const validate = config.validate as RouteValidatorConfig<{}, {}, {}>; const payloadValidation = validate[this.payload] as { validate(request: KibanaRequest): void }; @@ -91,6 +80,16 @@ export class MockRouter { public shouldThrow = (request: MockRouterRequest) => { expect(() => this.validateRoute(request)).toThrow(); }; + + findRouteRegistration = () => { + const routerCalls = this.router[this.method].mock.calls as any[]; + if (!routerCalls.length) throw new Error('No routes registered.'); + + const route = routerCalls.find(([router]: any) => router.path === this.path); + if (!route) throw new Error('No matching registered routes found - check method/path keys'); + + return route; + }; } /** From b43820e333e78bb98babf8c0f5d6eea832e3c207 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Tue, 2 Feb 2021 08:21:40 -0500 Subject: [PATCH 10/10] Private eyes --- .../plugins/enterprise_search/server/__mocks__/router.mock.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts index 98a6c5b98e16e4..819cabec44f070 100644 --- a/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts +++ b/x-pack/plugins/enterprise_search/server/__mocks__/router.mock.ts @@ -81,7 +81,7 @@ export class MockRouter { expect(() => this.validateRoute(request)).toThrow(); }; - findRouteRegistration = () => { + private findRouteRegistration = () => { const routerCalls = this.router[this.method].mock.calls as any[]; if (!routerCalls.length) throw new Error('No routes registered.');