[App Search] Add delete action to EnginesTable component (#92844)

* Add delete engine route to App Search

* Add new deleteEngine listener to EnginesLogic

* Convert EnginesTable Manage into a proper EuiBasicTable action

* Call EnginesLogic.actions.deleteEngine using new action in EnginesTable

* Manage action on EnginesTable should use eye icon

* Confirmation alert for delete action on EnginesTable

* Only display manage/delete actions to users with canManageEngines

* Add success message and reload after successful engine delete

* Jest tests for EngineTable actions

* Copy change for engine delete success message

* Fixing EnginesTable tests

* Adding more tests for DELETE engine route

* engineNameLink -> EngineNameLink

* Remove redundant test

* Convert Engine.type to enum EngineTypes

* Must use mountWithIntl

* Use platinum license instead of role ability check
This commit is contained in:
Byron Hulcher 2021-03-09 09:49:52 -05:00 committed by GitHub
parent 8e6d4ee2f8
commit cceed8ddd6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 369 additions and 51 deletions

View file

@ -18,6 +18,8 @@ jest.mock('../../app_logic', () => ({
}));
import { AppLogic } from '../../app_logic';
import { EngineTypes } from '../engine/types';
import { ApiTokenTypes } from './constants';
import { CredentialsLogic } from './credentials_logic';
@ -61,8 +63,8 @@ describe('CredentialsLogic', () => {
const credentialsDetails = {
engines: [
{ name: 'engine1', type: 'indexed', language: 'english', result_fields: {} },
{ name: 'engine1', type: 'indexed', language: 'english', result_fields: {} },
{ name: 'engine1', type: EngineTypes.indexed, language: 'english', result_fields: {} },
{ name: 'engine1', type: EngineTypes.indexed, language: 'english', result_fields: {} },
],
};

View file

@ -9,6 +9,8 @@ import { LogicMounter, mockHttpValues } from '../../../__mocks__';
import { nextTick } from '@kbn/test/jest';
import { EngineTypes } from './types';
import { EngineLogic } from './';
describe('EngineLogic', () => {
@ -17,7 +19,7 @@ describe('EngineLogic', () => {
const mockEngineData = {
name: 'some-engine',
type: 'default',
type: EngineTypes.default,
created_at: 'some date timestamp',
language: null,
document_count: 1,
@ -213,7 +215,7 @@ describe('EngineLogic', () => {
describe('isMetaEngine', () => {
it('should be set based on engine.type', () => {
const mockMetaEngine = { ...mockEngineData, type: 'meta' };
const mockMetaEngine = { ...mockEngineData, type: EngineTypes.meta };
mount({ engine: mockMetaEngine });
expect(EngineLogic.values).toEqual({

View file

@ -11,7 +11,7 @@ import { HttpLogic } from '../../../shared/http';
import { IIndexingStatus } from '../../../shared/types';
import { EngineDetails } from './types';
import { EngineDetails, EngineTypes } from './types';
interface EngineValues {
dataLoading: boolean;
@ -78,7 +78,7 @@ export const EngineLogic = kea<MakeLogicType<EngineValues, EngineActions>>({
],
},
selectors: ({ selectors }) => ({
isMetaEngine: [() => [selectors.engine], (engine) => engine?.type === 'meta'],
isMetaEngine: [() => [selectors.engine], (engine) => engine?.type === EngineTypes.meta],
isSampleEngine: [() => [selectors.engine], (engine) => !!engine?.sample],
hasSchemaConflicts: [
() => [selectors.engine],

View file

@ -8,9 +8,14 @@
import { Schema, SchemaConflicts, IIndexingStatus } from '../../../shared/types';
import { ApiToken } from '../credentials/types';
export enum EngineTypes {
default,
indexed,
meta,
}
export interface Engine {
name: string;
type: string;
type: EngineTypes;
language: string | null;
result_fields: {
[key: string]: ResultField;

View file

@ -23,6 +23,17 @@ export const CREATE_AN_ENGINE_BUTTON_LABEL = i18n.translate(
}
);
export const DELETE_ENGINE_MESSAGE = (engineName: string) =>
i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.delete.successMessage',
{
defaultMessage: 'Successfully deleted "{engineName}"',
values: {
engineName,
},
}
);
export const CREATE_A_META_ENGINE_BUTTON_LABEL = i18n.translate(
'xpack.enterpriseSearch.appSearch.engines.createAMetaEngineButton.ButtonLabel',
{

View file

@ -5,19 +5,20 @@
* 2.0.
*/
import { LogicMounter, mockHttpValues } from '../../../__mocks__';
import { LogicMounter, mockHttpValues, mockFlashMessageHelpers } from '../../../__mocks__';
import { nextTick } from '@kbn/test/jest';
import { DEFAULT_META } from '../../../shared/constants';
import { EngineDetails } from '../engine/types';
import { EngineDetails, EngineTypes } from '../engine/types';
import { EnginesLogic } from './';
describe('EnginesLogic', () => {
const { mount } = new LogicMounter(EnginesLogic);
const { http } = mockHttpValues;
const { flashAPIErrors, setSuccessMessage } = mockFlashMessageHelpers;
const DEFAULT_VALUES = {
dataLoading: true,
@ -123,6 +124,30 @@ describe('EnginesLogic', () => {
});
describe('listeners', () => {
describe('deleteEngine', () => {
it('calls the engine API endpoint then onDeleteEngineSuccess', async () => {
http.delete.mockReturnValueOnce(Promise.resolve({}));
mount();
jest.spyOn(EnginesLogic.actions, 'onDeleteEngineSuccess');
EnginesLogic.actions.deleteEngine(MOCK_ENGINE);
await nextTick();
expect(http.delete).toHaveBeenCalledWith('/api/app_search/engines/hello-world');
expect(EnginesLogic.actions.onDeleteEngineSuccess).toHaveBeenCalledWith(MOCK_ENGINE);
});
it('calls flashAPIErrors on API Error', async () => {
http.delete.mockReturnValueOnce(Promise.reject());
mount();
EnginesLogic.actions.deleteEngine(MOCK_ENGINE);
await nextTick();
expect(flashAPIErrors).toHaveBeenCalledTimes(1);
});
});
describe('loadEngines', () => {
it('should call the engines API endpoint and set state based on the results', async () => {
http.get.mockReturnValueOnce(Promise.resolve(MOCK_ENGINES_API_RESPONSE));
@ -164,6 +189,34 @@ describe('EnginesLogic', () => {
);
});
});
describe('onDeleteEngineSuccess', () => {
beforeEach(() => {
mount();
});
it('should call setSuccessMessage', () => {
EnginesLogic.actions.onDeleteEngineSuccess(MOCK_ENGINE);
expect(setSuccessMessage).toHaveBeenCalled();
});
it('should call loadEngines if engine.type === default', () => {
jest.spyOn(EnginesLogic.actions, 'loadEngines');
EnginesLogic.actions.onDeleteEngineSuccess({ ...MOCK_ENGINE, type: EngineTypes.default });
expect(EnginesLogic.actions.loadEngines).toHaveBeenCalled();
});
it('should call loadMetaEngines if engine.type === meta', () => {
jest.spyOn(EnginesLogic.actions, 'loadMetaEngines');
EnginesLogic.actions.onDeleteEngineSuccess({ ...MOCK_ENGINE, type: EngineTypes.meta });
expect(EnginesLogic.actions.loadMetaEngines).toHaveBeenCalled();
});
});
});
describe('selectors', () => {

View file

@ -9,10 +9,13 @@ import { kea, MakeLogicType } from 'kea';
import { Meta } from '../../../../../common/types';
import { DEFAULT_META } from '../../../shared/constants';
import { flashAPIErrors, setSuccessMessage } from '../../../shared/flash_messages';
import { HttpLogic } from '../../../shared/http';
import { updateMetaPageIndex } from '../../../shared/table_pagination';
import { EngineDetails } from '../engine/types';
import { EngineDetails, EngineTypes } from '../engine/types';
import { DELETE_ENGINE_MESSAGE } from './constants';
interface EnginesValues {
dataLoading: boolean;
@ -29,6 +32,8 @@ interface EnginesAPIResponse {
meta: Meta;
}
interface EnginesActions {
deleteEngine(engine: EngineDetails): { engine: EngineDetails };
onDeleteEngineSuccess(engine: EngineDetails): { engine: EngineDetails };
onEnginesLoad({ results, meta }: EnginesAPIResponse): EnginesAPIResponse;
onMetaEnginesLoad({ results, meta }: EnginesAPIResponse): EnginesAPIResponse;
onEnginesPagination(page: number): { page: number };
@ -40,6 +45,8 @@ interface EnginesActions {
export const EnginesLogic = kea<MakeLogicType<EnginesValues, EnginesActions>>({
path: ['enterprise_search', 'app_search', 'engines_logic'],
actions: {
deleteEngine: (engine) => ({ engine }),
onDeleteEngineSuccess: (engine) => ({ engine }),
onEnginesLoad: ({ results, meta }) => ({ results, meta }),
onMetaEnginesLoad: ({ results, meta }) => ({ results, meta }),
onEnginesPagination: (page) => ({ page }),
@ -96,6 +103,20 @@ export const EnginesLogic = kea<MakeLogicType<EnginesValues, EnginesActions>>({
],
},
listeners: ({ actions, values }) => ({
deleteEngine: async ({ engine }) => {
const { http } = HttpLogic.values;
let response;
try {
response = await http.delete(`/api/app_search/engines/${engine.name}`);
} catch (e) {
flashAPIErrors(e);
}
if (response) {
actions.onDeleteEngineSuccess(engine);
}
},
loadEngines: async () => {
const { http } = HttpLogic.values;
const { enginesMeta } = values;
@ -122,5 +143,13 @@ export const EnginesLogic = kea<MakeLogicType<EnginesValues, EnginesActions>>({
});
actions.onMetaEnginesLoad(response);
},
onDeleteEngineSuccess: async ({ engine }) => {
setSuccessMessage(DELETE_ENGINE_MESSAGE(engine.name));
if ([EngineTypes.default, EngineTypes.indexed].includes(engine.type)) {
actions.loadEngines();
} else if (engine.type === EngineTypes.meta) {
actions.loadMetaEngines();
}
},
}),
});

View file

@ -55,9 +55,13 @@ export const EnginesOverview: React.FC = () => {
metaEnginesLoading,
} = useValues(EnginesLogic);
const { loadEngines, loadMetaEngines, onEnginesPagination, onMetaEnginesPagination } = useActions(
EnginesLogic
);
const {
deleteEngine,
loadEngines,
loadMetaEngines,
onEnginesPagination,
onMetaEnginesPagination,
} = useActions(EnginesLogic);
useEffect(() => {
loadEngines();
@ -106,6 +110,7 @@ export const EnginesOverview: React.FC = () => {
hidePerPageOptions: true,
}}
onChange={handlePageChange(onEnginesPagination)}
onDeleteEngine={deleteEngine}
/>
</EuiPageContentBody>
@ -155,6 +160,7 @@ export const EnginesOverview: React.FC = () => {
/>
}
onChange={handlePageChange(onMetaEnginesPagination)}
onDeleteEngine={deleteEngine}
/>
</EuiPageContentBody>
</>

View file

@ -6,20 +6,27 @@
*/
import '../../../__mocks__/enterprise_search_url.mock';
import { mockTelemetryActions, mountWithIntl } from '../../../__mocks__';
import { mockTelemetryActions, mountWithIntl, setMockValues } from '../../../__mocks__';
import React from 'react';
import { EuiBasicTable, EuiPagination, EuiButtonEmpty } from '@elastic/eui';
import { ReactWrapper, shallow } from 'enzyme';
import { EuiBasicTable, EuiPagination, EuiButtonEmpty, EuiIcon, EuiTableRow } from '@elastic/eui';
import { KibanaLogic } from '../../../shared/kibana';
import { EuiLinkTo } from '../../../shared/react_router_helpers';
import { TelemetryLogic } from '../../../shared/telemetry';
import { EngineDetails } from '../engine/types';
import { EnginesLogic } from './engines_logic';
import { EnginesTable } from './engines_table';
describe('EnginesTable', () => {
const onChange = jest.fn();
const onDeleteEngine = jest.fn();
const data = [
{
name: 'test-engine',
@ -41,11 +48,22 @@ describe('EnginesTable', () => {
loading: false,
pagination,
onChange,
onDeleteEngine,
};
describe('basic table', () => {
const wrapper = mountWithIntl(<EnginesTable {...props} />);
const table = wrapper.find(EuiBasicTable);
let wrapper: ReactWrapper<any>;
let table: ReactWrapper<any>;
beforeAll(() => {
jest.clearAllMocks();
setMockValues({
// LicensingLogic
hasPlatinumLicense: false,
});
wrapper = mountWithIntl(<EnginesTable {...props} />);
table = wrapper.find(EuiBasicTable);
});
it('renders', () => {
expect(table).toHaveLength(1);
@ -83,7 +101,13 @@ describe('EnginesTable', () => {
describe('loading', () => {
it('passes the loading prop', () => {
jest.clearAllMocks();
setMockValues({
// LicensingLogic
hasPlatinumLicense: false,
});
const wrapper = mountWithIntl(<EnginesTable {...props} loading />);
expect(wrapper.find(EuiBasicTable).prop('loading')).toEqual(true);
});
});
@ -96,6 +120,14 @@ describe('EnginesTable', () => {
});
describe('language field', () => {
beforeAll(() => {
jest.clearAllMocks();
setMockValues({
// LicensingLogic
hasPlatinumLicense: false,
});
});
it('renders language when available', () => {
const wrapper = mountWithIntl(
<EnginesTable
@ -147,4 +179,71 @@ describe('EnginesTable', () => {
expect(tableContent).not.toContain('Universal');
});
});
describe('actions', () => {
it('will hide the action buttons if the user does not have a platinum license', () => {
jest.clearAllMocks();
setMockValues({
// LicensingLogic
hasPlatinumLicense: false,
});
const wrapper = shallow(<EnginesTable {...props} />);
const tableRow = wrapper.find(EuiTableRow).first();
expect(tableRow.find(EuiIcon)).toHaveLength(0);
});
describe('when user has a platinum license', () => {
let wrapper: ReactWrapper<any>;
let tableRow: ReactWrapper<any>;
let actions: ReactWrapper<any>;
beforeEach(() => {
jest.clearAllMocks();
setMockValues({
// LicensingLogic
hasPlatinumLicense: true,
});
wrapper = mountWithIntl(<EnginesTable {...props} />);
tableRow = wrapper.find(EuiTableRow).first();
actions = tableRow.find(EuiIcon);
EnginesLogic.mount();
});
it('renders a manage action', () => {
jest.spyOn(TelemetryLogic.actions, 'sendAppSearchTelemetry');
jest.spyOn(KibanaLogic.values, 'navigateToUrl');
actions.at(0).simulate('click');
expect(TelemetryLogic.actions.sendAppSearchTelemetry).toHaveBeenCalled();
expect(KibanaLogic.values.navigateToUrl).toHaveBeenCalledWith('/engines/test-engine');
});
describe('delete action', () => {
it('shows the user a confirm message when the action is clicked', () => {
jest.spyOn(global, 'confirm' as any).mockReturnValueOnce(true);
actions.at(1).simulate('click');
expect(global.confirm).toHaveBeenCalled();
});
it('clicking the action and confirming deletes the engine', () => {
jest.spyOn(global, 'confirm' as any).mockReturnValueOnce(true);
jest.spyOn(EnginesLogic.actions, 'deleteEngine');
actions.at(1).simulate('click');
expect(onDeleteEngine).toHaveBeenCalled();
});
it('clicking the action and not confirming does not delete the engine', () => {
jest.spyOn(global, 'confirm' as any).mockReturnValueOnce(false);
jest.spyOn(EnginesLogic.actions, 'deleteEngine');
actions.at(1).simulate('click');
expect(onDeleteEngine).toHaveBeenCalledTimes(0);
});
});
});
});
});

View file

@ -7,12 +7,19 @@
import React, { ReactNode } from 'react';
import { useActions } from 'kea';
import { useActions, useValues } from 'kea';
import { EuiBasicTable, EuiBasicTableColumn, CriteriaWithPagination } from '@elastic/eui';
import {
EuiBasicTable,
EuiBasicTableColumn,
CriteriaWithPagination,
EuiTableActionsColumnType,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage, FormattedNumber } from '@kbn/i18n/react';
import { FormattedNumber } from '@kbn/i18n/react';
import { KibanaLogic } from '../../../shared/kibana';
import { LicensingLogic } from '../../../shared/licensing';
import { EuiLinkTo } from '../../../shared/react_router_helpers';
import { TelemetryLogic } from '../../../shared/telemetry';
import { UNIVERSAL_LANGUAGE } from '../../constants';
@ -32,6 +39,7 @@ interface EnginesTableProps {
hidePerPageOptions: boolean;
};
onChange(criteria: CriteriaWithPagination<EngineDetails>): void;
onDeleteEngine(engine: EngineDetails): void;
}
export const EnginesTable: React.FC<EnginesTableProps> = ({
@ -40,17 +48,19 @@ export const EnginesTable: React.FC<EnginesTableProps> = ({
noItemsMessage,
pagination,
onChange,
onDeleteEngine,
}) => {
const { sendAppSearchTelemetry } = useActions(TelemetryLogic);
const { navigateToUrl } = useValues(KibanaLogic);
const { hasPlatinumLicense } = useValues(LicensingLogic);
const engineLinkProps = (engineName: string) => ({
to: generateEncodedPath(ENGINE_PATH, { engineName }),
onClick: () =>
sendAppSearchTelemetry({
action: 'clicked',
metric: 'engine_table_link',
}),
});
const generteEncodedEnginePath = (engineName: string) =>
generateEncodedPath(ENGINE_PATH, { engineName });
const sendEngineTableLinkClickTelemetry = () =>
sendAppSearchTelemetry({
action: 'clicked',
metric: 'engine_table_link',
});
const columns: Array<EuiBasicTableColumn<EngineDetails>> = [
{
@ -59,7 +69,11 @@ export const EnginesTable: React.FC<EnginesTableProps> = ({
defaultMessage: 'Name',
}),
render: (name: string) => (
<EuiLinkTo data-test-subj="engineNameLink" {...engineLinkProps(name)}>
<EuiLinkTo
data-test-subj="EngineNameLink"
to={generteEncodedEnginePath(name)}
onClick={sendEngineTableLinkClickTelemetry}
>
{name}
</EuiLinkTo>
),
@ -121,28 +135,74 @@ export const EnginesTable: React.FC<EnginesTableProps> = ({
render: (number: number) => <FormattedNumber value={number} />,
truncateText: true,
},
{
field: 'name',
name: i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.column.actions',
{
defaultMessage: 'Actions',
}
),
dataType: 'string',
render: (name: string) => (
<EuiLinkTo {...engineLinkProps(name)}>
<FormattedMessage
id="xpack.enterpriseSearch.appSearch.enginesOverview.table.action.manage"
defaultMessage="Manage"
/>
</EuiLinkTo>
),
align: 'right',
width: '100px',
},
];
const actionsColumn: EuiTableActionsColumnType<EngineDetails> = {
name: i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.table.column.actions', {
defaultMessage: 'Actions',
}),
actions: [
{
name: i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.manage',
{
defaultMessage: 'Manage',
}
),
description: i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.manage.buttonDescription',
{
defaultMessage: 'Manage this engine',
}
),
type: 'icon',
icon: 'eye',
onClick: (engineDetails) => {
sendEngineTableLinkClickTelemetry();
navigateToUrl(generteEncodedEnginePath(engineDetails.name));
},
},
{
name: i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.delete.buttonLabel',
{
defaultMessage: 'Delete',
}
),
description: i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.delete.buttonDescription',
{
defaultMessage: 'Delete this engine',
}
),
type: 'icon',
icon: 'trash',
onClick: (engine) => {
if (
window.confirm(
i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.delete.confirmationPopupMessage',
{
defaultMessage:
'Are you sure you want to permanently delete "{engineName}" and all of its content?',
values: {
engineName: engine.name,
},
}
)
)
) {
onDeleteEngine(engine);
}
},
},
],
};
if (hasPlatinumLicense) {
columns.push(actionsColumn);
}
return (
<EuiBasicTable
items={items}

View file

@ -199,6 +199,44 @@ describe('engine routes', () => {
});
});
describe('DELETE /api/app_search/engines/{name}', () => {
let mockRouter: MockRouter;
beforeEach(() => {
jest.clearAllMocks();
mockRouter = new MockRouter({
method: 'delete',
path: '/api/app_search/engines/{name}',
});
registerEnginesRoutes({
...mockDependencies,
router: mockRouter.router,
});
});
it('creates a request to enterprise search', () => {
expect(mockRequestHandler.createRequest).toHaveBeenCalledWith({
path: '/as/engines/:name',
});
});
it('validates correctly with name', () => {
const request = { params: { name: 'test-engine' } };
mockRouter.shouldValidate(request);
});
it('fails validation without name', () => {
const request = { params: {} };
mockRouter.shouldThrow(request);
});
it('fails validation with a non-string name', () => {
const request = { params: { name: 1 } };
mockRouter.shouldThrow(request);
});
});
describe('GET /api/app_search/engines/{name}/overview', () => {
let mockRouter: MockRouter;

View file

@ -69,6 +69,19 @@ export function registerEnginesRoutes({
path: '/as/engines/:name/details',
})
);
router.delete(
{
path: '/api/app_search/engines/{name}',
validate: {
params: schema.object({
name: schema.string(),
}),
},
},
enterpriseSearchRequestHandler.createRequest({
path: '/as/engines/:name',
})
);
router.get(
{
path: '/api/app_search/engines/{name}/overview',

View file

@ -20,12 +20,12 @@ export function AppSearchPageProvider({ getService, getPageObjects }: FtrProvide
async getEngineLinks(): Promise<WebElementWrapper[]> {
const engines = await testSubjects.find('appSearchEngines');
return await testSubjects.findAllDescendant('engineNameLink', engines);
return await testSubjects.findAllDescendant('EngineNameLink', engines);
},
async getMetaEngineLinks(): Promise<WebElementWrapper[]> {
const metaEngines = await testSubjects.find('appSearchMetaEngines');
return await testSubjects.findAllDescendant('engineNameLink', metaEngines);
return await testSubjects.findAllDescendant('EngineNameLink', metaEngines);
},
};
}