From 682e2e65a988e0b66d5e3dc6d7545109df3a4f05 Mon Sep 17 00:00:00 2001 From: Constance Date: Fri, 9 Jul 2021 09:23:18 -0700 Subject: [PATCH] [App Search] Relevance Tuning: Fix unsaved changes bug (#104951) * Fix unsavedChanges false positive when MultiInputRows is present - The fix for this is to change MultiInputRows from useEffect to useUpdateEffect, which prevents onChange from firing on initial mount/render (triggering updateBoostValue->unsavedChanges) @see https://github.com/streamich/react-use/blob/master/docs/useUpdateEffect.md * Fix precision tuner not triggering unsavedChanges --- .../multi_input_rows/multi_input_rows.test.tsx | 12 ++++++++++-- .../components/multi_input_rows/multi_input_rows.tsx | 5 +++-- .../relevance_tuning/relevance_tuning_logic.test.ts | 3 ++- .../relevance_tuning/relevance_tuning_logic.ts | 1 + 4 files changed, 16 insertions(+), 5 deletions(-) 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 index 3b8e1c96ff504..63952bc2a6de7 100644 --- 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 @@ -10,7 +10,7 @@ import '../../../__mocks__/shallow_useeffect.mock'; import React from 'react'; -import { shallow } from 'enzyme'; +import { shallow, ShallowWrapper } from 'enzyme'; import { rerender } from '../../../test_helpers'; @@ -162,10 +162,18 @@ describe('MultiInputRows', () => { }); describe('onChange', () => { + let wrapper: ShallowWrapper; const onChange = jest.fn(); + beforeEach(() => { + wrapper = shallow(); + }); + + it('does not call on change on mount', () => { + expect(onChange).not.toHaveBeenCalled(); + }); + it('returns the current values dynamically on change', () => { - const wrapper = shallow(); setMockValues({ ...values, values: ['updated'] }); rerender(wrapper); 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 index ac61e69eb44c4..257f4b637f3e0 100644 --- 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 @@ -5,9 +5,10 @@ * 2.0. */ -import React, { useEffect } from 'react'; +import React from 'react'; import { useValues, useActions } from 'kea'; +import useUpdateEffect from 'react-use/lib/useUpdateEffect'; import { EuiForm, EuiButton, EuiButtonEmpty, EuiSpacer } from '@elastic/eui'; @@ -49,7 +50,7 @@ export const MultiInputRows: React.FC = ({ const { values, addedNewRow, hasEmptyValues, hasOnlyOneValue } = useValues(logic); const { addValue, editValue, deleteValue } = useActions(logic); - useEffect(() => { + useUpdateEffect(() => { if (onChange) { onChange(filterEmptyValues(values)); } 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 4233a7b300d15..e2493b6404f7d 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 @@ -229,7 +229,7 @@ describe('RelevanceTuningLogic', () => { }); describe('updatePrecision', () => { - it('should set precision inside search settings', () => { + it('should set precision inside search settings and set unsavedChanges to true', () => { mount(); RelevanceTuningLogic.actions.updatePrecision(9); @@ -239,6 +239,7 @@ describe('RelevanceTuningLogic', () => { ...DEFAULT_VALUES.searchSettings, precision: 9, }, + unsavedChanges: true, }); }); }); 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 743bb1aa1502b..02903b4588ed4 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 @@ -191,6 +191,7 @@ export const RelevanceTuningLogic = kea< unsavedChanges: [ false, { + updatePrecision: () => true, setSearchSettings: () => true, setSearchSettingsResponse: () => false, },