Cleaned up a maybe method and fixed tests around it

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts

@@ -9,6 +9,7 @@ import {
  getEmptyFindResult,
  addPrepackagedRulesRequest,
  getFindResultWithSingleHit,
+ getAlertMock,
 } from '../__mocks__/request_responses';
 import { requestContextMock, serverMock, createMockConfig, mockGetCurrentUser } from '../__mocks__';
 import { AddPrepackagedRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema';
@@ -21,6 +22,7 @@ import { ExceptionListClient } from '../../../../../../lists/server';
 import { installPrepackagedTimelines } from '../../../timeline/routes/prepackaged_timelines/install_prepackaged_timelines';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks';
+import { getQueryRuleParams } from '../../schemas/rule_schemas.mock';
 
 jest.mock('../../rules/get_prepackaged_rules', () => {
  return {
@@ -90,6 +92,7 @@ describe('add_prepackaged_rules_route', () => {
  mockExceptionsClient = listMock.getExceptionListClient();
 
  clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit());
+ clients.rulesClient.update.mockResolvedValue(getAlertMock(getQueryRuleParams()));
 
  (installPrepackagedTimelines as jest.Mock).mockReset();
  (installPrepackagedTimelines as jest.Mock).mockResolvedValue({

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts

@@ -45,7 +45,7 @@ describe('import_rules_route', () => {
  ml = mlServicesMock.createSetupContract();
 
  clients.rulesClient.find.mockResolvedValue(getEmptyFindResult()); // no extant rules
-
+ clients.rulesClient.update.mockResolvedValue(getAlertMock(getQueryRuleParams()));
  context.core.elasticsearch.client.asCurrentUser.search.mockResolvedValue(
  elasticsearchClientMock.createSuccessTransportRequestPromise({ _shards: { total: 1 } })
  );

x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.test.ts

@@ -8,6 +8,9 @@
 import { patchRules } from './patch_rules';
 import { getPatchRulesOptionsMock, getPatchMlRulesOptionsMock } from './patch_rules.mock';
 import { PatchRulesOptions } from './types';
+import { RulesClientMock } from '../../../../../alerting/server/rules_client.mock';
+import { getAlertMock } from '../routes/__mocks__/request_responses';
+import { getQueryRuleParams } from '../schemas/rule_schemas.mock';
 
 describe('patchRules', () => {
  it('should call rulesClient.disable if the rule was enabled and enabled is false', async () => {
@@ -16,6 +19,9 @@ describe('patchRules', () => {
  ...rulesOptionsMock,
  enabled: false,
  };
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
  await patchRules(ruleOptions);
  expect(ruleOptions.rulesClient.disable).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -33,6 +39,9 @@ describe('patchRules', () => {
  if (ruleOptions.rule != null) {
  ruleOptions.rule.enabled = false;
  }
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
  await patchRules(ruleOptions);
  expect(ruleOptions.rulesClient.enable).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -50,6 +59,9 @@ describe('patchRules', () => {
  if (ruleOptions.rule != null) {
  ruleOptions.rule.enabled = false;
  }
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
  await patchRules(ruleOptions);
  expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -73,6 +85,9 @@ describe('patchRules', () => {
  if (ruleOptions.rule != null) {
  ruleOptions.rule.enabled = false;
  }
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
  await patchRules(ruleOptions);
  expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -102,6 +117,9 @@ describe('patchRules', () => {
  },
  ],
  };
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
  await patchRules(ruleOptions);
  expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -136,7 +154,9 @@ describe('patchRules', () => {
  },
  ];
  }
-
+ ((ruleOptions.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
  await patchRules(ruleOptions);
  expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith(
  expect.objectContaining({

x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts

@@ -7,7 +7,6 @@
 
 import { validate } from '@kbn/securitysolution-io-ts-utils';
 import { defaults } from 'lodash/fp';
-import { NOTIFICATION_THROTTLE_NO_ACTIONS } from '../../../../common/constants';
 import { PartialAlert } from '../../../../../alerting/server';
 import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions';
 import {
@@ -22,6 +21,7 @@ import {
  calculateInterval,
  calculateName,
  calculateVersion,
+ maybeMute,
  removeUndefined,
  transformToAlertThrottle,
  transformToNotifyWhen,
@@ -35,8 +35,6 @@ class PatchError extends Error {
  }
 }
 
-/* eslint complexity: ["error", 21]*/
-
 export const patchRules = async ({
  rulesClient,
  author,
@@ -210,15 +208,8 @@ export const patchRules = async ({
  data: validated,
  });
 
- // mutes or un-mutes the rule actions depending on the throttle
- if (rule.muteAll && throttle !== undefined && throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) {
- await rulesClient.unmuteAll({ id: update.id });
- } else if (
- !rule.muteAll &&
- throttle !== undefined &&
- throttle === NOTIFICATION_THROTTLE_NO_ACTIONS
- ) {
- await rulesClient.muteAll({ id: update.id });
+ if (throttle !== undefined) {
+ await maybeMute({ rulesClient, muteAll: rule.muteAll, throttle, id: update.id });
  }
 
  if (rule.enabled && enabled === false) {

x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.test.ts

@@ -18,6 +18,9 @@ describe('updateRules', () => {
  ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).get.mockResolvedValue(
  getAlertMock(getQueryRuleParams())
  );
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
 
  await updateRules(rulesOptionsMock);
 
@@ -36,6 +39,9 @@ describe('updateRules', () => {
  ...getAlertMock(getQueryRuleParams()),
  enabled: false,
  });
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getQueryRuleParams())
+ );
 
  await updateRules(rulesOptionsMock);
 
@@ -50,6 +56,10 @@ describe('updateRules', () => {
  const rulesOptionsMock = getUpdateMlRulesOptionsMock();
  rulesOptionsMock.ruleUpdate.enabled = true;
 
+ ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue(
+ getAlertMock(getMlRuleParams())
+ );
+
  ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).get.mockResolvedValue(
  getAlertMock(getMlRuleParams())
  );

x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.ts

@@ -7,10 +7,7 @@
 
 /* eslint-disable complexity */
 
-import {
- DEFAULT_MAX_SIGNALS,
- NOTIFICATION_THROTTLE_NO_ACTIONS,
-} from '../../../../common/constants';
+import { DEFAULT_MAX_SIGNALS } from '../../../../common/constants';
 import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions';
 import { PartialAlert } from '../../../../../alerting/server';
 import { readRules } from './read_rules';
@@ -19,7 +16,7 @@ import { addTags } from './add_tags';
 import { typeSpecificSnakeToCamel } from '../schemas/rule_converters';
 import { InternalRuleUpdate, RuleParams } from '../schemas/rule_schemas';
 import { enableRule } from './enable_rule';
-import { transformToAlertThrottle, transformToNotifyWhen } from './utils';
+import { maybeMute, transformToAlertThrottle, transformToNotifyWhen } from './utils';
 
 export const updateRules = async ({
  spaceId,
@@ -87,12 +84,12 @@ export const updateRules = async ({
  data: newInternalRule,
  });
 
- // mutes or unmutes the rule actions depending on the throttle
- if (existingRule.muteAll && ruleUpdate.throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) {
- await rulesClient.unmuteAll({ id: update.id });
- } else if (!existingRule.muteAll && ruleUpdate.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) {
- await rulesClient.muteAll({ id: update.id });
- }
+ await maybeMute({
+  rulesClient,
+ muteAll: existingRule.muteAll,
+  throttle: ruleUpdate.throttle,
+ id: update.id,
+ });
 
  if (existingRule.enabled && enabled === false) {
  await rulesClient.disable({ id: existingRule.id });

x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts

@@ -59,6 +59,7 @@ import {
  NOTIFICATION_THROTTLE_NO_ACTIONS,
  NOTIFICATION_THROTTLE_RULE,
 } from '../../../../common/constants';
+import { RulesClient } from '../../../../../alerting/server';
 
 export const calculateInterval = (
  interval: string | undefined,
@@ -228,3 +229,30 @@ export const transformFromAlertThrottle = (rule: SanitizedAlert<RuleParams>): st
  return rule.throttle;
  }
 };
+
+/**
+ * Mutes, unmutes, or does nothing to the alert if no changed is detected
+ * @param id The id of the alert to (un)mute
+ * @param rulesClient the rules client
+ * @param muteAll If the existing alert has all actions muted
+ * @param throttle If the existing alert has a throttle set
+ */
+export const maybeMute = async ({
+ id,
+ rulesClient,
+ muteAll,
+ throttle,
+}: {
+ id: SanitizedAlert['id'];
+ rulesClient: RulesClient;
+ muteAll: SanitizedAlert<RuleParams>['muteAll'];
+ throttle: string | null | undefined;
+}): Promise<void> => {
+ if (muteAll && throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) {
+ await rulesClient.unmuteAll({ id });
+ } else if (!muteAll && throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) {
+ await rulesClient.muteAll({ id });
+ } else {
+ // Do nothing, no-operation
+ }
+};