Skip to content

Commit

Permalink
[App Search] Add delete action to EnginesTable component (#92844)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
byronhulcher authored Mar 9, 2021
1 parent 8e6d4ee commit cceed8d
Show file tree
Hide file tree
Showing 13 changed files with 369 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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: {} },
],
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { LogicMounter, mockHttpValues } from '../../../__mocks__';

import { nextTick } from '@kbn/test/jest';

import { EngineTypes } from './types';

import { EngineLogic } from './';

describe('EngineLogic', () => {
Expand All @@ -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,
Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 };
Expand All @@ -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 }),
Expand Down Expand Up @@ -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;
Expand All @@ -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();
}
},
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -106,6 +110,7 @@ export const EnginesOverview: React.FC = () => {
hidePerPageOptions: true,
}}
onChange={handlePageChange(onEnginesPagination)}
onDeleteEngine={deleteEngine}
/>
</EuiPageContentBody>

Expand Down Expand Up @@ -155,6 +160,7 @@ export const EnginesOverview: React.FC = () => {
/>
}
onChange={handlePageChange(onMetaEnginesPagination)}
onDeleteEngine={deleteEngine}
/>
</EuiPageContentBody>
</>
Expand Down
Loading

0 comments on commit cceed8d

Please sign in to comment.