maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[App Search] Fix Relevance Tuning bugs (#95069)

Browse source
This commit is contained in:
Jason Stoltzfus 2021-03-23 13:37:17 -04:00 committed by GitHub
parent 8a42049acb
commit 97a03479e3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 162 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

59
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item.test.tsx Normal file
View file

@ -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 React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import { EuiAccordion } from '@elastic/eui';
import { BoostIcon } from '../boost_icon';
import { BoostType } from '../types';
import { ValueBadge } from '../value_badge';
import { BoostItem } from './boost_item';
import { BoostItemContent } from './boost_item_content';
describe('BoostItem', () => {
const boost = {
factor: 2,
type: BoostType.Value,
newBoost: true,
value: [''],
};
let wrapper: ShallowWrapper;
let accordian: ShallowWrapper;
beforeAll(() => {
wrapper = shallow(<BoostItem id="some_id" boost={boost} index={1} name="foo" />);
accordian = wrapper.find(EuiAccordion) as ShallowWrapper;
});
it('renders an accordion as open if it is a newly created boost', () => {
expect(accordian.prop('initialIsOpen')).toEqual(boost.newBoost);
});
it('renders an accordion button which shows a summary of the boost', () => {
const buttonContent = shallow(
accordian.prop('buttonContent') as React.ReactElement
) as ShallowWrapper;
expect(buttonContent.find(BoostIcon).prop('type')).toEqual('value');
expect(buttonContent.find(ValueBadge).children().text()).toEqual('2');
});
it('renders boost content inside of the accordion', () => {
const content = wrapper.find(BoostItemContent);
expect(content.props()).toEqual({
boost,
index: 1,
name: 'foo',
});
});
});

1
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item.tsx
View file

@ -32,6 +32,7 @@ export const BoostItem: React.FC<Props> = ({ id, boost, index, name }) => {
id={id}
className="boosts__item"
buttonContentClassName="boosts__itemButton"
initialIsOpen={!!boost.newBoost}
buttonContent={
<EuiFlexGroup responsive={false} alignItems="center">
<EuiFlexItem grow={false}>

1
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/boost_item_content.test.tsx
View file

@ -35,6 +35,7 @@ describe('BoostItemContent', () => {
const boost = {
factor: 2,
type: 'value' as BoostType,
value: [''],
};
const wrapper = shallow(<BoostItemContent boost={boost} index={3} name="foo" />);

14
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.test.tsx
View file

@ -50,20 +50,6 @@ describe('ValueBoostForm', () => {
expect(valueInput(wrapper, 2).prop('value')).toEqual('baz');
});
it('renders a single empty text box if the boost has no value', () => {
const wrapper = shallow(
<ValueBoostForm
boost={{
...boost,
value: undefined,
}}
index={3}
name="foo"
/>
);
expect(valueInput(wrapper, 0).prop('value')).toEqual('');
});
it('updates the corresponding value in state whenever a user changes the value in a text input', () => {
const wrapper = shallow(<ValueBoostForm boost={boost} index={3} name="foo" />);

2
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boost_item_content/value_boost_form.tsx
View file

@ -30,7 +30,7 @@ interface Props {
export const ValueBoostForm: React.FC<Props> = ({ boost, index, name }) => {
const { updateBoostValue, removeBoostValue, addBoostValue } = useActions(RelevanceTuningLogic);
const values = boost.value || [''];
const values = boost.value;
return (
<>

1
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/boosts/boosts.test.tsx
View file

@ -108,6 +108,7 @@ describe('Boosts', () => {
const boost1 = {
factor: 2,
type: 'value' as BoostType,
value: [''],
};
const boost2 = {
factor: 10,

1
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/constants.ts
View file

@ -83,6 +83,7 @@ export const BOOST_TYPE_TO_ICON_MAP = {
const EMPTY_VALUE_BOOST: ValueBoost = {
type: BoostType.Value,
factor: 1,
value: [''],
newBoost: true,
function: undefined,
operation: undefined,

2
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_form.test.tsx
View file

@ -34,6 +34,7 @@ describe('RelevanceTuningForm', () => {
{
factor: 2,
type: BoostType.Value,
value: [],
},
],
},
@ -85,6 +86,7 @@ describe('RelevanceTuningForm', () => {
{
factor: 2,
type: BoostType.Value,
value: [],
},
]);
expect(relevantTuningItems.at(1).prop('boosts')).toBeUndefined();

2
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_item.test.tsx
View file

@ -25,6 +25,7 @@ describe('RelevanceTuningItem', () => {
{
factor: 2,
type: BoostType.Value,
value: [''],
},
],
field: {
@ -54,6 +55,7 @@ describe('RelevanceTuningItem', () => {
{
factor: 2,
type: BoostType.Value,
value: [''],
},
{
factor: 3,

1
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_form/relevance_tuning_item_content/relevance_tuning_item_content.test.tsx
View file

@ -24,6 +24,7 @@ describe('RelevanceTuningItemContent', () => {
{
factor: 2,
type: BoostType.Value,
value: [''],
},
],
field: {

13
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.test.ts
View file

@ -23,6 +23,7 @@ describe('RelevanceTuningLogic', () => {
{
type: BoostType.Value,
factor: 5,
value: [],
},
],
},
@ -224,7 +225,7 @@ describe('RelevanceTuningLogic', () => {
describe('listeners', () => {
const { http } = mockHttpValues;
const { flashAPIErrors, setSuccessMessage } = mockFlashMessageHelpers;
const { flashAPIErrors, setSuccessMessage, clearFlashMessages } = mockFlashMessageHelpers;
let scrollToSpy: jest.SpyInstance;
let confirmSpy: jest.SpyInstance;
@ -316,7 +317,7 @@ describe('RelevanceTuningLogic', () => {
jest.useRealTimers();
});
it('should make an API call and set state based on the response', async () => {
it('should make an API call, set state based on the response, and clear flash messages', async () => {
const searchSettingsWithNewBoostProp = {
boosts: {
foo: [
@ -324,6 +325,7 @@ describe('RelevanceTuningLogic', () => {
type: BoostType.Value,
factor: 5,
newBoost: true, // This should be deleted before sent to the server
value: ['test'],
},
],
},
@ -341,6 +343,7 @@ describe('RelevanceTuningLogic', () => {
{
type: BoostType.Value,
factor: 5,
value: ['test'],
},
],
},
@ -373,6 +376,7 @@ describe('RelevanceTuningLogic', () => {
}
);
expect(RelevanceTuningLogic.actions.setSearchResults).toHaveBeenCalledWith(searchResults);
expect(clearFlashMessages).toHaveBeenCalled();
});
it("won't send boosts or search_fields on the API call if there are none", async () => {
@ -481,6 +485,7 @@ describe('RelevanceTuningLogic', () => {
type: BoostType.Value,
factor: 5,
newBoost: true, // This should be deleted before sent to the server
value: [''],
},
],
},
@ -492,6 +497,7 @@ describe('RelevanceTuningLogic', () => {
{
type: BoostType.Value,
factor: 5,
value: [''],
},
],
},
@ -698,6 +704,7 @@ describe('RelevanceTuningLogic', () => {
{
factor: 2,
type: BoostType.Value,
value: [''],
},
],
},
@ -714,6 +721,7 @@ describe('RelevanceTuningLogic', () => {
{
factor: 2,
type: BoostType.Value,
value: [''],
},
{
factor: 1,
@ -771,6 +779,7 @@ describe('RelevanceTuningLogic', () => {
{
factor: 2,
type: BoostType.Value,
value: [''],
},
],
},

12
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/relevance_tuning_logic.ts
View file

@ -8,7 +8,11 @@
import { kea, MakeLogicType } from 'kea';
import { omit, cloneDeep, isEmpty } from 'lodash';
import { setSuccessMessage, flashAPIErrors } from '../../../shared/flash_messages';
import {
setSuccessMessage,
flashAPIErrors,
clearFlashMessages,
} from '../../../shared/flash_messages';
import { HttpLogic } from '../../../shared/http';
import { Schema, SchemaConflicts } from '../../../shared/types';
@ -28,6 +32,7 @@ import {
parseBoostCenter,
removeBoostStateProps,
normalizeBoostValues,
removeEmptyValueBoosts,
} from './utils';
interface RelevanceTuningProps {
@ -273,18 +278,21 @@ export const RelevanceTuningLogic = kea<
actions.setResultsLoading(true);
const filteredBoosts = removeEmptyValueBoosts(boosts);
try {
const response = await http.post(url, {
query: {
query,
},
body: JSON.stringify({
boosts: isEmpty(boosts) ? undefined : boosts,
boosts: isEmpty(filteredBoosts) ? undefined : filteredBoosts,
search_fields: isEmpty(searchFields) ? undefined : searchFields,
}),
});
actions.setSearchResults(response.results);
clearFlashMessages();
} catch (e) {
flashAPIErrors(e);
}

2
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/types.ts
View file

@ -45,7 +45,7 @@ export interface RawBoost extends Omit<Boost, 'value'> {
}
export interface ValueBoost extends Boost {
value?: string[];
value: string[];
operation: undefined;
function: undefined;
}

33
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/utils.test.ts
View file

@ -4,12 +4,13 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { BoostType } from './types';
import { Boost, BoostType } from './types';
import {
filterIfTerm,
normalizeBoostValues,
removeBoostStateProps,
parseBoostCenter,
removeEmptyValueBoosts,
} from './utils';
describe('filterIfTerm', () => {
@ -42,6 +43,7 @@ describe('removeBoostStateProps', () => {
type: BoostType.Value,
factor: 5,
newBoost: true,
value: [''],
},
],
},
@ -58,6 +60,7 @@ describe('removeBoostStateProps', () => {
{
type: BoostType.Value,
factor: 5,
value: [''],
},
],
},
@ -152,3 +155,31 @@ describe('normalizeBoostValues', () => {
});
});
});
describe('removeEmptyValueBoosts', () => {
const boosts: Record<string, Boost[]> = {
bar: [
{ factor: 9.5, type: BoostType.Proximity },
{ type: BoostType.Functional, factor: 5 },
],
foo: [
{ factor: 9.5, type: BoostType.Value, value: ['1'] },
{ factor: 9.5, type: BoostType.Value, value: ['1', '', ' '] },
{ factor: 9.5, type: BoostType.Value, value: [] },
{ factor: 9.5, type: BoostType.Value, value: ['', '1'] },
],
baz: [{ factor: 9.5, type: BoostType.Value, value: [''] }],
};
expect(removeEmptyValueBoosts(boosts)).toEqual({
bar: [
{ factor: 9.5, type: BoostType.Proximity },
{ type: BoostType.Functional, factor: 5 },
],
foo: [
{ factor: 9.5, type: BoostType.Value, value: ['1'] },
{ factor: 9.5, type: BoostType.Value, value: ['1'] },
{ factor: 9.5, type: BoostType.Value, value: ['1'] },
],
});
});

40
x-pack/plugins/enterprise_search/public/applications/app_search/components/relevance_tuning/utils.ts
View file

@ -10,7 +10,7 @@ import { cloneDeep, omit } from 'lodash';
import { NUMBER } from '../../../shared/constants/field_types';
import { SchemaTypes } from '../../../shared/types';
import { RawBoost, Boost, SearchSettings, BoostType } from './types';
import { RawBoost, Boost, SearchSettings, BoostType, ValueBoost } from './types';
// If the user hasn't entered a filter, then we can skip filtering the array entirely
export const filterIfTerm = (array: string[], filterTerm: string): string[] => {
@ -61,3 +61,41 @@ export const normalizeBoostValues = (boosts: Record<string, RawBoost[]>): Record
[fieldName]: boostList.map(normalizeBoostValue),
};
}, {});
// Our model allows for empty values to be added to boosts. However, the server will not accept
// empty strings in values. To avoid that, we filter out empty values before sending them to the server.
// I.e., the server will not accept any of the following, so we need to filter them out
// value: undefined
// value: []
// value: ['']
// value: ['foo', '']
export const removeEmptyValueBoosts = (
boosts: Record<string, Boost[]>
): Record<string, Boost[]> => {
// before:
// { foo: { values: ['a', '', ' '] } }
// { foo: { values: [''] } }
// after:
// { foo: { values: ['a'] } }
const filterEmptyValueBoosts = (fieldBoosts: Boost[]) => {
return fieldBoosts.filter((boost: Boost) => {
if (boost.type !== BoostType.Value) return true;
const valueBoost = boost as ValueBoost;
const filteredValues = valueBoost.value.filter((value) => value.trim() !== '');
if (filteredValues.length) {
boost.value = filteredValues;
return true;
} else {
return false;
}
});
};
return Object.entries(boosts).reduce((acc, [fieldName, fieldBoosts]) => {
const updatedBoosts = filterEmptyValueBoosts(fieldBoosts);
return updatedBoosts.length ? { ...acc, [fieldName]: updatedBoosts } : acc;
}, {});
};