Skip to content

Commit

Permalink
[App Search] Relevance Tuning: Fix unsaved changes bug (elastic#104951)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Constance authored and kibanamachine committed Jul 9, 2021
1 parent a037e19 commit 160328f
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -162,10 +162,18 @@ describe('MultiInputRows', () => {
});

describe('onChange', () => {
let wrapper: ShallowWrapper;
const onChange = jest.fn();

beforeEach(() => {
wrapper = shallow(<MultiInputRows {...props} onChange={onChange} />);
});

it('does not call on change on mount', () => {
expect(onChange).not.toHaveBeenCalled();
});

it('returns the current values dynamically on change', () => {
const wrapper = shallow(<MultiInputRows {...props} onChange={onChange} />);
setMockValues({ ...values, values: ['updated'] });
rerender(wrapper);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -49,7 +50,7 @@ export const MultiInputRows: React.FC<Props> = ({
const { values, addedNewRow, hasEmptyValues, hasOnlyOneValue } = useValues(logic);
const { addValue, editValue, deleteValue } = useActions(logic);

useEffect(() => {
useUpdateEffect(() => {
if (onChange) {
onChange(filterEmptyValues(values));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -239,6 +239,7 @@ describe('RelevanceTuningLogic', () => {
...DEFAULT_VALUES.searchSettings,
precision: 9,
},
unsavedChanges: true,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export const RelevanceTuningLogic = kea<
unsavedChanges: [
false,
{
updatePrecision: () => true,
setSearchSettings: () => true,
setSearchSettingsResponse: () => false,
},
Expand Down

0 comments on commit 160328f

Please sign in to comment.