Merge branch '342121-move-save-button-to-vue-component' into 'master'

Move "save settings" code from integration_settings_form.js to Vue component

See merge request gitlab-org/gitlab!76409

app/assets/javascripts/integrations/constants.js

 import { s__, __ } from '~/locale';
 
-export const SAVE_INTEGRATION_EVENT = 'saveIntegration';
 export const VALIDATE_INTEGRATION_FORM_EVENT = 'validateIntegrationForm';
 
 export const integrationLevels = {

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

 <script>
 import { GlModal } from '@gitlab/ui';
-import { mapGetters } from 'vuex';
 import { __ } from '~/locale';
 
 export default {
   components: {
     GlModal,
   },
+
   computed: {
-    ...mapGetters(['isDisabled']),
     primaryProps() {
       return {
         text: __('Save'),
-        attributes: [
-          { variant: 'confirm' },
-          { category: 'primary' },
-          { disabled: this.isDisabled },
-        ],
+        attributes: [{ variant: 'confirm' }, { category: 'primary' }],
       };
     },
     cancelProps() {

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

@@ -4,7 +4,6 @@ import * as Sentry from '@sentry/browser';
 import { mapState, mapActions, mapGetters } from 'vuex';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import {
-  SAVE_INTEGRATION_EVENT,
   VALIDATE_INTEGRATION_FORM_EVENT,
   I18N_FETCH_TEST_SETTINGS_DEFAULT_ERROR_MESSAGE,
   I18N_DEFAULT_ERROR_MESSAGE,
@@ -54,12 +53,13 @@ export default {
   data() {
     return {
       integrationActive: false,
-      testingLoading: false,
+      isTesting: false,
+      isSaving: false,
     };
   },
   computed: {
-    ...mapGetters(['currentKey', 'propsSource', 'isDisabled']),
-    ...mapState(['defaultState', 'customState', 'override', 'isSaving', 'isResetting']),
+    ...mapGetters(['currentKey', 'propsSource']),
+    ...mapState(['defaultState', 'customState', 'override', 'isResetting']),
     isEditable() {
       return this.propsSource.editable;
     },
@@ -78,11 +78,8 @@ export default {
     showTestButton() {
       return this.propsSource.canTest;
     },
-    disableSaveButton() {
-      return Boolean(this.isResetting || this.testingLoading);
-    },
-    disableResetButton() {
-      return Boolean(this.isSaving || this.testingLoading);
+    disableButtons() {
+      return Boolean(this.isSaving || this.isResetting || this.isTesting);
     },
   },
   mounted() {
@@ -90,21 +87,20 @@ export default {
     this.form = document.querySelector(this.formSelector);
   },
   methods: {
-    ...mapActions([
-      'setOverride',
-      'setIsSaving',
-      'setIsResetting',
-      'fetchResetIntegration',
-      'requestJiraIssueTypes',
-    ]),
+    ...mapActions(['setOverride', 'fetchResetIntegration', 'requestJiraIssueTypes']),
     onSaveClick() {
-      this.setIsSaving(true);
+      this.isSaving = true;
+
+      if (this.integrationActive && !this.form.checkValidity()) {
+        this.isSaving = false;
+        eventHub.$emit(VALIDATE_INTEGRATION_FORM_EVENT);
+        return;
+      }
 
-      const formValid = this.form.checkValidity() || this.integrationActive === false;
-      eventHub.$emit(SAVE_INTEGRATION_EVENT, formValid);
+      this.form.submit();
     },
     onTestClick() {
-      this.testingLoading = true;
+      this.isTesting = true;
 
       if (!this.form.checkValidity()) {
         eventHub.$emit(VALIDATE_INTEGRATION_FORM_EVENT);
@@ -126,7 +122,7 @@ export default {
           Sentry.captureException(error);
         })
         .finally(() => {
-          this.testingLoading = false;
+          this.isTesting = false;
         });
     },
     onResetClick() {
@@ -211,7 +207,7 @@ export default {
               category="primary"
               variant="confirm"
               :loading="isSaving"
-              :disabled="disableSaveButton"
+              :disabled="disableButtons"
               data-qa-selector="save_changes_button"
             >
               {{ __('Save changes') }}
@@ -224,7 +220,7 @@ export default {
             variant="confirm"
             type="submit"
             :loading="isSaving"
-            :disabled="disableSaveButton"
+            :disabled="disableButtons"
             data-testid="save-button"
             data-qa-selector="save_changes_button"
             @click.prevent="onSaveClick"
@@ -236,8 +232,8 @@ export default {
             v-if="showTestButton"
             category="secondary"
             variant="confirm"
-            :loading="testingLoading"
-            :disabled="isDisabled"
+            :loading="isTesting"
+            :disabled="disableButtons"
             data-testid="test-button"
             @click.prevent="onTestClick"
           >
@@ -250,7 +246,7 @@ export default {
               category="secondary"
               variant="confirm"
               :loading="isResetting"
-              :disabled="disableResetButton"
+              :disabled="disableButtons"
               data-testid="reset-button"
             >
               {{ __('Reset') }}

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

 <script>
 import { GlModal } from '@gitlab/ui';
-import { mapGetters } from 'vuex';
 
 import { __ } from '~/locale';
 
@@ -9,15 +8,10 @@ export default {
     GlModal,
   },
   computed: {
-    ...mapGetters(['isDisabled']),
     primaryProps() {
       return {
         text: __('Reset'),
-        attributes: [
-          { variant: 'warning' },
-          { category: 'primary' },
-          { disabled: this.isDisabled },
-        ],
+        attributes: [{ variant: 'warning' }, { category: 'primary' }],
       };
     },
     cancelProps() {

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

@@ -85,28 +85,31 @@ function parseDatasetToProps(data) {
   };
 }
 
-export default (el, defaultEl, formSelector) => {
-  if (!el) {
+export default function initIntegrationSettingsForm(formSelector) {
+  const customSettingsEl = document.querySelector('.js-vue-integration-settings');
+  const defaultSettingsEl = document.querySelector('.js-vue-default-integration-settings');
+
+  if (!customSettingsEl) {
     return null;
   }
 
-  const props = parseDatasetToProps(el.dataset);
+  const customSettingsProps = parseDatasetToProps(customSettingsEl.dataset);
   const initialState = {
     defaultState: null,
-    customState: props,
+    customState: customSettingsProps,
   };
-  if (defaultEl) {
-    initialState.defaultState = Object.freeze(parseDatasetToProps(defaultEl.dataset));
+  if (defaultSettingsEl) {
+    initialState.defaultState = Object.freeze(parseDatasetToProps(defaultSettingsEl.dataset));
   }
 
   // Here, we capture the "helpHtml", so we can pass it to the Vue component
   // to position it where ever it wants.
   // Because this node is a _child_ of `el`, it will be removed when the Vue component is mounted,
   // so we don't need to manually remove it.
-  const helpHtml = el.querySelector('.js-integration-help-html')?.innerHTML;
+  const helpHtml = customSettingsEl.querySelector('.js-integration-help-html')?.innerHTML;
 
   return new Vue({
-    el,
+    el: customSettingsEl,
     store: createStore(initialState),
     render(createElement) {
       return createElement(IntegrationForm, {
@@ -117,4 +120,4 @@ export default (el, defaultEl, formSelector) => {
       });
     },
   });
-};
+}

app/assets/javascripts/integrations/edit/store/actions.js

@@ -10,7 +10,6 @@ import eventHub from '../event_hub';
 import * as types from './mutation_types';
 
 export const setOverride = ({ commit }, override) => commit(types.SET_OVERRIDE, override);
-export const setIsSaving = ({ commit }, isSaving) => commit(types.SET_IS_SAVING, isSaving);
 export const setIsResetting = ({ commit }, isResetting) =>
   commit(types.SET_IS_RESETTING, isResetting);
 

app/assets/javascripts/integrations/edit/store/getters.js

 export const isInheriting = (state) => (state.defaultState === null ? false : !state.override);
 
-export const isDisabled = (state) => state.isSaving || state.isResetting;
-
 export const propsSource = (state, getters) =>
   getters.isInheriting ? state.defaultState : state.customState;
 

app/assets/javascripts/integrations/edit/store/mutation_types.js

 export const SET_OVERRIDE = 'SET_OVERRIDE';
-export const SET_IS_SAVING = 'SET_IS_SAVING';
 export const SET_IS_RESETTING = 'SET_IS_RESETTING';
 
 export const SET_IS_LOADING_JIRA_ISSUE_TYPES = 'SET_IS_LOADING_JIRA_ISSUE_TYPES';

app/assets/javascripts/integrations/edit/store/mutations.js

@@ -4,9 +4,6 @@ export default {
   [types.SET_OVERRIDE](state, override) {
     state.override = override;
   },
-  [types.SET_IS_SAVING](state, isSaving) {
-    state.isSaving = isSaving;
-  },
   [types.SET_IS_RESETTING](state, isResetting) {
     state.isResetting = isResetting;
   },

app/assets/javascripts/integrations/integration_settings_form.js

-import { delay } from 'lodash';
-import initForm from './edit';
-import eventHub from './edit/event_hub';
-import { SAVE_INTEGRATION_EVENT, VALIDATE_INTEGRATION_FORM_EVENT } from './constants';
-
-export default class IntegrationSettingsForm {
-  constructor(formSelector) {
-    this.formSelector = formSelector;
-    this.$form = document.querySelector(formSelector);
-
-    this.vue = null;
-
-    // Form Metadata
-    this.testEndPoint = this.$form.dataset.testUrl;
-  }
-
-  init() {
-    // Init Vue component
-    this.vue = initForm(
-      document.querySelector('.js-vue-integration-settings'),
-      document.querySelector('.js-vue-default-integration-settings'),
-      this.formSelector,
-    );
-    eventHub.$on(SAVE_INTEGRATION_EVENT, (formValid) => {
-      this.saveIntegration(formValid);
-    });
-  }
-
-  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.
-
-    if (formValid) {
-      delay(() => {
-        this.$form.submit();
-      }, 100);
-    } else {
-      eventHub.$emit(VALIDATE_INTEGRATION_FORM_EVENT);
-      this.vue.$store.dispatch('setIsSaving', false);
-    }
-  }
-}

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

-import IntegrationSettingsForm from '~/integrations/integration_settings_form';
+import initIntegrationSettingsForm from '~/integrations/edit';
 import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics';
 
-function initIntegrations() {
-  const prometheusSettingsWrapper = document.querySelector('.js-prometheus-metrics-monitoring');
-  const integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form');
-  integrationSettingsForm.init();
+initIntegrationSettingsForm('.js-integration-settings-form');
 
-  if (prometheusSettingsWrapper) {
-    const prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring');
-    prometheusMetrics.loadActiveMetrics();
-  }
+const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring';
+const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector);
+if (prometheusSettingsWrapper) {
+  const prometheusMetrics = new PrometheusMetrics(prometheusSettingsSelector);
+  prometheusMetrics.loadActiveMetrics();
 }
-
-initIntegrations();

app/assets/javascripts/pages/groups/settings/integrations/edit/index.js

-import IntegrationSettingsForm from '~/integrations/integration_settings_form';
+import initIntegrationSettingsForm from '~/integrations/edit';
 import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics';
 
-const prometheusSettingsWrapper = document.querySelector('.js-prometheus-metrics-monitoring');
-const integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form');
-integrationSettingsForm.init();
+initIntegrationSettingsForm('.js-integration-settings-form');
 
+const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring';
+const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector);
 if (prometheusSettingsWrapper) {
-  const prometheusMetrics = new PrometheusMetrics('.js-prometheus-metrics-monitoring');
+  const prometheusMetrics = new PrometheusMetrics(prometheusSettingsSelector);
   prometheusMetrics.loadActiveMetrics();
 }

app/assets/javascripts/pages/projects/services/edit/index.js

-import IntegrationSettingsForm from '~/integrations/integration_settings_form';
+import initIntegrationSettingsForm from '~/integrations/edit';
 import PrometheusAlerts from '~/prometheus_alerts';
 import CustomMetrics from '~/prometheus_metrics/custom_metrics';
 
-const integrationSettingsForm = new IntegrationSettingsForm('.js-integration-settings-form');
-integrationSettingsForm.init();
+initIntegrationSettingsForm('.js-integration-settings-form');
 
 const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring';
 const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector);

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

@@ -18,7 +18,6 @@ import {
   integrationLevels,
   I18N_SUCCESSFUL_CONNECTION_MESSAGE,
   VALIDATE_INTEGRATION_FORM_EVENT,
-  SAVE_INTEGRATION_EVENT,
   I18N_DEFAULT_ERROR_MESSAGE,
 } from '~/integrations/constants';
 import { createStore } from '~/integrations/edit/store';
@@ -34,6 +33,7 @@ describe('IntegrationForm', () => {
   let wrapper;
   let dispatch;
   let mockAxios;
+  let mockForm;
 
   const createComponent = ({
     customStateProps = {},
@@ -69,11 +69,10 @@ describe('IntegrationForm', () => {
   };
 
   const createForm = ({ isValid = true } = {}) => {
-    const mockForm = document.createElement('form');
+    mockForm = document.createElement('form');
     jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
     jest.spyOn(mockForm, 'checkValidity').mockReturnValue(isValid);
-
-    return mockForm;
+    jest.spyOn(mockForm, 'submit');
   };
 
   const findOverrideDropdown = () => wrapper.findComponent(OverrideDropdown);
@@ -365,12 +364,8 @@ describe('IntegrationForm', () => {
     `(
       'when `toggle-integration-active` is emitted with $formActive',
       ({ formActive, novalidate }) => {
-        let mockForm;
-
         beforeEach(async () => {
-          mockForm = document.createElement('form');
-          jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
-
+          createForm();
           createComponent({
             customStateProps: {
               showActive: true,
@@ -389,41 +384,86 @@ describe('IntegrationForm', () => {
   });
 
   describe('when `save` button is clicked', () => {
-    let mockForm;
+    describe('buttons', () => {
+      beforeEach(async () => {
+        createForm();
+        createComponent({
+          customStateProps: {
+            showActive: true,
+            canTest: true,
+            initialActivated: true,
+          },
+        });
+
+        await findSaveButton().vm.$emit('click', new Event('click'));
+      });
+
+      it('sets save button `loading` prop to `true`', () => {
+        expect(findSaveButton().props('loading')).toBe(true);
+      });
+
+      it('sets test button `disabled` prop to `true`', () => {
+        expect(findTestButton().props('disabled')).toBe(true);
+      });
+    });
 
     describe.each`
-      checkValidityReturn | integrationActive | formValid
-      ${true}             | ${false}          | ${true}
-      ${true}             | ${true}           | ${true}
-      ${false}            | ${true}           | ${false}
-      ${false}            | ${false}          | ${true}
+      checkValidityReturn | integrationActive
+      ${true}             | ${false}
+      ${true}             | ${true}
+      ${false}            | ${false}
     `(
-      '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);
-
+      'when form is valid (checkValidity returns $checkValidityReturn and integrationActive is $integrationActive)',
+      ({ integrationActive, checkValidityReturn }) => {
+        beforeEach(async () => {
+          createForm({ isValid: checkValidityReturn });
           createComponent({
             customStateProps: {
               showActive: true,
+              canTest: true,
               initialActivated: integrationActive,
             },
           });
 
-          findSaveButton().vm.$emit('click', new Event('click'));
-        });
-
-        it('dispatches setIsSaving action', () => {
-          expect(dispatch).toHaveBeenCalledWith('setIsSaving', true);
+          await findSaveButton().vm.$emit('click', new Event('click'));
         });
 
-        it(`emits \`SAVE_INTEGRATION_EVENT\` event with payload \`${formValid}\``, () => {
-          expect(eventHub.$emit).toHaveBeenCalledWith(SAVE_INTEGRATION_EVENT, formValid);
+        it('submit form', () => {
+          expect(mockForm.submit).toHaveBeenCalledTimes(1);
         });
       },
     );
+
+    describe('when form is invalid (checkValidity returns false and integrationActive is true)', () => {
+      beforeEach(async () => {
+        createForm({ isValid: false });
+        createComponent({
+          customStateProps: {
+            showActive: true,
+            canTest: true,
+            initialActivated: true,
+          },
+        });
+
+        await findSaveButton().vm.$emit('click', new Event('click'));
+      });
+
+      it('does not submit form', () => {
+        expect(mockForm.submit).not.toHaveBeenCalled();
+      });
+
+      it('sets save button `loading` prop to `false`', () => {
+        expect(findSaveButton().props('loading')).toBe(false);
+      });
+
+      it('sets test button `disabled` prop to `false`', () => {
+        expect(findTestButton().props('disabled')).toBe(false);
+      });
+
+      it('emits `VALIDATE_INTEGRATION_FORM_EVENT`', () => {
+        expect(eventHub.$emit).toHaveBeenCalledWith(VALIDATE_INTEGRATION_FORM_EVENT);
+      });
+    });
   });
 
   describe('when `test` button is clicked', () => {

spec/frontend/integrations/edit/store/actions_spec.js

@@ -4,7 +4,6 @@ import testAction from 'helpers/vuex_action_helper';
 import { I18N_FETCH_TEST_SETTINGS_DEFAULT_ERROR_MESSAGE } from '~/integrations/constants';
 import {
   setOverride,
-  setIsSaving,
   setIsResetting,
   requestResetIntegration,
   receiveResetIntegrationSuccess,
@@ -39,12 +38,6 @@ describe('Integration form store actions', () => {
     });
   });
 
-  describe('setIsSaving', () => {
-    it('should commit isSaving mutation', () => {
-      return testAction(setIsSaving, true, state, [{ type: types.SET_IS_SAVING, payload: true }]);
-    });
-  });
-
   describe('setIsResetting', () => {
     it('should commit isResetting mutation', () => {
       return testAction(setIsResetting, true, state, [

spec/frontend/integrations/edit/store/getters_spec.js

-import {
-  currentKey,
-  isInheriting,
-  isDisabled,
-  propsSource,
-} from '~/integrations/edit/store/getters';
-import * as types from '~/integrations/edit/store/mutation_types';
-import mutations from '~/integrations/edit/store/mutations';
+import { currentKey, isInheriting, propsSource } from '~/integrations/edit/store/getters';
 import createState from '~/integrations/edit/store/state';
 import { mockIntegrationProps } from '../mock_data';
 
@@ -52,24 +45,6 @@ describe('Integration form store getters', () => {
     });
   });
 
-  describe('isDisabled', () => {
-    it.each`
-      isSaving | isResetting | expected
-      ${false} | ${false}    | ${false}
-      ${true}  | ${false}    | ${true}
-      ${false} | ${true}     | ${true}
-      ${true}  | ${true}     | ${true}
-    `(
-      'when isSaving = $isSaving, isResetting = $isResetting then isDisabled = $expected',
-      ({ isSaving, isResetting, expected }) => {
-        mutations[types.SET_IS_SAVING](state, isSaving);
-        mutations[types.SET_IS_RESETTING](state, isResetting);
-
-        expect(isDisabled(state)).toBe(expected);
-      },
-    );
-  });
-
   describe('propsSource', () => {
     beforeEach(() => {
       state.defaultState = defaultState;

spec/frontend/integrations/edit/store/mutations_spec.js

@@ -17,14 +17,6 @@ describe('Integration form store mutations', () => {
     });
   });
 
-  describe(`${types.SET_IS_SAVING}`, () => {
-    it('sets isSaving', () => {
-      mutations[types.SET_IS_SAVING](state, true);
-
-      expect(state.isSaving).toBe(true);
-    });
-  });
-
   describe(`${types.SET_IS_RESETTING}`, () => {
     it('sets isResetting', () => {
       mutations[types.SET_IS_RESETTING](state, true);

spec/frontend/integrations/integration_settings_form_spec.js

-import MockAdaptor from 'axios-mock-adapter';
-import IntegrationSettingsForm from '~/integrations/integration_settings_form';
-import eventHub from '~/integrations/edit/event_hub';
-import axios from '~/lib/utils/axios_utils';
-import { SAVE_INTEGRATION_EVENT } from '~/integrations/constants';
-import waitForPromises from 'helpers/wait_for_promises';
-
-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;
-
-  const mockStoreDispatch = () => jest.spyOn(integrationSettingsForm.vue.$store, 'dispatch');
-
-  beforeEach(() => {
-    loadFixtures(FIXTURE);
-
-    integrationSettingsForm = new IntegrationSettingsForm(mockFormSelector);
-    integrationSettingsForm.init();
-  });
-
-  afterEach(() => {
-    eventHub.dispose(); // clear event hub handlers
-  });
-
-  describe('constructor', () => {
-    it('should initialize form element refs on class object', () => {
-      expect(integrationSettingsForm.$form).toBeDefined();
-      expect(integrationSettingsForm.$form.nodeName).toBe('FORM');
-      expect(integrationSettingsForm.formSelector).toBe(mockFormSelector);
-    });
-
-    it('should initialize form metadata on class object', () => {
-      expect(integrationSettingsForm.testEndPoint).toBeDefined();
-    });
-  });
-
-  describe('event handling', () => {
-    let mockAxios;
-
-    beforeEach(() => {
-      mockAxios = new MockAdaptor(axios);
-      jest.spyOn(axios, 'put');
-      jest.spyOn(integrationSettingsForm.$form, 'submit');
-    });
-
-    afterEach(() => {
-      mockAxios.restore();
-    });
-
-    describe('when event hub receives `SAVE_INTEGRATION_EVENT`', () => {
-      describe('when form is valid', () => {
-        it('should submit the form', async () => {
-          eventHub.$emit(SAVE_INTEGRATION_EVENT, true);
-          await waitForPromises();
-
-          expect(integrationSettingsForm.$form.submit).toHaveBeenCalledTimes(1);
-        });
-      });
-
-      describe('when form is invalid', () => {
-        it('should dispatch `setIsSaving` with `false` and not submit form', async () => {
-          const dispatchSpy = mockStoreDispatch();
-
-          eventHub.$emit(SAVE_INTEGRATION_EVENT, false);
-
-          await waitForPromises();
-
-          expect(dispatchSpy).toHaveBeenCalledWith('setIsSaving', false);
-          expect(integrationSettingsForm.$form.submit).not.toHaveBeenCalled();
-        });
-      });
-    });
-  });
-});