Merge branch '332245-restrict-minutes-input-to-numbers' into 'master'

Update validation for escalation rule

See merge request gitlab-org/gitlab!62825

app/assets/stylesheets/page_bundles/escalation_policies.scss

@@ -6,10 +6,6 @@
   width: 240px;
 }
 
-.rule-elapsed-minutes {
-  width: 56px;
-}
-
 .rule-close-icon {
   right: 1rem;
 }

ee/app/assets/javascripts/escalation_policies/components/add_edit_escalation_policy_form.vue

@@ -81,14 +81,14 @@ export default {
       this.rules.push({ ...cloneDeep(defaultEscalationRule), key: this.getUid() });
     },
     updateEscalationRules(index, rule) {
-      this.rules[index] = rule;
-      this.emitUpdate();
+      this.rules[index] = { ...this.rules[index], ...rule };
+      this.emitRulesUpdate();
     },
     removeEscalationRule(index) {
       this.rules.splice(index, 1);
-      this.emitUpdate();
+      this.emitRulesUpdate();
     },
-    emitUpdate() {
+    emitRulesUpdate() {
       this.$emit('update-escalation-policy-form', { field: 'rules', value: this.rules });
     },
     getUid() {
@@ -146,7 +146,7 @@ export default {
         :index="index"
         :schedules="schedules"
         :schedules-loading="schedulesLoading"
-        :is-valid="validationState.rules[index]"
+        :validation-state="validationState.rules[index]"
         @update-escalation-rule="updateEscalationRules"
         @remove-escalation-rule="removeEscalationRule"
       />

ee/app/assets/javascripts/escalation_policies/components/add_edit_escalation_policy_modal.vue

@@ -61,15 +61,12 @@ export default {
       };
     },
     isFormValid() {
-      return this.validationState.name && this.validationState.rules.every(Boolean);
-    },
-    serializedData() {
-      const rules = this.form.rules.map(({ status, elapsedTimeSeconds, oncallScheduleIid }) => ({
-        status,
-        elapsedTimeSeconds,
-        oncallScheduleIid,
-      }));
-      return { ...this.form, rules };
+      return (
+        this.validationState.name &&
+        this.validationState.rules.every(
+          ({ isTimeValid, isScheduleValid }) => isTimeValid && isScheduleValid,
+        )
+      );
     },
   },
   methods: {
@@ -86,7 +83,7 @@ export default {
           variables: {
             input: {
               projectPath,
-              ...this.serializedData,
+              ...this.getRequestParams(),
             },
           },
         })
@@ -113,6 +110,15 @@ export default {
           this.loading = false;
         });
     },
+    getRequestParams() {
+      const rules = this.form.rules.map(({ status, elapsedTimeSeconds, oncallScheduleIid }) => ({
+        status,
+        elapsedTimeSeconds,
+        oncallScheduleIid,
+      }));
+
+      return { ...this.form, rules };
+    },
     validateForm(field) {
       if (field === 'name') {
         this.validationState.name = isNameFieldValid(this.form.name);

ee/app/assets/javascripts/escalation_policies/components/escalation_rule.vue

@@ -19,13 +19,16 @@ export const i18n = {
       condition: s__('EscalationPolicies|IF alert is not %{alertStatus} in %{minutes} minutes'),
       action: s__('EscalationPolicies|THEN %{doAction} %{schedule}'),
       selectSchedule: s__('EscalationPolicies|Select schedule'),
-      validationMsg: s__(
-        'EscalationPolicies|A schedule is required for adding an escalation policy.',
-      ),
       noSchedules: s__(
         'EscalationPolicies|A schedule is required for adding an escalation policy. Please create an on-call schedule first.',
       ),
       removeRuleLabel: s__('EscalationPolicies|Remove escalation rule'),
+      emptyScheduleValidationMsg: s__(
+        'EscalationPolicies|A schedule is required for adding an escalation policy.',
+      ),
+      invalidTimeValidationMsg: s__(
+        'EscalationPolicies|Elapsed time must be greater than or equal to zero.',
+      ),
     },
   },
 };
@@ -66,10 +69,10 @@ export default {
       type: Number,
       required: true,
     },
-    isValid: {
-      type: Boolean,
+    validationState: {
+      type: Object,
       required: false,
-      default: true,
+      default: () => {},
     },
   },
   data() {
@@ -90,6 +93,15 @@ export default {
     noSchedules() {
       return !this.schedulesLoading && !this.schedules.length;
     },
+    isValid() {
+      return this.isTimeValid && this.isScheduleValid;
+    },
+    isTimeValid() {
+      return this.validationState?.isTimeValid;
+    },
+    isScheduleValid() {
+      return this.validationState?.isScheduleValid;
+    },
   },
   methods: {
     setOncallSchedule({ iid }) {
@@ -123,11 +135,16 @@ export default {
       class="gl-absolute rule-close-icon"
       @click="$emit('remove-escalation-rule', index)"
     />
-    <gl-form-group
-      :invalid-feedback="$options.i18n.fields.rules.validationMsg"
-      :state="isValid"
-      class="gl-mb-0"
-    >
+    <gl-form-group :state="isValid" class="gl-mb-0">
+      <template #invalid-feedback>
+        <div v-if="!isScheduleValid">
+          {{ $options.i18n.fields.rules.emptyScheduleValidationMsg }}
+        </div>
+        <div v-if="!isTimeValid" class="gl-display-inline-block gl-mt-2">
+          {{ $options.i18n.fields.rules.invalidTimeValidationMsg }}
+        </div>
+      </template>
+
       <div class="gl-display-flex gl-align-items-center">
         <gl-sprintf :message="$options.i18n.fields.rules.condition">
           <template #alertStatus>
@@ -150,10 +167,10 @@ export default {
           <template #minutes>
             <gl-form-input
               v-model="elapsedTimeSeconds"
-              class="gl-mx-3 gl-inset-border-1-gray-200! rule-elapsed-minutes"
+              class="gl-mx-3 gl-inset-border-1-gray-200! gl-w-12"
               type="number"
               min="0"
-              @change="emitUpdate"
+              @input="emitUpdate"
             />
           </template>
         </gl-sprintf>

ee/app/assets/javascripts/escalation_policies/utils.js

@@ -15,5 +15,10 @@ export const isNameFieldValid = (name) => {
  * @returns {Array}
  */
 export const getRulesValidationState = (rules) => {
-  return rules.map((rule) => Boolean(rule.oncallScheduleIid));
+  return rules.map((rule) => {
+    return {
+      isTimeValid: parseInt(rule.elapsedTimeSeconds, 10) >= 0,
+      isScheduleValid: Boolean(rule.oncallScheduleIid),
+    };
+  });
 };

ee/spec/frontend/escalation_policies/add_edit_escalation_policy_form_spec.js

@@ -46,23 +46,9 @@ describe('AddEscalationPolicyForm', () => {
     wrapper.destroy();
   });
 
-  const findPolicyName = () => wrapper.findByTestId('escalation-policy-name');
   const findRules = () => wrapper.findAllComponents(EscalationRule);
   const findAddRuleLink = () => wrapper.findComponent(GlLink);
 
-  describe('Escalation policy form validation', () => {
-    it('should set correct validation state for validated controls', async () => {
-      createComponent({
-        props: {
-          validationState: { name: false, rules: [false] },
-        },
-      });
-      await wrapper.vm.$nextTick();
-      expect(findPolicyName().attributes('state')).toBeUndefined();
-      expect(findRules().at(0).attributes('is-valid')).toBeUndefined();
-    });
-  });
-
   describe('Escalation rules', () => {
     it('should render one default rule', () => {
       expect(findRules().length).toBe(1);
@@ -96,7 +82,7 @@ describe('AddEscalationPolicyForm', () => {
       };
       findRules().at(0).vm.$emit('update-escalation-rule', 0, updatedRule);
       expect(wrapper.emitted('update-escalation-policy-form')[0]).toEqual([
-        { field: 'rules', value: [updatedRule] },
+        { field: 'rules', value: [expect.objectContaining(updatedRule)] },
       ]);
     });
 

ee/spec/frontend/escalation_policies/escalation_rule_spec.js

 import { GlDropdownItem, GlFormGroup, GlSprintf } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import { cloneDeep } from 'lodash';
-import EscalationRule from 'ee/escalation_policies/components/escalation_rule.vue';
+import EscalationRule, { i18n } from 'ee/escalation_policies/components/escalation_rule.vue';
 import { defaultEscalationRule, ACTIONS, ALERT_STATUSES } from 'ee/escalation_policies/constants';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
@@ -11,6 +11,9 @@ const mockSchedules = [
   { id: 3, name: 'schedule3' },
 ];
 
+const emptyScheduleMsg = i18n.fields.rules.emptyScheduleValidationMsg;
+const invalidTimeMsg = i18n.fields.rules.invalidTimeValidationMsg;
+
 describe('EscalationRule', () => {
   let wrapper;
   const createComponent = ({ props = {} } = {}) => {
@@ -25,6 +28,7 @@ describe('EscalationRule', () => {
           ...props,
         },
         stubs: {
+          GlFormGroup,
           GlSprintf,
         },
       }),
@@ -96,17 +100,45 @@ describe('EscalationRule', () => {
   });
 
   describe('Validation', () => {
-    it.each`
-      isValid  | state
-      ${true}  | ${'true'}
-      ${false} | ${undefined}
-    `('when $isValid sets from group state to $state', ({ isValid, state }) => {
-      createComponent({
-        props: {
-          isValid,
-        },
+    describe.each`
+      validationState                                   | formState
+      ${{ isTimeValid: true, isScheduleValid: true }}   | ${'true'}
+      ${{ isTimeValid: false, isScheduleValid: true }}  | ${undefined}
+      ${{ isTimeValid: true, isScheduleValid: false }}  | ${undefined}
+      ${{ isTimeValid: false, isScheduleValid: false }} | ${undefined}
+    `(`when`, ({ validationState, formState }) => {
+      describe(`elapsed minutes control is ${
+        validationState.isTimeValid ? 'valid' : 'invalid'
+      } and schedule control is ${validationState.isScheduleValid ? 'valid' : 'invalid'}`, () => {
+        beforeEach(() => {
+          createComponent({
+            props: {
+              validationState,
+            },
+          });
+        });
+
+        it(`sets form group validation state to ${formState}`, () => {
+          expect(findFormGroup().attributes('state')).toBe(formState);
+        });
+
+        it(`does ${
+          validationState.isTimeValid ? 'not show' : 'show'
+        } invalid time error message && does ${
+          validationState.isScheduleValid ? 'not show' : 'show'
+        } invalid schedule error message `, () => {
+          if (validationState.isTimeValid) {
+            expect(findFormGroup().text()).not.toContain(invalidTimeMsg);
+          } else {
+            expect(findFormGroup().text()).toContain(invalidTimeMsg);
+          }
+          if (validationState.isScheduleValid) {
+            expect(findFormGroup().text()).not.toContain(emptyScheduleMsg);
+          } else {
+            expect(findFormGroup().text()).toContain(emptyScheduleMsg);
+          }
+        });
       });
-      expect(findFormGroup().attributes('state')).toBe(state);
     });
   });
 });

locale/gitlab.pot

@@ -13111,6 +13111,9 @@ msgstr ""
 msgid "EscalationPolicies|Edit escalation policy"
 msgstr ""
 
+msgid "EscalationPolicies|Elapsed time must be greater than or equal to zero."
+msgstr ""
+
 msgid "EscalationPolicies|Email on-call user in schedule"
 msgstr ""