Move "Integration active" toggle to integration_form.vue

app/assets/javascripts/integrations/constants.js

@@ -2,7 +2,6 @@ import { s__, __ } from '~/locale';
 
 export const TEST_INTEGRATION_EVENT = 'testIntegration';
 export const SAVE_INTEGRATION_EVENT = 'saveIntegration';
-export const TOGGLE_INTEGRATION_EVENT = 'toggleIntegration';
 export const VALIDATE_INTEGRATION_FORM_EVENT = 'validateIntegrationForm';
 
 export const integrationLevels = {

app/assets/javascripts/integrations/edit/components/active_checkbox.vue

 <script>
 import { GlFormGroup, GlFormCheckbox } from '@gitlab/ui';
 import { mapGetters } from 'vuex';
-import { TOGGLE_INTEGRATION_EVENT } from '~/integrations/constants';
-import eventHub from '../event_hub';
 
 export default {
   name: 'ActiveCheckbox',
@@ -20,14 +18,11 @@ export default {
   },
   mounted() {
     this.activated = this.propsSource.initialActivated;
-    // Initialize view
-    this.$nextTick(() => {
-      this.onChange(this.activated);
-    });
+    this.onChange(this.activated);
   },
   methods: {
-    onChange(e) {
-      eventHub.$emit(TOGGLE_INTEGRATION_EVENT, e);
+    onChange(isChecked) {
+      this.$emit('toggle-integration-active', isChecked);
     },
   },
 };

app/assets/javascripts/integrations/edit/components/integration_form.vue

@@ -37,12 +37,21 @@ export default {
   },
   mixins: [glFeatureFlagsMixin()],
   props: {
+    formSelector: {
+      type: String,
+      required: true,
+    },
     helpHtml: {
       type: String,
       required: false,
       default: '',
     },
   },
+  data() {
+    return {
+      integrationActive: false,
+    };
+  },
   computed: {
     ...mapGetters(['currentKey', 'propsSource', 'isDisabled']),
     ...mapState([
@@ -71,7 +80,7 @@ export default {
   },
   mounted() {
     // this form element is defined in Haml
-    this.form = document.querySelector('.js-integration-settings-form');
+    this.form = document.querySelector(this.formSelector);
   },
   methods: {
     ...mapActions([
@@ -84,11 +93,15 @@ export default {
     ]),
     onSaveClick() {
       this.setIsSaving(true);
-      eventHub.$emit(SAVE_INTEGRATION_EVENT);
+
+      const formValid = this.form.checkValidity() || this.integrationActive === false;
+      eventHub.$emit(SAVE_INTEGRATION_EVENT, formValid);
     },
     onTestClick() {
       this.setIsTesting(true);
-      eventHub.$emit(TEST_INTEGRATION_EVENT);
+
+      const formValid = this.form.checkValidity();
+      eventHub.$emit(TEST_INTEGRATION_EVENT, formValid);
     },
     onResetClick() {
       this.fetchResetIntegration();
@@ -97,6 +110,19 @@ export default {
       const formData = new FormData(this.form);
       this.requestJiraIssueTypes(formData);
     },
+    onToggleIntegrationState(integrationActive) {
+      this.integrationActive = integrationActive;
+      if (!this.form) {
+        return;
+      }
+
+      // If integration will be active, enable form validation.
+      if (integrationActive) {
+        this.form.removeAttribute('novalidate');
+      } else {
+        this.form.setAttribute('novalidate', true);
+      }
+    },
   },
   helpHtmlConfig: {
     ADD_ATTR: ['target'], // allow external links, can be removed after https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1427 is implemented
@@ -123,7 +149,11 @@ export default {
         <!-- helpHtml is trusted input -->
         <div v-if="helpHtml" v-safe-html:[$options.helpHtmlConfig]="helpHtml"></div>
 
-        <active-checkbox v-if="propsSource.showActive" :key="`${currentKey}-active-checkbox`" />
+        <active-checkbox
+          v-if="propsSource.showActive"
+          :key="`${currentKey}-active-checkbox`"
+          @toggle-integration-active="onToggleIntegrationState"
+        />
         <jira-trigger-fields
           v-if="isJira"
           :key="`${currentKey}-jira-trigger-fields`"
@@ -167,6 +197,7 @@ export default {
             type="submit"
             :loading="isSaving"
             :disabled="isDisabled"
+            data-testid="save-button"
             data-qa-selector="save_changes_button"
             @click.prevent="onSaveClick"
           >
@@ -180,6 +211,7 @@ export default {
             :loading="isTesting"
             :disabled="isDisabled"
             :href="propsSource.testPath"
+            data-testid="test-button"
             @click.prevent="onTestClick"
           >
             {{ __('Test settings') }}

app/assets/javascripts/integrations/edit/index.js

@@ -85,7 +85,7 @@ function parseDatasetToProps(data) {
   };
 }
 
-export default (el, defaultEl) => {
+export default (el, defaultEl, formSelector) => {
   if (!el) {
     return null;
   }
@@ -112,6 +112,7 @@ export default (el, defaultEl) => {
       return createElement(IntegrationForm, {
         props: {
           helpHtml,
+          formSelector,
         },
       });
     },

app/assets/javascripts/integrations/integration_settings_form.js

@@ -5,7 +5,6 @@ import eventHub from './edit/event_hub';
 import {
   TEST_INTEGRATION_EVENT,
   SAVE_INTEGRATION_EVENT,
-  TOGGLE_INTEGRATION_EVENT,
   VALIDATE_INTEGRATION_FORM_EVENT,
   I18N_DEFAULT_ERROR_MESSAGE,
   I18N_SUCCESSFUL_CONNECTION_MESSAGE,
@@ -14,8 +13,8 @@ import { testIntegrationSettings } from './edit/api';
 
 export default class IntegrationSettingsForm {
   constructor(formSelector) {
+    this.formSelector = formSelector;
     this.$form = document.querySelector(formSelector);
-    this.formActive = false;
 
     this.vue = null;
 
@@ -28,26 +27,22 @@ export default class IntegrationSettingsForm {
     this.vue = initForm(
       document.querySelector('.js-vue-integration-settings'),
       document.querySelector('.js-vue-default-integration-settings'),
+      this.formSelector,
     );
-    eventHub.$on(TOGGLE_INTEGRATION_EVENT, (active) => {
-      this.formActive = active;
-      this.toggleServiceState();
+    eventHub.$on(TEST_INTEGRATION_EVENT, (formValid) => {
+      this.testIntegration(formValid);
     });
-    eventHub.$on(TEST_INTEGRATION_EVENT, () => {
-      this.testIntegration();
-    });
-    eventHub.$on(SAVE_INTEGRATION_EVENT, () => {
-      this.saveIntegration();
+    eventHub.$on(SAVE_INTEGRATION_EVENT, (formValid) => {
+      this.saveIntegration(formValid);
     });
   }
 
-  saveIntegration() {
+  saveIntegration(formValid) {
     // Save Service if not active and check the following if active;
     // 1) If form contents are valid
     // 2) If this service can be saved
     // If both conditions are true, we override form submission
     // and save the service using provided configuration.
-    const formValid = this.$form.checkValidity() || this.formActive === false;
 
     if (formValid) {
       delay(() => {
@@ -59,13 +54,13 @@ export default class IntegrationSettingsForm {
     }
   }
 
-  testIntegration() {
+  testIntegration(formValid) {
     // Service was marked active so now we check;
     // 1) If form contents are valid
     // 2) If this service can be tested
     // If both conditions are true, we override form submission
     // and test the service using provided configuration.
-    if (this.$form.checkValidity()) {
+    if (formValid) {
       this.testSettings(new FormData(this.$form));
     } else {
       eventHub.$emit(VALIDATE_INTEGRATION_FORM_EVENT);
@@ -73,17 +68,6 @@ export default class IntegrationSettingsForm {
     }
   }
 
-  /**
-   * Change Form's validation enforcement based on service status (active/inactive)
-   */
-  toggleServiceState() {
-    if (this.formActive) {
-      this.$form.removeAttribute('novalidate');
-    } else if (!this.$form.getAttribute('novalidate')) {
-      this.$form.setAttribute('novalidate', 'novalidate');
-    }
-  }
-
   /**
    * Get a list of Jira issue types for the currently configured project
    *

spec/frontend/integrations/edit/components/active_checkbox_spec.js

@@ -34,16 +34,22 @@ describe('ActiveCheckbox', () => {
       });
     });
 
-    describe('initialActivated is false', () => {
-      it('renders GlFormCheckbox as unchecked', () => {
+    describe('initialActivated is `false`', () => {
+      beforeEach(() => {
         createComponent({
           initialActivated: false,
         });
+      });
 
+      it('renders GlFormCheckbox as unchecked', () => {
         expect(findGlFormCheckbox().exists()).toBe(true);
         expect(findGlFormCheckbox().vm.$attrs.checked).toBe(false);
         expect(findInputInCheckbox().attributes('disabled')).toBeUndefined();
       });
+
+      it('emits `toggle-integration-active` event with `false` on mount', () => {
+        expect(wrapper.emitted('toggle-integration-active')[0]).toEqual([false]);
+      });
     });
 
     describe('initialActivated is true', () => {
@@ -63,10 +69,21 @@ describe('ActiveCheckbox', () => {
           findInputInCheckbox().trigger('click');
 
           await wrapper.vm.$nextTick();
-
           expect(findGlFormCheckbox().vm.$attrs.checked).toBe(false);
         });
       });
+
+      it('emits `toggle-integration-active` event with `true` on mount', () => {
+        expect(wrapper.emitted('toggle-integration-active')[0]).toEqual([true]);
+      });
+
+      describe('on checkbox `change` event', () => {
+        it('emits `toggle-integration-active` event', () => {
+          findGlFormCheckbox().vm.$emit('change', false);
+
+          expect(wrapper.emitted('toggle-integration-active')[1]).toEqual([false]);
+        });
+      });
     });
   });
 });

spec/frontend/integrations/edit/components/integration_form_spec.js

@@ -11,8 +11,15 @@ import JiraTriggerFields from '~/integrations/edit/components/jira_trigger_field
 import OverrideDropdown from '~/integrations/edit/components/override_dropdown.vue';
 import ResetConfirmationModal from '~/integrations/edit/components/reset_confirmation_modal.vue';
 import TriggerFields from '~/integrations/edit/components/trigger_fields.vue';
-import { integrationLevels } from '~/integrations/constants';
+import {
+  integrationLevels,
+  TEST_INTEGRATION_EVENT,
+  SAVE_INTEGRATION_EVENT,
+} from '~/integrations/constants';
 import { createStore } from '~/integrations/edit/store';
+import eventHub from '~/integrations/edit/event_hub';
+
+jest.mock('~/integrations/edit/event_hub');
 
 describe('IntegrationForm', () => {
   let wrapper;
@@ -31,7 +38,7 @@ describe('IntegrationForm', () => {
     dispatch = jest.spyOn(store, 'dispatch').mockImplementation();
 
     wrapper = shallowMountExtended(IntegrationForm, {
-      propsData: { ...props },
+      propsData: { ...props, formSelector: '.test' },
       store,
       stubs: {
         OverrideDropdown,
@@ -55,31 +62,13 @@ describe('IntegrationForm', () => {
   const findConfirmationModal = () => wrapper.findComponent(ConfirmationModal);
   const findResetConfirmationModal = () => wrapper.findComponent(ResetConfirmationModal);
   const findResetButton = () => wrapper.findByTestId('reset-button');
+  const findSaveButton = () => wrapper.findByTestId('save-button');
+  const findTestButton = () => wrapper.findByTestId('test-button');
   const findJiraTriggerFields = () => wrapper.findComponent(JiraTriggerFields);
   const findJiraIssuesFields = () => wrapper.findComponent(JiraIssuesFields);
   const findTriggerFields = () => wrapper.findComponent(TriggerFields);
 
   describe('template', () => {
-    describe('showActive is true', () => {
-      it('renders ActiveCheckbox', () => {
-        createComponent();
-
-        expect(findActiveCheckbox().exists()).toBe(true);
-      });
-    });
-
-    describe('showActive is false', () => {
-      it('does not render ActiveCheckbox', () => {
-        createComponent({
-          customStateProps: {
-            showActive: false,
-          },
-        });
-
-        expect(findActiveCheckbox().exists()).toBe(false);
-      });
-    });
-
     describe('integrationLevel is instance', () => {
       it('renders ConfirmationModal', () => {
         createComponent({
@@ -323,4 +312,122 @@ describe('IntegrationForm', () => {
       });
     });
   });
+
+  describe('ActiveCheckbox', () => {
+    describe.each`
+      showActive
+      ${true}
+      ${false}
+    `('when `showActive` is $showActive', ({ showActive }) => {
+      it(`${showActive ? 'renders' : 'does not render'} ActiveCheckbox`, () => {
+        createComponent({
+          customStateProps: {
+            showActive,
+          },
+        });
+
+        expect(findActiveCheckbox().exists()).toBe(showActive);
+      });
+    });
+
+    describe.each`
+      formActive | novalidate
+      ${true}    | ${null}
+      ${false}   | ${'true'}
+    `(
+      'when `toggle-integration-active` is emitted with $formActive',
+      ({ formActive, novalidate }) => {
+        let mockForm;
+
+        beforeEach(async () => {
+          mockForm = document.createElement('form');
+          jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
+
+          createComponent({
+            customStateProps: {
+              showActive: true,
+              initialActivated: false,
+            },
+          });
+
+          await findActiveCheckbox().vm.$emit('toggle-integration-active', formActive);
+        });
+
+        it(`sets noValidate to ${novalidate}`, () => {
+          expect(mockForm.getAttribute('novalidate')).toBe(novalidate);
+        });
+      },
+    );
+  });
+
+  describe('when `save` button is clicked', () => {
+    let mockForm;
+
+    describe.each`
+      checkValidityReturn | integrationActive | formValid
+      ${true}             | ${false}          | ${true}
+      ${true}             | ${true}           | ${true}
+      ${false}            | ${true}           | ${false}
+      ${false}            | ${false}          | ${true}
+    `(
+      'when form checkValidity returns $checkValidityReturn and integrationActive is $integrationActive',
+      ({ formValid, integrationActive, checkValidityReturn }) => {
+        beforeEach(() => {
+          mockForm = document.createElement('form');
+          jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
+          jest.spyOn(mockForm, 'checkValidity').mockReturnValue(checkValidityReturn);
+
+          createComponent({
+            customStateProps: {
+              showActive: true,
+              initialActivated: integrationActive,
+            },
+          });
+
+          findSaveButton().vm.$emit('click', new Event('click'));
+        });
+
+        it('dispatches setIsSaving action', () => {
+          expect(dispatch).toHaveBeenCalledWith('setIsSaving', true);
+        });
+
+        it(`emits \`SAVE_INTEGRATION_EVENT\` event with payload \`${formValid}\``, () => {
+          expect(eventHub.$emit).toHaveBeenCalledWith(SAVE_INTEGRATION_EVENT, formValid);
+        });
+      },
+    );
+  });
+
+  describe('when `test` button is clicked', () => {
+    let mockForm;
+
+    describe.each`
+      formValid
+      ${true}
+      ${false}
+    `('when form checkValidity returns $formValid', ({ formValid }) => {
+      beforeEach(() => {
+        mockForm = document.createElement('form');
+        jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
+        jest.spyOn(mockForm, 'checkValidity').mockReturnValue(formValid);
+
+        createComponent({
+          customStateProps: {
+            showActive: true,
+            canTest: true,
+          },
+        });
+
+        findTestButton().vm.$emit('click', new Event('click'));
+      });
+
+      it('dispatches setIsTesting action', () => {
+        expect(dispatch).toHaveBeenCalledWith('setIsTesting', true);
+      });
+
+      it(`emits \`TEST_INTEGRATION_EVENT\` event with payload \`${formValid}\``, () => {
+        expect(eventHub.$emit).toHaveBeenCalledWith(TEST_INTEGRATION_EVENT, formValid);
+      });
+    });
+  });
 });

spec/frontend/integrations/integration_settings_form_spec.js

@@ -6,7 +6,6 @@ import toast from '~/vue_shared/plugins/global_toast';
 import {
   I18N_SUCCESSFUL_CONNECTION_MESSAGE,
   I18N_DEFAULT_ERROR_MESSAGE,
-  TOGGLE_INTEGRATION_EVENT,
   TEST_INTEGRATION_EVENT,
   SAVE_INTEGRATION_EVENT,
 } from '~/integrations/constants';
@@ -16,6 +15,7 @@ jest.mock('~/vue_shared/plugins/global_toast');
 jest.mock('lodash/delay', () => (callback) => callback());
 
 const FIXTURE = 'services/edit_service.html';
+const mockFormSelector = '.js-integration-settings-form';
 
 describe('IntegrationSettingsForm', () => {
   let integrationSettingsForm;
@@ -25,7 +25,7 @@ describe('IntegrationSettingsForm', () => {
   beforeEach(() => {
     loadFixtures(FIXTURE);
 
-    integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form');
+    integrationSettingsForm = new IntegrationSettingsForm(mockFormSelector);
     integrationSettingsForm.init();
   });
 
@@ -33,7 +33,7 @@ describe('IntegrationSettingsForm', () => {
     it('should initialize form element refs on class object', () => {
       expect(integrationSettingsForm.$form).toBeDefined();
       expect(integrationSettingsForm.$form.nodeName).toBe('FORM');
-      expect(integrationSettingsForm.formActive).toBeDefined();
+      expect(integrationSettingsForm.formSelector).toBe(mockFormSelector);
     });
 
     it('should initialize form metadata on class object', () => {
@@ -47,6 +47,8 @@ describe('IntegrationSettingsForm', () => {
     beforeEach(() => {
       mockAxios = new MockAdaptor(axios);
       jest.spyOn(axios, 'put');
+      jest.spyOn(integrationSettingsForm, 'testSettings');
+      jest.spyOn(integrationSettingsForm.$form, 'submit');
     });
 
     afterEach(() => {
@@ -54,28 +56,10 @@ describe('IntegrationSettingsForm', () => {
       eventHub.dispose(); // clear event hub handlers
     });
 
-    describe('when event hub receives `TOGGLE_INTEGRATION_EVENT`', () => {
-      it('should remove `novalidate` attribute to form when called with `true`', () => {
-        eventHub.$emit(TOGGLE_INTEGRATION_EVENT, true);
-
-        expect(integrationSettingsForm.$form.getAttribute('novalidate')).toBe(null);
-      });
-
-      it('should set `novalidate` attribute to form when called with `false`', () => {
-        eventHub.$emit(TOGGLE_INTEGRATION_EVENT, false);
-
-        expect(integrationSettingsForm.$form.getAttribute('novalidate')).toBe('novalidate');
-      });
-    });
-
     describe('when event hub receives `TEST_INTEGRATION_EVENT`', () => {
       describe('when form is valid', () => {
-        beforeEach(() => {
-          jest.spyOn(integrationSettingsForm.$form, 'checkValidity').mockReturnValue(true);
-        });
-
         it('should make an ajax request with provided `formData`', async () => {
-          eventHub.$emit(TEST_INTEGRATION_EVENT);
+          eventHub.$emit(TEST_INTEGRATION_EVENT, true);
           await waitForPromises();
 
           expect(axios.put).toHaveBeenCalledWith(
@@ -91,7 +75,7 @@ describe('IntegrationSettingsForm', () => {
             error: false,
           });
 
-          eventHub.$emit(TEST_INTEGRATION_EVENT);
+          eventHub.$emit(TEST_INTEGRATION_EVENT, true);
           await waitForPromises();
 
           expect(toast).toHaveBeenCalledWith(I18N_SUCCESSFUL_CONNECTION_MESSAGE);
@@ -108,7 +92,7 @@ describe('IntegrationSettingsForm', () => {
             test_failed: false,
           });
 
-          eventHub.$emit(TEST_INTEGRATION_EVENT);
+          eventHub.$emit(TEST_INTEGRATION_EVENT, true);
           await waitForPromises();
 
           expect(toast).toHaveBeenCalledWith(`${errorMessage} ${serviceResponse}`);
@@ -117,7 +101,7 @@ describe('IntegrationSettingsForm', () => {
         it('should show error message if ajax request failed', async () => {
           mockAxios.onPut(integrationSettingsForm.testEndPoint).networkError();
 
-          eventHub.$emit(TEST_INTEGRATION_EVENT);
+          eventHub.$emit(TEST_INTEGRATION_EVENT, true);
           await waitForPromises();
 
           expect(toast).toHaveBeenCalledWith(I18N_DEFAULT_ERROR_MESSAGE);
@@ -127,7 +111,7 @@ describe('IntegrationSettingsForm', () => {
           const dispatchSpy = mockStoreDispatch();
           mockAxios.onPut(integrationSettingsForm.testEndPoint).networkError();
 
-          eventHub.$emit(TEST_INTEGRATION_EVENT);
+          eventHub.$emit(TEST_INTEGRATION_EVENT, true);
           await waitForPromises();
 
           expect(dispatchSpy).toHaveBeenCalledWith('setIsTesting', false);
@@ -135,15 +119,10 @@ describe('IntegrationSettingsForm', () => {
       });
 
       describe('when form is invalid', () => {
-        beforeEach(() => {
-          jest.spyOn(integrationSettingsForm.$form, 'checkValidity').mockReturnValue(false);
-          jest.spyOn(integrationSettingsForm, 'testSettings');
-        });
-
         it('should dispatch `setIsTesting` with `false` and not call `testSettings`', async () => {
           const dispatchSpy = mockStoreDispatch();
 
-          eventHub.$emit(TEST_INTEGRATION_EVENT);
+          eventHub.$emit(TEST_INTEGRATION_EVENT, false);
           await waitForPromises();
 
           expect(dispatchSpy).toHaveBeenCalledWith('setIsTesting', false);
@@ -154,13 +133,8 @@ describe('IntegrationSettingsForm', () => {
 
     describe('when event hub receives `SAVE_INTEGRATION_EVENT`', () => {
       describe('when form is valid', () => {
-        beforeEach(() => {
-          jest.spyOn(integrationSettingsForm.$form, 'checkValidity').mockReturnValue(true);
-          jest.spyOn(integrationSettingsForm.$form, 'submit');
-        });
-
         it('should submit the form', async () => {
-          eventHub.$emit(SAVE_INTEGRATION_EVENT);
+          eventHub.$emit(SAVE_INTEGRATION_EVENT, true);
           await waitForPromises();
 
           expect(integrationSettingsForm.$form.submit).toHaveBeenCalled();
@@ -169,15 +143,10 @@ describe('IntegrationSettingsForm', () => {
       });
 
       describe('when form is invalid', () => {
-        beforeEach(() => {
-          jest.spyOn(integrationSettingsForm.$form, 'checkValidity').mockReturnValue(false);
-          jest.spyOn(integrationSettingsForm.$form, 'submit');
-        });
-
         it('should dispatch `setIsSaving` with `false` and not submit form', async () => {
           const dispatchSpy = mockStoreDispatch();
 
-          eventHub.$emit(SAVE_INTEGRATION_EVENT);
+          eventHub.$emit(SAVE_INTEGRATION_EVENT, false);
 
           await waitForPromises();