[Search Session][Management] Rename "cancel" button and delete "Reload" button (#90015)

* Rename management button to "delete"

* fix jest

* Delete reload action from management

* Added both cancel and delete session

* Improve texts

* fix test

* ts

* doc

* fix jest
This commit is contained in:
Liza Katz 2021-02-04 16:57:26 +02:00 committed by GitHub
parent f3a9c763df
commit b5ce8ba26f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 137 additions and 118 deletions

View file

@ -40,5 +40,6 @@ export function createSearchRequestHandlerContext() {
updateSession: jest.fn(),
extendSession: jest.fn(),
cancelSession: jest.fn(),
deleteSession: jest.fn(),
};
}

View file

@ -307,9 +307,8 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
return strategy.extend(id, keepAlive, options, deps);
};
private cancelSession = async (deps: SearchStrategyDependencies, sessionId: string) => {
private cancelSessionSearches = async (deps: SearchStrategyDependencies, sessionId: string) => {
const searchIdMapping = await deps.searchSessionsClient.getSearchIdMapping(sessionId);
const response = await deps.searchSessionsClient.cancel(sessionId);
for (const [searchId, strategyName] of searchIdMapping.entries()) {
const searchOptions = {
@ -319,10 +318,19 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
};
this.cancel(deps, searchId, searchOptions);
}
};
private cancelSession = async (deps: SearchStrategyDependencies, sessionId: string) => {
const response = await deps.searchSessionsClient.cancel(sessionId);
this.cancelSessionSearches(deps, sessionId);
return response;
};
private deleteSession = async (deps: SearchStrategyDependencies, sessionId: string) => {
this.cancelSessionSearches(deps, sessionId);
return deps.searchSessionsClient.delete(sessionId);
};
private extendSession = async (
deps: SearchStrategyDependencies,
sessionId: string,
@ -372,6 +380,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
updateSession: searchSessionsClient.update,
extendSession: this.extendSession.bind(this, deps),
cancelSession: this.cancelSession.bind(this, deps),
deleteSession: this.deleteSession.bind(this, deps),
};
};
};

View file

@ -21,5 +21,6 @@ export function createSearchSessionsClientMock<T = unknown>(): jest.Mocked<
update: jest.fn(),
cancel: jest.fn(),
extend: jest.fn(),
delete: jest.fn(),
};
}

View file

@ -41,6 +41,9 @@ export class SearchSessionService implements ISearchSessionService {
cancel: async () => {
throw new Error('cancel not implemented in OSS search session service');
},
delete: async () => {
throw new Error('delete not implemented in OSS search session service');
},
});
}
}

View file

@ -29,6 +29,7 @@ export interface IScopedSearchSessionsClient<T = unknown> {
find: (options: Omit<SavedObjectsFindOptions, 'type'>) => Promise<SavedObjectsFindResponse<T>>;
update: (sessionId: string, attributes: Partial<T>) => Promise<SavedObjectsUpdateResponse<T>>;
cancel: (sessionId: string) => Promise<{}>;
delete: (sessionId: string) => Promise<{}>;
extend: (sessionId: string, expires: Date) => Promise<SavedObjectsUpdateResponse<T>>;
}

View file

@ -92,6 +92,7 @@ export interface IScopedSearchClient extends ISearchClient {
findSessions: IScopedSearchSessionsClient['find'];
updateSession: IScopedSearchSessionsClient['update'];
cancelSession: IScopedSearchSessionsClient['cancel'];
deleteSession: IScopedSearchSessionsClient['delete'];
extendSession: IScopedSearchSessionsClient['extend'];
}

View file

@ -1254,6 +1254,7 @@ export class SearchSessionService implements ISearchSessionService {
update: () => Promise<never>;
extend: () => Promise<never>;
cancel: () => Promise<never>;
delete: () => Promise<never>;
};
}
@ -1430,7 +1431,7 @@ export function usageProvider(core: CoreSetup_2): SearchUsage;
// src/plugins/data/server/index.ts:266:1 - (ae-forgotten-export) The symbol "calcAutoIntervalLessThan" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index_patterns/index_patterns_service.ts:59:14 - (ae-forgotten-export) The symbol "IndexPatternsService" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/plugin.ts:79:74 - (ae-forgotten-export) The symbol "DataEnhancements" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/search/types.ts:113:5 - (ae-forgotten-export) The symbol "ISearchStartSearchSource" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/search/types.ts:114:5 - (ae-forgotten-export) The symbol "ISearchStartSearchSource" needs to be exported by the entry point index.d.ts
// (No @packageDocumentation comment for this package)

View file

@ -13,31 +13,31 @@ import { SearchSessionsMgmtAPI } from '../../lib/api';
import { TableText } from '../';
import { OnActionComplete } from './types';
interface CancelButtonProps {
interface DeleteButtonProps {
id: string;
name: string;
api: SearchSessionsMgmtAPI;
onActionComplete: OnActionComplete;
}
const CancelConfirm = ({
onConfirmDismiss,
const DeleteConfirm = ({
onConfirmCancel,
...props
}: CancelButtonProps & { onConfirmDismiss: () => void }) => {
}: DeleteButtonProps & { onConfirmCancel: () => void }) => {
const { id, name, api, onActionComplete } = props;
const [isLoading, setIsLoading] = useState(false);
const title = i18n.translate('xpack.data.mgmt.searchSessions.cancelModal.title', {
defaultMessage: 'Cancel search session',
defaultMessage: 'Delete search session',
});
const confirm = i18n.translate('xpack.data.mgmt.searchSessions.cancelModal.cancelButton', {
const confirm = i18n.translate('xpack.data.mgmt.searchSessions.cancelModal.deleteButton', {
defaultMessage: 'Delete',
});
const cancel = i18n.translate('xpack.data.mgmt.searchSessions.cancelModal.cancelButton', {
defaultMessage: 'Cancel',
});
const cancel = i18n.translate('xpack.data.mgmt.searchSessions.cancelModal.dontCancelButton', {
defaultMessage: 'Dismiss',
});
const message = i18n.translate('xpack.data.mgmt.searchSessions.cancelModal.message', {
defaultMessage: `Canceling the search session \'{name}\' will expire any cached results, so that quick restore will no longer be available. You will still be able to re-run it, using the reload action.`,
defaultMessage: `Deleting the search session \'{name}\' deletes all cached results.`,
values: {
name,
},
@ -47,7 +47,7 @@ const CancelConfirm = ({
<EuiOverlayMask>
<EuiConfirmModal
title={title}
onCancel={onConfirmDismiss}
onCancel={onConfirmCancel}
onConfirm={async () => {
setIsLoading(true);
await api.sendCancel(id);
@ -65,14 +65,14 @@ const CancelConfirm = ({
);
};
export const CancelButton = (props: CancelButtonProps) => {
export const DeleteButton = (props: DeleteButtonProps) => {
const [showConfirm, setShowConfirm] = useState(false);
const onClick = () => {
setShowConfirm(true);
};
const onConfirmDismiss = () => {
const onConfirmCancel = () => {
setShowConfirm(false);
};
@ -80,11 +80,11 @@ export const CancelButton = (props: CancelButtonProps) => {
<>
<TableText onClick={onClick}>
<FormattedMessage
id="xpack.data.mgmt.searchSessions.actionCancel"
defaultMessage="Cancel"
id="xpack.data.mgmt.searchSessions.actionDelete"
defaultMessage="Delete"
/>
</TableText>
{showConfirm ? <CancelConfirm {...props} onConfirmDismiss={onConfirmDismiss} /> : null}
{showConfirm ? <DeleteConfirm {...props} onConfirmCancel={onConfirmCancel} /> : null}
</>
);
};

View file

@ -10,30 +10,22 @@ import { IClickActionDescriptor } from '../';
import extendSessionIcon from '../../icons/extend_session.svg';
import { SearchSessionsMgmtAPI } from '../../lib/api';
import { UISession } from '../../types';
import { CancelButton } from './cancel_button';
import { DeleteButton } from './delete_button';
import { ExtendButton } from './extend_button';
import { ReloadButton } from './reload_button';
import { ACTION, OnActionComplete } from './types';
export const getAction = (
api: SearchSessionsMgmtAPI,
actionType: string,
{ id, name, expires, reloadUrl }: UISession,
{ id, name, expires }: UISession,
onActionComplete: OnActionComplete
): IClickActionDescriptor | null => {
switch (actionType) {
case ACTION.CANCEL:
case ACTION.DELETE:
return {
iconType: 'crossInACircleFilled',
textColor: 'default',
label: <CancelButton api={api} id={id} name={name} onActionComplete={onActionComplete} />,
};
case ACTION.RELOAD:
return {
iconType: 'refresh',
textColor: 'default',
label: <ReloadButton api={api} reloadUrl={reloadUrl} />,
label: <DeleteButton api={api} id={id} name={name} onActionComplete={onActionComplete} />,
};
case ACTION.EXTEND:

View file

@ -90,7 +90,7 @@ export const PopoverActionsMenu = ({ api, onActionComplete, session }: PopoverAc
// add a line above the delete action (when there are multiple)
// NOTE: Delete action MUST be the final action[] item
if (actions.length > 1 && actionType === ACTION.CANCEL) {
if (actions.length > 1 && actionType === ACTION.DELETE) {
itemSet.push({ isSeparator: true, key: 'separadorable' });
}

View file

@ -1,33 +0,0 @@
/*
* 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 { FormattedMessage } from '@kbn/i18n/react';
import React from 'react';
import { TableText } from '../';
import { SearchSessionsMgmtAPI } from '../../lib/api';
interface ReloadButtonProps {
api: SearchSessionsMgmtAPI;
reloadUrl: string;
}
export const ReloadButton = (props: ReloadButtonProps) => {
function onClick() {
props.api.reloadSearchSession(props.reloadUrl);
}
return (
<>
<TableText onClick={onClick}>
<FormattedMessage
id="xpack.data.mgmt.searchSessions.actionReload"
defaultMessage="Reload"
/>
</TableText>
</>
);
};

View file

@ -9,6 +9,5 @@ export type OnActionComplete = () => void;
export enum ACTION {
EXTEND = 'extend',
CANCEL = 'cancel',
RELOAD = 'reload',
DELETE = 'delete',
}

View file

@ -61,9 +61,8 @@ describe('Search Sessions Management API', () => {
Array [
Object {
"actions": Array [
"reload",
"extend",
"cancel",
"delete",
],
"appId": "pizza",
"created": undefined,
@ -146,7 +145,7 @@ describe('Search Sessions Management API', () => {
await api.sendCancel('abc-123-cool-session-ID');
expect(mockCoreStart.notifications.toasts.addSuccess).toHaveBeenCalledWith({
title: 'The search session was canceled and expired.',
title: 'The search session was deleted.',
});
});
@ -162,37 +161,11 @@ describe('Search Sessions Management API', () => {
expect(mockCoreStart.notifications.toasts.addError).toHaveBeenCalledWith(
new Error('implementation is so bad'),
{ title: 'Failed to cancel the search session!' }
{ title: 'Failed to delete the search session!' }
);
});
});
describe('reload', () => {
beforeEach(() => {
sessionsClient.find = jest.fn().mockImplementation(async () => {
return {
saved_objects: [
{
id: 'hello-pizza-123',
attributes: { name: 'Veggie', appId: 'pizza', status: SearchSessionStatus.COMPLETE },
},
],
} as SavedObjectsFindResponse;
});
});
test('send cancel calls the cancel endpoint with a session ID', async () => {
const api = new SearchSessionsMgmtAPI(sessionsClient, mockConfig, {
urls: mockUrls,
notifications: mockCoreStart.notifications,
application: mockCoreStart.application,
});
await api.reloadSearchSession('www.myurl.com');
expect(mockCoreStart.application.navigateToUrl).toHaveBeenCalledWith('www.myurl.com');
});
});
describe('extend', () => {
beforeEach(() => {
sessionsClient.find = jest.fn().mockImplementation(async () => {

View file

@ -21,10 +21,9 @@ type UrlGeneratorsStart = SharePluginStart['urlGenerators'];
function getActions(status: SearchSessionStatus) {
const actions: ACTION[] = [];
actions.push(ACTION.RELOAD);
if (status === SearchSessionStatus.IN_PROGRESS || status === SearchSessionStatus.COMPLETE) {
actions.push(ACTION.EXTEND);
actions.push(ACTION.CANCEL);
actions.push(ACTION.DELETE);
}
return actions;
}
@ -162,8 +161,8 @@ export class SearchSessionsMgmtAPI {
await this.sessionsClient.delete(id);
this.deps.notifications.toasts.addSuccess({
title: i18n.translate('xpack.data.mgmt.searchSessions.api.canceled', {
defaultMessage: 'The search session was canceled and expired.',
title: i18n.translate('xpack.data.mgmt.searchSessions.api.deleted', {
defaultMessage: 'The search session was deleted.',
}),
});
} catch (err) {
@ -171,8 +170,8 @@ export class SearchSessionsMgmtAPI {
console.error(err);
this.deps.notifications.toasts.addError(err, {
title: i18n.translate('xpack.data.mgmt.searchSessions.api.cancelError', {
defaultMessage: 'Failed to cancel the search session!',
title: i18n.translate('xpack.data.mgmt.searchSessions.api.deletedError', {
defaultMessage: 'Failed to delete the search session!',
}),
});
}

View file

@ -16,6 +16,13 @@ import type {
import { dataPluginMock } from '../../../../../src/plugins/data/server/mocks';
import { registerSessionRoutes } from './session';
enum PostHandlerIndex {
SAVE,
FIND,
CANCEL,
EXTEND,
}
describe('registerSessionRoutes', () => {
let mockCoreSetup: MockedKeys<CoreSetup<{}, DataPluginStart>>;
let mockContext: jest.Mocked<DataRequestHandlerContext>;
@ -37,7 +44,7 @@ describe('registerSessionRoutes', () => {
const mockResponse = httpServerMock.createResponseFactory();
const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
const [[, saveHandler]] = mockRouter.post.mock.calls;
const [, saveHandler] = mockRouter.post.mock.calls[PostHandlerIndex.SAVE];
saveHandler(mockContext, mockRequest, mockResponse);
@ -71,7 +78,7 @@ describe('registerSessionRoutes', () => {
const mockResponse = httpServerMock.createResponseFactory();
const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
const [, [, findHandler]] = mockRouter.post.mock.calls;
const [, findHandler] = mockRouter.post.mock.calls[PostHandlerIndex.FIND];
findHandler(mockContext, mockRequest, mockResponse);
@ -89,14 +96,14 @@ describe('registerSessionRoutes', () => {
const mockResponse = httpServerMock.createResponseFactory();
const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
const [[, updateHandler]] = mockRouter.put.mock.calls;
const [, updateHandler] = mockRouter.put.mock.calls[0];
updateHandler(mockContext, mockRequest, mockResponse);
expect(mockContext.search!.updateSession).toHaveBeenCalledWith(id, body);
});
it('delete calls cancelSession with id', async () => {
it('cancel calls cancelSession with id', async () => {
const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
const params = { id };
@ -104,13 +111,28 @@ describe('registerSessionRoutes', () => {
const mockResponse = httpServerMock.createResponseFactory();
const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
const [[, deleteHandler]] = mockRouter.delete.mock.calls;
const [, cancelHandler] = mockRouter.post.mock.calls[PostHandlerIndex.CANCEL];
deleteHandler(mockContext, mockRequest, mockResponse);
cancelHandler(mockContext, mockRequest, mockResponse);
expect(mockContext.search!.cancelSession).toHaveBeenCalledWith(id);
});
it('delete calls deleteSession with id', async () => {
const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
const params = { id };
const mockRequest = httpServerMock.createKibanaRequest({ params });
const mockResponse = httpServerMock.createResponseFactory();
const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
const [, deleteHandler] = mockRouter.delete.mock.calls[0];
await deleteHandler(mockContext, mockRequest, mockResponse);
expect(mockContext.search!.deleteSession).toHaveBeenCalledWith(id);
});
it('extend calls extendSession with id', async () => {
const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
const expires = new Date().toISOString();
@ -121,7 +143,7 @@ describe('registerSessionRoutes', () => {
const mockResponse = httpServerMock.createResponseFactory();
const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
const [, , [, extendHandler]] = mockRouter.post.mock.calls;
const [, extendHandler] = mockRouter.post.mock.calls[PostHandlerIndex.EXTEND];
extendHandler(mockContext, mockRequest, mockResponse);

View file

@ -129,6 +129,29 @@ export function registerSessionRoutes(router: DataEnhancedPluginRouter, logger:
}),
},
},
async (context, request, res) => {
const { id } = request.params;
try {
await context.search!.deleteSession(id);
return res.ok();
} catch (e) {
const err = e.output?.payload || e;
logger.error(err);
return reportServerError(res, err);
}
}
);
router.post(
{
path: '/internal/session/{id}/cancel',
validate: {
params: schema.object({
id: schema.string(),
}),
},
},
async (context, request, res) => {
const { id } = request.params;
try {

View file

@ -226,6 +226,11 @@ export class SearchSessionService
});
};
// TODO: Throw an error if this session doesn't belong to this user
public delete = ({ savedObjectsClient }: SearchSessionDependencies, sessionId: string) => {
return savedObjectsClient.delete(SEARCH_SESSION_TYPE, sessionId);
};
/**
* Tracks the given search request/search ID in the saved session.
* @internal
@ -308,6 +313,7 @@ export class SearchSessionService
update: this.update.bind(this, deps),
extend: this.extend.bind(this, deps),
cancel: this.cancel.bind(this, deps),
delete: this.delete.bind(this, deps),
};
};
};

View file

@ -45,10 +45,29 @@ export default function ({ getService }: FtrProviderContext) {
await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200);
});
it('should fail to cancel an unknown session', async () => {
it('should fail to delete an unknown session', async () => {
await supertest.delete(`/internal/session/123`).set('kbn-xsrf', 'foo').expect(404);
});
it('should create and delete a session', async () => {
const sessionId = `my-session-${Math.random()}`;
await supertest
.post(`/internal/session`)
.set('kbn-xsrf', 'foo')
.send({
sessionId,
name: 'My Session',
appId: 'discover',
expires: '123',
urlGeneratorId: 'discover',
})
.expect(200);
await supertest.delete(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200);
await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(404);
});
it('should create and cancel a session', async () => {
const sessionId = `my-session-${Math.random()}`;
await supertest
@ -63,7 +82,10 @@ export default function ({ getService }: FtrProviderContext) {
})
.expect(200);
await supertest.delete(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200);
await supertest
.post(`/internal/session/${sessionId}/cancel`)
.set('kbn-xsrf', 'foo')
.expect(200);
const resp = await supertest
.get(`/internal/session/${sessionId}`)

View file

@ -49,11 +49,11 @@ export function SearchSessionsPageProvider({ getService, getPageObjects }: FtrPr
'[data-test-subj="sessionManagementPopoverAction-reload"]'
);
},
cancel: async () => {
log.debug('management ui: cancel the session');
delete: async () => {
log.debug('management ui: delete the session');
await actionsCell.click();
await find.clickByCssSelector(
'[data-test-subj="sessionManagementPopoverAction-cancel"]'
'[data-test-subj="sessionManagementPopoverAction-delete"]'
);
await PageObjects.common.clickConfirmOnModal();
},

View file

@ -88,15 +88,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await searchSessions.expectState('completed');
});
it('Cancels a session from management', async () => {
it('Deletes a session from management', async () => {
await PageObjects.searchSessionsManagement.goTo();
const searchSessionList = await PageObjects.searchSessionsManagement.getList();
expect(searchSessionList.length).to.be(1);
await searchSessionList[0].cancel();
await searchSessionList[0].delete();
// TODO: update this once canceling doesn't delete the object!
await retry.waitFor(`wait for list to be empty`, async function () {
const s = await PageObjects.searchSessionsManagement.getList();