Merge branch '300827-split-alert-integrations-form' into 'master'

UX cleanup: Adding an alert integration - split form into tabs

See merge request gitlab-org/gitlab!54842

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

 <script>
+import { GlButton } from '@gitlab/ui';
 import createHttpIntegrationMutation from 'ee_else_ce/alerts_settings/graphql/mutations/create_http_integration.mutation.graphql';
 import updateHttpIntegrationMutation from 'ee_else_ce/alerts_settings/graphql/mutations/update_http_integration.mutation.graphql';
 import createFlash, { FLASH_TYPES } from '~/flash';
@@ -31,9 +32,7 @@ import {
 import IntegrationsList from './alerts_integrations_list.vue';
 import AlertSettingsForm from './alerts_settings_form.vue';
 
-export default {
-  typeSet,
-  i18n: {
+export const i18n = {
   changesSaved: s__(
     'AlertsIntegrations|The integration has been successfully saved. Alerts from this new integration should now appear on your alerts list.',
   ),
@@ -41,10 +40,16 @@ export default {
   alertSent: s__(
     'AlertsIntegrations|The test alert has been successfully sent, and should now be visible on your alerts list.',
   ),
-  },
+  addNewIntegration: s__('AlertSettings|Add new integration'),
+};
+
+export default {
+  typeSet,
+  i18n,
   components: {
     IntegrationsList,
     AlertSettingsForm,
+    GlButton,
   },
   inject: {
     generic: {
@@ -116,10 +121,10 @@ export default {
   data() {
     return {
       isUpdating: false,
-      testAlertPayload: null,
       integrations: {},
       httpIntegrations: {},
       currentIntegration: null,
+      formVisible: false,
     };
   },
   computed: {
@@ -161,18 +166,6 @@ export default {
             return createFlash({ message: error });
           }
 
-          if (this.testAlertPayload) {
-            const integration =
-              httpIntegrationCreate?.integration || prometheusIntegrationCreate?.integration;
-
-            const payload = {
-              ...this.testAlertPayload,
-              endpoint: integration.url,
-              token: integration.token,
-            };
-            return this.validateAlertPayload(payload);
-          }
-
           return createFlash({
             message: this.$options.i18n.changesSaved,
             type: FLASH_TYPES.SUCCESS,
@@ -203,10 +196,6 @@ export default {
             return createFlash({ message: error });
           }
 
-          if (this.testAlertPayload) {
-            return this.validateAlertPayload();
-          }
-
           this.clearCurrentIntegration({ type });
 
           return createFlash({
@@ -219,7 +208,6 @@ export default {
         })
         .finally(() => {
           this.isUpdating = false;
-          this.testAlertPayload = null;
         });
     },
     resetToken({ type, variables }) {
@@ -276,6 +264,7 @@ export default {
           : updateCurrentPrometheusIntegrationMutation,
         variables: currentIntegration,
       });
+      this.setFormVisibility(true);
     },
     deleteIntegration({ id, type }) {
       const { projectPath } = this;
@@ -308,19 +297,19 @@ export default {
         });
     },
     clearCurrentIntegration({ type }) {
+      if (type) {
         this.$apollo.mutate({
           mutation: this.isHttp(type)
             ? updateCurrentHttpIntegrationMutation
             : updateCurrentPrometheusIntegrationMutation,
           variables: {},
         });
+      }
+      this.setFormVisibility(false);
     },
-    setTestAlertPayload(payload) {
-      this.testAlertPayload = payload;
-    },
-    validateAlertPayload(payload) {
+    testAlertPayload(payload) {
       return service
-        .updateTestAlert(payload ?? this.testAlertPayload)
+        .updateTestAlert(payload)
         .then(() => {
           return createFlash({
             message: this.$options.i18n.alertSent,
@@ -331,6 +320,9 @@ export default {
           createFlash({ message: INTEGRATION_PAYLOAD_TEST_ERROR });
         });
     },
+    setFormVisibility(visible) {
+      this.formVisible = visible;
+    },
   },
 };
 </script>
@@ -343,7 +335,18 @@ export default {
       @edit-integration="editIntegration"
       @delete-integration="deleteIntegration"
     />
+    <gl-button
+      v-if="canAddIntegration && !formVisible"
+      category="secondary"
+      variant="confirm"
+      data-testid="add-integration-btn"
+      class="gl-mt-3"
+      @click="setFormVisibility(true)"
+    >
+      {{ $options.i18n.addNewIntegration }}
+    </gl-button>
     <alert-settings-form
+      v-if="formVisible"
       :loading="isUpdating"
       :can-add-integration="canAddIntegration"
       :alert-fields="alertFields"
@@ -351,7 +354,7 @@ export default {
       @update-integration="updateIntegration"
       @reset-token="resetToken"
       @clear-current-integration="clearCurrentIntegration"
-      @set-test-alert-payload="setTestAlertPayload"
+      @test-alert-payload="testAlertPayload"
     />
   </div>
 </template>

app/assets/javascripts/alerts_settings/constants.js

-import { s__ } from '~/locale';
+import { s__, __ } from '~/locale';
 
 // TODO: Remove this as part of the form old removal
 export const i18n = {
@@ -38,6 +38,74 @@ export const i18n = {
     'AlertSettings|Authorization key has been successfully reset. Please save your changes now.',
   ),
   integration: s__('AlertSettings|Integration'),
+  integrationTabs: {
+    configureDetails: s__('AlertSettings|Configure details'),
+    viewCredentials: s__('AlertSettings|View credentials'),
+    sendTestAlert: s__('AlertSettings|Send test alert'),
+  },
+  integrationFormSteps: {
+    selectType: {
+      label: s__('AlertSettings|Select integration type'),
+      enterprise: s__(
+        'AlertSettings|In free versions of GitLab, only one integration for each type can be added. %{linkStart}Upgrade your subscription%{linkEnd} to add additional integrations.',
+      ),
+    },
+    nameIntegration: {
+      label: s__('AlertSettings|Name integration'),
+      placeholder: s__('AlertSettings|Enter integration name'),
+      activeToggle: __('Active'),
+    },
+    setupCredentials: {
+      help: s__(
+        "AlertSettings|Utilize the URL and authorization key below to authorize an external service to send alerts to GitLab. Review your external service's documentation to learn where to add these details, and the %{linkStart}GitLab documentation%{linkEnd} to learn more about configuring your endpoint.",
+      ),
+      prometheusHelp: s__(
+        'AlertSettings|Utilize the URL and authorization key below to authorize Prometheus to send alerts to GitLab. Review the Prometheus documentation to learn where to add these details, and the %{linkStart}GitLab documentation%{linkEnd} to learn more about configuring your endpoint.',
+      ),
+      webhookUrl: s__('AlertSettings|Webhook URL'),
+      authorizationKey: s__('AlertSettings|Authorization key'),
+      reset: s__('AlertSettings|Reset Key'),
+    },
+    setSamplePayload: {
+      label: s__('AlertSettings|Sample alert payload (optional)'),
+      testPayloadHelpHttp: s__(
+        'AlertSettings|Provide an example payload from the monitoring tool you intend to integrate with. This payload can be used to create a custom mapping (optional).',
+      ),
+      testPayloadHelp: s__(
+        'AlertSettings|Provide an example payload from the monitoring tool you intend to integrate with. This will allow you to send an alert to an active GitLab alerting point.',
+      ),
+      placeholder: s__('AlertSettings|{ "events": [{ "application": "Name of application" }] }'),
+      resetHeader: s__('AlertSettings|Reset the mapping'),
+      resetBody: s__(
+        "AlertSettings|If you edit the payload, the stored mapping will be reset, and you'll need to re-map the fields.",
+      ),
+      resetOk: s__('AlertSettings|Proceed with editing'),
+      editPayload: s__('AlertSettings|Edit payload'),
+      parsePayload: s__('AlertSettings|Parse payload for custom mapping'),
+      payloadParsedSucessMsg: s__(
+        'AlertSettings|Sample payload has been parsed. You can now map the fields.',
+      ),
+    },
+    mapFields: {
+      label: s__('AlertSettings|Map fields (optional)'),
+      intro: s__(
+        "AlertSettings|If you've provided a sample alert payload, you can create a custom mapping for your endpoint. The default GitLab alert keys are listed below. Please define which payload key should map to the specified GitLab key.",
+      ),
+    },
+    prometheusFormUrl: {
+      label: s__('AlertSettings|Prometheus API base URL'),
+      help: s__('AlertSettings|URL cannot be blank and must start with http or https'),
+    },
+    restKeyInfo: {
+      label: s__(
+        'AlertSettings|Resetting the authorization key for this project will require updating the authorization key in every alert source it is enabled in.',
+      ),
+    },
+  },
+  saveIntegration: s__('AlertSettings|Save integration'),
+  cancelAndClose: __('Cancel and close'),
+  send: s__('AlertSettings|Send'),
+  copy: __('Copy'),
 };
 
 export const integrationTypes = {

app/assets/javascripts/alerts_settings/utils/error_messages.js

@@ -17,5 +17,5 @@ export const RESET_INTEGRATION_TOKEN_ERROR = s__(
 );
 
 export const INTEGRATION_PAYLOAD_TEST_ERROR = s__(
-  'AlertsIntegrations|Integration payload is invalid. You can still save your changes.',
+  'AlertsIntegrations|Integration payload is invalid.',
 );

changelogs/unreleased/300827-split-alert-integrations-form.yml

+---
+title: Regroup alerts integration form into tabs
+merge_request: 54842
+author:
+type: changed

locale/gitlab.pot

@@ -2820,21 +2820,6 @@ msgstr ""
 msgid "AlertMappingBuilder|Title is a required field for alerts in GitLab. Should the payload field you specified not be available, specifiy which field we should use instead. "
 msgstr ""
 
-msgid "AlertSettings|1. Select integration type"
-msgstr ""
-
-msgid "AlertSettings|2. Name integration"
-msgstr ""
-
-msgid "AlertSettings|3. Set up webhook"
-msgstr ""
-
-msgid "AlertSettings|4. Sample alert payload (optional)"
-msgstr ""
-
-msgid "AlertSettings|5. Map fields (optional)"
-msgstr ""
-
 msgid "AlertSettings|API URL"
 msgstr ""
 
@@ -2844,7 +2829,7 @@ msgstr ""
 msgid "AlertSettings|Add URL and auth key to your Prometheus config file"
 msgstr ""
 
-msgid "AlertSettings|Add new integrations"
+msgid "AlertSettings|Add new integration"
 msgstr ""
 
 msgid "AlertSettings|Alert test payload"
@@ -2856,6 +2841,9 @@ msgstr ""
 msgid "AlertSettings|Authorization key has been successfully reset. Please save your changes now."
 msgstr ""
 
+msgid "AlertSettings|Configure details"
+msgstr ""
+
 msgid "AlertSettings|Copy"
 msgstr ""
 
@@ -2892,19 +2880,25 @@ msgstr ""
 msgid "AlertSettings|Learn more about our our upcoming %{linkStart}integrations%{linkEnd}"
 msgstr ""
 
-msgid "AlertSettings|Proceed with editing"
+msgid "AlertSettings|Map fields (optional)"
 msgstr ""
 
-msgid "AlertSettings|Prometheus"
+msgid "AlertSettings|Name integration"
+msgstr ""
+
+msgid "AlertSettings|Parse payload for custom mapping"
+msgstr ""
+
+msgid "AlertSettings|Proceed with editing"
 msgstr ""
 
 msgid "AlertSettings|Prometheus API base URL"
 msgstr ""
 
-msgid "AlertSettings|Provide an example payload from the monitoring tool you intend to integrate with. This payload can be used to create a custom mapping (optional), or to test the integration (also optional)."
+msgid "AlertSettings|Provide an example payload from the monitoring tool you intend to integrate with. This payload can be used to create a custom mapping (optional)."
 msgstr ""
 
-msgid "AlertSettings|Provide an example payload from the monitoring tool you intend to integrate with. This payload can be used to test the integration (optional)."
+msgid "AlertSettings|Provide an example payload from the monitoring tool you intend to integrate with. This will allow you to send an alert to an active GitLab alerting point."
 msgstr ""
 
 msgid "AlertSettings|Reset Key"
@@ -2922,10 +2916,10 @@ msgstr ""
 msgid "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."
 msgstr ""
 
-msgid "AlertSettings|Sample payload has been parsed. You can now map the fields."
+msgid "AlertSettings|Sample alert payload (optional)"
 msgstr ""
 
-msgid "AlertSettings|Save and test payload"
+msgid "AlertSettings|Sample payload has been parsed. You can now map the fields."
 msgstr ""
 
 msgid "AlertSettings|Save integration"
@@ -2934,7 +2928,10 @@ msgstr ""
 msgid "AlertSettings|Select integration type"
 msgstr ""
 
-msgid "AlertSettings|Submit payload"
+msgid "AlertSettings|Send"
+msgstr ""
+
+msgid "AlertSettings|Send test alert"
 msgstr ""
 
 msgid "AlertSettings|Test alert payload"
@@ -2961,6 +2958,9 @@ msgstr ""
 msgid "AlertSettings|Utilize the URL and authorization key below to authorize an external service to send alerts to GitLab. Review your external service's documentation to learn where to add these details, and the %{linkStart}GitLab documentation%{linkEnd} to learn more about configuring your endpoint."
 msgstr ""
 
+msgid "AlertSettings|View credentials"
+msgstr ""
+
 msgid "AlertSettings|Webhook URL"
 msgstr ""
 
@@ -2991,7 +2991,7 @@ msgstr ""
 msgid "AlertsIntegrations|Integration Name"
 msgstr ""
 
-msgid "AlertsIntegrations|Integration payload is invalid. You can still save your changes."
+msgid "AlertsIntegrations|Integration payload is invalid."
 msgstr ""
 
 msgid "AlertsIntegrations|No integrations have been added yet"
@@ -5418,6 +5418,9 @@ msgstr ""
 msgid "Cancel"
 msgstr ""
 
+msgid "Cancel and close"
+msgstr ""
+
 msgid "Cancel index deletion"
 msgstr ""
 

spec/features/alerts_settings/user_views_alerts_settings_spec.rb

@@ -30,8 +30,8 @@ RSpec.describe 'Alert integrations settings form', :js do
         end
       end
 
-      it 'shows the new alerts setting form' do
-        expect(page).to have_content('1. Select integration type')
+      it 'shows the integrations list title' do
+        expect(page).to have_content('Current integrations')
       end
     end
   end
@@ -44,7 +44,7 @@ RSpec.describe 'Alert integrations settings form', :js do
       wait_for_requests
     end
 
-    it 'shows the old alerts setting form' do
+    it 'does not have rights to access the setting form' do
       expect(page).not_to have_selector('.incident-management-list')
       expect(page).not_to have_selector('#js-alert-management-settings')
     end

spec/frontend/alerts_settings/components/alerts_settings_form_spec.js

-import {
-  GlForm,
-  GlFormSelect,
-  GlCollapse,
-  GlFormInput,
-  GlToggle,
-  GlFormTextarea,
-} from '@gitlab/ui';
+import { GlForm, GlFormSelect, GlFormInput, GlToggle, GlFormTextarea, GlTab } from '@gitlab/ui';
 import { mount } from '@vue/test-utils';
 import waitForPromises from 'helpers/wait_for_promises';
 import MappingBuilder from '~/alerts_settings/components/alert_mapping_builder.vue';
@@ -52,18 +45,18 @@ describe('AlertsSettingsForm', () => {
 
   const findForm = () => wrapper.find(GlForm);
   const findSelect = () => wrapper.find(GlFormSelect);
-  const findFormSteps = () => wrapper.find(GlCollapse);
   const findFormFields = () => wrapper.findAll(GlFormInput);
   const findFormToggle = () => wrapper.find(GlToggle);
-  const findTestPayloadSection = () => wrapper.find(`[id = "test-integration"]`);
+  const findSamplePayloadSection = () => wrapper.find('[data-testid="sample-payload-section"]');
   const findMappingBuilderSection = () => wrapper.find(`[id = "mapping-builder"]`);
   const findMappingBuilder = () => wrapper.findComponent(MappingBuilder);
   const findSubmitButton = () => wrapper.find(`[type = "submit"]`);
   const findMultiSupportText = () =>
     wrapper.find(`[data-testid="multi-integrations-not-supported"]`);
-  const findJsonTestSubmit = () => wrapper.find(`[data-testid="integration-test-and-submit"]`);
+  const findJsonTestSubmit = () => wrapper.find(`[data-testid="send-test-alert"]`);
   const findJsonTextArea = () => wrapper.find(`[id = "test-payload"]`);
   const findActionBtn = () => wrapper.find(`[data-testid="payload-action-btn"]`);
+  const findTabs = () => wrapper.findAll(GlTab);
 
   afterEach(() => {
     if (wrapper) {
@@ -95,7 +88,7 @@ describe('AlertsSettingsForm', () => {
       expect(findForm().exists()).toBe(true);
       expect(findSelect().exists()).toBe(true);
       expect(findMultiSupportText().exists()).toBe(false);
-      expect(findFormSteps().attributes('visible')).toBeUndefined();
+      expect(findFormFields()).toHaveLength(0);
     });
 
     it('shows the rest of the form when the dropdown is used', async () => {
@@ -110,11 +103,40 @@ describe('AlertsSettingsForm', () => {
       expect(findMultiSupportText().exists()).toBe(true);
     });
 
-    it('disabled the name input when the selected value is prometheus', async () => {
+    it('hides the name input when the selected value is prometheus', async () => {
       createComponent();
       await selectOptionAtIndex(2);
+      expect(findFormFields().at(0).attributes('id')).not.toBe('name-integration');
+    });
+
+    describe('form tabs', () => {
+      it('renders 3 tabs', () => {
+        expect(findTabs()).toHaveLength(3);
+      });
 
-      expect(findFormFields().at(0).attributes('disabled')).toBe('disabled');
+      it('only first tab is enabled on integration create', () => {
+        createComponent({
+          data: {
+            currentIntegration: null,
+          },
+        });
+        const tabs = findTabs();
+        expect(tabs.at(0).find('[role="tabpanel"]').classes('disabled')).toBe(false);
+        expect(tabs.at(1).find('[role="tabpanel"]').classes('disabled')).toBe(true);
+        expect(tabs.at(2).find('[role="tabpanel"]').classes('disabled')).toBe(true);
+      });
+
+      it('all tabs are enabled on integration edit', () => {
+        createComponent({
+          data: {
+            currentIntegration: { id: 1 },
+          },
+        });
+        const tabs = findTabs();
+        expect(tabs.at(0).find('[role="tabpanel"]').classes('disabled')).toBe(false);
+        expect(tabs.at(1).find('[role="tabpanel"]').classes('disabled')).toBe(false);
+        expect(tabs.at(2).find('[role="tabpanel"]').classes('disabled')).toBe(false);
+      });
     });
   });
 
@@ -195,14 +217,9 @@ describe('AlertsSettingsForm', () => {
     describe('PROMETHEUS', () => {
       it('create', async () => {
         createComponent();
-
         await selectOptionAtIndex(2);
-
         const apiUrl = 'https://test.com';
-        enableIntegration(1, apiUrl);
-
-        findFormToggle().trigger('click');
-
+        enableIntegration(0, apiUrl);
         const submitBtn = findSubmitButton();
         expect(submitBtn.exists()).toBe(true);
         expect(submitBtn.text()).toBe('Save integration');
@@ -226,7 +243,7 @@ describe('AlertsSettingsForm', () => {
         });
 
         const apiUrl = 'https://test-post.com';
-        enableIntegration(1, apiUrl);
+        enableIntegration(0, apiUrl);
 
         const submitBtn = findSubmitButton();
         expect(submitBtn.exists()).toBe(true);
@@ -264,7 +281,7 @@ describe('AlertsSettingsForm', () => {
 
       const jsonTestSubmit = findJsonTestSubmit();
       expect(jsonTestSubmit.exists()).toBe(true);
-      expect(jsonTestSubmit.text()).toBe('Save and test payload');
+      expect(jsonTestSubmit.text()).toBe('Send');
       expect(jsonTestSubmit.props('disabled')).toBe(true);
     });
 
@@ -313,16 +330,14 @@ describe('AlertsSettingsForm', () => {
 
       it(`textarea should be ${enabledState} when payload reset ${payloadResetMsg} and current integration is ${activeState}`, async () => {
         wrapper.setData({
-          currentIntegration: {
-            type: typeSet.http,
-            payloadExample: validSamplePayload,
-            payloadAttributeMappings: [],
-          },
+          selectedIntegration: typeSet.http,
           active,
           resetPayloadAndMappingConfirmed,
         });
         await wrapper.vm.$nextTick();
-        expect(findTestPayloadSection().find(GlFormTextarea).attributes('disabled')).toBe(disabled);
+        expect(findSamplePayloadSection().find(GlFormTextarea).attributes('disabled')).toBe(
+          disabled,
+        );
       });
     });
 
@@ -330,9 +345,9 @@ describe('AlertsSettingsForm', () => {
       describe.each`
         resetPayloadAndMappingConfirmed | payloadExample        | caption
         ${false}                        | ${validSamplePayload} | ${'Edit payload'}
-        ${true}                         | ${emptySamplePayload} | ${'Submit payload'}
-        ${true}                         | ${validSamplePayload} | ${'Submit payload'}
-        ${false}                        | ${emptySamplePayload} | ${'Submit payload'}
+        ${true}                         | ${emptySamplePayload} | ${'Parse payload for custom mapping'}
+        ${true}                         | ${validSamplePayload} | ${'Parse payload for custom mapping'}
+        ${false}                        | ${emptySamplePayload} | ${'Parse payload for custom mapping'}
       `('', ({ resetPayloadAndMappingConfirmed, payloadExample, caption }) => {
         const samplePayloadMsg = payloadExample ? 'was provided' : 'was not provided';
         const payloadResetMsg = resetPayloadAndMappingConfirmed
@@ -386,7 +401,7 @@ describe('AlertsSettingsForm', () => {
 
         await waitForPromises();
 
-        expect(findTestPayloadSection().find('.invalid-feedback').text()).toBe(errorMessage);
+        expect(findSamplePayloadSection().find('.invalid-feedback').text()).toBe(errorMessage);
       });
     });
   });