[Security Solution] Rule bulk schedule fixes and test coverage expans…

…ion (#141604) (#141946)

## Summary

Fixes issues, nits and [expands test coverage](https://docs.google.com/document/d/116x7ITTTJQ6cTiwaGK831_f6Ox7XB3qyLiHxC3Cmf8w/edit#) for PR: #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)

(cherry picked from commit 9738b04)

Co-authored-by: Juan Pablo Djeredjian <[email protected]>

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);

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) &&

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();

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"]';

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);

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

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 }}
     />
   ),

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,
           },

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',
       });