From b2b11abc2905523241a6af21180b2f094049c191 Mon Sep 17 00:00:00 2001 From: Jason Stoltzfus Date: Wed, 31 Mar 2021 14:37:40 -0400 Subject: [PATCH] Remaining sample work --- .../sample_response/sample_response.test.tsx | 20 +- .../sample_response/sample_response.tsx | 22 +- .../sample_response_logic.test.ts | 211 +++++++++++++----- .../sample_response/sample_response_logic.ts | 120 +++++----- .../server/routes/app_search/index.ts | 2 + .../app_search/sample_response_search.test.ts | 72 ++++++ .../app_search/sample_response_search.ts | 33 +++ 7 files changed, 353 insertions(+), 127 deletions(-) create mode 100644 x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.test.ts create mode 100644 x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.ts diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.test.tsx index 2808e28d91943d..e324150a2d52a7 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.test.tsx @@ -5,6 +5,7 @@ * 2.0. */ +import '../../../../__mocks__/shallow_useeffect.mock'; import { setMockActions, setMockValues } from '../../../../__mocks__'; import React from 'react'; @@ -18,9 +19,11 @@ import { SampleResponse } from './sample_response'; describe('SampleResponse', () => { const actions = { queryChanged: jest.fn(), + getSearchResults: jest.fn(), }; const values = { + reducedServerResultFields: {}, query: 'foo', response: { bar: 'baz', @@ -44,16 +47,29 @@ describe('SampleResponse', () => { expect(actions.queryChanged).toHaveBeenCalledWith('bar'); }); + it('will call getSearchResults with the current value of query and reducedServerResultFields in a useEffect, which updates the displayed response', () => { + const wrapper = shallow(); + expect(wrapper.find(EuiFieldSearch).prop('value')).toEqual('foo'); + }); + it('renders the response from the given user "query" in a code block', () => { const wrapper = shallow(); expect(wrapper.find(EuiCodeBlock).prop('children')).toEqual('{\n "bar": "baz"\n}'); }); - it('renders an empty string the code block if no response exists yet', () => { + it('renders a plain old string in the code block if the response is a string', () => { + setMockValues({ + response: 'No results.', + }); + const wrapper = shallow(); + expect(wrapper.find(EuiCodeBlock).prop('children')).toEqual('No results.'); + }); + + it('will not render a code block at all if there is no response yet', () => { setMockValues({ response: null, }); const wrapper = shallow(); - expect(wrapper.find(EuiCodeBlock).prop('children')).toEqual(''); + expect(wrapper.find(EuiCodeBlock).exists()).toEqual(false); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.tsx index 88e3b96c240230..63e4018aae5b08 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React from 'react'; +import React, { useEffect } from 'react'; import { useActions, useValues } from 'kea'; @@ -20,11 +20,19 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import { ResultSettingsLogic } from '../result_settings_logic'; + import { SampleResponseLogic } from './sample_response_logic'; export const SampleResponse: React.FC = () => { + const { reducedServerResultFields } = useValues(ResultSettingsLogic); + const { query, response } = useValues(SampleResponseLogic); - const { queryChanged } = useActions(SampleResponseLogic); + const { queryChanged, getSearchResults } = useActions(SampleResponseLogic); + + useEffect(() => { + getSearchResults(query, reducedServerResultFields); + }, [query, reducedServerResultFields]); return ( @@ -54,11 +62,11 @@ export const SampleResponse: React.FC = () => { data-test-subj="ResultSettingsQuerySampleResponse" /> - - {/* TODO No results messsage */} - {/* TODO Query on load */} - {response ? JSON.stringify(response, null, 2) : ''} - + {!!response && ( + + {typeof response === 'string' ? response : JSON.stringify(response, null, 2)} + + )} ); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.test.ts index a91a29f828ea14..3fa847815d6807 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.test.ts @@ -5,19 +5,22 @@ * 2.0. */ -import { LogicMounter } from '../../../../__mocks__'; +import { LogicMounter, mockHttpValues } from '../../../../__mocks__'; +import '../../../__mocks__/engine_logic.mock'; import { nextTick } from '@kbn/test/jest'; +import { flashAPIErrors } from '../../../../shared/flash_messages'; + import { SampleResponseLogic } from './sample_response_logic'; describe('SampleResponseLogic', () => { const { mount } = new LogicMounter(SampleResponseLogic); + const { http } = mockHttpValues; const DEFAULT_VALUES = { query: '', response: null, - isLoading: false, }; beforeEach(() => { @@ -33,10 +36,9 @@ describe('SampleResponseLogic', () => { describe('actions', () => { describe('queryChanged', () => { - it('updates query and sets isLoading to true', () => { + it('updates the query', () => { mount({ - query: {}, - isLoading: false, + query: '', }); SampleResponseLogic.actions.queryChanged('foo'); @@ -44,16 +46,14 @@ describe('SampleResponseLogic', () => { expect(SampleResponseLogic.values).toEqual({ ...DEFAULT_VALUES, query: 'foo', - isLoading: true, }); }); }); describe('getSearchResultsSuccess', () => { - it('sets the response from a search API request and sets isLoading to false', () => { + it('sets the response from a search API request', () => { mount({ response: null, - isLoading: true, }); SampleResponseLogic.actions.getSearchResultsSuccess({}); @@ -61,85 +61,178 @@ describe('SampleResponseLogic', () => { expect(SampleResponseLogic.values).toEqual({ ...DEFAULT_VALUES, response: {}, - isLoading: false, + }); + }); + + it('sets the response to null if no response is passed', () => { + mount({ + response: null, + }); + + SampleResponseLogic.actions.getSearchResultsSuccess(); + + expect(SampleResponseLogic.values).toEqual({ + ...DEFAULT_VALUES, + response: null, }); }); }); describe('getSearchResultsFailure', () => { - it('sets isLoading to false', () => { + it('sets a string response from a search API request', () => { mount({ - isLoading: true, + response: null, + }); + + SampleResponseLogic.actions.getSearchResultsFailure('An error occured.'); + + expect(SampleResponseLogic.values).toEqual({ + ...DEFAULT_VALUES, + response: 'An error occured.', + }); + }); + + it('sets the response to null if no response is passed', () => { + mount({ + response: null, }); SampleResponseLogic.actions.getSearchResultsFailure(); expect(SampleResponseLogic.values).toEqual({ ...DEFAULT_VALUES, - isLoading: false, + response: null, }); }); }); }); describe('listeners', () => { - describe('queryChanged', () => { - // TODO it('makes an API request') + describe('getSearchResults', () => { + beforeAll(() => jest.useFakeTimers()); + afterAll(() => jest.useRealTimers()); - it('calls getSearchResultsSuccess with the result of the response from the search API request', async () => { + it('makes a search API request and calls getSearchResultsSuccess with the first result of the response', async () => { mount(); jest.spyOn(SampleResponseLogic.actions, 'getSearchResultsSuccess'); - SampleResponseLogic.actions.queryChanged('foo'); + http.post.mockReturnValue( + Promise.resolve({ + results: [ + { id: { raw: 'foo' }, _meta: {} }, + { id: { raw: 'bar' }, _meta: {} }, + { id: { raw: 'baz' }, _meta: {} }, + ], + }) + ); + + SampleResponseLogic.actions.getSearchResults('foo', { foo: { raw: true } }); + jest.runAllTimers(); await nextTick(); expect(SampleResponseLogic.actions.getSearchResultsSuccess).toHaveBeenCalledWith({ - visitors: { - raw: 776218, - }, - nps_image_url: { - raw: - 'https://www.nps.gov/common/uploads/banner_image/imr/homepage/9E7FC0DB-1DD8-B71B-0BC3880DC2250415.jpg', - }, - square_km: { - raw: 1366.2, - }, - world_heritage_site: { - raw: 'false', - }, - date_established: { - raw: '1964-09-12T05:00:00+00:00', - }, - image_url: { - raw: - 'https://storage.googleapis.com/public-demo-assets.swiftype.info/swiftype-dot-com-search-ui-national-parks-demo/9E7FC0DB-1DD8-B71B-0BC3880DC2250415.jpg', - }, - description: { - raw: - 'This landscape was eroded into a maze of canyons, buttes, and mesas by the combined efforts of the Colorado River, Green River, and their tributaries, which divide the park into three districts. The park also contains rock pinnacles and arches, as well as artifacts from Ancient Pueblo peoples.', - }, - location: { - raw: '38.2,-109.93', - }, - acres: { - raw: '337597.83', - }, - title: { - raw: 'Canyonlands', - }, - nps_link: { - raw: 'https://www.nps.gov/cany/index.htm', - }, - states: { - raw: ['Utah'], - }, - id: { - raw: 'park_canyonlands', - }, + // Note that the _meta field was stripped from the result + id: { raw: 'foo' }, }); }); - // TODO it('handles errors') + it('calls getSearchResultsSuccess with a "No Results." message if there are no results', async () => { + mount(); + jest.spyOn(SampleResponseLogic.actions, 'getSearchResultsSuccess'); + + http.post.mockReturnValue( + Promise.resolve({ + results: [], + }) + ); + + SampleResponseLogic.actions.getSearchResults('foo', { foo: { raw: true } }); + jest.runAllTimers(); + await nextTick(); + + expect(SampleResponseLogic.actions.getSearchResultsSuccess).toHaveBeenCalledWith( + 'No results.' + ); + }); + + it('handles 500 errors by setting a generic error response and showing a flash message error', async () => { + mount(); + jest.spyOn(SampleResponseLogic.actions, 'getSearchResultsFailure'); + + const error = { + response: { + status: 500, + }, + }; + + http.post.mockReturnValueOnce(Promise.reject(error)); + + SampleResponseLogic.actions.getSearchResults('foo', { foo: { raw: true } }); + jest.runAllTimers(); + await nextTick(); + + expect(flashAPIErrors).toHaveBeenCalledWith(error); + expect(SampleResponseLogic.actions.getSearchResultsFailure).toHaveBeenCalledWith( + 'An error occured.' + ); + }); + + it('handles 400 errors by setting the response message, but does not show a flash error message', async () => { + mount(); + jest.spyOn(SampleResponseLogic.actions, 'getSearchResultsFailure'); + + http.post.mockReturnValueOnce( + Promise.reject({ + response: { + status: 400, + }, + body: { + message: 'A validation error occurred.', + }, + }) + ); + + SampleResponseLogic.actions.getSearchResults('foo', { foo: { raw: true } }); + jest.runAllTimers(); + await nextTick(); + + expect(SampleResponseLogic.actions.getSearchResultsFailure).toHaveBeenCalledWith( + 'A validation error occurred.' + ); + }); + + it('sets a generic message on a 400 error if no custom message is provided in the response', async () => { + mount(); + jest.spyOn(SampleResponseLogic.actions, 'getSearchResultsFailure'); + + http.post.mockReturnValueOnce( + Promise.reject({ + response: { + status: 400, + }, + }) + ); + + SampleResponseLogic.actions.getSearchResults('foo', { foo: { raw: true } }); + jest.runAllTimers(); + await nextTick(); + + expect(SampleResponseLogic.actions.getSearchResultsFailure).toHaveBeenCalledWith( + 'An error occured.' + ); + }); + + it('does nothing if an empty object is passed for the resultFields parameter', async () => { + mount(); + jest.spyOn(SampleResponseLogic.actions, 'getSearchResultsSuccess'); + + SampleResponseLogic.actions.getSearchResults('foo', {}); + + jest.runAllTimers(); + await nextTick(); + + expect(SampleResponseLogic.actions.getSearchResultsSuccess).not.toHaveBeenCalled(); + }); }); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.ts index 9a107150f0fb96..1a1a90ba99aa9a 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/sample_response/sample_response_logic.ts @@ -7,18 +7,38 @@ import { kea, MakeLogicType } from 'kea'; -import { SampleSearchResponse } from '../types'; +import { i18n } from '@kbn/i18n'; + +import { flashAPIErrors } from '../../../../shared/flash_messages'; + +import { HttpLogic } from '../../../../shared/http'; +import { EngineLogic } from '../../engine'; + +import { SampleSearchResponse, ServerFieldResultSettingObject } from '../types'; + +const NO_RESULTS_MESSAGE = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.resultSettings.sampleResponse.noResultsMessage', + { defaultMessage: 'No results.' } +); + +const ERROR_MESSAGE = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.resultSettings.sampleResponse.errorMessage', + { defaultMessage: 'An error occured.' } +); interface SampleResponseValues { query: string; - response?: SampleSearchResponse; - isLoading: boolean; + response: SampleSearchResponse | string | null; } interface SampleResponseActions { queryChanged: (query: string) => { query: string }; - getSearchResultsSuccess: (response: SampleSearchResponse) => { response: SampleSearchResponse }; - getSearchResultsFailure: () => void; + getSearchResultsSuccess: (response?: SampleSearchResponse) => { response?: SampleSearchResponse }; + getSearchResultsFailure: (response?: string) => { response?: string }; + getSearchResults: ( + query: string, + resultFields: ServerFieldResultSettingObject + ) => { query: string; resultFields: ServerFieldResultSettingObject }; } export const SampleResponseLogic = kea>({ @@ -26,70 +46,52 @@ export const SampleResponseLogic = kea ({ query }), getSearchResultsSuccess: (response) => ({ response }), - getSearchResultsFailure: true, + getSearchResultsFailure: (response) => ({ response }), + getSearchResults: (query, resultFields) => ({ query, resultFields }), }, reducers: { query: ['', { queryChanged: (_, { query }) => query }], - response: [null, { getSearchResultsSuccess: (_, { response }) => response }], - isLoading: [ - false, + response: [ + null, { - queryChanged: () => true, - getSearchResultsSuccess: () => false, - getSearchResultsFailure: () => false, + getSearchResultsSuccess: (_, { response }) => response || null, + getSearchResultsFailure: (_, { response }) => response || null, }, ], }, listeners: ({ actions }) => ({ - queryChanged: async () => { + getSearchResults: async ({ query, resultFields }, breakpoint) => { + if (Object.keys(resultFields).length < 1) return; + await breakpoint(250); + + const { http } = HttpLogic.values; + const { engineName } = EngineLogic.values; + + const url = `/api/app_search/engines/${engineName}/sample_response_search`; + try { - const response = await Promise.resolve({ - visitors: { - raw: 776218, - }, - nps_image_url: { - raw: - 'https://www.nps.gov/common/uploads/banner_image/imr/homepage/9E7FC0DB-1DD8-B71B-0BC3880DC2250415.jpg', - }, - square_km: { - raw: 1366.2, - }, - world_heritage_site: { - raw: 'false', - }, - date_established: { - raw: '1964-09-12T05:00:00+00:00', - }, - image_url: { - raw: - 'https://storage.googleapis.com/public-demo-assets.swiftype.info/swiftype-dot-com-search-ui-national-parks-demo/9E7FC0DB-1DD8-B71B-0BC3880DC2250415.jpg', - }, - description: { - raw: - 'This landscape was eroded into a maze of canyons, buttes, and mesas by the combined efforts of the Colorado River, Green River, and their tributaries, which divide the park into three districts. The park also contains rock pinnacles and arches, as well as artifacts from Ancient Pueblo peoples.', - }, - location: { - raw: '38.2,-109.93', - }, - acres: { - raw: '337597.83', - }, - title: { - raw: 'Canyonlands', - }, - nps_link: { - raw: 'https://www.nps.gov/cany/index.htm', - }, - states: { - raw: ['Utah'], - }, - id: { - raw: 'park_canyonlands', - }, + const response = await http.post(url, { + body: JSON.stringify({ + query, + result_fields: resultFields, + }), }); - actions.getSearchResultsSuccess(response as SampleSearchResponse); - } catch (error) { - actions.getSearchResultsFailure(); + + const result = response.results?.[0]; + actions.getSearchResultsSuccess( + result ? { ...result, _meta: undefined } : NO_RESULTS_MESSAGE + ); + } catch (e) { + if (e.response.status >= 500) { + // 4XX Validation errors are expected, as a user could enter something like 2 as a size, which is out of valid range. + // In this case, we simply render the message from the server as the response. + // + // 5xx Server errors are unexpected, and need to be reported in a flash message. + flashAPIErrors(e); + actions.getSearchResultsFailure(ERROR_MESSAGE); + } else { + actions.getSearchResultsFailure(e.body?.message || ERROR_MESSAGE); + } } }, }), 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 1d48614e733741..71267d465a5d5c 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 @@ -16,6 +16,7 @@ import { registerEnginesRoutes } from './engines'; import { registerOnboardingRoutes } from './onboarding'; import { registerResultSettingsRoutes } from './result_settings'; import { registerRoleMappingsRoutes } from './role_mappings'; +import { registerSampleResponseSearchRoute } from './sample_response_search'; import { registerSearchSettingsRoutes } from './search_settings'; import { registerSettingsRoutes } from './settings'; @@ -28,6 +29,7 @@ export const registerAppSearchRoutes = (dependencies: RouteDependencies) => { registerDocumentRoutes(dependencies); registerCurationsRoutes(dependencies); registerSearchSettingsRoutes(dependencies); + registerSampleResponseSearchRoute(dependencies); registerRoleMappingsRoutes(dependencies); registerResultSettingsRoutes(dependencies); registerApiLogsRoutes(dependencies); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.test.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.test.ts new file mode 100644 index 00000000000000..afb4bdbcab3da2 --- /dev/null +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.test.ts @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { MockRouter, mockRequestHandler, mockDependencies } from '../../__mocks__'; + +import { registerSampleResponseSearchRoute } from './sample_response_search'; + +const resultFields = { + id: { + raw: {}, + }, + hp: { + raw: {}, + }, + name: { + raw: {}, + }, +}; + +describe('sample response search route', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('POST /api/app_search/engines/{name}/sample_response_search', () => { + const mockRouter = new MockRouter({ + method: 'post', + path: '/api/app_search/engines/{engineName}/sample_response_search', + }); + + beforeEach(() => { + registerSampleResponseSearchRoute({ + ...mockDependencies, + router: mockRouter.router, + }); + }); + + it('creates a request to enterprise search', () => { + mockRouter.callRoute({ + params: { engineName: 'some-engine' }, + body: { + query: 'test', + result_fields: resultFields, + }, + }); + + expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({ + path: '/as/engines/:engineName/sample_response_search', + }); + }); + + describe('validates', () => { + it('correctly', () => { + const request = { + body: { + query: 'test', + result_fields: resultFields, + }, + }; + mockRouter.shouldValidate(request); + }); + it('missing required fields', () => { + const request = { body: {} }; + mockRouter.shouldThrow(request); + }); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.ts b/x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.ts new file mode 100644 index 00000000000000..20eb57e754260e --- /dev/null +++ b/x-pack/plugins/enterprise_search/server/routes/app_search/sample_response_search.ts @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { schema } from '@kbn/config-schema'; + +import { RouteDependencies } from '../../plugin'; + +export function registerSampleResponseSearchRoute({ + router, + enterpriseSearchRequestHandler, +}: RouteDependencies) { + router.post( + { + path: '/api/app_search/engines/{engineName}/sample_response_search', + validate: { + params: schema.object({ + engineName: schema.string(), + }), + body: schema.object({ + query: schema.string(), + result_fields: schema.recordOf(schema.string(), schema.object({}, { unknowns: 'allow' })), + }), + }, + }, + enterpriseSearchRequestHandler.createRequest({ + path: '/as/engines/:engineName/sample_response_search', + }) + ); +}