Skip to content

Commit

Permalink
[App Search] General UX Improvements for Curations and Suggestions (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
byronhulcher authored Oct 7, 2021
1 parent a5f4304 commit 920ea03
Show file tree
Hide file tree
Showing 35 changed files with 417 additions and 199 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const columns: Array<EuiBasicTableColumn<CurationSuggestion>> = [
field: 'promoted',
name: i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.suggestionsTable.column.promotedDocumentsTableHeader',
{ defaultMessage: 'Promoted documents' }
{ defaultMessage: 'Promoted results' }
),
render: (promoted: string[]) => <span>{promoted.length}</span>,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ export const QUERY_INPUTS_PLACEHOLDER = i18n.translate(
{ defaultMessage: 'Enter a query' }
);

export const DELETE_MESSAGE = i18n.translate(
export const DELETE_CONFIRMATION_MESSAGE = i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.deleteConfirmation',
{ defaultMessage: 'Are you sure you want to remove this curation?' }
);
export const SUCCESS_MESSAGE = i18n.translate(
export const DELETE_SUCCESS_MESSAGE = i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.deleteSuccessMessage',
{ defaultMessage: 'Your curation was deleted' }
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import React from 'react';

import { shallow, ShallowWrapper } from 'enzyme';

import { EuiBadge, EuiTab } from '@elastic/eui';
import { EuiBadge, EuiButton, EuiLoadingSpinner, EuiTab } from '@elastic/eui';

import { getPageHeaderActions, getPageHeaderTabs, getPageTitle } from '../../../../test_helpers';

Expand All @@ -25,6 +25,7 @@ import { AppSearchPageTemplate } from '../../layout';
import { AutomatedCuration } from './automated_curation';
import { CurationLogic } from './curation_logic';

import { DeleteCurationButton } from './delete_curation_button';
import { PromotedDocuments, OrganicDocuments } from './documents';

describe('AutomatedCuration', () => {
Expand All @@ -33,6 +34,8 @@ describe('AutomatedCuration', () => {
queries: ['query A', 'query B'],
isFlyoutOpen: false,
curation: {
promoted: [],
hidden: [],
suggestion: {
status: 'applied',
},
Expand Down Expand Up @@ -89,13 +92,29 @@ describe('AutomatedCuration', () => {
expect(pageTitle.find(EuiBadge)).toHaveLength(1);
});

it('displays a spinner in the title when loading', () => {
setMockValues({ ...values, dataLoading: true });

const wrapper = shallow(<AutomatedCuration />);
const pageTitle = shallow(<div>{getPageTitle(wrapper)}</div>);

expect(pageTitle.find(EuiLoadingSpinner)).toHaveLength(1);
});

it('contains a button to delete the curation', () => {
const wrapper = shallow(<AutomatedCuration />);
const pageHeaderActions = getPageHeaderActions(wrapper);

expect(pageHeaderActions.find(DeleteCurationButton)).toHaveLength(1);
});

describe('convert to manual button', () => {
let convertToManualButton: ShallowWrapper;
let confirmSpy: jest.SpyInstance;

beforeAll(() => {
const wrapper = shallow(<AutomatedCuration />);
convertToManualButton = getPageHeaderActions(wrapper).childAt(0);
convertToManualButton = getPageHeaderActions(wrapper).find(EuiButton);

confirmSpy = jest.spyOn(window, 'confirm');
});
Expand All @@ -107,12 +126,14 @@ describe('AutomatedCuration', () => {
it('converts the curation upon user confirmation', () => {
confirmSpy.mockReturnValueOnce(true);
convertToManualButton.simulate('click');

expect(actions.convertToManual).toHaveBeenCalled();
});

it('does not convert the curation if the user cancels', () => {
confirmSpy.mockReturnValueOnce(false);
convertToManualButton.simulate('click');

expect(actions.convertToManual).not.toHaveBeenCalled();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { useParams } from 'react-router-dom';

import { useValues, useActions } from 'kea';

import { EuiSpacer, EuiButton, EuiBadge } from '@elastic/eui';
import { EuiButton, EuiBadge, EuiLoadingSpinner, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';

import { AppSearchPageTemplate } from '../../layout';
import { AutomatedIcon } from '../components/automated_icon';
Expand All @@ -24,22 +24,25 @@ import { getCurationsBreadcrumbs } from '../utils';

import { HIDDEN_DOCUMENTS_TITLE, PROMOTED_DOCUMENTS_TITLE } from './constants';
import { CurationLogic } from './curation_logic';
import { DeleteCurationButton } from './delete_curation_button';
import { PromotedDocuments, OrganicDocuments } from './documents';

export const AutomatedCuration: React.FC = () => {
const { curationId } = useParams<{ curationId: string }>();
const logic = CurationLogic({ curationId });
const { convertToManual } = useActions(logic);
const { activeQuery, dataLoading, queries } = useValues(logic);
const { activeQuery, dataLoading, queries, curation } = useValues(logic);

// This tab group is meant to visually mirror the dynamic group of tags in the ManualCuration component
const pageTabs = [
{
label: PROMOTED_DOCUMENTS_TITLE,
append: <EuiBadge>{curation.promoted.length}</EuiBadge>,
isSelected: true,
},
{
label: HIDDEN_DOCUMENTS_TITLE,
append: <EuiBadge isDisabled>0</EuiBadge>,
isSelected: false,
disabled: true,
},
Expand All @@ -51,30 +54,36 @@ export const AutomatedCuration: React.FC = () => {
pageHeader={{
pageTitle: (
<>
{activeQuery}{' '}
{dataLoading ? <EuiLoadingSpinner size="l" /> : activeQuery}{' '}
<EuiBadge iconType={AutomatedIcon} color="accent">
{AUTOMATED_LABEL}
</EuiBadge>
</>
),
rightSideItems: [
<EuiButton
color="primary"
fill
iconType="exportAction"
onClick={() => {
if (window.confirm(CONVERT_TO_MANUAL_CONFIRMATION)) convertToManual();
}}
>
{COVERT_TO_MANUAL_BUTTON_LABEL}
</EuiButton>,
<EuiFlexGroup gutterSize="s" responsive={false}>
<EuiFlexItem grow={false}>
<DeleteCurationButton />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton
color="primary"
fill
iconType="exportAction"
onClick={() => {
if (window.confirm(CONVERT_TO_MANUAL_CONFIRMATION)) convertToManual();
}}
>
{COVERT_TO_MANUAL_BUTTON_LABEL}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>,
],
tabs: pageTabs,
}}
isLoading={dataLoading}
>
<PromotedDocuments />
<EuiSpacer />
<OrganicDocuments />
</AppSearchPageTemplate>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import { i18n } from '@kbn/i18n';

export const PROMOTED_DOCUMENTS_TITLE = i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.promotedDocuments.title',
{ defaultMessage: 'Promoted documents' }
{ defaultMessage: 'Promoted results' }
);

export const HIDDEN_DOCUMENTS_TITLE = i18n.translate(
'xpack.enterpriseSearch.appSearch.engine.curations.hiddenDocuments.title',
{ defaultMessage: 'Hidden documents' }
{ defaultMessage: 'Hidden results' }
);
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('CurationLogic', () => {
const { mount } = new LogicMounter(CurationLogic);
const { http } = mockHttpValues;
const { navigateToUrl } = mockKibanaValues;
const { clearFlashMessages, flashAPIErrors } = mockFlashMessageHelpers;
const { clearFlashMessages, flashAPIErrors, flashSuccessToast } = mockFlashMessageHelpers;

const MOCK_CURATION_RESPONSE = {
id: 'cur-123456789',
Expand Down Expand Up @@ -249,23 +249,6 @@ describe('CurationLogic', () => {
});
});

describe('resetCuration', () => {
it('should clear promotedIds & hiddenIds & set dataLoading to true', () => {
mount({ promotedIds: ['hello'], hiddenIds: ['world'] });

CurationLogic.actions.resetCuration();

expect(CurationLogic.values).toEqual({
...DEFAULT_VALUES,
dataLoading: true,
promotedIds: [],
promotedDocumentsLoading: true,
hiddenIds: [],
hiddenDocumentsLoading: true,
});
});
});

describe('onSelectPageTab', () => {
it('should set the selected page tab', () => {
mount({
Expand Down Expand Up @@ -336,6 +319,33 @@ describe('CurationLogic', () => {
});
});

describe('deleteCuration', () => {
it('should make an API call and navigate to the curations page', async () => {
http.delete.mockReturnValueOnce(Promise.resolve());
mount({}, { curationId: 'cur-123456789' });
jest.spyOn(CurationLogic.actions, 'onCurationLoad');

CurationLogic.actions.deleteCuration();
await nextTick();

expect(http.delete).toHaveBeenCalledWith(
'/internal/app_search/engines/some-engine/curations/cur-123456789'
);
expect(flashSuccessToast).toHaveBeenCalled();
expect(navigateToUrl).toHaveBeenCalledWith('/engines/some-engine/curations');
});

it('flashes any errors', async () => {
http.delete.mockReturnValueOnce(Promise.reject('error'));
mount({}, { curationId: 'cur-404' });

CurationLogic.actions.deleteCuration();
await nextTick();

expect(flashAPIErrors).toHaveBeenCalledWith('error');
});
});

describe('loadCuration', () => {
it('should set dataLoading state', () => {
mount({ dataLoading: false }, { curationId: 'cur-123456789' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@

import { kea, MakeLogicType } from 'kea';

import { clearFlashMessages, flashAPIErrors } from '../../../../shared/flash_messages';
import {
clearFlashMessages,
flashAPIErrors,
flashSuccessToast,
} from '../../../../shared/flash_messages';
import { HttpLogic } from '../../../../shared/http';
import { KibanaLogic } from '../../../../shared/kibana';
import { ENGINE_CURATIONS_PATH } from '../../../routes';
import { EngineLogic, generateEnginePath } from '../../engine';
import { DELETE_SUCCESS_MESSAGE } from '../constants';

import { Curation } from '../types';
import { addDocument, removeDocument } from '../utils';
Expand All @@ -35,6 +40,7 @@ interface CurationValues {

interface CurationActions {
convertToManual(): void;
deleteCuration(): void;
loadCuration(): void;
onCurationLoad(curation: Curation): { curation: Curation };
updateCuration(): void;
Expand All @@ -48,7 +54,6 @@ interface CurationActions {
addHiddenId(id: string): { id: string };
removeHiddenId(id: string): { id: string };
clearHiddenIds(): void;
resetCuration(): void;
onSelectPageTab(pageTab: CurationPageTabs): { pageTab: CurationPageTabs };
}

Expand All @@ -60,6 +65,7 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
path: ['enterprise_search', 'app_search', 'curation_logic'],
actions: () => ({
convertToManual: true,
deleteCuration: true,
loadCuration: true,
onCurationLoad: (curation) => ({ curation }),
updateCuration: true,
Expand All @@ -73,15 +79,13 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
addHiddenId: (id) => ({ id }),
removeHiddenId: (id) => ({ id }),
clearHiddenIds: true,
resetCuration: true,
onSelectPageTab: (pageTab) => ({ pageTab }),
}),
reducers: () => ({
dataLoading: [
true,
{
loadCuration: () => true,
resetCuration: () => true,
onCurationLoad: () => false,
onCurationError: () => false,
},
Expand Down Expand Up @@ -204,6 +208,21 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
flashAPIErrors(e);
}
},
deleteCuration: async () => {
const { http } = HttpLogic.values;
const { engineName } = EngineLogic.values;
const { navigateToUrl } = KibanaLogic.values;

try {
await http.delete(
`/internal/app_search/engines/${engineName}/curations/${props.curationId}`
);
navigateToUrl(generateEnginePath(ENGINE_CURATIONS_PATH));
flashSuccessToast(DELETE_SUCCESS_MESSAGE);
} catch (e) {
flashAPIErrors(e);
}
},
loadCuration: async () => {
const { http } = HttpLogic.values;
const { engineName } = EngineLogic.values;
Expand Down Expand Up @@ -260,9 +279,5 @@ export const CurationLogic = kea<MakeLogicType<CurationValues, CurationActions,
addHiddenId: () => actions.updateCuration(),
removeHiddenId: () => actions.updateCuration(),
clearHiddenIds: () => actions.updateCuration(),
resetCuration: () => {
actions.clearPromotedIds();
actions.clearHiddenIds();
},
}),
});
Loading

0 comments on commit 920ea03

Please sign in to comment.