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

Fixed UI/UX issues: alerts delete confirmation, combobox behaviors #60703

Merged
merged 9 commits into from
Mar 21, 2020

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { EuiConfirmModal, EuiOverlayMask } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { HttpSetup } from 'kibana/public';
import { useAppDependencies } from '../app_context';

export const DeleteModalConfirmation = ({
idsToDelete,
apiDeleteCall,
onDeleted,
onCancel,
singleTitle,
multipleTitle,
}: {
idsToDelete: string[];
apiDeleteCall: ({
ids,
http,
}: {
ids: string[];
http: HttpSetup;
}) => Promise<{ successes: string[]; errors: string[] }>;
onDeleted: (deleted: string[]) => void;
onCancel: () => void;
singleTitle: string;
multipleTitle: string;
}) => {
const { http, toastNotifications } = useAppDependencies();
const numIdsToDelete = idsToDelete.length;
if (!numIdsToDelete) {
return null;
}
const confirmModalText = i18n.translate(
'xpack.triggersActionsUI.deleteSelectedIdsConfirmModal.descriptionText',
{
defaultMessage:
"You can't recover {numIdsToDelete, plural, one {a deleted {singleTitle}} other {deleted {multipleTitle}}}.",
values: { numIdsToDelete, singleTitle, multipleTitle },
}
);
const confirmButtonText = i18n.translate(
'xpack.triggersActionsUI.deleteSelectedIdsConfirmModal.deleteButtonLabel',
{
defaultMessage:
'Delete {numIdsToDelete, plural, one {{singleTitle}} other {# {multipleTitle}}} ',
values: { numIdsToDelete, singleTitle, multipleTitle },
}
);
const cancelButtonText = i18n.translate(
'xpack.triggersActionsUI.deleteSelectedIdsConfirmModal.cancelButtonLabel',
{
defaultMessage: 'Cancel',
}
);
return (
<EuiOverlayMask>
<EuiConfirmModal
buttonColor="danger"
data-test-subj="deleteIdsConfirmation"
title={confirmButtonText}
onCancel={() => onCancel()}
onConfirm={async () => {
const { successes, errors } = await apiDeleteCall({ ids: idsToDelete, http });
const numSuccesses = successes.length;
const numErrors = errors.length;
onDeleted(successes);
if (numSuccesses > 0) {
toastNotifications.addSuccess(
i18n.translate(
'xpack.triggersActionsUI.components.deleteSelectedIdsSuccessNotification.descriptionText',
{
defaultMessage:
'Deleted {numSuccesses, number} {numSuccesses, plural, one {{singleTitle}} other {{multipleTitle}}}',
values: { numSuccesses, singleTitle, multipleTitle },
}
)
);
}

if (numErrors > 0) {
toastNotifications.addDanger(
i18n.translate(
'xpack.triggersActionsUI.components.deleteSelectedIdsErrorNotification.descriptionText',
{
defaultMessage:
'Failed to delete {numErrors, number} {numErrors, plural, one {{singleTitle}} other {{multipleTitle}}}',
values: { numErrors, singleTitle, multipleTitle },
}
)
);
}
}}
cancelButtonText={cancelButtonText}
confirmButtonText={confirmButtonText}
>
{confirmModalText}
</EuiConfirmModal>
</EuiOverlayMask>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { Alert, AlertType } from '../../types';
import { httpServiceMock } from '../../../../../../src/core/public/mocks';
import {
createAlert,
deleteAlert,
deleteAlerts,
disableAlerts,
enableAlerts,
Expand Down Expand Up @@ -347,24 +346,11 @@ describe('loadAlerts', () => {
});
});

describe('deleteAlert', () => {
test('should call delete API for alert', async () => {
const id = '1';
const result = await deleteAlert({ http, id });
expect(result).toEqual(undefined);
expect(http.delete.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"/api/alert/1",
]
`);
});
});

describe('deleteAlerts', () => {
test('should call delete API for each alert', async () => {
const ids = ['1', '2', '3'];
const result = await deleteAlerts({ http, ids });
expect(result).toEqual(undefined);
expect(result).toEqual({ errors: [], successes: [undefined, undefined, undefined] });
expect(http.delete.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,24 @@ export async function loadAlerts({
});
}

export async function deleteAlert({ id, http }: { id: string; http: HttpSetup }): Promise<void> {
await http.delete(`${BASE_ALERT_API_PATH}/${id}`);
}

export async function deleteAlerts({
ids,
http,
}: {
ids: string[];
http: HttpSetup;
}): Promise<void> {
await Promise.all(ids.map(id => deleteAlert({ http, id })));
}): Promise<{ successes: string[]; errors: string[] }> {
const successes: string[] = [];
const errors: string[] = [];
await Promise.all(ids.map(id => http.delete(`${BASE_ALERT_API_PATH}/${id}`))).then(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if one delete fails, they will all report as failing. Seems like this needs to be restructured so that the delete's are wrapped in a function that never fails, but internally, it updates successes and errors, so that it can report a combination of successes and failures.

Seems survivable for 7.7, and should probably just be a new tech debt issue.

function(fulfilled) {
successes.push(...fulfilled);
},
function(rejected) {
errors.push(...rejected);
}
);
return { successes, errors };
}

export async function createAlert({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import {
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { useAppDependencies } from '../../../app_context';
import { loadAllActions, loadActionTypes } from '../../../lib/action_connector_api';
import { loadAllActions, loadActionTypes, deleteActions } from '../../../lib/action_connector_api';
import { ConnectorAddFlyout, ConnectorEditFlyout } from '../../action_connector_form';
import { hasDeleteActionsCapability, hasSaveActionsCapability } from '../../../lib/capabilities';
import { DeleteConnectorsModal } from '../../../components/delete_connectors_modal';
import { DeleteModalConfirmation } from '../../../components/delete_modal_confirmation';
import { ActionsConnectorsContextProvider } from '../../../context/actions_connectors_context';
import { checkActionTypeEnabled } from '../../../lib/check_action_type_enabled';
import './actions_connectors_list.scss';
Expand Down Expand Up @@ -378,29 +378,38 @@ export const ActionsConnectorsList: React.FunctionComponent = () => {

return (
<section data-test-subj="actionsList">
<DeleteConnectorsModal
callback={(deleted?: string[]) => {
if (deleted) {
if (selectedItems.length === 0 || selectedItems.length === deleted.length) {
const updatedActions = actions.filter(
action => action.id && !connectorsToDelete.includes(action.id)
);
setActions(updatedActions);
setSelectedItems([]);
} else {
toastNotifications.addDanger({
title: i18n.translate(
'xpack.triggersActionsUI.sections.actionsConnectorsList.failedToDeleteActionsMessage',
{ defaultMessage: 'Failed to delete action(s)' }
),
});
// Refresh the actions from the server, some actions may have beend deleted
loadActions();
}
<DeleteModalConfirmation
onDeleted={(deleted: string[]) => {
if (selectedItems.length === 0 || selectedItems.length === deleted.length) {
const updatedActions = actions.filter(
action => action.id && !connectorsToDelete.includes(action.id)
);
setActions(updatedActions);
setSelectedItems([]);
}
setConnectorsToDelete([]);
}}
connectorsToDelete={connectorsToDelete}
onCancel={async () => {
toastNotifications.addDanger({
title: i18n.translate(
'xpack.triggersActionsUI.sections.actionsConnectorsList.failedToDeleteActionsMessage',
{ defaultMessage: 'Failed to delete action(s)' }
),
});
// Refresh the actions from the server, some actions may have beend deleted
await loadActions();
setConnectorsToDelete([]);
}}
apiDeleteCall={deleteActions}
idsToDelete={connectorsToDelete}
singleTitle={i18n.translate(
'xpack.triggersActionsUI.sections.actionsConnectorsList.singleTitle',
{ defaultMessage: 'connector' }
)}
multipleTitle={i18n.translate(
'xpack.triggersActionsUI.sections.actionsConnectorsList.multipleTitle',
{ defaultMessage: 'connectors' }
)}
/>
<EuiSpacer size="m" />
{/* Render the view based on if there's data or if they can save */}
Expand Down
Loading