Merge branch...

Merge branch '31904-form-for-creating-an-alert-should-be-validated-before-submitting-in-monitor-dashboard' into 'master'

Monitor charts: Validate form for creating an alert before submitting

See merge request gitlab-org/gitlab!17109

ee/app/assets/javascripts/monitoring/components/alert_widget.vue

@@ -54,9 +54,6 @@ export default {
         .map(this.formatAlertSummary)
         .join(', ');
     },
-    formDisabled() {
-      return Boolean(this.errorMessage || this.isLoading);
-    },
     supportsComputedAlerts() {
       return gon.features && gon.features.prometheusComputedAlerts;
     },
@@ -103,6 +100,7 @@ export default {
       return `${alertQuery.label} ${alert.operator} ${alert.threshold}`;
     },
     hideModal() {
+      this.errorMessage = null;
       this.$root.$emit('bv::hide::modal', this.modalId);
     },
     handleSetApiAction(apiAction) {
@@ -165,7 +163,7 @@ export default {
     </span>
     <alert-widget-form
       ref="widgetForm"
-      :disabled="formDisabled"
+      :disabled="isLoading"
       :alerts-to-manage="alertsToManage"
       :relevant-queries="relevantQueries"
       :error-message="errorMessage"

ee/app/assets/javascripts/monitoring/components/alert_widget_form.vue

@@ -152,17 +152,18 @@ export default {
         this.threshold = existingAlert.threshold;
       } else {
         this.selectedAlert = {};
-        this.operator = null;
+        this.operator = this.operators.greaterThan;
         this.threshold = null;
       }
 
       this.prometheusMetricId = queryId;
     },
-    handleCancel() {
+    handleHidden() {
       this.resetAlertData();
       this.$emit('cancel');
     },
-    handleSubmit() {
+    handleSubmit(e) {
+      e.preventDefault();
       this.$emit(this.submitAction, {
         alert: this.selectedAlert.alert_path,
         operator: this.operator,
@@ -196,19 +197,20 @@ export default {
     :ok-variant="submitAction === 'delete' ? 'danger' : 'success'"
     :ok-title="submitActionText"
     :ok-disabled="formDisabled"
-    class="prometheus-alert-widget d-flex align-items-center"
     @ok="handleSubmit"
+    @hidden="handleHidden"
   >
-    <span v-if="errorMessage" class="alert-error-message"> {{ errorMessage }} </span>
+    <div v-if="errorMessage" class="alert-modal-message danger_message">{{ errorMessage }}</div>
     <div class="alert-form">
       <gl-form-group
         v-if="supportsComputedAlerts"
         :label="$options.alertQueryText.label"
+        label-for="alert-query-input"
         :valid-feedback="$options.alertQueryText.validFeedback"
         :invalid-feedback="$options.alertQueryText.invalidFeedback"
         :state="isValidQuery"
       >
-        <gl-form-input v-model.trim="alertQuery" :state="isValidQuery" />
+        <gl-form-input id="alert-query-input" v-model.trim="alertQuery" :state="isValidQuery" />
         <template #description>
           <div class="d-flex align-items-center">
             {{ __('Single or combined queries') }}
@@ -220,20 +222,21 @@ export default {
           </div>
         </template>
       </gl-form-group>
-      <gl-dropdown
-        v-else
-        :text="queryDropdownLabel"
-        class="form-group"
-        toggle-class="dropdown-menu-toggle"
-      >
-        <gl-dropdown-item
-          v-for="query in relevantQueries"
-          :key="query.metricId"
-          @click="selectQuery(query.metricId)"
+      <gl-form-group v-else label-for="alert-query-dropdown" :label="$options.alertQueryText.label">
+        <gl-dropdown
+          id="alert-query-dropdown"
+          :text="queryDropdownLabel"
+          toggle-class="dropdown-menu-toggle"
         >
-          {{ query.label }}
-        </gl-dropdown-item>
-      </gl-dropdown>
+          <gl-dropdown-item
+            v-for="query in relevantQueries"
+            :key="query.metricId"
+            @click="selectQuery(query.metricId)"
+          >
+            {{ query.label }}
+          </gl-dropdown-item>
+        </gl-dropdown>
+      </gl-form-group>
       <gl-button-group class="mb-2" :label="s__('PrometheusAlerts|Operator')">
         <gl-button
           :class="{ active: operator === operators.greaterThan }"

ee/app/assets/stylesheets/pages/environments.scss

@@ -117,13 +117,13 @@
   vertical-align: middle;
 }
 
-.alert-form {
-  padding: $gl-padding $gl-padding $gl-padding-8;
-
-  label {
-    font-weight: normal;
-  }
+.alert-modal-message {
+  margin-left: -1rem;
+  margin-right: -3rem;
+  margin-top: -1rem;
+}
 
+.alert-form {
   .btn-group,
   .action-group {
     display: flex;

ee/changelogs/unreleased/31904-form-for-creating-an-alert-should-be-validated-before-submitting.yml

+---
+title: 'Monitor charts: Validate form for creating an alert before submitting'
+merge_request: 17109
+author:
+type: fixed

ee/spec/frontend/monitoring/components/alert_widget_form_spec.js

@@ -36,6 +36,13 @@ describe('AlertWidgetForm', () => {
   const modal = () => wrapper.find(GlModal);
   const modalTitle = () => modal().attributes('title');
   const submitText = () => modal().attributes('ok-title');
+  const e = {
+    preventDefault: jest.fn(),
+  };
+
+  beforeEach(() => {
+    e.preventDefault.mockReset();
+  });
 
   afterEach(() => {
     if (wrapper) wrapper.destroy();
@@ -62,10 +69,9 @@ describe('AlertWidgetForm', () => {
     createComponent();
 
     wrapper.vm.selectQuery('9');
-    wrapper.vm.operator = '>';
     wrapper.vm.threshold = 900;
 
-    wrapper.vm.handleSubmit();
+    wrapper.vm.handleSubmit(e);
 
     expect(wrapper.emitted().create[0]).toEqual([
       {
@@ -75,6 +81,22 @@ describe('AlertWidgetForm', () => {
         prometheus_metric_id: '9',
       },
     ]);
+    expect(e.preventDefault).toHaveBeenCalledTimes(1);
+  });
+
+  it('resets form when modal is dismissed (hidden)', () => {
+    createComponent();
+
+    wrapper.vm.selectQuery('9');
+    wrapper.vm.selectQuery('>');
+    wrapper.vm.threshold = 800;
+
+    wrapper.find(GlModal).vm.$emit('hidden');
+
+    expect(wrapper.vm.selectedAlert).toEqual({});
+    expect(wrapper.vm.operator).toBe(null);
+    expect(wrapper.vm.threshold).toBe(null);
+    expect(wrapper.vm.prometheusMetricId).toBe(null);
   });
 
   describe('with existing alert', () => {
@@ -90,7 +112,7 @@ describe('AlertWidgetForm', () => {
     });
 
     it('emits "delete" event when form values unchanged', () => {
-      wrapper.vm.handleSubmit();
+      wrapper.vm.handleSubmit(e);
 
       expect(wrapper.emitted().delete[0]).toEqual([
         {
@@ -100,12 +122,13 @@ describe('AlertWidgetForm', () => {
           prometheus_metric_id: '8',
         },
       ]);
+      expect(e.preventDefault).toHaveBeenCalledTimes(1);
     });
 
     it('emits "update" event when form changed', () => {
       wrapper.vm.threshold = 11;
 
-      wrapper.vm.handleSubmit();
+      wrapper.vm.handleSubmit(e);
 
       expect(wrapper.emitted().update[0]).toEqual([
         {
@@ -115,6 +138,7 @@ describe('AlertWidgetForm', () => {
           prometheus_metric_id: '8',
         },
       ]);
+      expect(e.preventDefault).toHaveBeenCalledTimes(1);
     });
   });
 });

ee/spec/javascripts/monitoring/alert_widget_spec.js

@@ -61,6 +61,8 @@ describe('AlertWidget', () => {
 
     // expect loading spinner to exist during fetch
     expect(vm.isLoading).toBeTruthy();
+    expect(vm.$refs.widgetForm.$props.disabled).toBe(true);
+
     expect(vm.$el.querySelector('.loading-container')).toBeVisible();
 
     resolveReadAlert({ operator: '=', threshold: 42 });
@@ -70,6 +72,7 @@ describe('AlertWidget', () => {
       vm.$nextTick(() => {
         expect(vm.isLoading).toEqual(false);
         expect(vm.$el.querySelector('.loading-container')).toBeHidden();
+        expect(vm.$refs.widgetForm.$props.disabled).toBe(false);
         done();
       }),
     );
@@ -155,6 +158,17 @@ describe('AlertWidget', () => {
       .catch(done.fail);
   });
 
+  it('dismisses error message when action is cancelled', () => {
+    vm = mountComponent(AlertWidgetComponent, props);
+    vm.$on('setAlerts', mockSetAlerts);
+    vm.errorMessage = 'Mock error message.';
+
+    // widget modal is dismissed
+    vm.$refs.widgetForm.$emit('cancel');
+
+    expect(vm.errorMessage).toBe(null);
+  });
+
   it('updates an alert with an appropriate handler', done => {
     const alertParams = { operator: '<', threshold: 4, alert_path: alertPath };
     const newAlertParams = { operator: '=', threshold: 12 };