Use gl-form in integration_form.vue

This commit moves the <form> element
for integrations from HAML to Vue.

No user-facing changes should
be expected

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

 <script>
-import { GlButton, GlModalDirective, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
+import { GlButton, GlModalDirective, GlSafeHtmlDirective as SafeHtml, GlForm } from '@gitlab/ui';
 import axios from 'axios';
 import * as Sentry from '@sentry/browser';
 import { mapState, mapActions, mapGetters } from 'vuex';
@@ -12,6 +12,7 @@ import {
   integrationLevels,
 } from '~/integrations/constants';
 import { refreshCurrentPage } from '~/lib/utils/url_utility';
+import csrf from '~/lib/utils/csrf';
 import eventHub from '../event_hub';
 import { testIntegrationSettings } from '../api';
 import ActiveCheckbox from './active_checkbox.vue';
@@ -35,6 +36,7 @@ export default {
     ConfirmationModal,
     ResetConfirmationModal,
     GlButton,
+    GlForm,
   },
   directives: {
     GlModal: GlModalDirective,
@@ -42,10 +44,6 @@ export default {
   },
   mixins: [glFeatureFlagsMixin()],
   props: {
-    formSelector: {
-      type: String,
-      required: true,
-    },
     helpHtml: {
       type: String,
       required: false,
@@ -84,16 +82,14 @@ export default {
     disableButtons() {
       return Boolean(this.isSaving || this.isResetting || this.isTesting);
     },
+    form() {
+      return this.$refs.integrationForm.$el;
     },
-  mounted() {
-    // this form element is defined in Haml
-    this.form = document.querySelector(this.formSelector);
   },
   methods: {
     ...mapActions(['setOverride', 'fetchResetIntegration', 'requestJiraIssueTypes']),
     onSaveClick() {
       this.isSaving = true;
-
       if (this.integrationActive && !this.form.checkValidity()) {
         this.isSaving = false;
         eventHub.$emit(VALIDATE_INTEGRATION_FORM_EVENT);
@@ -152,16 +148,6 @@ export default {
     },
     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: {
@@ -169,11 +155,27 @@ export default {
     ADD_TAGS: ['use'], // to support icon SVGs
     FORBID_ATTR: [], // This is trusted input so we can override the default config to allow data-* attributes
   },
+  csrf,
 };
 </script>
 
 <template>
-  <div class="gl-mb-3">
+  <gl-form
+    ref="integrationForm"
+    method="post"
+    class="gl-mb-3 gl-show-field-errors integration-settings-form"
+    :action="propsSource.formPath"
+    :novalidate="!integrationActive"
+  >
+    <input type="hidden" name="_method" value="put" />
+    <input type="hidden" name="authenticity_token" :value="$options.csrf.token" />
+    <input
+      type="hidden"
+      name="redirect_to"
+      :value="propsSource.redirectTo"
+      data-testid="redirect-to-field"
+    />
+
     <override-dropdown
       v-if="defaultState !== null"
       :inherit-from-id="defaultState.id"
@@ -282,5 +284,5 @@ export default {
         </div>
       </div>
     </div>
-  </div>
+  </gl-form>
 </template>

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

@@ -28,9 +28,11 @@ function parseDatasetToProps(data) {
     cancelPath,
     testPath,
     resetPath,
+    formPath,
     vulnerabilitiesIssuetype,
     jiraIssueTransitionAutomatic,
     jiraIssueTransitionId,
+    redirectTo,
     ...booleanAttributes
   } = data;
   const {
@@ -57,6 +59,7 @@ function parseDatasetToProps(data) {
     canTest,
     testPath,
     resetPath,
+    formPath,
     triggerFieldsProps: {
       initialTriggerCommit: commitEvents,
       initialTriggerMergeRequest: mergeRequestEvents,
@@ -82,10 +85,11 @@ function parseDatasetToProps(data) {
     inheritFromId: parseInt(inheritFromId, 10),
     integrationLevel,
     id: parseInt(id, 10),
+    redirectTo,
   };
 }
 
-export default function initIntegrationSettingsForm(formSelector) {
+export default function initIntegrationSettingsForm() {
   const customSettingsEl = document.querySelector('.js-vue-integration-settings');
   const defaultSettingsEl = document.querySelector('.js-vue-default-integration-settings');
 
@@ -115,7 +119,6 @@ export default function initIntegrationSettingsForm(formSelector) {
       return createElement(IntegrationForm, {
         props: {
           helpHtml,
-          formSelector,
         },
       });
     },

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

 import initIntegrationSettingsForm from '~/integrations/edit';
 import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics';
 
-initIntegrationSettingsForm('.js-integration-settings-form');
+initIntegrationSettingsForm();
 
 const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring';
 const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector);

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

 import initIntegrationSettingsForm from '~/integrations/edit';
 import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics';
 
-initIntegrationSettingsForm('.js-integration-settings-form');
+initIntegrationSettingsForm();
 
 const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring';
 const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector);

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

@@ -2,7 +2,7 @@ import initIntegrationSettingsForm from '~/integrations/edit';
 import PrometheusAlerts from '~/prometheus_alerts';
 import CustomMetrics from '~/prometheus_metrics/custom_metrics';
 
-initIntegrationSettingsForm('.js-integration-settings-form');
+initIntegrationSettingsForm();
 
 const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring';
 const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector);

app/helpers/integrations_helper.rb

@@ -90,7 +90,9 @@ module IntegrationsHelper
       cancel_path: scoped_integrations_path(project: project, group: group),
       can_test: integration.testable?.to_s,
       test_path: scoped_test_integration_path(integration, project: project, group: group),
-      reset_path: scoped_reset_integration_path(integration, group: group)
+      reset_path: scoped_reset_integration_path(integration, group: group),
+      form_path: scoped_integration_path(integration, project: project, group: group),
+      redirect_to: request.referer
     }
 
     if integration.is_a?(Integrations::Jira)

app/views/projects/services/_form.html.haml

@@ -6,10 +6,7 @@
   - if integration.operating?
     = sprite_icon('check', css_class: 'gl-text-green-500')
 
-= form_for(integration, as: :service, url: scoped_integration_path(integration, project: @project, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_integration_path(@project, integration) } }) do |form|
-  = render 'shared/service_settings', form: form, integration: integration
-  %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referer }
-
+= render 'shared/service_settings', integration: integration
 - if lookup_context.template_exists?('show', "projects/services/#{integration.to_param}", true)
   %hr
   = render "projects/services/#{integration.to_param}/show", integration: integration

app/views/shared/integrations/_form.html.haml

 - integration = local_assigns.fetch(:integration)
 
-= form_for integration, as: :service, url: scoped_integration_path(integration, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration, group: @group) } } do |form|
-  = render 'shared/service_settings', form: form, integration: integration
+= render 'shared/service_settings', integration: integration

ee/spec/features/projects/settings/slack_application_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe 'Slack application' do
   it 'I can edit slack integration' do
     visit slack_application_form_path
 
-    within '.js-integration-settings-form' do
+    within '.service-settings' do
       click_link 'Edit'
     end
 
@@ -31,7 +31,7 @@ RSpec.describe 'Slack application' do
 
     expect(page).to have_content('The project alias was updated successfully')
 
-    within '.js-integration-settings-form' do
+    within '.service-settings' do
       expect(page).to have_content('alias-edited')
     end
   end

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

+import { GlForm } from '@gitlab/ui';
 import axios from 'axios';
 import MockAdapter from 'axios-mock-adapter';
 import * as Sentry from '@sentry/browser';
 import { setHTMLFixture } from 'helpers/fixtures';
-import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import { mountExtended } from 'helpers/vue_test_utils_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import ActiveCheckbox from '~/integrations/edit/components/active_checkbox.vue';
 import ConfirmationModal from '~/integrations/edit/components/confirmation_modal.vue';
@@ -35,7 +36,6 @@ describe('IntegrationForm', () => {
   let wrapper;
   let dispatch;
   let mockAxios;
-  let mockForm;
 
   const createComponent = ({
     customStateProps = {},
@@ -49,7 +49,7 @@ describe('IntegrationForm', () => {
     });
     dispatch = jest.spyOn(store, 'dispatch').mockImplementation();
 
-    wrapper = shallowMountExtended(IntegrationForm, {
+    wrapper = mountExtended(IntegrationForm, {
       propsData: { ...props, formSelector: '.test' },
       provide: {
         glFeatures: featureFlags,
@@ -70,13 +70,6 @@ describe('IntegrationForm', () => {
     });
   };
 
-  const createForm = ({ isValid = true } = {}) => {
-    mockForm = document.createElement('form');
-    jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
-    jest.spyOn(mockForm, 'checkValidity').mockReturnValue(isValid);
-    jest.spyOn(mockForm, 'submit');
-  };
-
   const findOverrideDropdown = () => wrapper.findComponent(OverrideDropdown);
   const findActiveCheckbox = () => wrapper.findComponent(ActiveCheckbox);
   const findConfirmationModal = () => wrapper.findComponent(ConfirmationModal);
@@ -88,6 +81,8 @@ describe('IntegrationForm', () => {
   const findJiraTriggerFields = () => wrapper.findComponent(JiraTriggerFields);
   const findJiraIssuesFields = () => wrapper.findComponent(JiraIssuesFields);
   const findTriggerFields = () => wrapper.findComponent(TriggerFields);
+  const findGlForm = () => wrapper.findComponent(GlForm);
+  const findRedirectToField = () => wrapper.findByTestId('redirect-to-field');
 
   beforeEach(() => {
     mockAxios = new MockAdapter(axios);
@@ -341,6 +336,16 @@ describe('IntegrationForm', () => {
         });
       });
     });
+
+    it('renders hidden fields', () => {
+      createComponent({
+        customStateProps: {
+          redirectTo: '/services',
+        },
+      });
+
+      expect(findRedirectToField().attributes('value')).toBe('/services');
+    });
   });
 
   describe('ActiveCheckbox', () => {
@@ -362,13 +367,12 @@ describe('IntegrationForm', () => {
 
     describe.each`
       formActive | novalidate
-      ${true}    | ${null}
-      ${false}   | ${'true'}
+      ${true}    | ${undefined}
+      ${false}   | ${'novalidate'}
     `(
       'when `toggle-integration-active` is emitted with $formActive',
       ({ formActive, novalidate }) => {
         beforeEach(async () => {
-          createForm();
           createComponent({
             customStateProps: {
               showActive: true,
@@ -380,7 +384,7 @@ describe('IntegrationForm', () => {
         });
 
         it(`sets noValidate to ${novalidate}`, () => {
-          expect(mockForm.getAttribute('novalidate')).toBe(novalidate);
+          expect(findGlForm().attributes('novalidate')).toBe(novalidate);
         });
       },
     );
@@ -389,7 +393,6 @@ describe('IntegrationForm', () => {
   describe('when `save` button is clicked', () => {
     describe('buttons', () => {
       beforeEach(async () => {
-        createForm();
         createComponent({
           customStateProps: {
             showActive: true,
@@ -419,7 +422,6 @@ describe('IntegrationForm', () => {
       'when form is valid (checkValidity returns $checkValidityReturn and integrationActive is $integrationActive)',
       ({ integrationActive, checkValidityReturn }) => {
         beforeEach(async () => {
-          createForm({ isValid: checkValidityReturn });
           createComponent({
             customStateProps: {
               showActive: true,
@@ -427,19 +429,20 @@ describe('IntegrationForm', () => {
               initialActivated: integrationActive,
             },
           });
+          jest.spyOn(findGlForm().element, 'submit');
+          jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(checkValidityReturn);
 
           await findProjectSaveButton().vm.$emit('click', new Event('click'));
         });
 
         it('submit form', () => {
-          expect(mockForm.submit).toHaveBeenCalledTimes(1);
+          expect(findGlForm().element.submit).toHaveBeenCalledTimes(1);
         });
       },
     );
 
     describe('when form is invalid (checkValidity returns false and integrationActive is true)', () => {
       beforeEach(async () => {
-        createForm({ isValid: false });
         createComponent({
           customStateProps: {
             showActive: true,
@@ -447,12 +450,14 @@ describe('IntegrationForm', () => {
             initialActivated: true,
           },
         });
+        jest.spyOn(findGlForm().element, 'submit');
+        jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(false);
 
         await findProjectSaveButton().vm.$emit('click', new Event('click'));
       });
 
       it('does not submit form', () => {
-        expect(mockForm.submit).not.toHaveBeenCalled();
+        expect(findGlForm().element.submit).not.toHaveBeenCalled();
       });
 
       it('sets save button `loading` prop to `false`', () => {
@@ -472,13 +477,13 @@ describe('IntegrationForm', () => {
   describe('when `test` button is clicked', () => {
     describe('when form is invalid', () => {
       it('emits `VALIDATE_INTEGRATION_FORM_EVENT` event to the event hub', () => {
-        createForm({ isValid: false });
         createComponent({
           customStateProps: {
             showActive: true,
             canTest: true,
           },
         });
+        jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(false);
 
         findTestButton().vm.$emit('click', new Event('click'));
 
@@ -490,7 +495,6 @@ describe('IntegrationForm', () => {
       const mockTestPath = '/test';
 
       beforeEach(() => {
-        createForm({ isValid: true });
         createComponent({
           customStateProps: {
             showActive: true,
@@ -498,6 +502,7 @@ describe('IntegrationForm', () => {
             testPath: mockTestPath,
           },
         });
+        jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(true);
       });
 
       describe('buttons', () => {

spec/helpers/integrations_helper_spec.rb

@@ -20,6 +20,12 @@ RSpec.describe IntegrationsHelper do
   end
 
   describe '#integration_form_data' do
+    before do
+      allow(helper).to receive_messages(
+        request: double(referer: '/services')
+      )
+    end
+
     let(:fields) do
       [
         :id,
@@ -39,7 +45,8 @@ RSpec.describe IntegrationsHelper do
         :cancel_path,
         :can_test,
         :test_path,
-        :reset_path
+        :reset_path,
+        :redirect_to
       ]
     end
 
@@ -61,6 +68,10 @@ RSpec.describe IntegrationsHelper do
       specify do
         expect(subject[:reset_path]).to eq(helper.scoped_reset_integration_path(integration))
       end
+
+      specify do
+        expect(subject[:redirect_to]).to eq('/services')
+      end
     end
 
     context 'Jira service' do

spec/views/projects/services/_form.haml_spec.rb

@@ -23,10 +23,6 @@ RSpec.describe 'projects/services/_form' do
   context 'commit_events and merge_request_events' do
     it 'display merge_request_events and commit_events descriptions' do
       allow(Integrations::Redmine).to receive(:supported_events).and_return(%w(commit merge_request))
-
-      render
-
-      expect(rendered).to have_css("input[name='redirect_to'][value='/services']", count: 1, visible: false)
     end
   end
 end