From 2a281c99c6dd98f1f89a44f5492ff3b95f15bb1a Mon Sep 17 00:00:00 2001 From: Constance Date: Wed, 14 Apr 2021 15:11:25 -0700 Subject: [PATCH] [App Search] Refactor out a shared MultiInputRows component (#96881) * Add new reusable MultiInputRows component - basically the CurationQuery component, but with a generic values var & allows passing in custom text for every string * Update CurationQueries with MultiInputRows * Update MultiInputRows to support on change behavior - for upcoming Relevance Tuning usage * Update Relevance Tuning value boost form to use new component - relevance_tuning_form.test.tsx fix: was getting test errors with mount(), so I switched to shallow() * Change submitOnChange to onChange fn - more flexible - allows for either an onSubmit or onChange, or even potentially both * Convert MultiInputRowsLogic to keyed Kea logic - so that we can have multiple instances on the same page - primarily the value boosts use case * Update LogicMounter helper & tests to handle keyed logic w/ props * [Misc] LogicMounter helper - fix typing, perf - Use Kea's types instead of trying to rewrite my own LogicFile - Add an early return for tests that pass `{}` to values as well for performance * PR feedback: Change values prop to initialValues + bonus - add a fallback for initially empty components + add a test to check that the logic was mounted correctly * PR feedback: Remove useRef/on mount onChange catch for now - We don't currently need the extra catch for any live components, and it's confusing --- .../public/applications/__mocks__/kea.mock.ts | 44 +++--- .../curation_queries.test.tsx | 102 -------------- .../curation_queries/curation_queries.tsx | 72 ---------- .../curation_queries_logic.test.ts | 94 ------------- .../curation_queries_logic.ts | 53 ------- .../components/curation_queries/utils.test.ts | 15 -- .../components/curations/components/index.ts | 8 -- .../components/curations/constants.ts | 9 ++ .../queries/manage_queries_modal.test.tsx | 12 +- .../curation/queries/manage_queries_modal.tsx | 10 +- .../views/curation_creation.test.tsx | 8 +- .../curations/views/curation_creation.tsx | 15 +- .../components/multi_input_rows/constants.ts | 23 +++ .../index.ts | 2 +- .../input_row.scss} | 2 +- .../input_row.test.tsx} | 30 ++-- .../input_row.tsx} | 30 ++-- .../multi_input_rows.test.tsx | 133 ++++++++++++++++++ .../multi_input_rows/multi_input_rows.tsx | 93 ++++++++++++ .../multi_input_rows_logic.test.ts | 102 ++++++++++++++ .../multi_input_rows_logic.ts | 59 ++++++++ .../components/multi_input_rows/utils.test.ts | 15 ++ .../utils.ts | 4 +- .../value_boost_form.test.tsx | 43 ++---- .../boost_item_content/value_boost_form.tsx | 61 ++------ .../relevance_tuning_form.test.tsx | 12 +- .../relevance_tuning_logic.test.ts | 127 +---------------- .../relevance_tuning_logic.ts | 64 +-------- 28 files changed, 559 insertions(+), 683 deletions(-) delete mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.test.tsx delete mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.tsx delete mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.test.ts delete mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.ts delete mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.test.ts delete mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/index.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/constants.ts rename x-pack/plugins/enterprise_search/public/applications/app_search/components/{curations/components/curation_queries => multi_input_rows}/index.ts (82%) rename x-pack/plugins/enterprise_search/public/applications/app_search/components/{curations/components/curation_queries/curation_queries.scss => multi_input_rows/input_row.scss} (60%) rename x-pack/plugins/enterprise_search/public/applications/app_search/components/{curations/components/curation_queries/curation_query.test.tsx => multi_input_rows/input_row.test.tsx} (55%) rename x-pack/plugins/enterprise_search/public/applications/app_search/components/{curations/components/curation_queries/curation_query.tsx => multi_input_rows/input_row.tsx} (59%) create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.test.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.test.ts rename x-pack/plugins/enterprise_search/public/applications/app_search/components/{curations/components/curation_queries => multi_input_rows}/utils.ts (70%) diff --git a/x-pack/plugins/enterprise_search/public/applications/__mocks__/kea.mock.ts b/x-pack/plugins/enterprise_search/public/applications/__mocks__/kea.mock.ts index 2579d0b728c15..4ebb9edd20c0e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/__mocks__/kea.mock.ts +++ b/x-pack/plugins/enterprise_search/public/applications/__mocks__/kea.mock.ts @@ -84,13 +84,10 @@ export const setMockActions = (actions: object) => { * unmount(); * }); */ -import { resetContext, Logic, LogicInput } from 'kea'; +import { resetContext, LogicWrapper } from 'kea'; + +type LogicFile = LogicWrapper; -interface LogicFile { - inputs: Array>; - build(props?: object): void; - mount(): Function; -} export class LogicMounter { private logicFile: LogicFile; private unmountFn!: Function; @@ -100,24 +97,39 @@ export class LogicMounter { } // Reset context with optional default value overrides - public resetContext = (values?: object) => { - if (!values) { + public resetContext = (values?: object, props?: object) => { + if (!values || !Object.keys(values).length) { resetContext({}); } else { - const path = this.logicFile.inputs[0].path as string[]; // example: ['x', 'y', 'z'] - const defaults = path.reduceRight((value: object, key: string) => ({ [key]: value }), values); // example: { x: { y: { z: values } } } + let { path, key } = this.logicFile.inputs[0]; + + // For keyed logic files, both key and path should be functions + if (this.logicFile._isKeaWithKey) { + key = key(props); + path = path(key); + } + + // Generate the correct nested defaults obj based on the file path + // example path: ['x', 'y', 'z'] + // example defaults: { x: { y: { z: values } } } + const defaults = path.reduceRight( + (value: object, name: string) => ({ [name]: value }), + values + ); resetContext({ defaults }); } }; // Automatically reset context & mount the logic file public mount = (values?: object, props?: object) => { - this.resetContext(values); - if (props) this.logicFile.build(props); + this.resetContext(values, props); + + const logicWithProps = this.logicFile.build(props); + this.unmountFn = logicWithProps.mount(); - const unmount = this.logicFile.mount(); - this.unmountFn = unmount; - return unmount; // Keep Kea behavior of returning an unmount fn from mount + return logicWithProps; + // NOTE: Unlike kea's mount(), this returns the current + // built logic instance with props, NOT the unmount fn }; // Also add unmount as a class method that can be destructured on init without becoming stale later @@ -146,7 +158,7 @@ export class LogicMounter { const { listeners } = this.logicFile.inputs[0]; return typeof listeners === 'function' - ? (listeners as Function)(listenersArgs) // e.g., listeners({ values, actions, props }) => ({ ... }) + ? listeners(listenersArgs) // e.g., listeners({ values, actions, props }) => ({ ... }) : listeners; // handles simpler logic files that just define listeners: { ... } }; } diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.test.tsx deleted file mode 100644 index e55b944f7bebc..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.test.tsx +++ /dev/null @@ -1,102 +0,0 @@ -/* - * 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 { setMockActions, setMockValues } from '../../../../../__mocks__'; - -import React from 'react'; - -import { shallow } from 'enzyme'; - -import { CurationQuery } from './curation_query'; - -import { CurationQueries } from './'; - -describe('CurationQueries', () => { - const props = { - queries: ['a', 'b', 'c'], - onSubmit: jest.fn(), - }; - const values = { - queries: ['a', 'b', 'c'], - hasEmptyQueries: false, - hasOnlyOneQuery: false, - }; - const actions = { - addQuery: jest.fn(), - editQuery: jest.fn(), - deleteQuery: jest.fn(), - }; - - beforeEach(() => { - jest.clearAllMocks(); - setMockValues(values); - setMockActions(actions); - }); - - it('renders a CurationQuery row for each query', () => { - const wrapper = shallow(); - - expect(wrapper.find(CurationQuery)).toHaveLength(3); - expect(wrapper.find(CurationQuery).at(0).prop('queryValue')).toEqual('a'); - expect(wrapper.find(CurationQuery).at(1).prop('queryValue')).toEqual('b'); - expect(wrapper.find(CurationQuery).at(2).prop('queryValue')).toEqual('c'); - }); - - it('calls editQuery when the CurationQuery value changes', () => { - const wrapper = shallow(); - wrapper.find(CurationQuery).at(0).simulate('change', 'new query value'); - - expect(actions.editQuery).toHaveBeenCalledWith(0, 'new query value'); - }); - - it('calls deleteQuery when the CurationQuery calls onDelete', () => { - const wrapper = shallow(); - wrapper.find(CurationQuery).at(2).simulate('delete'); - - expect(actions.deleteQuery).toHaveBeenCalledWith(2); - }); - - it('calls addQuery when the Add Query button is clicked', () => { - const wrapper = shallow(); - wrapper.find('[data-test-subj="addCurationQueryButton"]').simulate('click'); - - expect(actions.addQuery).toHaveBeenCalled(); - }); - - it('disables the add button if any query fields are empty', () => { - setMockValues({ - ...values, - queries: ['a', '', 'c'], - hasEmptyQueries: true, - }); - const wrapper = shallow(); - const button = wrapper.find('[data-test-subj="addCurationQueryButton"]'); - - expect(button.prop('isDisabled')).toEqual(true); - }); - - it('calls the passed onSubmit callback when the submit button is clicked', () => { - setMockValues({ ...values, queries: ['some query'] }); - const wrapper = shallow(); - wrapper.find('[data-test-subj="submitCurationQueriesButton"]').simulate('click'); - - expect(props.onSubmit).toHaveBeenCalledWith(['some query']); - }); - - it('disables the submit button if no query fields have been filled', () => { - setMockValues({ - ...values, - queries: [''], - hasOnlyOneQuery: true, - hasEmptyQueries: true, - }); - const wrapper = shallow(); - const button = wrapper.find('[data-test-subj="submitCurationQueriesButton"]'); - - expect(button.prop('isDisabled')).toEqual(true); - }); -}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.tsx deleted file mode 100644 index bd9ba592e7224..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.tsx +++ /dev/null @@ -1,72 +0,0 @@ -/* - * 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 React from 'react'; - -import { useValues, useActions } from 'kea'; - -import { EuiButton, EuiButtonEmpty, EuiSpacer } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; - -import { CONTINUE_BUTTON_LABEL } from '../../../../../shared/constants'; - -import { Curation } from '../../types'; - -import { CurationQueriesLogic } from './curation_queries_logic'; -import { CurationQuery } from './curation_query'; -import { filterEmptyQueries } from './utils'; -import './curation_queries.scss'; - -interface Props { - queries: Curation['queries']; - onSubmit(queries: Curation['queries']): void; - submitButtonText?: string; -} - -export const CurationQueries: React.FC = ({ - queries: initialQueries, - onSubmit, - submitButtonText = CONTINUE_BUTTON_LABEL, -}) => { - const logic = CurationQueriesLogic({ queries: initialQueries }); - const { queries, hasEmptyQueries, hasOnlyOneQuery } = useValues(logic); - const { addQuery, editQuery, deleteQuery } = useActions(logic); - - return ( - <> - {queries.map((query: string, index) => ( - editQuery(index, newValue)} - onDelete={() => deleteQuery(index)} - disableDelete={hasOnlyOneQuery} - /> - ))} - - {i18n.translate('xpack.enterpriseSearch.appSearch.engine.curations.addQueryButtonLabel', { - defaultMessage: 'Add query', - })} - - - onSubmit(filterEmptyQueries(queries))} - data-test-subj="submitCurationQueriesButton" - > - {submitButtonText} - - - ); -}; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.test.ts deleted file mode 100644 index 766ab78b283be..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.test.ts +++ /dev/null @@ -1,94 +0,0 @@ -/* - * 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 { LogicMounter } from '../../../../../__mocks__'; - -import { CurationQueriesLogic } from './curation_queries_logic'; - -describe('CurationQueriesLogic', () => { - const { mount } = new LogicMounter(CurationQueriesLogic); - - const MOCK_QUERIES = ['a', 'b', 'c']; - - const DEFAULT_PROPS = { queries: MOCK_QUERIES }; - const DEFAULT_VALUES = { - queries: MOCK_QUERIES, - hasEmptyQueries: false, - hasOnlyOneQuery: false, - }; - - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('has expected default values passed from props', () => { - mount({}, DEFAULT_PROPS); - expect(CurationQueriesLogic.values).toEqual(DEFAULT_VALUES); - }); - - describe('actions', () => { - afterEach(() => { - // Should not mutate the original array - expect(CurationQueriesLogic.values.queries).not.toBe(MOCK_QUERIES); // Would fail if we did not clone a new array - }); - - describe('addQuery', () => { - it('appends an empty string to the queries array', () => { - mount(DEFAULT_VALUES); - CurationQueriesLogic.actions.addQuery(); - - expect(CurationQueriesLogic.values).toEqual({ - ...DEFAULT_VALUES, - hasEmptyQueries: true, - queries: ['a', 'b', 'c', ''], - }); - }); - }); - - describe('deleteQuery', () => { - it('deletes the query string at the specified array index', () => { - mount(DEFAULT_VALUES); - CurationQueriesLogic.actions.deleteQuery(1); - - expect(CurationQueriesLogic.values).toEqual({ - ...DEFAULT_VALUES, - queries: ['a', 'c'], - }); - }); - }); - - describe('editQuery', () => { - it('edits the query string at the specified array index', () => { - mount(DEFAULT_VALUES); - CurationQueriesLogic.actions.editQuery(2, 'z'); - - expect(CurationQueriesLogic.values).toEqual({ - ...DEFAULT_VALUES, - queries: ['a', 'b', 'z'], - }); - }); - }); - }); - - describe('selectors', () => { - describe('hasEmptyQueries', () => { - it('returns true if queries has any empty strings', () => { - mount({}, { queries: ['', '', ''] }); - - expect(CurationQueriesLogic.values.hasEmptyQueries).toEqual(true); - }); - }); - - describe('hasOnlyOneQuery', () => { - it('returns true if queries only has one item', () => { - mount({}, { queries: ['test'] }); - - expect(CurationQueriesLogic.values.hasOnlyOneQuery).toEqual(true); - }); - }); - }); -}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.ts deleted file mode 100644 index 98109657d61a3..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries_logic.ts +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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 { kea, MakeLogicType } from 'kea'; - -interface CurationQueriesValues { - queries: string[]; - hasEmptyQueries: boolean; - hasOnlyOneQuery: boolean; -} - -interface CurationQueriesActions { - addQuery(): void; - deleteQuery(indexToDelete: number): { indexToDelete: number }; - editQuery(index: number, newQueryValue: string): { index: number; newQueryValue: string }; -} - -export const CurationQueriesLogic = kea< - MakeLogicType ->({ - path: ['enterprise_search', 'app_search', 'curation_queries_logic'], - actions: () => ({ - addQuery: true, - deleteQuery: (indexToDelete) => ({ indexToDelete }), - editQuery: (index, newQueryValue) => ({ index, newQueryValue }), - }), - reducers: ({ props }) => ({ - queries: [ - props.queries, - { - addQuery: (state) => [...state, ''], - deleteQuery: (state, { indexToDelete }) => { - const newState = [...state]; - newState.splice(indexToDelete, 1); - return newState; - }, - editQuery: (state, { index, newQueryValue }) => { - const newState = [...state]; - newState[index] = newQueryValue; - return newState; - }, - }, - ], - }), - selectors: { - hasEmptyQueries: [(selectors) => [selectors.queries], (queries) => queries.indexOf('') >= 0], - hasOnlyOneQuery: [(selectors) => [selectors.queries], (queries) => queries.length <= 1], - }, -}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.test.ts deleted file mode 100644 index d84649f090691..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.test.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * 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 { filterEmptyQueries } from './utils'; - -describe('filterEmptyQueries', () => { - it('filters out all empty strings from a queries array', () => { - const queries = ['', 'a', '', 'b', '', 'c', '']; - expect(filterEmptyQueries(queries)).toEqual(['a', 'b', 'c']); - }); -}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/index.ts deleted file mode 100644 index 4f9136d15d6c3..0000000000000 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/index.ts +++ /dev/null @@ -1,8 +0,0 @@ -/* - * 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. - */ - -export { CurationQueries } from './curation_queries'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts index 99f3d340f8430..37c1e9a7a1a2e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/constants.ts @@ -25,6 +25,15 @@ export const MANAGE_CURATION_TITLE = i18n.translate( { defaultMessage: 'Manage curation' } ); +export const QUERY_INPUTS_BUTTON = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.curations.addQueryButtonLabel', + { defaultMessage: 'Add query' } +); +export const QUERY_INPUTS_PLACEHOLDER = i18n.translate( + 'xpack.enterpriseSearch.appSearch.engine.curations.queryPlaceholder', + { defaultMessage: 'Enter a query' } +); + export const DELETE_MESSAGE = i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.curations.deleteConfirmation', { defaultMessage: 'Are you sure you want to remove this curation?' } diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.test.tsx index 3555a9333a789..7fe992cdd96e2 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.test.tsx @@ -13,7 +13,7 @@ import { shallow, ShallowWrapper } from 'enzyme'; import { EuiButton, EuiModal } from '@elastic/eui'; -import { CurationQueries } from '../../components'; +import { MultiInputRows } from '../../../multi_input_rows'; import { ManageQueriesModal } from './'; @@ -66,13 +66,13 @@ describe('ManageQueriesModal', () => { expect(wrapper.find(EuiModal)).toHaveLength(0); }); - it('renders the CurationQueries form component', () => { - expect(wrapper.find(CurationQueries)).toHaveLength(1); - expect(wrapper.find(CurationQueries).prop('queries')).toEqual(['hello', 'world']); + it('renders the MultiInputRows component with curation queries', () => { + expect(wrapper.find(MultiInputRows)).toHaveLength(1); + expect(wrapper.find(MultiInputRows).prop('initialValues')).toEqual(['hello', 'world']); }); - it('calls updateCuration and closes the modal on CurationQueries form submit', () => { - wrapper.find(CurationQueries).simulate('submit', ['new', 'queries']); + it('calls updateCuration and closes the modal on MultiInputRows form submit', () => { + wrapper.find(MultiInputRows).simulate('submit', ['new', 'queries']); expect(actions.updateQueries).toHaveBeenCalledWith(['new', 'queries']); expect(wrapper.find(EuiModal)).toHaveLength(0); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.tsx index 471fab8413b38..5ab349115a265 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/curation/queries/manage_queries_modal.tsx @@ -21,8 +21,9 @@ import { import { i18n } from '@kbn/i18n'; import { SAVE_BUTTON_LABEL } from '../../../../../shared/constants'; +import { MultiInputRows } from '../../../multi_input_rows'; -import { CurationQueries } from '../../components'; +import { QUERY_INPUTS_BUTTON, QUERY_INPUTS_PLACEHOLDER } from '../../constants'; import { CurationLogic } from '../curation_logic'; export const ManageQueriesModal: React.FC = () => { @@ -61,8 +62,11 @@ export const ManageQueriesModal: React.FC = () => {

- { updateQueries(newQueries); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx index e6ddbb9c1b7a9..258d0ec6231fc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.test.tsx @@ -11,7 +11,7 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { CurationQueries } from '../components'; +import { MultiInputRows } from '../../multi_input_rows'; import { CurationCreation } from './curation_creation'; @@ -28,12 +28,12 @@ describe('CurationCreation', () => { it('renders', () => { const wrapper = shallow(); - expect(wrapper.find(CurationQueries)).toHaveLength(1); + expect(wrapper.find(MultiInputRows)).toHaveLength(1); }); - it('calls createCuration on CurationQueries submit', () => { + it('calls createCuration on submit', () => { const wrapper = shallow(); - wrapper.find(CurationQueries).simulate('submit', ['some query']); + wrapper.find(MultiInputRows).simulate('submit', ['some query']); expect(actions.createCuration).toHaveBeenCalledWith(['some query']); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx index 10f1fc093e60f..32d46775a2125 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/views/curation_creation.tsx @@ -13,9 +13,13 @@ import { EuiPageHeader, EuiPageContent, EuiTitle, EuiText, EuiSpacer } from '@el import { i18n } from '@kbn/i18n'; import { FlashMessages } from '../../../../shared/flash_messages'; +import { MultiInputRows } from '../../multi_input_rows'; -import { CurationQueries } from '../components'; -import { CREATE_NEW_CURATION_TITLE } from '../constants'; +import { + CREATE_NEW_CURATION_TITLE, + QUERY_INPUTS_BUTTON, + QUERY_INPUTS_PLACEHOLDER, +} from '../constants'; import { CurationsLogic } from '../index'; export const CurationCreation: React.FC = () => { @@ -46,7 +50,12 @@ export const CurationCreation: React.FC = () => {

- createCuration(queries)} /> + createCuration(queries)} + /> ); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/constants.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/constants.ts new file mode 100644 index 0000000000000..f0c077c5bfaf2 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/constants.ts @@ -0,0 +1,23 @@ +/* + * 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 { i18n } from '@kbn/i18n'; + +export const ADD_VALUE_BUTTON_LABEL = i18n.translate( + 'xpack.enterpriseSearch.appSearch.multiInputRows.addValueButtonLabel', + { defaultMessage: 'Add value' } +); + +export const DELETE_VALUE_BUTTON_LABEL = i18n.translate( + 'xpack.enterpriseSearch.appSearch.multiInputRows.removeValueButtonLabel', + { defaultMessage: 'Remove value' } +); + +export const INPUT_ROW_PLACEHOLDER = i18n.translate( + 'xpack.enterpriseSearch.appSearch.multiInputRows.inputRowPlaceholder', + { defaultMessage: 'Enter a value' } +); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/index.ts similarity index 82% rename from x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/index.ts rename to x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/index.ts index 4f9136d15d6c3..553bf23f21d30 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/index.ts @@ -5,4 +5,4 @@ * 2.0. */ -export { CurationQueries } from './curation_queries'; +export { MultiInputRows } from './multi_input_rows'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.scss b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.scss similarity index 60% rename from x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.scss rename to x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.scss index c242cf29fd37d..8c256c66a8dbf 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_queries.scss +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.scss @@ -1,3 +1,3 @@ -.curationQueryRow { +.inputRow { margin-bottom: $euiSizeXS; } diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_query.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.test.tsx similarity index 55% rename from x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_query.test.tsx rename to x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.test.tsx index 64fbec59382a0..03b0c0e4a0d91 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_query.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.test.tsx @@ -11,14 +11,16 @@ import { shallow } from 'enzyme'; import { EuiFieldText } from '@elastic/eui'; -import { CurationQuery } from './curation_query'; +import { InputRow } from './input_row'; -describe('CurationQuery', () => { +describe('InputRow', () => { const props = { - queryValue: 'some query', + value: 'some value', + placeholder: 'Enter a value', onChange: jest.fn(), onDelete: jest.fn(), disableDelete: false, + deleteLabel: 'Delete value', }; beforeEach(() => { @@ -26,29 +28,33 @@ describe('CurationQuery', () => { }); it('renders', () => { - const wrapper = shallow(); + const wrapper = shallow(); expect(wrapper.find(EuiFieldText)).toHaveLength(1); - expect(wrapper.find(EuiFieldText).prop('value')).toEqual('some query'); + expect(wrapper.find(EuiFieldText).prop('value')).toEqual('some value'); + expect(wrapper.find(EuiFieldText).prop('placeholder')).toEqual('Enter a value'); + expect(wrapper.find('[data-test-subj="deleteInputRowButton"]').prop('title')).toEqual( + 'Delete value' + ); }); it('calls onChange when the input value changes', () => { - const wrapper = shallow(); - wrapper.find(EuiFieldText).simulate('change', { target: { value: 'new query value' } }); + const wrapper = shallow(); + wrapper.find(EuiFieldText).simulate('change', { target: { value: 'new value' } }); - expect(props.onChange).toHaveBeenCalledWith('new query value'); + expect(props.onChange).toHaveBeenCalledWith('new value'); }); it('calls onDelete when the delete button is clicked', () => { - const wrapper = shallow(); - wrapper.find('[data-test-subj="deleteCurationQueryButton"]').simulate('click'); + const wrapper = shallow(); + wrapper.find('[data-test-subj="deleteInputRowButton"]').simulate('click'); expect(props.onDelete).toHaveBeenCalled(); }); it('disables the delete button if disableDelete is passed', () => { - const wrapper = shallow(); - const button = wrapper.find('[data-test-subj="deleteCurationQueryButton"]'); + const wrapper = shallow(); + const button = wrapper.find('[data-test-subj="deleteInputRowButton"]'); expect(button.prop('isDisabled')).toEqual(true); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_query.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.tsx similarity index 59% rename from x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_query.tsx rename to x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.tsx index 3ec1f9b8bf3b6..5f2a82ae945ed 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/curation_query.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/input_row.tsx @@ -8,33 +8,34 @@ import React from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiFieldText, EuiButtonIcon } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; - -import { DELETE_BUTTON_LABEL } from '../../../../../shared/constants'; interface Props { - queryValue: string; + value: string; + placeholder: string; onChange(newValue: string): void; onDelete(): void; disableDelete: boolean; + deleteLabel: string; } -export const CurationQuery: React.FC = ({ - queryValue, +import './input_row.scss'; + +export const InputRow: React.FC = ({ + value, + placeholder, onChange, onDelete, disableDelete, + deleteLabel, }) => ( - + onChange(e.target.value)} + autoFocus /> @@ -43,8 +44,9 @@ export const CurationQuery: React.FC = ({ color="danger" onClick={onDelete} isDisabled={disableDelete} - aria-label={DELETE_BUTTON_LABEL} - data-test-subj="deleteCurationQueryButton" + aria-label={deleteLabel} + title={deleteLabel} + data-test-subj="deleteInputRowButton" /> diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.test.tsx new file mode 100644 index 0000000000000..f832ceb8c8842 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.test.tsx @@ -0,0 +1,133 @@ +/* + * 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 { setMockActions, setMockValues, rerender } from '../../../__mocks__'; +import '../../../__mocks__/shallow_useeffect.mock'; + +import React from 'react'; + +import { shallow } from 'enzyme'; + +import { InputRow } from './input_row'; + +jest.mock('./multi_input_rows_logic', () => ({ + MultiInputRowsLogic: jest.fn(), +})); +import { MultiInputRowsLogic } from './multi_input_rows_logic'; + +import { MultiInputRows } from './'; + +describe('MultiInputRows', () => { + const props = { + id: 'test', + }; + const values = { + values: ['a', 'b', 'c'], + hasEmptyValues: false, + hasOnlyOneValue: false, + }; + const actions = { + addValue: jest.fn(), + editValue: jest.fn(), + deleteValue: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + setMockValues(values); + setMockActions(actions); + }); + + it('initializes MultiInputRowsLogic with a keyed ID and initialValues', () => { + shallow(); + expect(MultiInputRowsLogic).toHaveBeenCalledWith({ id: 'lorem', values: ['ipsum'] }); + }); + + it('renders a InputRow row for each value', () => { + const wrapper = shallow(); + + expect(wrapper.find(InputRow)).toHaveLength(3); + expect(wrapper.find(InputRow).at(0).prop('value')).toEqual('a'); + expect(wrapper.find(InputRow).at(1).prop('value')).toEqual('b'); + expect(wrapper.find(InputRow).at(2).prop('value')).toEqual('c'); + }); + + it('calls editValue when the InputRow value changes', () => { + const wrapper = shallow(); + wrapper.find(InputRow).at(0).simulate('change', 'new value'); + + expect(actions.editValue).toHaveBeenCalledWith(0, 'new value'); + }); + + it('calls deleteValue when the InputRow calls onDelete', () => { + const wrapper = shallow(); + wrapper.find(InputRow).at(2).simulate('delete'); + + expect(actions.deleteValue).toHaveBeenCalledWith(2); + }); + + it('calls addValue when the Add Value button is clicked', () => { + const wrapper = shallow(); + wrapper.find('[data-test-subj="addInputRowButton"]').simulate('click'); + + expect(actions.addValue).toHaveBeenCalled(); + }); + + it('disables the add button if any value fields are empty', () => { + setMockValues({ + ...values, + values: ['a', '', 'c'], + hasEmptyValues: true, + }); + const wrapper = shallow(); + const button = wrapper.find('[data-test-subj="addInputRowButton"]'); + + expect(button.prop('isDisabled')).toEqual(true); + }); + + describe('onSubmit', () => { + const onSubmit = jest.fn(); + + it('does not render the submit button if onSubmit is not passed', () => { + const wrapper = shallow(); + expect(wrapper.find('[data-test-subj="submitInputValuesButton"]').exists()).toBe(false); + }); + + it('calls the passed onSubmit callback when the submit button is clicked', () => { + setMockValues({ ...values, values: ['some value'] }); + const wrapper = shallow(); + wrapper.find('[data-test-subj="submitInputValuesButton"]').simulate('click'); + + expect(onSubmit).toHaveBeenCalledWith(['some value']); + }); + + it('disables the submit button if no value fields have been filled', () => { + setMockValues({ + ...values, + values: [''], + hasOnlyOneValue: true, + hasEmptyValues: true, + }); + const wrapper = shallow(); + const button = wrapper.find('[data-test-subj="submitInputValuesButton"]'); + + expect(button.prop('isDisabled')).toEqual(true); + }); + }); + + describe('onChange', () => { + const onChange = jest.fn(); + + it('returns the current values dynamically on change', () => { + const wrapper = shallow(); + setMockValues({ ...values, values: ['updated'] }); + rerender(wrapper); + + expect(onChange).toHaveBeenCalledWith(['updated']); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.tsx new file mode 100644 index 0000000000000..aa2f0977594c4 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows.tsx @@ -0,0 +1,93 @@ +/* + * 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 React, { useEffect } from 'react'; + +import { useValues, useActions } from 'kea'; + +import { EuiButton, EuiButtonEmpty, EuiSpacer } from '@elastic/eui'; + +import { CONTINUE_BUTTON_LABEL } from '../../../shared/constants'; + +import { + ADD_VALUE_BUTTON_LABEL, + DELETE_VALUE_BUTTON_LABEL, + INPUT_ROW_PLACEHOLDER, +} from './constants'; +import { InputRow } from './input_row'; +import { MultiInputRowsLogic } from './multi_input_rows_logic'; +import { filterEmptyValues } from './utils'; + +interface Props { + id: string; + initialValues?: string[]; + onSubmit?(values: string[]): void; + onChange?(values: string[]): void; + submitButtonText?: string; + addRowText?: string; + deleteRowLabel?: string; + inputPlaceholder?: string; +} + +export const MultiInputRows: React.FC = ({ + id, + initialValues = [''], + onSubmit, + onChange, + submitButtonText = CONTINUE_BUTTON_LABEL, + addRowText = ADD_VALUE_BUTTON_LABEL, + deleteRowLabel = DELETE_VALUE_BUTTON_LABEL, + inputPlaceholder = INPUT_ROW_PLACEHOLDER, +}) => { + const logic = MultiInputRowsLogic({ id, values: initialValues }); + const { values, hasEmptyValues, hasOnlyOneValue } = useValues(logic); + const { addValue, editValue, deleteValue } = useActions(logic); + + useEffect(() => { + if (onChange) { + onChange(filterEmptyValues(values)); + } + }, [values]); + + return ( + <> + {values.map((value: string, index: number) => ( + editValue(index, newValue)} + onDelete={() => deleteValue(index)} + disableDelete={hasOnlyOneValue} + deleteLabel={deleteRowLabel} + /> + ))} + + {addRowText} + + {onSubmit && ( + <> + + onSubmit(filterEmptyValues(values))} + data-test-subj="submitInputValuesButton" + > + {submitButtonText} + + + )} + + ); +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.test.ts new file mode 100644 index 0000000000000..b84db6775820c --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.test.ts @@ -0,0 +1,102 @@ +/* + * 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 { LogicMounter } from '../../../__mocks__'; + +import { Logic } from 'kea'; + +import { MultiInputRowsLogic } from './multi_input_rows_logic'; + +describe('MultiInputRowsLogic', () => { + const { mount } = new LogicMounter(MultiInputRowsLogic); + + const MOCK_VALUES = ['a', 'b', 'c']; + + const DEFAULT_PROPS = { + id: 'test', + values: MOCK_VALUES, + }; + const DEFAULT_VALUES = { + values: MOCK_VALUES, + hasEmptyValues: false, + hasOnlyOneValue: false, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('has expected default values passed from props', () => { + const logic = mount({}, DEFAULT_PROPS); + expect(logic.values).toEqual(DEFAULT_VALUES); + }); + + describe('actions', () => { + let logic: Logic; + + beforeEach(() => { + logic = mount({}, DEFAULT_PROPS); + }); + + afterEach(() => { + // Should not mutate the original array + expect(logic.values.values).not.toBe(MOCK_VALUES); // Would fail if we did not clone a new array + }); + + describe('addValue', () => { + it('appends an empty string to the values array', () => { + logic.actions.addValue(); + + expect(logic.values).toEqual({ + ...DEFAULT_VALUES, + hasEmptyValues: true, + values: ['a', 'b', 'c', ''], + }); + }); + }); + + describe('deleteValue', () => { + it('deletes the value at the specified array index', () => { + logic.actions.deleteValue(1); + + expect(logic.values).toEqual({ + ...DEFAULT_VALUES, + values: ['a', 'c'], + }); + }); + }); + + describe('editValue', () => { + it('edits the value at the specified array index', () => { + logic.actions.editValue(2, 'z'); + + expect(logic.values).toEqual({ + ...DEFAULT_VALUES, + values: ['a', 'b', 'z'], + }); + }); + }); + }); + + describe('selectors', () => { + describe('hasEmptyValues', () => { + it('returns true if values has any empty strings', () => { + const logic = mount({}, { ...DEFAULT_PROPS, values: ['', '', ''] }); + + expect(logic.values.hasEmptyValues).toEqual(true); + }); + }); + + describe('hasOnlyOneValue', () => { + it('returns true if values only has one item', () => { + const logic = mount({}, { ...DEFAULT_PROPS, values: ['test'] }); + + expect(logic.values.hasOnlyOneValue).toEqual(true); + }); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.ts new file mode 100644 index 0000000000000..6cc392598a61f --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/multi_input_rows_logic.ts @@ -0,0 +1,59 @@ +/* + * 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 { kea, MakeLogicType } from 'kea'; + +interface MultiInputRowsValues { + values: string[]; + hasEmptyValues: boolean; + hasOnlyOneValue: boolean; +} + +interface MultiInputRowsActions { + addValue(): void; + deleteValue(indexToDelete: number): { indexToDelete: number }; + editValue(index: number, newValueValue: string): { index: number; newValueValue: string }; +} + +interface MultiInputRowsProps { + values: string[]; + id: string; +} + +export const MultiInputRowsLogic = kea< + MakeLogicType +>({ + path: (key: string) => ['enterprise_search', 'app_search', 'multi_input_rows_logic', key], + key: (props) => props.id, + actions: () => ({ + addValue: true, + deleteValue: (indexToDelete) => ({ indexToDelete }), + editValue: (index, newValueValue) => ({ index, newValueValue }), + }), + reducers: ({ props }) => ({ + values: [ + props.values, + { + addValue: (state) => [...state, ''], + deleteValue: (state, { indexToDelete }) => { + const newState = [...state]; + newState.splice(indexToDelete, 1); + return newState; + }, + editValue: (state, { index, newValueValue }) => { + const newState = [...state]; + newState[index] = newValueValue; + return newState; + }, + }, + ], + }), + selectors: { + hasEmptyValues: [(selectors) => [selectors.values], (values) => values.indexOf('') >= 0], + hasOnlyOneValue: [(selectors) => [selectors.values], (values) => values.length <= 1], + }, +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.test.ts new file mode 100644 index 0000000000000..0946890c40dfa --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.test.ts @@ -0,0 +1,15 @@ +/* + * 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 { filterEmptyValues } from './utils'; + +describe('filterEmptyValues', () => { + it('filters out all empty strings from the array', () => { + const values = ['', 'a', '', 'b', '', 'c', '']; + expect(filterEmptyValues(values)).toEqual(['a', 'b', 'c']); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.ts similarity index 70% rename from x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.ts rename to x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.ts index 505e9641d778e..5ecefb240e17d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/curations/components/curation_queries/utils.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/multi_input_rows/utils.ts @@ -5,6 +5,6 @@ * 2.0. */ -export const filterEmptyQueries = (queries: string[]) => { - return queries.filter((query) => query.length); +export const filterEmptyValues = (values: string[]) => { + return values.filter((value) => value.length); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.test.tsx index 6fbf90e6a2000..6f9284891e711 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.test.tsx @@ -9,9 +9,9 @@ import { setMockActions } from '../../../../../__mocks__/kea.mock'; import React from 'react'; -import { shallow, ShallowWrapper } from 'enzyme'; +import { shallow } from 'enzyme'; -import { EuiButton, EuiButtonIcon, EuiFieldText } from '@elastic/eui'; +import { MultiInputRows } from '../../../multi_input_rows'; import { ValueBoost, BoostType } from '../../types'; @@ -23,13 +23,11 @@ describe('ValueBoostForm', () => { function: undefined, factor: 2, type: 'value' as BoostType, - value: ['bar', '', 'baz'], + value: [], }; const actions = { - removeBoostValue: jest.fn(), updateBoostValue: jest.fn(), - addBoostValue: jest.fn(), }; beforeEach(() => { @@ -37,40 +35,15 @@ describe('ValueBoostForm', () => { setMockActions(actions); }); - const valueInput = (wrapper: ShallowWrapper, index: number) => - wrapper.find(EuiFieldText).at(index); - const removeButton = (wrapper: ShallowWrapper, index: number) => - wrapper.find(EuiButtonIcon).at(index); - const addButton = (wrapper: ShallowWrapper) => wrapper.find(EuiButton); - - it('renders a text input for each value from the boost', () => { - const wrapper = shallow(); - expect(valueInput(wrapper, 0).prop('value')).toEqual('bar'); - expect(valueInput(wrapper, 1).prop('value')).toEqual(''); - expect(valueInput(wrapper, 2).prop('value')).toEqual('baz'); - }); - - it('updates the corresponding value in state whenever a user changes the value in a text input', () => { - const wrapper = shallow(); - - valueInput(wrapper, 2).simulate('change', { target: { value: 'new value' } }); - - expect(actions.updateBoostValue).toHaveBeenCalledWith('foo', 3, 2, 'new value'); - }); - - it('deletes a boost value when the Remove Value button is clicked', () => { + it('renders', () => { const wrapper = shallow(); - - removeButton(wrapper, 2).simulate('click'); - - expect(actions.removeBoostValue).toHaveBeenCalledWith('foo', 3, 2); + expect(wrapper.find(MultiInputRows).exists()).toBe(true); }); - it('adds a new boost value when the Add Value is button clicked', () => { + it('updates the boost value whenever the MultiInputRows form component updates', () => { const wrapper = shallow(); + wrapper.find(MultiInputRows).simulate('change', ['bar', 'baz']); - addButton(wrapper).simulate('click'); - - expect(actions.addBoostValue).toHaveBeenCalledWith('foo', 3); + expect(actions.updateBoostValue).toHaveBeenCalledWith('foo', 3, ['bar', 'baz']); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.tsx index 48d9749029a7e..4f6c1c4248fe6 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.tsx @@ -9,17 +9,9 @@ import React from 'react'; import { useActions } from 'kea'; -import { - EuiButton, - EuiButtonIcon, - EuiFieldText, - EuiFlexGroup, - EuiFlexItem, - EuiSpacer, -} from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; +import { MultiInputRows } from '../../../multi_input_rows'; -import { RelevanceTuningLogic } from '../..'; +import { RelevanceTuningLogic } from '../../index'; import { ValueBoost } from '../../types'; interface Props { @@ -29,51 +21,14 @@ interface Props { } export const ValueBoostForm: React.FC = ({ boost, index, name }) => { - const { updateBoostValue, removeBoostValue, addBoostValue } = useActions(RelevanceTuningLogic); + const { updateBoostValue } = useActions(RelevanceTuningLogic); const values = boost.value; return ( - <> - {values.map((value, valueIndex) => ( - - - updateBoostValue(name, index, valueIndex, e.target.value)} - aria-label={i18n.translate( - 'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.boosts.value.valueNameAriaLabel', - { - defaultMessage: 'Value name', - } - )} - autoFocus - /> - - - removeBoostValue(name, index, valueIndex)} - aria-label={i18n.translate( - 'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.boosts.value.removeValueAriaLabel', - { - defaultMessage: 'Remove value', - } - )} - /> - - - ))} - - addBoostValue(name, index)}> - {i18n.translate( - 'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.boosts.value.addValueButtonLabel', - { - defaultMessage: 'Add value', - } - )} - - + updateBoostValue(name, index, updatedValues)} + id={`${name}BoostValue-${index}`} + /> ); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.test.tsx index 68d1b7439be5c..a1a241b8856a5 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.test.tsx @@ -9,14 +9,14 @@ import { setMockValues, setMockActions } from '../../../../__mocks__/kea.mock'; import React from 'react'; -import { shallow, mount, ReactWrapper, ShallowWrapper } from 'enzyme'; +import { shallow, ShallowWrapper } from 'enzyme'; import { EuiFieldSearch } from '@elastic/eui'; import { BoostType } from '../types'; import { RelevanceTuningForm } from './relevance_tuning_form'; -import { RelevanceTuningItem } from './relevance_tuning_item'; +import { RelevanceTuningItemContent } from './relevance_tuning_item_content'; describe('RelevanceTuningForm', () => { const values = { @@ -55,14 +55,14 @@ describe('RelevanceTuningForm', () => { }); describe('fields', () => { - let wrapper: ReactWrapper; + let wrapper: ShallowWrapper; let relevantTuningItems: any; beforeAll(() => { setMockValues(values); - wrapper = mount(); - relevantTuningItems = wrapper.find(RelevanceTuningItem); + wrapper = shallow(); + relevantTuningItems = wrapper.find(RelevanceTuningItemContent); }); it('renders a list of fields that may or may not have been filterd by user input', () => { @@ -112,7 +112,7 @@ describe('RelevanceTuningForm', () => { filteredSchemaFieldsWithConflicts: ['fe', 'fi', 'fo'], }); - const wrapper = mount(); + const wrapper = shallow(); expect(wrapper.find('[data-test-subj="DisabledFieldsSection"]').exists()).toBe(true); expect(wrapper.find('[data-test-subj="DisabledField"]').map((f) => f.text())).toEqual([ 'fe', 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 4ec38d314a259..97030e08e2a9f 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 @@ -891,7 +891,7 @@ describe('RelevanceTuningLogic', () => { }); describe('updateBoostValue', () => { - it('will update the boost value and update search reuslts', () => { + it('will update the boost value and update search results', () => { mount({ searchSettings: searchSettingsWithBoost({ factor: 1, @@ -901,33 +901,13 @@ describe('RelevanceTuningLogic', () => { }); jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - RelevanceTuningLogic.actions.updateBoostValue('foo', 1, 1, 'a'); + RelevanceTuningLogic.actions.updateBoostValue('foo', 1, ['x', 'y', 'z']); expect(RelevanceTuningLogic.actions.setSearchSettings).toHaveBeenCalledWith( searchSettingsWithBoost({ factor: 1, type: BoostType.Functional, - value: ['a', 'a', 'c'], - }) - ); - }); - - it('will create a new array if no array exists yet for value', () => { - mount({ - searchSettings: searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - }), - }); - jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - - RelevanceTuningLogic.actions.updateBoostValue('foo', 1, 0, 'a'); - - expect(RelevanceTuningLogic.actions.setSearchSettings).toHaveBeenCalledWith( - searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a'], + value: ['x', 'y', 'z'], }) ); }); @@ -959,107 +939,6 @@ describe('RelevanceTuningLogic', () => { }); }); - describe('addBoostValue', () => { - it('will add an empty boost value', () => { - mount({ - searchSettings: searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a'], - }), - }); - jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - - RelevanceTuningLogic.actions.addBoostValue('foo', 1); - - expect(RelevanceTuningLogic.actions.setSearchSettings).toHaveBeenCalledWith( - searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a', ''], - }) - ); - }); - - it('will add two empty boost values if none exist yet', () => { - mount({ - searchSettings: searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - }), - }); - jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - - RelevanceTuningLogic.actions.addBoostValue('foo', 1); - - expect(RelevanceTuningLogic.actions.setSearchSettings).toHaveBeenCalledWith( - searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['', ''], - }) - ); - }); - - it('will still work if the boost index is out of range', () => { - mount({ - searchSettings: searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a', ''], - }), - }); - jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - - RelevanceTuningLogic.actions.addBoostValue('foo', 10); - - expect(RelevanceTuningLogic.actions.setSearchSettings).toHaveBeenCalledWith( - searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a', ''], - }) - ); - }); - }); - - describe('removeBoostValue', () => { - it('will remove a boost value', () => { - mount({ - searchSettings: searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a', 'b', 'c'], - }), - }); - jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - - RelevanceTuningLogic.actions.removeBoostValue('foo', 1, 1); - - expect(RelevanceTuningLogic.actions.setSearchSettings).toHaveBeenCalledWith( - searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - value: ['a', 'c'], - }) - ); - }); - - it('will do nothing if boost values do not exist', () => { - mount({ - searchSettings: searchSettingsWithBoost({ - factor: 1, - type: BoostType.Functional, - }), - }); - jest.spyOn(RelevanceTuningLogic.actions, 'setSearchSettings'); - - RelevanceTuningLogic.actions.removeBoostValue('foo', 1, 1); - - expect(RelevanceTuningLogic.actions.setSearchSettings).not.toHaveBeenCalled(); - }); - }); - describe('updateBoostSelectOption', () => { it('will update the boost', () => { mount({ diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts index b87fef91c7d21..4787ef89c0119 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts @@ -69,20 +69,13 @@ interface RelevanceTuningActions { updateBoostValue( name: string, boostIndex: number, - valueIndex: number, - value: string - ): { name: string; boostIndex: number; valueIndex: number; value: string }; + updatedValues: string[] + ): { name: string; boostIndex: number; updatedValues: string[] }; updateBoostCenter( name: string, boostIndex: number, value: string | number ): { name: string; boostIndex: number; value: string | number }; - addBoostValue(name: string, boostIndex: number): { name: string; boostIndex: number }; - removeBoostValue( - name: string, - boostIndex: number, - valueIndex: number - ): { name: string; boostIndex: number; valueIndex: number }; updateBoostSelectOption( name: string, boostIndex: number, @@ -141,15 +134,8 @@ export const RelevanceTuningLogic = kea< addBoost: (name, type) => ({ name, type }), deleteBoost: (name, index) => ({ name, index }), updateBoostFactor: (name, index, factor) => ({ name, index, factor }), - updateBoostValue: (name, boostIndex, valueIndex, value) => ({ - name, - boostIndex, - valueIndex, - value, - }), + updateBoostValue: (name, boostIndex, updatedValues) => ({ name, boostIndex, updatedValues }), updateBoostCenter: (name, boostIndex, value) => ({ name, boostIndex, value }), - addBoostValue: (name, boostIndex) => ({ name, boostIndex }), - removeBoostValue: (name, boostIndex, valueIndex) => ({ name, boostIndex, valueIndex }), updateBoostSelectOption: (name, boostIndex, optionType, value) => ({ name, boostIndex, @@ -430,16 +416,11 @@ export const RelevanceTuningLogic = kea< }, }); }, - updateBoostValue: ({ name, boostIndex, valueIndex, value }) => { + updateBoostValue: ({ name, boostIndex, updatedValues }) => { const { searchSettings } = values; const { boosts } = searchSettings; const updatedBoosts: Boost[] = cloneDeep(boosts[name]); - const existingValue = updatedBoosts[boostIndex].value; - if (existingValue === undefined) { - updatedBoosts[boostIndex].value = [value]; - } else { - existingValue[valueIndex] = value; - } + updatedBoosts[boostIndex].value = updatedValues; actions.setSearchSettings({ ...searchSettings, @@ -464,41 +445,6 @@ export const RelevanceTuningLogic = kea< }, }); }, - addBoostValue: ({ name, boostIndex }) => { - const { searchSettings } = values; - const { boosts } = searchSettings; - const updatedBoosts = cloneDeep(boosts[name]); - const updatedBoost = updatedBoosts[boostIndex]; - if (updatedBoost) { - updatedBoost.value = Array.isArray(updatedBoost.value) ? updatedBoost.value : ['']; - updatedBoost.value.push(''); - } - - actions.setSearchSettings({ - ...searchSettings, - boosts: { - ...boosts, - [name]: updatedBoosts, - }, - }); - }, - removeBoostValue: ({ name, boostIndex, valueIndex }) => { - const { searchSettings } = values; - const { boosts } = searchSettings; - const updatedBoosts = cloneDeep(boosts[name]); - const boostValue = updatedBoosts[boostIndex].value; - - if (boostValue === undefined) return; - - boostValue.splice(valueIndex, 1); - actions.setSearchSettings({ - ...searchSettings, - boosts: { - ...boosts, - [name]: updatedBoosts, - }, - }); - }, updateBoostSelectOption: ({ name, boostIndex, optionType, value }) => { const { searchSettings } = values; const { boosts } = searchSettings;