[Security Solution] Rule bulk schedule fixes and test coverage expansion (#141604)

## Summary Fixes issues, nits and [expands test coverage](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#) for PR: https://github.com/elastic/kibana/pull/140166 - Extends definition of `TimeUnit` type and its tests - Adds e2e test to test default values of Bulk Schedule flyout - Corrects copy as reported by @elastic/security-docs - Corrects validation for Interval field when editing rule schedule individually ### Checklist Delete any items that are not applicable to this PR. - [ ] 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/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
2025-04-23 17:28:26 -04:00 · 2022-09-27 16:01:23 +02:00 · 2022-09-27 16:01:23 +02:00 · 9738b04524
commit 9738b04524
parent e24ca7a89b
9 changed files with 66 additions and 15 deletions
--- a/packages/kbn-securitysolution-io-ts-types/src/time_duration/index.test.ts
+++ b/packages/kbn-securitysolution-io-ts-types/src/time_duration/index.test.ts
@ -11,7 +11,7 @@ import { left } from 'fp-ts/lib/Either';
 import { TimeDuration } from '.';
 import { foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts-utils';

-describe('time_unit', () => {
+describe('TimeDuration', () => {
  test('it should validate a correctly formed TimeDuration with time unit of seconds', () => {
    const payload = '1s';
    const decoded = TimeDuration.decode(payload);
@ -39,6 +39,17 @@ describe('time_unit', () => {
    expect(message.schema).toEqual(payload);
  });

+  test('it should NOT validate a TimeDuration of 0 length', () => {
+    const payload = '0s';
+    const decoded = TimeDuration.decode(payload);
+    const message = pipe(decoded, foldLeftRight);
+
+    expect(getPaths(left(message.errors))).toEqual([
+      'Invalid value "0s" supplied to "TimeDuration"',
+    ]);
+    expect(message.schema).toEqual({});
+  });
+
  test('it should NOT validate a negative TimeDuration', () => {
    const payload = '-10s';
    const decoded = TimeDuration.decode(payload);
@ -50,6 +61,17 @@ describe('time_unit', () => {
    expect(message.schema).toEqual({});
  });

+  test('it should NOT validate a decimal TimeDuration', () => {
+    const payload = '1.5s';
+    const decoded = TimeDuration.decode(payload);
+    const message = pipe(decoded, foldLeftRight);
+
+    expect(getPaths(left(message.errors))).toEqual([
+      'Invalid value "1.5s" supplied to "TimeDuration"',
+    ]);
+    expect(message.schema).toEqual({});
+  });
+
  test('it should NOT validate a TimeDuration with some other time unit', () => {
    const payload = '10000000w';
    const decoded = TimeDuration.decode(payload);
--- a/packages/kbn-securitysolution-io-ts-types/src/time_duration/index.ts
+++ b/packages/kbn-securitysolution-io-ts-types/src/time_duration/index.ts
@ -21,8 +21,12 @@ export const TimeDuration = new t.Type<string, string, unknown>(
    if (typeof input === 'string' && input.trim() !== '') {
      try {
        const inputLength = input.length;
-        const time = parseInt(input.trim().substring(0, inputLength - 1), 10);
        const unit = input.trim().at(-1);
+        const time = parseFloat(input.trim().substring(0, inputLength - 1));
+
+        if (!Number.isInteger(time)) {
+          return t.failure(input, context);
+        }
        if (
          time >= 1 &&
          Number.isSafeInteger(time) &&
--- a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts
+++ b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts
@ -71,6 +71,7 @@ import {
  setScheduleIntervalTimeUnit,
  assertRuleScheduleValues,
  assertUpdateScheduleWarningExists,
+  assertDefaultValuesAreAppliedToScheduleFields,
 } from '../../tasks/rules_bulk_edit';

 import { hasIndexPatterns, getDetails } from '../../tasks/rule_details';
@ -493,6 +494,18 @@ describe('Detection rules, bulk edit', () => {
  });

  describe('Schedule', () => {
+    it('Default values are applied to bulk edit schedule fields', () => {
+      selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited);
+      clickUpdateScheduleMenuItem();
+
+      assertUpdateScheduleWarningExists(expectedNumberOfCustomRulesToBeEdited);
+
+      assertDefaultValuesAreAppliedToScheduleFields({
+        interval: 5,
+        lookback: 1,
+      });
+    });
+
    it('Updates schedule for custom rules', () => {
      selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited);
      clickUpdateScheduleMenuItem();
--- a/x-pack/plugins/security_solution/cypress/screens/rules_bulk_edit.ts
+++ b/x-pack/plugins/security_solution/cypress/screens/rules_bulk_edit.ts
@ -56,4 +56,6 @@ export const UPDATE_SCHEDULE_INTERVAL_INPUT =
 export const UPDATE_SCHEDULE_LOOKBACK_INPUT =
  '[data-test-subj="bulkEditRulesScheduleLookbackSelector"]';

+export const UPDATE_SCHEDULE_TIME_INTERVAL = '[data-test-subj="interval"]';
+
 export const UPDATE_SCHEDULE_TIME_UNIT_SELECT = '[data-test-subj="timeType"]';
--- a/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts
+++ b/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts
@ -193,6 +193,19 @@ export const typeScheduleLookback = (lookback: string) => {
    .blur();
 };

+interface ScheduleFormFields {
+  interval: number;
+  lookback: number;
+}
+
+export const assertDefaultValuesAreAppliedToScheduleFields = ({
+  interval,
+  lookback,
+}: ScheduleFormFields) => {
+  cy.get(UPDATE_SCHEDULE_INTERVAL_INPUT).find('input').should('have.value', interval);
+  cy.get(UPDATE_SCHEDULE_LOOKBACK_INPUT).find('input').should('have.value', lookback);
+};
+
 type TimeUnit = 'Seconds' | 'Minutes' | 'Hours';
 export const setScheduleIntervalTimeUnit = (timeUnit: TimeUnit) => {
  cy.get(UPDATE_SCHEDULE_INTERVAL_INPUT).within(() => {
@ -209,17 +222,15 @@ export const setScheduleLookbackTimeUnit = (timeUnit: TimeUnit) => {
 export const assertUpdateScheduleWarningExists = (expectedNumberOfNotMLRules: number) => {
  cy.get(RULES_BULK_EDIT_SCHEDULES_WARNING).should(
    'have.text',
-    `You're about to apply changes to ${expectedNumberOfNotMLRules} selected rules. The changes you made will be overwritten to the existing Rule schedules and additional look-back time (if any).`
+    `You're about to apply changes to ${expectedNumberOfNotMLRules} selected rules. The changes you make will overwrite the existing rule schedules and additional look-back time (if any).`
  );
 };
-
-export const assertRuleScheduleValues = ({
-  interval,
-  lookback,
-}: {
+interface RuleSchedule {
  interval: string;
  lookback: string;
-}) => {
+}
+
+export const assertRuleScheduleValues = ({ interval, lookback }: RuleSchedule) => {
  cy.get(SCHEDULE_DETAILS).within(() => {
    cy.get('dd').eq(0).should('contain.text', interval);
    cy.get('dd').eq(1).should('contain.text', lookback);
--- a/x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx
+++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_schedule_rule/index.tsx
@ -95,6 +95,7 @@ const StepScheduleRuleComponent: FC<StepScheduleRuleProps> = ({
              idAria: 'detectionEngineStepScheduleRuleInterval',
              isDisabled: isLoading,
              dataTestSubj: 'detectionEngineStepScheduleRuleInterval',
+              minimumValue: 1,
            }}
          />
          <UseField
--- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/translations.tsx
+++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/translations.tsx
@ -129,7 +129,7 @@ export const bulkSetSchedule = {
  warningCalloutMessage: (rulesCount: number): JSX.Element => (
    <FormattedMessage
      id="xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.setSchedule.warningCalloutMessage"
-      defaultMessage="You're about to apply changes to {rulesCount, plural, one {# selected rule} other {# selected rules}}. The changes you made will be overwritten to the existing Rule schedules and additional look-back time (if any)."
+      defaultMessage="You're about to apply changes to {rulesCount, plural, one {# selected rule} other {# selected rules}}. The changes you make will overwrite the existing rule schedules and additional look-back time (if any)."
      values={{ rulesCount }}
    />
  ),
--- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/bulk_actions/action_to_rules_client_operation.ts
+++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/bulk_actions/action_to_rules_client_operation.ts
@ -93,8 +93,6 @@ export const bulkEditActionToRulesClientOperation = (
        {
          field: 'schedule',
          operation: 'set',
-          // We need to pass a pure Interval object
-          // i.e. get rid of the meta property
          value: {
            interval: action.value.interval,
          },
--- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/bulk_actions/rule_params_modifier.test.ts
+++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/bulk_actions/rule_params_modifier.test.ts
@ -353,7 +353,7 @@ describe('ruleParamsModifier', () => {
  });

  describe('schedule', () => {
-    test('should set timeline', () => {
+    test('should set schedule', () => {
      const INTERVAL_IN_MINUTES = 5;
      const LOOKBACK_IN_MINUTES = 1;
      const FROM_IN_SECONDS = (INTERVAL_IN_MINUTES + LOOKBACK_IN_MINUTES) * 60;
@ -367,8 +367,8 @@ describe('ruleParamsModifier', () => {
        },
      ]);

-      // @ts-expect-error
-      expect(editedRuleParams.interval).toBeUndefined();
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      expect((editedRuleParams as any).interval).toBeUndefined();
      expect(editedRuleParams.meta).toStrictEqual({
        from: '1m',
      });