Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[App Search] Implement various Relevance Tuning states and form actions #92644

Merged
merged 20 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,26 @@
* 2.0.
*/

import { EngineDetails } from '../components/engine/types';
import { generateEncodedPath } from '../utils/encode_path_params';

export const mockEngineValues = {
engineName: 'some-engine',
engine: {},
engine: {} as EngineDetails,
};

export const mockEngineActions = {
initializeEngine: jest.fn(),
};

export const mockGenerateEnginePath = jest.fn((path, pathParams = {}) =>
generateEncodedPath(path, { engineName: mockEngineValues.engineName, ...pathParams })
);

jest.mock('../components/engine', () => ({
EngineLogic: { values: mockEngineValues },
EngineLogic: {
values: mockEngineValues,
actions: mockEngineActions,
},
generateEnginePath: mockGenerateEnginePath,
}));
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
* 2.0.
*/

export { mockEngineValues } from './engine_logic.mock';
export { mockEngineValues, mockEngineActions } from './engine_logic.mock';
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,14 @@ export const BoostItem: React.FC<Props> = ({ id, boost, index, name }) => {
className="boosts__item"
buttonContentClassName="boosts__itemButton"
buttonContent={
<EuiFlexGroup responsive={false} wrap>
<EuiFlexItem>
<EuiFlexGroup responsive={false}>
<EuiFlexItem grow={false}>
<BoostIcon type={boost.type} />
</EuiFlexItem>
<EuiFlexItem grow={false}>{BOOST_TYPE_TO_DISPLAY_MAP[boost.type]}</EuiFlexItem>
<EuiHideFor sizes={['xs', 's', 'm', 'l']}>
<EuiFlexItem>{summary}</EuiFlexItem>
</EuiHideFor>
</EuiFlexGroup>
<EuiFlexGroup responsive={false} alignItems="center">
<EuiFlexItem grow={false}>
<BoostIcon type={boost.type} />
</EuiFlexItem>
<EuiFlexItem grow={false}>{BOOST_TYPE_TO_DISPLAY_MAP[boost.type]}</EuiFlexItem>
<EuiHideFor sizes={['xs', 's', 'm', 'l']}>
<EuiFlexItem className="eui-textBreakAll">{summary}</EuiFlexItem>
</EuiHideFor>
<EuiFlexItem grow={false}>
<ValueBadge>{boost.factor}</ValueBadge>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ describe('BoostItemContent', () => {
expect(actions.updateBoostFactor).toHaveBeenCalledWith('foo', 3, 2);
});

it("will delete the current boost if the 'Delete Boost' button is clicked", () => {
it("will delete the current boost if the 'Delete boost' button is clicked", () => {
const boost = {
factor: 8,
type: 'proximity' as BoostType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const BoostItemContent: React.FC<Props> = ({ boost, index, name }) => {
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.boosts.deleteBoostButtonLabel',
{
defaultMessage: 'Delete Boost',
defaultMessage: 'Delete boost',
}
)}
</EuiButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export const ValueBoostForm: React.FC<Props> = ({ boost, index, name }) => {
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.boosts.value.addValueButtonLabel',
{
defaultMessage: 'Add Value',
defaultMessage: 'Add value',
}
)}
</EuiButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,85 @@
* 2.0.
*/
import '../../../__mocks__/shallow_useeffect.mock';
import { setMockActions } from '../../../__mocks__/kea.mock';
import { setMockActions, setMockValues } from '../../../__mocks__/kea.mock';

import React from 'react';

import { shallow, ShallowWrapper } from 'enzyme';
import { shallow } from 'enzyme';

import { EuiEmptyPrompt } from '@elastic/eui';

import { Loading } from '../../../shared/loading';
import { UnsavedChangesPrompt } from '../../../shared/unsaved_changes_prompt';

import { RelevanceTuning } from './relevance_tuning';
import { RelevanceTuningForm } from './relevance_tuning_form';

describe('RelevanceTuning', () => {
let wrapper: ShallowWrapper;
const values = {
engineHasSchemaFields: true,
engine: {
invalidBoosts: false,
unsearchedUnconfirmedFields: false,
},
schemaFieldsWithConflicts: [],
unsavedChanges: false,
dataLoading: false,
};

const actions = {
initializeRelevanceTuning: jest.fn(),
updateSearchSettings: jest.fn(),
resetSearchSettings: jest.fn(),
};

const subject = () => shallow(<RelevanceTuning engineBreadcrumb={['test']} />);

beforeEach(() => {
jest.clearAllMocks();
setMockValues(values);
setMockActions(actions);
wrapper = shallow(<RelevanceTuning engineBreadcrumb={['test']} />);
});

it('renders', () => {
const wrapper = subject();
expect(wrapper.find(RelevanceTuningForm).exists()).toBe(true);
expect(wrapper.find(Loading).exists()).toBe(false);
expect(wrapper.find('EmptyCallout').exists()).toBe(false);
});

it('initializes relevance tuning data', () => {
subject();
expect(actions.initializeRelevanceTuning).toHaveBeenCalled();
});

it('will render an empty message when the engine has no schema', () => {
setMockValues({
...values,
engineHasSchemaFields: false,
});
const wrapper = subject();
expect(wrapper.find('EmptyCallout').dive().find(EuiEmptyPrompt).exists()).toBe(true);
expect(wrapper.find(Loading).exists()).toBe(false);
expect(wrapper.find(RelevanceTuningForm).exists()).toBe(false);
});

it('will show a loading message if data is loading', () => {
setMockValues({
...values,
dataLoading: true,
});
const wrapper = subject();
expect(wrapper.find(Loading).exists()).toBe(true);
expect(wrapper.find('EmptyCallout').exists()).toBe(false);
expect(wrapper.find(RelevanceTuningForm).exists()).toBe(false);
});

it('will prevent user from leaving the page if there are unsaved changes', () => {
setMockValues({
...values,
unsavedChanges: true,
});
expect(subject().find(UnsavedChangesPrompt).prop('hasUnsavedChanges')).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,67 +7,93 @@

import React, { useEffect } from 'react';

import { useActions } from 'kea';

import {
EuiPageHeader,
EuiPageHeaderSection,
EuiTitle,
EuiText,
EuiSpacer,
EuiFlexGroup,
EuiFlexItem,
EuiTextColor,
} from '@elastic/eui';
import { useActions, useValues } from 'kea';

import { EuiButton, EuiEmptyPrompt, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { FlashMessages } from '../../../shared/flash_messages';
import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
import { Loading } from '../../../shared/loading';
import { UnsavedChangesPrompt } from '../../../shared/unsaved_changes_prompt';
import { DOCS_PREFIX } from '../../routes';

import { RELEVANCE_TUNING_TITLE } from './constants';
import { RelevanceTuningForm } from './relevance_tuning_form';
import { RelevanceTuningLogic } from './relevance_tuning_logic';
import { RelevanceTuningLayout } from './relevance_tuning_layout';

import { RelevanceTuningLogic } from '.';

interface Props {
engineBreadcrumb: string[];
}

const EmptyCallout: React.FC = () => {
return (
<EuiEmptyPrompt
title={
<h2>
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.emptyErrorMessageTitle',
{
defaultMessage: 'Tuning requires schema fields',
}
)}
</h2>
}
body={i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.emptyErrorMessage',
{
defaultMessage: 'Index documents to tune relevance.',
}
)}
actions={
<EuiButton
size="s"
color="primary"
href={`${DOCS_PREFIX}/relevance-tuning-guide.html`}
fill
>
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.emptyButtonLabel',
{
defaultMessage: 'Read the relevance tuning guide',
}
)}
</EuiButton>
}
/>
);
};

export const RelevanceTuning: React.FC<Props> = ({ engineBreadcrumb }) => {
const { dataLoading, engineHasSchemaFields, unsavedChanges } = useValues(RelevanceTuningLogic);
const { initializeRelevanceTuning } = useActions(RelevanceTuningLogic);

useEffect(() => {
initializeRelevanceTuning();
}, []);

return (
<>
<SetPageChrome trail={[...engineBreadcrumb, RELEVANCE_TUNING_TITLE]} />
<EuiPageHeader>
<EuiPageHeaderSection>
<EuiTitle size="l">
<h1>{RELEVANCE_TUNING_TITLE}</h1>
</EuiTitle>
<EuiText>
<EuiTextColor color="subdued">
{i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.relevanceTuning.description',
{
defaultMessage: 'Set field weights and boosts',
}
)}
</EuiTextColor>
</EuiText>
</EuiPageHeaderSection>
</EuiPageHeader>
<EuiSpacer />
<FlashMessages />
const body = () => {
if (dataLoading) {
return <Loading />;
}

if (!engineHasSchemaFields) {
return <EmptyCallout />;
}
Comment on lines +75 to +81
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also optional, but I think Scotty stated a preference for single line returns. I'm fine with whatever though lol, not a big deal to me, we can yolo it and come back to returns later


return (
<EuiFlexGroup>
<EuiFlexItem>
<RelevanceTuningForm />
</EuiFlexItem>
<EuiFlexItem />
</EuiFlexGroup>
</>
);
};

return (
<RelevanceTuningLayout engineBreadcrumb={engineBreadcrumb}>
<UnsavedChangesPrompt hasUnsavedChanges={unsavedChanges} />
Comment on lines +94 to +95
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[optional, not a blocker] I think you could probably bake UnsavedChangesPrompt into the RelevanceTuningLayout, unless you think that's too hidden or confusing. Up to you!

{body()}
</RelevanceTuningLayout>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* 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 '../../__mocks__/engine_logic.mock';
import { setMockValues } from '../../../__mocks__/kea.mock';

import React from 'react';

import { shallow } from 'enzyme';

import { RelevanceTuningCallouts } from './relevance_tuning_callouts';

describe('RelevanceTuningCallouts', () => {
const values = {
engineHasSchemaFields: true,
engine: {
invalidBoosts: false,
unsearchedUnconfirmedFields: false,
},
schemaFieldsWithConflicts: [],
};

beforeEach(() => {
jest.clearAllMocks();
setMockValues(values);
});

const subject = () => shallow(<RelevanceTuningCallouts />);

it('renders', () => {
const wrapper = subject();
expect(wrapper.find('[data-test-subj="RelevanceTuningInvalidBoostsCallout"]').exists()).toBe(
false
);
expect(wrapper.find('[data-test-subj="RelevanceTuningUnsearchedFieldsCallout"]').exists()).toBe(
false
);
expect(subject().find('[data-test-subj="SchemaConflictsCallout"]').exists()).toBe(false);
});

it('shows a message when there are invalid boosts', () => {
// An invalid boost would be if a user creats a functional boost on a number field, then that
// field later changes to text. At this point, the boost still exists but is invalid for
// a text field.
setMockValues({
...values,
engine: {
invalidBoosts: true,
unsearchedUnconfirmedFields: false,
},
});
expect(subject().find('[data-test-subj="RelevanceTuningInvalidBoostsCallout"]').exists()).toBe(
true
);
});

it('shows a message when there are unconfirmed fields', () => {
// An invalid boost would be if a user creats a functional boost on a number field, then that
// field later changes to text. At this point, the boost still exists but is invalid for
// a text field.
setMockValues({
...values,
engine: {
invalidBoosts: false,
unsearchedUnconfirmedFields: true,
},
});
expect(
subject().find('[data-test-subj="RelevanceTuningUnsearchedFieldsCallout"]').exists()
).toBe(true);
});

it('shows a message when there are schema field conflicts', () => {
// Schema conflicts occur when a meta engine has fields in source engines with have differing types,
// hence relevance tuning cannot be applied as we don't know the actual type
setMockValues({
...values,
schemaFieldsWithConflicts: ['fe', 'fi', 'fo'],
});
expect(subject().find('[data-test-subj="SchemaConflictsCallout"]').exists()).toBe(true);
});
});
Loading