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] Add delete action to EnginesTable component #92844

Merged
merged 19 commits into from
Mar 9, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
84406e3
Add delete engine route to App Search
byronhulcher Feb 25, 2021
d780524
Add new deleteEngine listener to EnginesLogic
byronhulcher Feb 25, 2021
7d28cb0
Convert EnginesTable Manage into a proper EuiBasicTable action
byronhulcher Feb 25, 2021
8349997
Call EnginesLogic.actions.deleteEngine using new action in EnginesTable
byronhulcher Feb 25, 2021
90bfb1a
Manage action on EnginesTable should use eye icon
byronhulcher Feb 25, 2021
274cfca
Confirmation alert for delete action on EnginesTable
byronhulcher Feb 25, 2021
d62c67d
Only display manage/delete actions to users with canManageEngines
byronhulcher Feb 25, 2021
aada8f4
Add success message and reload after successful engine delete
byronhulcher Feb 25, 2021
097d26a
Jest tests for EngineTable actions
byronhulcher Feb 25, 2021
bf1ec7c
Copy change for engine delete success message
byronhulcher Mar 1, 2021
4222c97
Merge remote-tracking branch 'origin/master' into delete-engine-action
byronhulcher Mar 1, 2021
0f879eb
Fixing EnginesTable tests
byronhulcher Mar 1, 2021
c393ee8
Adding more tests for DELETE engine route
byronhulcher Mar 1, 2021
c9b37dc
engineNameLink -> EngineNameLink
byronhulcher Mar 1, 2021
e5285ab
Remove redundant test
byronhulcher Mar 1, 2021
846ae27
Convert Engine.type to enum EngineTypes
byronhulcher Mar 1, 2021
b6c66d0
Must use mountWithIntl
byronhulcher Mar 1, 2021
2b01ad3
Merge remote-tracking branch 'origin/master' into delete-engine-action
byronhulcher Mar 8, 2021
4bd7ce9
Use platinum license instead of role ability check
byronhulcher Mar 8, 2021
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 @@ -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 @@ -22,3 +22,14 @@ export const CREATE_AN_ENGINE_BUTTON_LABEL = i18n.translate(
defaultMessage: 'Create an engine',
}
);

export const DELETE_ENGINE_MESSAGE = (engineName: string) =>
i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.table.action.delete.successMessage',
{
defaultMessage: 'Successfully deleted "{engineName}"',
values: {
engineName,
},
}
);
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 @@ -37,6 +37,7 @@ import './engines_overview.scss';

export const EnginesOverview: React.FC = () => {
const { hasPlatinumLicense } = useValues(LicensingLogic);

const {
dataLoading,
engines,
Expand All @@ -46,9 +47,14 @@ export const EnginesOverview: React.FC = () => {
metaEnginesMeta,
metaEnginesLoading,
} = useValues(EnginesLogic);
const { loadEngines, loadMetaEngines, onEnginesPagination, onMetaEnginesPagination } = useActions(
EnginesLogic
);

const {
deleteEngine,
loadEngines,
loadMetaEngines,
onEnginesPagination,
onMetaEnginesPagination,
} = useActions(EnginesLogic);
JasonStoltz marked this conversation as resolved.
Show resolved Hide resolved

useEffect(() => {
loadEngines();
Expand Down Expand Up @@ -97,6 +103,7 @@ export const EnginesOverview: React.FC = () => {
hidePerPageOptions: true,
}}
onChange={handlePageChange(onEnginesPagination)}
onDeleteEngine={deleteEngine}
/>
</EuiPageContentBody>

Expand All @@ -119,6 +126,7 @@ export const EnginesOverview: React.FC = () => {
hidePerPageOptions: true,
}}
onChange={handlePageChange(onMetaEnginesPagination)}
onDeleteEngine={deleteEngine}
/>
</EuiPageContentBody>
</>
Expand Down
Loading