Merge branch '262633-integrations-form-cleanup' into 'master'

Alert integrations form cleanup

See merge request gitlab-org/gitlab!45304

app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue

@@ -191,9 +191,12 @@ export default {
     this.authKey = this.selectedService.authKey ?? '';
   },
   methods: {
-    createUserErrorMessage(errors = { error: [''] }) {
-      // eslint-disable-next-line prefer-destructuring
-      this.serverError = errors.error[0];
+    createUserErrorMessage(errors = {}) {
+      const error = Object.entries(errors)?.[0];
+      if (error) {
+        const [field, [msg]] = error;
+        this.serverError = `${field} ${msg}`;
+      }
     },
     setOpsgenieAsDefault() {
       this.options = this.options.map(el => {
@@ -267,7 +270,7 @@ export default {
         .catch(({ response: { data: { errors } = {} } = {} }) => {
           this.createUserErrorMessage(errors);
           this.setFeedback({
-            feedbackMessage: `${this.$options.i18n.errorMsg}.`,
+            feedbackMessage: this.$options.i18n.errorMsg,
             variant: 'danger',
           });
         })
@@ -298,7 +301,7 @@ export default {
         .catch(({ response: { data: { errors } = {} } = {} }) => {
           this.createUserErrorMessage(errors);
           this.setFeedback({
-            feedbackMessage: `${this.$options.i18n.errorMsg}.`,
+            feedbackMessage: this.$options.i18n.errorMsg,
             variant: 'danger',
           });
         })
@@ -368,6 +371,11 @@ export default {
 
 <template>
   <div>
+    <integrations-list :integrations="integrations" />
+
+    <gl-form @submit.prevent="onSubmit" @reset.prevent="onReset">
+      <h5 class="gl-font-lg gl-my-5">{{ $options.i18n.integrationsLabel }}</h5>
+
       <gl-alert v-if="showFeedbackMsg" :variant="feedback.variant" @dismiss="dismissFeedback">
         {{ feedback.feedbackMessage }}
         <br />
@@ -383,13 +391,7 @@ export default {
         </gl-button>
       </gl-alert>
 
-    <integrations-list :integrations="integrations" />
-
-    <gl-form @submit.prevent="onSubmit" @reset.prevent="onReset">
-      <h5 class="gl-font-lg">{{ $options.i18n.integrationsLabel }}</h5>
-
-      <gl-form-group label-for="integrations">
-        <div data-testid="alert-settings-description" class="gl-mt-5">
+      <div data-testid="alert-settings-description">
         <p v-for="section in sections" :key="section.text">
           <gl-sprintf :message="section.text">
             <template #link="{ content }">
@@ -398,18 +400,21 @@ export default {
           </gl-sprintf>
         </p>
       </div>
+
+      <gl-form-group label-for="integration-type" :label="$options.i18n.integration">
         <gl-form-select
+          id="integration-type"
           v-model="selectedEndpoint"
           :options="options"
           data-testid="alert-settings-select"
           @change="resetFormValues"
         />
-        <span class="gl-text-gray-200">
+        <span class="gl-text-gray-500">
           <gl-sprintf :message="$options.i18n.integrationsInfo">
             <template #link="{ content }">
               <gl-link
                 class="gl-display-inline-block"
-                href="https://gitlab.com/groups/gitlab-org/-/epics/3362"
+                href="https://gitlab.com/groups/gitlab-org/-/epics/4390"
                 target="_blank"
                 >{{ content }}</gl-link
               >
@@ -438,7 +443,7 @@ export default {
           :placeholder="baseUrlPlaceholder"
           :disabled="!active"
         />
-        <span class="gl-text-gray-200">
+        <span class="gl-text-gray-500">
           {{ $options.i18n.apiBaseUrlHelpText }}
         </span>
       </gl-form-group>
@@ -453,7 +458,7 @@ export default {
               />
             </template>
           </gl-form-input-group>
-          <span class="gl-text-gray-200">
+          <span class="gl-text-gray-500">
             {{ prometheusInfo }}
           </span>
         </gl-form-group>

app/assets/javascripts/alerts_settings/constants.js

@@ -7,7 +7,7 @@ export const i18n = {
   setupSection: s__(
     "AlertSettings|Review your external service's documentation to learn where to provide this information to your external service, and the %{linkStart}GitLab documentation%{linkEnd} to learn more about configuring your endpoint.",
   ),
-  errorMsg: s__('AlertSettings|There was an error updating the alert settings'),
+  errorMsg: s__('AlertSettings|There was an error updating the alert settings.'),
   errorKeyMsg: s__(
     'AlertSettings|There was an error while trying to reset the key. Please refresh the page to try again.',
   ),
@@ -17,7 +17,7 @@ export const i18n = {
   changesSaved: s__('AlertSettings|Your integration was successfully updated.'),
   prometheusInfo: s__('AlertSettings|Add URL and auth key to your Prometheus config file'),
   integrationsInfo: s__(
-    'AlertSettings|Learn more about our %{linkStart}upcoming integrations%{linkEnd}',
+    'AlertSettings|Learn more about our improvements for %{linkStart}integrations%{linkEnd}',
   ),
   resetKey: s__('AlertSettings|Reset key'),
   copyToClipboard: s__('AlertSettings|Copy'),
@@ -37,6 +37,7 @@ export const i18n = {
   authKeyRest: s__(
     'AlertSettings|Authorization key has been successfully reset. Please save your changes now.',
   ),
+  integration: s__('AlertSettings|Integration'),
 };
 
 export const serviceOptions = [

app/helpers/operations_helper.rb

@@ -27,7 +27,7 @@ module OperationsHelper
       'authorization_key' => alerts_service.token,
       'prometheus_url' => notify_project_prometheus_alerts_url(@project, format: :json),
       'url' => alerts_service.url,
-      'alerts_setup_url' => help_page_path('user/project/integrations/generic_alerts.md', anchor: 'setting-up-generic-alerts'),
+      'alerts_setup_url' => help_page_path('operations/incident_management/alert_integrations.md', anchor: 'generic-http-endpoint'),
       'alerts_usage_url' => project_alert_management_index_path(@project),
       'disabled' => disabled.to_s
     }

changelogs/unreleased/262633-integrations-form-cleanup.yml

+---
+title: Fix documentation link, spacing, and error handling in alert integrations list
+merge_request: 45304
+author:
+type: other

locale/gitlab.pot

@@ -2514,7 +2514,10 @@ msgstr ""
 msgid "AlertSettings|HTTP endpoint"
 msgstr ""
 
-msgid "AlertSettings|Learn more about our %{linkStart}upcoming integrations%{linkEnd}"
+msgid "AlertSettings|Integration"
+msgstr ""
+
+msgid "AlertSettings|Learn more about our improvements for %{linkStart}integrations%{linkEnd}"
 msgstr ""
 
 msgid "AlertSettings|Opsgenie"
@@ -2538,7 +2541,7 @@ msgstr ""
 msgid "AlertSettings|Test failed. Do you still want to save your changes anyway?"
 msgstr ""
 
-msgid "AlertSettings|There was an error updating the alert settings"
+msgid "AlertSettings|There was an error updating the alert settings."
 msgstr ""
 
 msgid "AlertSettings|There was an error while trying to reset the key. Please refresh the page to try again."

spec/frontend/alert_settings/__snapshots__/alert_settings_form_spec.js.snap

@@ -2,12 +2,11 @@
 
 exports[`AlertsSettingsForm with default values renders the initial template 1`] = `
 "<div>
-  <!---->
   <integrations-list-stub integrations=\\"[object Object],[object Object]\\"></integrations-list-stub>
   <gl-form-stub>
-    <h5 class=\\"gl-font-lg\\">Add new integrations</h5>
-    <gl-form-group-stub label-for=\\"integrations\\">
-      <div data-testid=\\"alert-settings-description\\" class=\\"gl-mt-5\\">
+    <h5 class=\\"gl-font-lg gl-my-5\\">Add new integrations</h5>
+    <!---->
+    <div data-testid=\\"alert-settings-description\\">
       <p>
         <gl-sprintf-stub message=\\"You must provide this URL and authorization key to authorize an external service to send alerts to GitLab. You can provide this URL and key to multiple services. After configuring an external service, alerts from your service will display on the GitLab %{linkStart}Alerts%{linkEnd} page.\\"></gl-sprintf-stub>
       </p>
@@ -15,14 +14,15 @@ exports[`AlertsSettingsForm with default values renders the initial template 1`]
         <gl-sprintf-stub message=\\"Review your external service's documentation to learn where to provide this information to your external service, and the %{linkStart}GitLab documentation%{linkEnd} to learn more about configuring your endpoint.\\"></gl-sprintf-stub>
       </p>
     </div>
-      <gl-form-select-stub options=\\"[object Object],[object Object],[object Object]\\" data-testid=\\"alert-settings-select\\" value=\\"generic\\"></gl-form-select-stub> <span class=\\"gl-text-gray-200\\"><gl-sprintf-stub message=\\"Learn more about our %{linkStart}upcoming integrations%{linkEnd}\\"></gl-sprintf-stub></span>
+    <gl-form-group-stub label-for=\\"integration-type\\" label=\\"Integration\\">
+      <gl-form-select-stub id=\\"integration-type\\" options=\\"[object Object],[object Object],[object Object]\\" data-testid=\\"alert-settings-select\\" value=\\"generic\\"></gl-form-select-stub> <span class=\\"gl-text-gray-500\\"><gl-sprintf-stub message=\\"Learn more about our improvements for %{linkStart}integrations%{linkEnd}\\"></gl-sprintf-stub></span>
     </gl-form-group-stub>
     <gl-form-group-stub label=\\"Active\\" label-for=\\"activated\\">
       <toggle-button-stub id=\\"activated\\"></toggle-button-stub>
     </gl-form-group-stub>
     <!---->
     <gl-form-group-stub label=\\"Webhook URL\\" label-for=\\"url\\">
-      <gl-form-input-group-stub value=\\"/alerts/notify.json\\" predefinedoptions=\\"[object Object]\\" id=\\"url\\" readonly=\\"\\"></gl-form-input-group-stub> <span class=\\"gl-text-gray-200\\">
+      <gl-form-input-group-stub value=\\"/alerts/notify.json\\" predefinedoptions=\\"[object Object]\\" id=\\"url\\" readonly=\\"\\"></gl-form-input-group-stub> <span class=\\"gl-text-gray-500\\">
 
         </span>
     </gl-form-group-stub>

spec/frontend/alert_settings/alert_settings_form_spec.js

-import axios from 'axios';
-import MockAdapter from 'axios-mock-adapter';
 import { shallowMount } from '@vue/test-utils';
 import { GlModal, GlAlert } from '@gitlab/ui';
 import AlertsSettingsForm from '~/alerts_settings/components/alerts_settings_form.vue';
 import IntegrationsList from '~/alerts_settings/components/alerts_integrations_list.vue';
 import ToggleButton from '~/vue_shared/components/toggle_button.vue';
+import { i18n } from '~/alerts_settings/constants';
+import service from '~/alerts_settings/services';
+
+jest.mock('~/alerts_settings/services');
 
 const PROMETHEUS_URL = '/prometheus/alerts/notify.json';
 const GENERIC_URL = '/alerts/notify.json';
@@ -14,7 +16,6 @@ const ACTIVATED = false;
 
 describe('AlertsSettingsForm', () => {
   let wrapper;
-  let mockAxios;
 
   const createComponent = ({ methods } = {}, data) => {
     wrapper = shallowMount(AlertsSettingsForm, {
@@ -54,7 +55,6 @@ describe('AlertsSettingsForm', () => {
   const findApiUrl = () => wrapper.find('#api-url');
 
   beforeEach(() => {
-    mockAxios = new MockAdapter(axios);
     setFixtures(`
     <div>
       <span class="js-service-active-status fa fa-circle" data-value="true"></span>
@@ -64,7 +64,6 @@ describe('AlertsSettingsForm', () => {
 
   afterEach(() => {
     wrapper.destroy();
-    mockAxios.restore();
   });
 
   describe('with default values', () => {
@@ -105,8 +104,7 @@ describe('AlertsSettingsForm', () => {
     });
 
     it('shows a alert message on error', () => {
-      const formPath = 'some/path';
-      mockAxios.onPut(formPath).replyOnce(404);
+      service.updateGenericKey.mockRejectedValueOnce({});
 
       createComponent();
 
@@ -128,8 +126,7 @@ describe('AlertsSettingsForm', () => {
 
     describe('error is encountered', () => {
       it('restores previous value', () => {
-        const formPath = 'some/path';
-        mockAxios.onPut(formPath).replyOnce(500);
+        service.updateGenericKey.mockRejectedValueOnce({});
         createComponent();
         return wrapper.vm.resetKey().then(() => {
           expect(wrapper.find(ToggleButton).props('value')).toBe(false);
@@ -199,18 +196,34 @@ describe('AlertsSettingsForm', () => {
     });
 
     describe('alert service is toggled', () => {
-      it('should show a error alert if failed', () => {
-        const formPath = 'some/path';
+      describe('error handling', () => {
         const toggleService = true;
-        mockAxios.onPut(formPath).replyOnce(422, {
-          errors: 'Error message to display',
-        });
+
+        it('should show generic error', async () => {
+          service.updateGenericActive.mockRejectedValueOnce({});
 
           createComponent();
 
-        return wrapper.vm.toggleActivated(toggleService).then(() => {
+          await wrapper.vm.toggleActivated(toggleService);
           expect(wrapper.vm.active).toBe(false);
           expect(wrapper.find(GlAlert).attributes('variant')).toBe('danger');
+          expect(wrapper.find(GlAlert).text()).toBe(i18n.errorMsg);
+        });
+
+        it('should show first field specific error when available', async () => {
+          const err1 = "can't be blank";
+          const err2 = 'is not a valid URL';
+          const key = 'api_url';
+          service.updateGenericActive.mockRejectedValueOnce({
+            response: { data: { errors: { [key]: [err1, err2] } } },
+          });
+
+          createComponent();
+
+          await wrapper.vm.toggleActivated(toggleService);
+
+          expect(wrapper.find(GlAlert).text()).toContain(i18n.errorMsg);
+          expect(wrapper.find(GlAlert).text()).toContain(`${key} ${err1}`);
         });
       });
     });

spec/helpers/operations_helper_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe OperationsHelper do
           'url' => alerts_service.url,
           'authorization_key' => nil,
           'form_path' => project_service_path(project, alerts_service),
-          'alerts_setup_url' => help_page_path('user/project/integrations/generic_alerts.md', anchor: 'setting-up-generic-alerts'),
+          'alerts_setup_url' => help_page_path('operations/incident_management/alert_integrations.md', anchor: 'generic-http-endpoint'),
           'alerts_usage_url' => project_alert_management_index_path(project),
           'prometheus_form_path' => project_service_path(project, prometheus_service),
           'prometheus_reset_key_path' => reset_alerting_token_project_settings_operations_path(project),