Skip to content

Commit

Permalink
[Security Solutions][Detections] - Fix bug, last response not showing…
Browse files Browse the repository at this point in the history 
… for disabled rules (#81783)

## Summary

**Bug fix addressed in this PR:**
- fixes #63203
- in the backend, we were only fetching status data for enabled rules, changed to fetch status data regardles of whether rule is enabled or disabled
  • Loading branch information
@yctercero
yctercero authored Oct 28, 2020
1 parent 0793089 commit 2788dca
Show file tree
Hide file tree
Showing 8 changed files with 243 additions and 63 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/security_solution/cypress/screens/alerts_detection_rules.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ export const RULE_CHECKBOX = '.euiTableRow .euiCheckbox__input';

export const RULE_NAME = '[data-test-subj="ruleName"]';

export const RULE_SWITCH = '[data-test-subj="rule-switch"]';
export const RULE_SWITCH = '[data-test-subj="ruleSwitch"]';

export const RULE_SWITCH_LOADER = '[data-test-subj="rule-switch-loader"]';
export const RULE_SWITCH_LOADER = '[data-test-subj="ruleSwitchLoader"]';

export const RULES_TABLE = '[data-test-subj="rules-table"]';

Expand Down
20 changes: 0 additions & 20 deletions ...solution/public/detections/components/rules/rule_switch/__snapshots__/index.test.tsx.snap
View file

This file was deleted.

167 changes: 162 additions & 5 deletions ...k/plugins/security_solution/public/detections/components/rules/rule_switch/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,173 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { shallow } from 'enzyme';
import { mount } from 'enzyme';
import React from 'react';
import { ThemeProvider } from 'styled-components';
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
import { waitFor } from '@testing-library/react';

import { enableRules } from '../../../containers/detection_engine/rules';
import { enableRulesAction } from '../../../pages/detection_engine/rules/all/actions';
import { RuleSwitchComponent } from './index';
import { getRulesSchemaMock } from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks';
import { RulesSchema } from '../../../../../common/detection_engine/schemas/response/rules_schema';
import { useStateToaster, displayErrorToast } from '../../../../common/components/toasters';

jest.mock('../../../../common/components/toasters');
jest.mock('../../../containers/detection_engine/rules');
jest.mock('../../../pages/detection_engine/rules/all/actions');

describe('RuleSwitch', () => {
test('renders correctly against snapshot', () => {
const wrapper = shallow(
<RuleSwitchComponent optionLabel="rule-switch" enabled={true} id={'7'} isLoading={false} />
beforeEach(() => {
(useStateToaster as jest.Mock).mockImplementation(() => [[], jest.fn()]);
(enableRules as jest.Mock).mockResolvedValue([getRulesSchemaMock()]);
});

afterEach(() => {
jest.clearAllMocks();
});

test('it renders loader if "isLoading" is true', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent optionLabel="rule-switch" enabled={true} id={'7'} isLoading />
</ThemeProvider>
);

expect(wrapper.find('[data-test-subj="ruleSwitchLoader"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="ruleSwitch"]').exists()).toBeFalsy();
});

test('it renders switch disabled if "isDisabled" is true', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent optionLabel="rule-switch" enabled={true} id={'7'} isDisabled />
</ThemeProvider>
);

expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props().disabled).toBeTruthy();
});

test('it renders switch enabled if "enabled" is true', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent optionLabel="rule-switch" enabled id={'7'} />
</ThemeProvider>
);
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props().checked).toBeTruthy();
});

test('it renders switch disabled if "enabled" is false', () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent optionLabel="rule-switch" enabled={false} id={'7'} />
</ThemeProvider>
);
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(0).props().checked).toBeFalsy();
});

test('it renders an off switch enabled on click', async () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent
optionLabel="rule-switch"
enabled={false}
isDisabled={false}
id={'7'}
/>
</ThemeProvider>
);
wrapper.find('[data-test-subj="ruleSwitch"]').at(2).simulate('click');

await waitFor(() => {
wrapper.update();
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(1).props().checked).toBeTruthy();
});
});

test('it renders an on switch off on click', async () => {
const rule: RulesSchema = { ...getRulesSchemaMock(), enabled: false };

(enableRules as jest.Mock).mockResolvedValue([rule]);

const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent optionLabel="rule-switch" enabled isDisabled={false} id={'7'} />
</ThemeProvider>
);
wrapper.find('[data-test-subj="ruleSwitch"]').at(2).simulate('click');

await waitFor(() => {
wrapper.update();
expect(wrapper.find('[data-test-subj="ruleSwitch"]').at(1).props().checked).toBeFalsy();
});
});

test('it dispatches error toaster if "enableRules" call rejects', async () => {
const mockError = new Error('uh oh');
(enableRules as jest.Mock).mockRejectedValue(mockError);

const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent
optionLabel="rule-switch"
enabled={false}
isDisabled={false}
id={'7'}
/>
</ThemeProvider>
);
expect(wrapper).toMatchSnapshot();
wrapper.find('[data-test-subj="ruleSwitch"]').at(2).simulate('click');

await waitFor(() => {
wrapper.update();
expect(displayErrorToast).toHaveBeenCalledTimes(1);
});
});

test('it dispatches error toaster if "enableRules" call resolves with some errors', async () => {
(enableRules as jest.Mock).mockResolvedValue([
getRulesSchemaMock(),
{ error: { status_code: 400, message: 'error' } },
{ error: { status_code: 400, message: 'error' } },
]);

const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent
optionLabel="rule-switch"
enabled={false}
isDisabled={false}
id={'7'}
/>
</ThemeProvider>
);
wrapper.find('[data-test-subj="ruleSwitch"]').at(2).simulate('click');

await waitFor(() => {
wrapper.update();
expect(displayErrorToast).toHaveBeenCalledTimes(1);
});
});

test('it invokes "enableRulesAction" if dispatch is passed through', async () => {
const wrapper = mount(
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>
<RuleSwitchComponent
optionLabel="rule-switch"
enabled
isDisabled={false}
id={'7'}
dispatch={jest.fn()}
/>
</ThemeProvider>
);
wrapper.find('[data-test-subj="ruleSwitch"]').at(2).simulate('click');

await waitFor(() => {
wrapper.update();
expect(enableRulesAction).toHaveBeenCalledTimes(1);
});
});
});
29 changes: 16 additions & 13 deletions x-pack/plugins/security_solution/public/detections/components/rules/rule_switch/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from '@elastic/eui';
import { isEmpty } from 'lodash/fp';
import styled from 'styled-components';
import React, { useCallback, useState, useEffect } from 'react';
import React, { useMemo, useCallback, useState, useEffect } from 'react';

import * as i18n from '../../../pages/detection_engine/rules/translations';
import { enableRules } from '../../../containers/detection_engine/rules';
Expand Down Expand Up @@ -63,8 +63,11 @@ export const RuleSwitchComponent = ({
if (dispatch != null) {
await enableRulesAction([id], event.target.checked!, dispatch, dispatchToaster);
} else {
const enabling = event.target.checked!;
const title = enabling
? i18n.BATCH_ACTION_ACTIVATE_SELECTED_ERROR(1)
: i18n.BATCH_ACTION_DEACTIVATE_SELECTED_ERROR(1);
try {
const enabling = event.target.checked!;
const response = await enableRules({
ids: [id],
enabled: enabling,
Expand All @@ -73,9 +76,7 @@ export const RuleSwitchComponent = ({

if (errors.length > 0) {
setMyIsLoading(false);
const title = enabling
? i18n.BATCH_ACTION_ACTIVATE_SELECTED_ERROR(1)
: i18n.BATCH_ACTION_DEACTIVATE_SELECTED_ERROR(1);

displayErrorToast(
title,
errors.map((e) => e.error.message),
Expand All @@ -88,8 +89,9 @@ export const RuleSwitchComponent = ({
onChange(rule.enabled);
}
}
} catch {
} catch (err) {
setMyIsLoading(false);
displayErrorToast(title, err.message, dispatchToaster);
}
}
setMyIsLoading(false);
Expand All @@ -105,21 +107,22 @@ export const RuleSwitchComponent = ({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [enabled]);

useEffect(() => {
const showLoader = useMemo((): boolean => {
if (myIsLoading !== isLoading) {
setMyIsLoading(isLoading ?? false);
return isLoading ?? false;
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isLoading]);

return myIsLoading;
}, [myIsLoading, isLoading]);

return (
<EuiFlexGroup alignItems="center" justifyContent="spaceAround">
<EuiFlexItem grow={false}>
{myIsLoading ? (
<EuiLoadingSpinner size="m" data-test-subj="rule-switch-loader" />
{showLoader ? (
<EuiLoadingSpinner size="m" data-test-subj="ruleSwitchLoader" />
) : (
<StaticSwitch
data-test-subj="rule-switch"
data-test-subj="ruleSwitch"
label={optionLabel ?? ''}
showLabel={!isEmpty(optionLabel)}
disabled={isDisabled}
Expand Down
7 changes: 1 addition & 6 deletions ...gins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,7 @@ export const patchRulesBulkRoute = (router: IRouter, ml: SetupPlugins['ml']) =>
search: rule.id,
searchFields: ['alertId'],
});
return transformValidateBulkError(
rule.id,
rule,
ruleActions,
ruleStatuses.saved_objects[0]
);
return transformValidateBulkError(rule.id, rule, ruleActions, ruleStatuses);
} else {
return getIdBulkError({ id, ruleId });
}
Expand Down
8 changes: 1 addition & 7 deletions ...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const updateRulesBulkRoute = (router: IRouter, ml: SetupPlugins['ml']) =>
if (!siemClient || !alertsClient) {
return siemResponse.error({ statusCode: 404 });
}

const mlAuthz = buildMlAuthz({ license: context.licensing.license, ml, request });
const ruleStatusClient = ruleStatusSavedObjectsClientFactory(savedObjectsClient);
const rules = await Promise.all(
Expand Down Expand Up @@ -192,12 +191,7 @@ export const updateRulesBulkRoute = (router: IRouter, ml: SetupPlugins['ml']) =>
search: rule.id,
searchFields: ['alertId'],
});
return transformValidateBulkError(
rule.id,
rule,
ruleActions,
ruleStatuses.saved_objects[0]
);
return transformValidateBulkError(rule.id, rule, ruleActions, ruleStatuses);
} else {
return getIdBulkError({ id, ruleId });
}
Expand Down
Loading

0 comments on commit 2788dca

Please sign in to comment.