Skip to content

Commit

Permalink
[SIEM] Use bulk actions API when updating or deleting rules (elastic#…
Browse files Browse the repository at this point in the history
…54521)

## Summary

This PR updates the `All Rules Table` actions to use the new bulk API introduced in elastic#53543. More robust error reporting has also been added to let the user know exactly which operation has failed. Note that individual `update`/`delete` requests now also go through the bulk API as this simplifies the implementation and error handling.

Additional features:
* Adds toast error when failing to activate, deactivate or delete a rule (related elastic#54515)
* Extracted commonly used toast utility for better re-use
* Removes ability to delete `immutable` rules


##### Activate/Deactivate Before:
![bulk_activate_before](https://user-images.githubusercontent.com/2946766/72196245-0ea50300-33d4-11ea-8d49-5ebdb63db1a1.gif)
(Ignore failed requests from test env -- request count is important here)


##### Activate/Deactivate After:
![bulk_activate_after](https://user-images.githubusercontent.com/2946766/72196361-c0443400-33d4-11ea-9a42-11f66c64e925.gif)



##### Delete Before:
![bulk_delete_before](https://user-images.githubusercontent.com/2946766/72196249-149ae400-33d4-11ea-80fc-b2f7fb83245f.gif)
(Ignore failed requests from test env -- request count is important here)

##### Delete After:
![bulk_delete_after](https://user-images.githubusercontent.com/2946766/72196366-c803d880-33d4-11ea-90d8-f1917b18035f.gif)

### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

- [x] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)
- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] ~[Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
- [ ] ~This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~

### For maintainers

- [ ] ~This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
- [ ] ~This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~
  • Loading branch information
spong committed Jan 13, 2020
1 parent 93093a1 commit 62d5ecf
Show file tree
Hide file tree
Showing 17 changed files with 500 additions and 120 deletions.
Expand Up @@ -15,10 +15,10 @@ import { DEFAULT_INDEX_KEY } from '../../../common/constants';
import { getIndexPatternTitleIdMapping } from '../../hooks/api/helpers';
import { useIndexPatterns } from '../../hooks/use_index_patterns';
import { Loader } from '../loader';
import { useStateToaster } from '../toasters';
import { displayErrorToast, useStateToaster } from '../toasters';
import { Embeddable } from './embeddable';
import { EmbeddableHeader } from './embeddable_header';
import { createEmbeddable, displayErrorToast } from './embedded_map_helpers';
import { createEmbeddable } from './embedded_map_helpers';
import { IndexPatternsMissingPrompt } from './index_patterns_missing_prompt';
import { MapToolTip } from './map_tool_tip/map_tool_tip';
import * as i18n from './translations';
Expand Down Expand Up @@ -134,7 +134,7 @@ export const EmbeddedMapComponent = ({
}
} catch (e) {
if (isSubscribed) {
displayErrorToast(i18n.ERROR_CREATING_EMBEDDABLE, e.message, dispatchToaster);
displayErrorToast(i18n.ERROR_CREATING_EMBEDDABLE, [e.message], dispatchToaster);
setIsError(true);
}
}
Expand Down
Expand Up @@ -4,19 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { createEmbeddable, displayErrorToast } from './embedded_map_helpers';
import { createEmbeddable } from './embedded_map_helpers';
import { createUiNewPlatformMock } from 'ui/new_platform/__mocks__/helpers';
import { createPortalNode } from 'react-reverse-portal';

jest.mock('ui/new_platform');

jest.mock('uuid', () => {
return {
v1: jest.fn(() => '27261ae0-0bbb-11ea-b0ea-db767b07ea47'),
v4: jest.fn(() => '9e1f72a9-7c73-4b7f-a562-09940f7daf4a'),
};
});

const { npStart } = createUiNewPlatformMock();
npStart.plugins.embeddable.getEmbeddableFactory = jest.fn().mockImplementation(() => ({
createFromState: () => ({
Expand All @@ -25,24 +18,6 @@ npStart.plugins.embeddable.getEmbeddableFactory = jest.fn().mockImplementation((
}));

describe('embedded_map_helpers', () => {
describe('displayErrorToast', () => {
test('dispatches toast with correct title and message', () => {
const mockToast = {
toast: {
color: 'danger',
errors: ['message'],
iconType: 'alert',
id: '9e1f72a9-7c73-4b7f-a562-09940f7daf4a',
title: 'Title',
},
type: 'addToaster',
};
const dispatchToasterMock = jest.fn();
displayErrorToast('Title', 'message', dispatchToasterMock);
expect(dispatchToasterMock.mock.calls[0][0]).toEqual(mockToast);
});
});

describe('createEmbeddable', () => {
test('attaches refresh action', async () => {
const setQueryMock = jest.fn();
Expand Down
Expand Up @@ -7,7 +7,6 @@
import uuid from 'uuid';
import React from 'react';
import { OutPortal, PortalNode } from 'react-reverse-portal';
import { ActionToaster, AppToast } from '../toasters';
import { ViewMode } from '../../../../../../../src/legacy/core_plugins/embeddable_api/public/np_ready/public';
import {
IndexPatternMapping,
Expand All @@ -22,31 +21,6 @@ import { MAP_SAVED_OBJECT_TYPE } from '../../../../maps/common/constants';
import * as i18n from './translations';
import { Query, esFilters } from '../../../../../../../src/plugins/data/public';

/**
* Displays an error toast for the provided title and message
*
* @param errorTitle Title of error to display in toaster and modal
* @param errorMessage Message to display in error modal when clicked
* @param dispatchToaster provided by useStateToaster()
*/
export const displayErrorToast = (
errorTitle: string,
errorMessage: string,
dispatchToaster: React.Dispatch<ActionToaster>
) => {
const toast: AppToast = {
id: uuid.v4(),
title: errorTitle,
color: 'danger',
iconType: 'alert',
errors: [errorMessage],
};
dispatchToaster({
type: 'addToaster',
toast,
});
};

/**
* Creates MapEmbeddable with provided initial configuration
*
Expand Down
Expand Up @@ -8,7 +8,20 @@ import { cloneDeep, set } from 'lodash/fp';
import { mount } from 'enzyme';
import React, { useEffect } from 'react';

import { AppToast, useStateToaster, ManageGlobalToaster, GlobalToaster } from '.';
import {
AppToast,
useStateToaster,
ManageGlobalToaster,
GlobalToaster,
displayErrorToast,
} from '.';

jest.mock('uuid', () => {
return {
v1: jest.fn(() => '27261ae0-0bbb-11ea-b0ea-db767b07ea47'),
v4: jest.fn(() => '9e1f72a9-7c73-4b7f-a562-09940f7daf4a'),
};
});

const mockToast: AppToast = {
color: 'danger',
Expand Down Expand Up @@ -270,4 +283,22 @@ describe('Toaster', () => {
expect(wrapper.find('.euiToastHeader__title').text()).toBe('Test & Test');
});
});

describe('displayErrorToast', () => {
test('dispatches toast with correct title and message', () => {
const mockErrorToast = {
toast: {
color: 'danger',
errors: ['message'],
iconType: 'alert',
id: '9e1f72a9-7c73-4b7f-a562-09940f7daf4a',
title: 'Title',
},
type: 'addToaster',
};
const dispatchToasterMock = jest.fn();
displayErrorToast('Title', ['message'], dispatchToasterMock);
expect(dispatchToasterMock.mock.calls[0][0]).toEqual(mockErrorToast);
});
});
});
26 changes: 26 additions & 0 deletions x-pack/legacy/plugins/siem/public/components/toasters/index.tsx
Expand Up @@ -8,6 +8,7 @@ import { EuiGlobalToastList, EuiGlobalToastListToast as Toast, EuiButton } from
import { noop } from 'lodash/fp';
import React, { createContext, Dispatch, useReducer, useContext, useState } from 'react';
import styled from 'styled-components';
import uuid from 'uuid';

import { ModalAllErrors } from './modal_all_errors';
import * as i18n from './translations';
Expand Down Expand Up @@ -122,3 +123,28 @@ const ErrorToastContainer = styled.div`
`;

ErrorToastContainer.displayName = 'ErrorToastContainer';

/**
* Displays an error toast for the provided title and message
*
* @param errorTitle Title of error to display in toaster and modal
* @param errorMessages Message to display in error modal when clicked
* @param dispatchToaster provided by useStateToaster()
*/
export const displayErrorToast = (
errorTitle: string,
errorMessages: string[],
dispatchToaster: React.Dispatch<ActionToaster>
) => {
const toast: AppToast = {
id: uuid.v4(),
title: errorTitle,
color: 'danger',
iconType: 'alert',
errors: errorMessages,
};
dispatchToaster({
type: 'addToaster',
toast,
});
};
Expand Up @@ -16,6 +16,7 @@ import {
Rule,
FetchRuleProps,
BasicFetchProps,
RuleError,
} from './types';
import { throwIfNotOk } from '../../../hooks/api/api';
import {
Expand Down Expand Up @@ -122,50 +123,50 @@ export const fetchRuleById = async ({ id, signal }: FetchRuleProps): Promise<Rul
*
* @param ids array of Rule ID's (not rule_id) to enable/disable
* @param enabled to enable or disable
*
* @throws An error if response is not OK
*/
export const enableRules = async ({ ids, enabled }: EnableRulesProps): Promise<Rule[]> => {
const requests = ids.map(id =>
fetch(`${chrome.getBasePath()}${DETECTION_ENGINE_RULES_URL}`, {
const response = await fetch(
`${chrome.getBasePath()}${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
{
method: 'PUT',
credentials: 'same-origin',
headers: {
'content-type': 'application/json',
'kbn-xsrf': 'true',
},
body: JSON.stringify({ id, enabled }),
})
body: JSON.stringify(ids.map(id => ({ id, enabled }))),
}
);

const responses = await Promise.all(requests);
await responses.map(response => throwIfNotOk(response));
return Promise.all(
responses.map<Promise<Rule>>(response => response.json())
);
await throwIfNotOk(response);
return response.json();
};

/**
* Deletes provided Rule ID's
*
* @param ids array of Rule ID's (not rule_id) to delete
*
* @throws An error if response is not OK
*/
export const deleteRules = async ({ ids }: DeleteRulesProps): Promise<Rule[]> => {
// TODO: Don't delete if immutable!
const requests = ids.map(id =>
fetch(`${chrome.getBasePath()}${DETECTION_ENGINE_RULES_URL}?id=${id}`, {
export const deleteRules = async ({ ids }: DeleteRulesProps): Promise<Array<Rule | RuleError>> => {
const response = await fetch(
`${chrome.getBasePath()}${DETECTION_ENGINE_RULES_URL}/_bulk_delete`,
{
method: 'DELETE',
credentials: 'same-origin',
headers: {
'content-type': 'application/json',
'kbn-xsrf': 'true',
},
})
body: JSON.stringify(ids.map(id => ({ id }))),
}
);

const responses = await Promise.all(requests);
await responses.map(response => throwIfNotOk(response));
return Promise.all(
responses.map<Promise<Rule>>(response => response.json())
);
await throwIfNotOk(response);
return response.json();
};

/**
Expand Down
Expand Up @@ -78,9 +78,11 @@ export const RuleSchema = t.intersection([
updated_by: t.string,
}),
t.partial({
output_index: t.string,
saved_id: t.string,
timeline_id: t.string,
timeline_title: t.string,
version: t.number,
}),
]);

Expand All @@ -89,6 +91,16 @@ export const RulesSchema = t.array(RuleSchema);
export type Rule = t.TypeOf<typeof RuleSchema>;
export type Rules = t.TypeOf<typeof RulesSchema>;

export interface RuleError {
rule_id: string;
error: { status_code: number; message: string };
}

export interface RuleResponseBuckets {
rules: Rule[];
errors: RuleError[];
}

export interface PaginationOptions {
page: number;
perPage: number;
Expand Down

0 comments on commit 62d5ecf

Please sign in to comment.