diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.test.tsx index edd417cc1ffe8..9ed6e17c2bcd9 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.test.tsx @@ -9,7 +9,7 @@ import { setMockActions, setMockValues } from '../../../__mocks__/kea.mock'; import React from 'react'; -import { shallow } from 'enzyme'; +import { shallow, ShallowWrapper } from 'enzyme'; import { EuiPageHeader } from '@elastic/eui'; @@ -33,9 +33,11 @@ describe('RelevanceTuningLayout', () => { }); const subject = () => shallow(); + const findButtons = (wrapper: ShallowWrapper) => + wrapper.find(EuiPageHeader).prop('rightSideItems') as React.ReactElement[]; it('renders a Save button that will save the current changes', () => { - const buttons = subject().find(EuiPageHeader).prop('rightSideItems') as React.ReactElement[]; + const buttons = findButtons(subject()); expect(buttons.length).toBe(2); const saveButton = shallow(buttons[0]); saveButton.simulate('click'); @@ -43,7 +45,7 @@ describe('RelevanceTuningLayout', () => { }); it('renders a Reset button that will remove all weights and boosts', () => { - const buttons = subject().find(EuiPageHeader).prop('rightSideItems') as React.ReactElement[]; + const buttons = findButtons(subject()); expect(buttons.length).toBe(2); const resetButton = shallow(buttons[1]); resetButton.simulate('click'); @@ -55,7 +57,7 @@ describe('RelevanceTuningLayout', () => { ...values, engineHasSchemaFields: false, }); - const buttons = subject().find(EuiPageHeader).prop('rightSideItems') as React.ReactElement[]; + const buttons = findButtons(subject()); expect(buttons.length).toBe(0); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.tsx index 0ea38b0d9fa36..f29cc12f20a98 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_layout.tsx @@ -37,7 +37,7 @@ export const RelevanceTuningLayout: React.FC = ({ engineBreadcrumb, child description={i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.description', { - defaultMessage: 'Set field weights and boosts', + defaultMessage: 'Set field weights and boosts.', } )} rightSideItems={ diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.test.ts index ca9b0a886fdd1..4ec38d314a259 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.test.ts @@ -586,10 +586,9 @@ describe('RelevanceTuningLogic', () => { confirmSpy.mockImplementation(() => false); RelevanceTuningLogic.actions.resetSearchSettings(); + await nextTick(); - expect(http.post).not.toHaveBeenCalledWith( - '/api/app_search/engines/test-engine/search_settings/reset' - ); + expect(http.post).not.toHaveBeenCalled(); }); it('handles errors', async () => { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.test.tsx index 9eda1362e04fc..5365cc0f029f8 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.test.tsx @@ -11,7 +11,9 @@ import { setMockValues, setMockActions } from '../../../__mocks__'; import React from 'react'; -import { shallow } from 'enzyme'; +import { shallow, ShallowWrapper } from 'enzyme'; + +import { EuiPageHeader } from '@elastic/eui'; import { ResultSettings } from './result_settings'; import { ResultSettingsTable } from './result_settings_table'; @@ -24,6 +26,9 @@ describe('RelevanceTuning', () => { const actions = { initializeResultSettingsData: jest.fn(), + saveResultSettings: jest.fn(), + confirmResetAllFields: jest.fn(), + clearAllFields: jest.fn(), }; beforeEach(() => { @@ -32,8 +37,12 @@ describe('RelevanceTuning', () => { jest.clearAllMocks(); }); + const subject = () => shallow(); + const findButtons = (wrapper: ShallowWrapper) => + wrapper.find(EuiPageHeader).prop('rightSideItems') as React.ReactElement[]; + it('renders', () => { - const wrapper = shallow(); + const wrapper = subject(); expect(wrapper.find(ResultSettingsTable).exists()).toBe(true); expect(wrapper.find(SampleResponse).exists()).toBe(true); }); @@ -47,8 +56,32 @@ describe('RelevanceTuning', () => { setMockValues({ dataLoading: true, }); - const wrapper = shallow(); + const wrapper = subject(); expect(wrapper.find(ResultSettingsTable).exists()).toBe(false); expect(wrapper.find(SampleResponse).exists()).toBe(false); }); + + it('renders a "save" button that will save the current changes', () => { + const buttons = findButtons(subject()); + expect(buttons.length).toBe(3); + const saveButton = shallow(buttons[0]); + saveButton.simulate('click'); + expect(actions.saveResultSettings).toHaveBeenCalled(); + }); + + it('renders a "restore defaults" button that will reset all values to their defaults', () => { + const buttons = findButtons(subject()); + expect(buttons.length).toBe(3); + const resetButton = shallow(buttons[1]); + resetButton.simulate('click'); + expect(actions.confirmResetAllFields).toHaveBeenCalled(); + }); + + it('renders a "clear" button that will remove all selected options', () => { + const buttons = findButtons(subject()); + expect(buttons.length).toBe(3); + const clearButton = shallow(buttons[2]); + clearButton.simulate('click'); + expect(actions.clearAllFields).toHaveBeenCalled(); + }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.tsx index 336f3f663119f..a513d0c1b9f34 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings.tsx @@ -9,12 +9,15 @@ import React, { useEffect } from 'react'; import { useActions, useValues } from 'kea'; -import { EuiPageHeader, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { EuiPageHeader, EuiFlexGroup, EuiFlexItem, EuiButton, EuiButtonEmpty } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; + +import { SAVE_BUTTON_LABEL } from '../../../shared/constants'; import { FlashMessages } from '../../../shared/flash_messages'; import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; - import { Loading } from '../../../shared/loading'; +import { RESTORE_DEFAULTS_BUTTON_LABEL } from '../../constants'; import { RESULT_SETTINGS_TITLE } from './constants'; import { ResultSettingsTable } from './result_settings_table'; @@ -23,13 +26,23 @@ import { SampleResponse } from './sample_response'; import { ResultSettingsLogic } from '.'; +const CLEAR_BUTTON_LABEL = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.resultSettings.clearButtonLabel', + { defaultMessage: 'Clear all values' } +); + interface Props { engineBreadcrumb: string[]; } export const ResultSettings: React.FC = ({ engineBreadcrumb }) => { const { dataLoading } = useValues(ResultSettingsLogic); - const { initializeResultSettingsData } = useActions(ResultSettingsLogic); + const { + initializeResultSettingsData, + saveResultSettings, + confirmResetAllFields, + clearAllFields, + } = useActions(ResultSettingsLogic); useEffect(() => { initializeResultSettingsData(); @@ -40,7 +53,33 @@ export const ResultSettings: React.FC = ({ engineBreadcrumb }) => { return ( <> - + + {SAVE_BUTTON_LABEL} + , + + {RESTORE_DEFAULTS_BUTTON_LABEL} + , + + {CLEAR_BUTTON_LABEL} + , + ]} + /> diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.test.ts index a9c161b2bb5be..8d9c33e3c9e68 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.test.ts @@ -15,7 +15,7 @@ import { nextTick } from '@kbn/test/jest'; import { Schema, SchemaConflicts, SchemaTypes } from '../../../shared/types'; -import { OpenModal, ServerFieldResultSettingObject } from './types'; +import { ServerFieldResultSettingObject } from './types'; import { ResultSettingsLogic } from '.'; @@ -25,7 +25,6 @@ describe('ResultSettingsLogic', () => { const DEFAULT_VALUES = { dataLoading: true, saving: false, - openModal: OpenModal.None, resultFields: {}, lastSavedResultFields: {}, schema: {}, @@ -83,7 +82,6 @@ describe('ResultSettingsLogic', () => { mount({ dataLoading: true, saving: true, - openModal: OpenModal.ConfirmSaveModal, }); ResultSettingsLogic.actions.initializeResultFields( @@ -139,8 +137,6 @@ describe('ResultSettingsLogic', () => { snippetFallback: false, }, }, - // The modal should be reset back to closed if it had been opened previously - openModal: OpenModal.None, // Stores the provided schema details schema, schemaConflicts, @@ -156,47 +152,6 @@ describe('ResultSettingsLogic', () => { }); }); - describe('openConfirmSaveModal', () => { - mount({ - openModal: OpenModal.None, - }); - - ResultSettingsLogic.actions.openConfirmSaveModal(); - - expect(resultSettingLogicValues()).toEqual({ - ...DEFAULT_VALUES, - openModal: OpenModal.ConfirmSaveModal, - }); - }); - - describe('openConfirmResetModal', () => { - mount({ - openModal: OpenModal.None, - }); - - ResultSettingsLogic.actions.openConfirmResetModal(); - - expect(resultSettingLogicValues()).toEqual({ - ...DEFAULT_VALUES, - openModal: OpenModal.ConfirmResetModal, - }); - }); - - describe('closeModals', () => { - it('should close open modals', () => { - mount({ - openModal: OpenModal.ConfirmSaveModal, - }); - - ResultSettingsLogic.actions.closeModals(); - - expect(resultSettingLogicValues()).toEqual({ - ...DEFAULT_VALUES, - openModal: OpenModal.None, - }); - }); - }); - describe('clearAllFields', () => { it('should remove all settings that have been set for each field', () => { mount({ @@ -237,19 +192,6 @@ describe('ResultSettingsLogic', () => { }, }); }); - - it('should close open modals', () => { - mount({ - openModal: OpenModal.ConfirmSaveModal, - }); - - ResultSettingsLogic.actions.resetAllFields(); - - expect(resultSettingLogicValues()).toEqual({ - ...DEFAULT_VALUES, - openModal: OpenModal.None, - }); - }); }); describe('updateField', () => { @@ -297,7 +239,7 @@ describe('ResultSettingsLogic', () => { }); describe('saving', () => { - it('sets saving to true and close any open modals', () => { + it('sets saving to true', () => { mount({ saving: false, }); @@ -307,7 +249,6 @@ describe('ResultSettingsLogic', () => { expect(resultSettingLogicValues()).toEqual({ ...DEFAULT_VALUES, saving: true, - openModal: OpenModal.None, }); }); }); @@ -563,6 +504,12 @@ describe('ResultSettingsLogic', () => { describe('listeners', () => { const { http } = mockHttpValues; const { flashAPIErrors } = mockFlashMessageHelpers; + let confirmSpy: jest.SpyInstance; + + beforeAll(() => { + confirmSpy = jest.spyOn(window, 'confirm'); + }); + afterAll(() => confirmSpy.mockRestore()); const serverFieldResultSettings = { foo: { @@ -864,20 +811,55 @@ describe('ResultSettingsLogic', () => { }); }); + describe('confirmResetAllFields', () => { + it('will reset all fields as long as the user confirms the action', async () => { + mount(); + confirmSpy.mockImplementation(() => true); + jest.spyOn(ResultSettingsLogic.actions, 'resetAllFields'); + + ResultSettingsLogic.actions.confirmResetAllFields(); + + expect(ResultSettingsLogic.actions.resetAllFields).toHaveBeenCalled(); + }); + + it('will do nothing if the user cancels the action', async () => { + mount(); + confirmSpy.mockImplementation(() => false); + jest.spyOn(ResultSettingsLogic.actions, 'resetAllFields'); + + ResultSettingsLogic.actions.confirmResetAllFields(); + + expect(ResultSettingsLogic.actions.resetAllFields).not.toHaveBeenCalled(); + }); + }); + describe('saveResultSettings', () => { + beforeEach(() => { + confirmSpy.mockImplementation(() => true); + }); + it('should make an API call to update result settings and update state accordingly', async () => { + const resultFields = { + foo: { raw: true, rawSize: 100 }, + }; + + const serverResultFields = { + foo: { raw: { size: 100 } }, + }; + mount({ schema, + resultFields, }); http.put.mockReturnValueOnce( Promise.resolve({ - result_fields: serverFieldResultSettings, + result_fields: serverResultFields, }) ); jest.spyOn(ResultSettingsLogic.actions, 'saving'); jest.spyOn(ResultSettingsLogic.actions, 'initializeResultFields'); - ResultSettingsLogic.actions.saveResultSettings(serverFieldResultSettings); + ResultSettingsLogic.actions.saveResultSettings(); expect(ResultSettingsLogic.actions.saving).toHaveBeenCalled(); @@ -887,12 +869,12 @@ describe('ResultSettingsLogic', () => { '/api/app_search/engines/test-engine/result_settings', { body: JSON.stringify({ - result_fields: serverFieldResultSettings, + result_fields: serverResultFields, }), } ); expect(ResultSettingsLogic.actions.initializeResultFields).toHaveBeenCalledWith( - serverFieldResultSettings, + serverResultFields, schema ); }); @@ -901,11 +883,21 @@ describe('ResultSettingsLogic', () => { mount(); http.put.mockReturnValueOnce(Promise.reject('error')); - ResultSettingsLogic.actions.saveResultSettings(serverFieldResultSettings); + ResultSettingsLogic.actions.saveResultSettings(); await nextTick(); expect(flashAPIErrors).toHaveBeenCalledWith('error'); }); + + it('does nothing if the user does not confirm', async () => { + mount(); + confirmSpy.mockImplementation(() => false); + + ResultSettingsLogic.actions.saveResultSettings(); + await nextTick(); + + expect(http.put).not.toHaveBeenCalled(); + }); }); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.ts index c345ae7e02e8d..f518fc945bfbf 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_logic.ts @@ -19,7 +19,6 @@ import { DEFAULT_SNIPPET_SIZE } from './constants'; import { FieldResultSetting, FieldResultSettingObject, - OpenModal, ServerFieldResultSettingObject, } from './types'; @@ -34,9 +33,6 @@ import { } from './utils'; interface ResultSettingsActions { - openConfirmResetModal(): void; - openConfirmSaveModal(): void; - closeModals(): void; initializeResultFields( serverResultFields: ServerFieldResultSettingObject, schema: Schema, @@ -62,15 +58,13 @@ interface ResultSettingsActions { updateRawSizeForField(fieldName: string, size: number): { fieldName: string; size: number }; updateSnippetSizeForField(fieldName: string, size: number): { fieldName: string; size: number }; initializeResultSettingsData(): void; - saveResultSettings( - resultFields: ServerFieldResultSettingObject - ): { resultFields: ServerFieldResultSettingObject }; + confirmResetAllFields(): void; + saveResultSettings(): void; } interface ResultSettingsValues { dataLoading: boolean; saving: boolean; - openModal: OpenModal; resultFields: FieldResultSettingObject; lastSavedResultFields: FieldResultSettingObject; schema: Schema; @@ -86,12 +80,25 @@ interface ResultSettingsValues { queryPerformanceScore: number; } +const SAVE_CONFIRMATION_MESSAGE = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.resultSettings.confirmSaveMessage', + { + defaultMessage: + 'The changes will start immediately. Make sure your applications are ready to accept the new search results!', + } +); + +const RESET_CONFIRMATION_MESSAGE = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.resultSettings.confirmResetMessage', + { + defaultMessage: + 'This will revert your settings back to the default: all fields set to raw. The default will take over immediately and impact your search results.', + } +); + export const ResultSettingsLogic = kea>({ path: ['enterprise_search', 'app_search', 'result_settings_logic'], actions: () => ({ - openConfirmResetModal: () => true, - openConfirmSaveModal: () => true, - closeModals: () => true, initializeResultFields: (serverResultFields, schema, schemaConflicts) => { const resultFields = convertServerResultFieldsToResultFields(serverResultFields, schema); @@ -113,7 +120,8 @@ export const ResultSettingsLogic = kea ({ fieldName, size }), updateSnippetSizeForField: (fieldName, size) => ({ fieldName, size }), initializeResultSettingsData: () => true, - saveResultSettings: (resultFields) => ({ resultFields }), + confirmResetAllFields: () => true, + saveResultSettings: () => true, }), reducers: () => ({ dataLoading: [ @@ -129,17 +137,6 @@ export const ResultSettingsLogic = kea true, }, ], - openModal: [ - OpenModal.None, - { - initializeResultFields: () => OpenModal.None, - closeModals: () => OpenModal.None, - resetAllFields: () => OpenModal.None, - openConfirmResetModal: () => OpenModal.ConfirmResetModal, - openConfirmSaveModal: () => OpenModal.ConfirmSaveModal, - saving: () => OpenModal.None, - }, - ], resultFields: [ {}, { @@ -308,35 +305,42 @@ export const ResultSettingsLogic = kea { - actions.saving(); + confirmResetAllFields: () => { + if (window.confirm(RESET_CONFIRMATION_MESSAGE)) { + actions.resetAllFields(); + } + }, + saveResultSettings: async () => { + if (window.confirm(SAVE_CONFIRMATION_MESSAGE)) { + actions.saving(); - const { http } = HttpLogic.values; - const { engineName } = EngineLogic.values; - const url = `/api/app_search/engines/${engineName}/result_settings`; + const { http } = HttpLogic.values; + const { engineName } = EngineLogic.values; + const url = `/api/app_search/engines/${engineName}/result_settings`; - actions.saving(); + actions.saving(); - let response; - try { - response = await http.put(url, { - body: JSON.stringify({ - result_fields: resultFields, - }), - }); - } catch (e) { - flashAPIErrors(e); - } + let response; + try { + response = await http.put(url, { + body: JSON.stringify({ + result_fields: values.reducedServerResultFields, + }), + }); + } catch (e) { + flashAPIErrors(e); + } - actions.initializeResultFields(response.result_fields, values.schema); - setSuccessMessage( - i18n.translate( - 'xpack.enterpriseSearch.appSearch.engine.resultSettings.saveSuccessMessage', - { - defaultMessage: 'Result settings have been saved successfully.', - } - ) - ); + actions.initializeResultFields(response.result_fields, values.schema); + setSuccessMessage( + i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.resultSettings.saveSuccessMessage', + { + defaultMessage: 'Result settings have been saved successfully.', + } + ) + ); + } }, }), }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/types.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/types.ts index 18843112f46bf..1174f65523d99 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/types.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/types.ts @@ -7,11 +7,6 @@ import { FieldValue } from '../result/types'; -export enum OpenModal { - None, - ConfirmResetModal, - ConfirmSaveModal, -} export interface ServerFieldResultSetting { raw?: | {