Add feature flag for new integrations form

This commit wraps the new, vue-based integration
form element in a feature flag. When disabled,
we render the old, haml-based form element.

The feature flag is disabled by default.

app/assets/javascripts/integrations/constants.js

@@ -26,3 +26,5 @@ export const I18N_SUCCESSFUL_CONNECTION_MESSAGE = s__('Integrations|Connection s
 
 export const settingsTabTitle = __('Settings');
 export const overridesTabTitle = s__('Integrations|Projects using custom settings');
+
+export const INTEGRATION_FORM_SELECTOR = '.js-integration-settings-form';

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

@@ -9,6 +9,7 @@ import {
   I18N_FETCH_TEST_SETTINGS_DEFAULT_ERROR_MESSAGE,
   I18N_DEFAULT_ERROR_MESSAGE,
   I18N_SUCCESSFUL_CONNECTION_MESSAGE,
+  INTEGRATION_FORM_SELECTOR,
   integrationLevels,
 } from '~/integrations/constants';
 import { refreshCurrentPage } from '~/lib/utils/url_utility';
@@ -82,14 +83,34 @@ export default {
     disableButtons() {
       return Boolean(this.isSaving || this.isResetting || this.isTesting);
     },
-    form() {
-      return this.$refs.integrationForm.$el;
+    useVueForm() {
+      return this.glFeatures?.vueIntegrationForm;
+    },
+    formContainerProps() {
+      return this.useVueForm
+        ? {
+            ref: 'integrationForm',
+            method: 'post',
+            class: 'gl-mb-3 gl-show-field-errors integration-settings-form',
+            action: this.propsSource.formPath,
+            novalidate: !this.integrationActive,
+          }
+        : {};
+    },
+    formContainer() {
+      return this.useVueForm ? GlForm : 'div';
     },
   },
+  mounted() {
+    this.form = this.useVueForm
+      ? this.$refs.integrationForm.$el
+      : document.querySelector(INTEGRATION_FORM_SELECTOR);
+  },
   methods: {
     ...mapActions(['setOverride', 'fetchResetIntegration', 'requestJiraIssueTypes']),
     onSaveClick() {
       this.isSaving = true;
+
       if (this.integrationActive && !this.form.checkValidity()) {
         this.isSaving = false;
         eventHub.$emit(VALIDATE_INTEGRATION_FORM_EVENT);
@@ -148,6 +169,16 @@ export default {
     },
     onToggleIntegrationState(integrationActive) {
       this.integrationActive = integrationActive;
+      if (!this.form || this.useVueForm) {
+        return;
+      }
+
+      // If integration will be active, enable form validation.
+      if (integrationActive) {
+        this.form.removeAttribute('novalidate');
+      } else {
+        this.form.setAttribute('novalidate', true);
+      }
     },
   },
   helpHtmlConfig: {
@@ -160,13 +191,8 @@ export default {
 </script>
 
 <template>
-  <gl-form
-    ref="integrationForm"
-    method="post"
-    class="gl-mb-3 gl-show-field-errors integration-settings-form"
-    :action="propsSource.formPath"
-    :novalidate="!integrationActive"
-  >
+  <component :is="formContainer" v-bind="formContainerProps">
+    <template v-if="useVueForm">
       <input type="hidden" name="_method" value="put" />
       <input type="hidden" name="authenticity_token" :value="$options.csrf.token" />
       <input
@@ -175,6 +201,7 @@ export default {
         :value="propsSource.redirectTo"
         data-testid="redirect-to-field"
       />
+    </template>
 
     <override-dropdown
       v-if="defaultState !== null"
@@ -284,5 +311,5 @@ export default {
         </div>
       </div>
     </div>
-  </gl-form>
+  </component>
 </template>

app/controllers/concerns/integrations/actions.rb

@@ -8,6 +8,9 @@ module Integrations::Actions
     include IntegrationsHelper
 
     before_action :integration, only: [:edit, :update, :overrides, :test]
+    before_action do
+      push_frontend_feature_flag(:vue_integration_form, current_user, default_enabled: :yaml)
+    end
 
     urgency :low, [:test]
   end

app/controllers/projects/services_controller.rb

@@ -12,6 +12,9 @@ class Projects::ServicesController < Projects::ApplicationController
   before_action :web_hook_logs, only: [:edit, :update]
   before_action :set_deprecation_notice_for_prometheus_integration, only: [:edit, :update]
   before_action :redirect_deprecated_prometheus_integration, only: [:update]
+  before_action do
+    push_frontend_feature_flag(:vue_integration_form, current_user, default_enabled: :yaml)
+  end
 
   respond_to :html
 

app/helpers/integrations_helper.rb

@@ -228,6 +228,10 @@ module IntegrationsHelper
       name: integration.to_param
     }
   end
+
+  def vue_integration_form_enabled?
+    Feature.enabled?(:vue_integration_form, current_user, default_enabled: :yaml)
+  end
 end
 
 IntegrationsHelper.prepend_mod_with('IntegrationsHelper')

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

@@ -6,7 +6,13 @@
   - if integration.operating?
     = sprite_icon('check', css_class: 'gl-text-green-500')
 
-= render 'shared/integration_settings', integration: integration
+- if vue_integration_form_enabled?
+  = render 'shared/integration_settings', integration: integration
+- else
+  = 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), testid: 'integration-form' } }) do |form|
+    = render 'shared/integration_settings', form: form, integration: integration
+    %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referer }
+
 - 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), testid: 'integration-form' } } do |form|
+  = render 'shared/integration_settings', form: form, integration: integration

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

@@ -7,4 +7,7 @@
   = @integration.title
 
 = render 'shared/integrations/tabs', integration: @integration, active_tab: 'edit' do
+  - if vue_integration_form_enabled?
     = render 'shared/integration_settings', integration: @integration
+  - else
+    = render 'shared/integrations/form', integration: @integration

config/feature_flags/development/vue_integration_form.yml

+---
+name: vue_integration_form
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77934
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350444
+milestone: '14.7'
+type: development
+group: group::integrations
+default_enabled: false

spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb

@@ -19,4 +19,19 @@ RSpec.describe 'User activates the instance-level Mattermost Slash Command integ
     expect(page).to have_link('Settings', href: edit_path)
     expect(page).to have_link('Projects using custom settings', href: overrides_path)
   end
+
+  it 'does not render integration form element' do
+    expect(page).not_to have_selector('[data-testid="integration-form"]')
+  end
+
+  context 'when `vue_integration_form` feature flag is disabled' do
+    before do
+      stub_feature_flags(vue_integration_form: false)
+      visit_instance_integration('Mattermost slash commands')
+    end
+
+    it 'renders integration form element' do
+      expect(page).to have_selector('[data-testid="integration-form"]')
+    end
+  end
 end

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

@@ -36,10 +36,16 @@ describe('IntegrationForm', () => {
   let wrapper;
   let dispatch;
   let mockAxios;
+  let mockForm;
+  let vueIntegrationFormFeatureFlag;
+
+  const createForm = () => {
+    mockForm = document.createElement('form');
+    jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
+  };
 
   const createComponent = ({
     customStateProps = {},
-    featureFlags = {},
     initialState = {},
     props = {},
     mountFn = shallowMountExtended,
@@ -50,11 +56,12 @@ describe('IntegrationForm', () => {
     });
     dispatch = jest.spyOn(store, 'dispatch').mockImplementation();
 
+    if (!vueIntegrationFormFeatureFlag) {
+      createForm();
+    }
+
     wrapper = mountFn(IntegrationForm, {
-      propsData: { ...props, formSelector: '.test' },
-      provide: {
-        glFeatures: featureFlags,
-      },
+      propsData: { ...props },
       store,
       stubs: {
         OverrideDropdown,
@@ -68,6 +75,11 @@ describe('IntegrationForm', () => {
           show: mockToastShow,
         },
       },
+      provide: {
+        glFeatures: {
+          vueIntegrationForm: vueIntegrationFormFeatureFlag,
+        },
+      },
     });
   };
 
@@ -84,6 +96,12 @@ describe('IntegrationForm', () => {
   const findTriggerFields = () => wrapper.findComponent(TriggerFields);
   const findGlForm = () => wrapper.findComponent(GlForm);
   const findRedirectToField = () => wrapper.findByTestId('redirect-to-field');
+  const findFormElement = () => (vueIntegrationFormFeatureFlag ? findGlForm().element : mockForm);
+
+  const mockFormFunctions = ({ checkValidityReturn }) => {
+    jest.spyOn(findFormElement(), 'checkValidity').mockReturnValue(checkValidityReturn);
+    jest.spyOn(findFormElement(), 'submit');
+  };
 
   beforeEach(() => {
     mockAxios = new MockAdapter(axios);
@@ -339,7 +357,9 @@ describe('IntegrationForm', () => {
       });
     });
 
+    describe('when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled', () => {
       it('renders hidden fields', () => {
+        vueIntegrationFormFeatureFlag = true;
         createComponent({
           customStateProps: {
             redirectTo: '/services',
@@ -349,6 +369,7 @@ describe('IntegrationForm', () => {
         expect(findRedirectToField().attributes('value')).toBe('/services');
       });
     });
+  });
 
   describe('ActiveCheckbox', () => {
     describe.each`
@@ -368,13 +389,17 @@ describe('IntegrationForm', () => {
     });
 
     describe.each`
-      formActive | novalidate
-      ${true}    | ${undefined}
-      ${false}   | ${'novalidate'}
+      formActive | vueIntegrationFormEnabled | novalidate
+      ${true}    | ${true}                   | ${null}
+      ${false}   | ${true}                   | ${'novalidate'}
+      ${true}    | ${false}                  | ${null}
+      ${false}   | ${false}                  | ${'true'}
     `(
-      'when `toggle-integration-active` is emitted with $formActive',
-      ({ formActive, novalidate }) => {
+      'when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled and `toggle-integration-active` is emitted with $formActive',
+      ({ formActive, vueIntegrationFormEnabled, novalidate }) => {
         beforeEach(async () => {
+          vueIntegrationFormFeatureFlag = vueIntegrationFormEnabled;
+
           createComponent({
             customStateProps: {
               showActive: true,
@@ -382,17 +407,29 @@ describe('IntegrationForm', () => {
             },
             mountFn: mountExtended,
           });
+          mockFormFunctions({ checkValidityReturn: false });
 
           await findActiveCheckbox().vm.$emit('toggle-integration-active', formActive);
         });
 
         it(`sets noValidate to ${novalidate}`, () => {
-          expect(findGlForm().attributes('novalidate')).toBe(novalidate);
+          expect(findFormElement().getAttribute('novalidate')).toBe(novalidate);
         });
       },
     );
   });
 
+  describe.each`
+    vueIntegrationFormEnabled
+    ${true}
+    ${false}
+  `(
+    'when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled',
+    ({ vueIntegrationFormEnabled }) => {
+      beforeEach(() => {
+        vueIntegrationFormFeatureFlag = vueIntegrationFormEnabled;
+      });
+
       describe('when `save` button is clicked', () => {
         describe('buttons', () => {
           beforeEach(async () => {
@@ -434,14 +471,14 @@ describe('IntegrationForm', () => {
                 },
                 mountFn: mountExtended,
               });
-          jest.spyOn(findGlForm().element, 'submit');
-          jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(checkValidityReturn);
+
+              mockFormFunctions({ checkValidityReturn });
 
               await findProjectSaveButton().vm.$emit('click', new Event('click'));
             });
 
-        it('submit form', () => {
-          expect(findGlForm().element.submit).toHaveBeenCalledTimes(1);
+            it('submits form', () => {
+              expect(findFormElement().submit).toHaveBeenCalledTimes(1);
             });
           },
         );
@@ -456,14 +493,13 @@ describe('IntegrationForm', () => {
               },
               mountFn: mountExtended,
             });
-        jest.spyOn(findGlForm().element, 'submit');
-        jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(false);
+            mockFormFunctions({ checkValidityReturn: false });
 
             await findProjectSaveButton().vm.$emit('click', new Event('click'));
           });
 
           it('does not submit form', () => {
-        expect(findGlForm().element.submit).not.toHaveBeenCalled();
+            expect(findFormElement().submit).not.toHaveBeenCalled();
           });
 
           it('sets save button `loading` prop to `false`', () => {
@@ -490,7 +526,7 @@ describe('IntegrationForm', () => {
               },
               mountFn: mountExtended,
             });
-        jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(false);
+            mockFormFunctions({ checkValidityReturn: false });
 
             findTestButton().vm.$emit('click', new Event('click'));
 
@@ -510,7 +546,7 @@ describe('IntegrationForm', () => {
               },
               mountFn: mountExtended,
             });
-        jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(true);
+            mockFormFunctions({ checkValidityReturn: true });
           });
 
           describe('buttons', () => {
@@ -561,6 +597,8 @@ describe('IntegrationForm', () => {
           });
         });
       });
+    },
+  );
 
   describe('when `reset-confirmation-modal` emits `reset` event', () => {
     const mockResetPath = '/reset';

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'projects/services/_form' do
+  let(:project) { create(:redmine_project) }
+  let(:user) { create(:admin) }
+
+  before do
+    assign(:project, project)
+
+    allow(controller).to receive(:current_user).and_return(user)
+
+    allow(view).to receive_messages(
+      current_user: user,
+      can?: true,
+      current_application_settings: Gitlab::CurrentSettings.current_application_settings,
+      integration: project.redmine_integration,
+      request: double(referer: '/services')
+    )
+  end
+
+  context 'integrations form' do
+    it 'does not render form element' do
+      render
+
+      expect(rendered).not_to have_selector('[data-testid="integration-form"]')
+    end
+
+    context 'when vue_integration_form feature flag is disabled' do
+      before do
+        stub_feature_flags(vue_integration_form: false)
+      end
+
+      it 'renders form element' do
+        render
+
+        expect(rendered).to have_selector('[data-testid="integration-form"]')
+      end
+
+      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
+  end
+end