Merge branch 'justin_ho-extract-integration-trigger-fields-to-vue' into 'master'

Extract integration trigger fields to Vue

See merge request gitlab-org/gitlab!31074

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

 <script>
 import ActiveToggle from './active_toggle.vue';
 import JiraTriggerFields from './jira_trigger_fields.vue';
+import TriggerFields from './trigger_fields.vue';
 
 export default {
   name: 'IntegrationForm',
   components: {
     ActiveToggle,
     JiraTriggerFields,
+    TriggerFields,
   },
   props: {
     activeToggleProps: {
@@ -21,6 +23,11 @@ export default {
       type: Object,
       required: true,
     },
+    triggerEvents: {
+      type: Array,
+      required: false,
+      default: () => [],
+    },
     type: {
       type: String,
       required: true,
@@ -38,5 +45,6 @@ export default {
   <div>
     <active-toggle v-if="showActive" v-bind="activeToggleProps" />
     <jira-trigger-fields v-if="isJira" v-bind="triggerFieldsProps" />
+    <trigger-fields v-else-if="triggerEvents.length" :events="triggerEvents" :type="type" />
   </div>
 </template>

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

+<script>
+import { startCase } from 'lodash';
+import { __ } from '~/locale';
+import { GlFormGroup, GlFormCheckbox, GlFormInput } from '@gitlab/ui';
+
+const typeWithPlaceholder = {
+  SLACK: 'slack',
+  MATTERMOST: 'mattermost',
+};
+
+const placeholderForType = {
+  [typeWithPlaceholder.SLACK]: __('Slack channels (e.g. general, development)'),
+  [typeWithPlaceholder.MATTERMOST]: __('Channel handle (e.g. town-square)'),
+};
+
+export default {
+  name: 'TriggerFields',
+  components: {
+    GlFormGroup,
+    GlFormCheckbox,
+    GlFormInput,
+  },
+  props: {
+    events: {
+      type: Array,
+      required: false,
+      default: null,
+    },
+    type: {
+      type: String,
+      required: true,
+    },
+  },
+  computed: {
+    placeholder() {
+      return placeholderForType[this.type];
+    },
+  },
+  methods: {
+    checkboxName(name) {
+      return `service[${name}]`;
+    },
+    fieldName(name) {
+      return `service[${name}]`;
+    },
+    startCase,
+  },
+};
+</script>
+
+<template>
+  <gl-form-group
+    class="gl-pt-3"
+    :label="__('Trigger')"
+    label-for="trigger-fields"
+    data-testid="trigger-fields-group"
+  >
+    <div id="trigger-fields" class="gl-pt-3">
+      <gl-form-group v-for="event in events" :key="event.title" :description="event.description">
+        <input :name="checkboxName(event.name)" type="hidden" value="false" />
+        <gl-form-checkbox v-model="event.value" :name="checkboxName(event.name)">
+          {{ startCase(event.title) }}
+        </gl-form-checkbox>
+        <gl-form-input
+          v-if="event.field"
+          v-model="event.field.value"
+          :name="fieldName(event.field.name)"
+          :placeholder="placeholder"
+        />
+      </gl-form-group>
+    </div>
+  </gl-form-group>
+</template>

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

@@ -15,7 +15,7 @@ export default el => {
     return result;
   }
 
-  const { type, commentDetail, ...booleanAttributes } = el.dataset;
+  const { type, commentDetail, triggerEvents, ...booleanAttributes } = el.dataset;
   const {
     showActive,
     activated,
@@ -40,6 +40,7 @@ export default el => {
             initialEnableComments: enableComments,
             initialCommentDetail: commentDetail,
           },
+          triggerEvents: JSON.parse(triggerEvents),
         },
       });
     },

app/serializers/service_event_entity.rb

+# frozen_string_literal: true
+
+class ServiceEventEntity < Grape::Entity
+  include RequestAwareEntity
+
+  expose :title do |event|
+    event
+  end
+
+  expose :event_field_name, as: :name
+
+  expose :value do |event|
+    service[event_field_name]
+  end
+
+  expose :description do |event|
+    service.class.event_description(event)
+  end
+
+  expose :field, if: -> (_, _) { event_field } do
+    expose :name do |event|
+      event_field[:name]
+    end
+    expose :value do |event|
+      service.public_send(event_field[:name]) # rubocop:disable GitlabSecurity/PublicSend
+    end
+  end
+
+  private
+
+  alias_method :event, :object
+
+  def event_field_name
+    ServicesHelper.service_event_field_name(event)
+  end
+
+  def event_field
+    @event_field ||= service.event_field(event)
+  end
+
+  def service
+    request.service
+  end
+end

app/serializers/service_event_serializer.rb

+# frozen_string_literal: true
+
+class ServiceEventSerializer < BaseSerializer
+  entity ServiceEventEntity
+end

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

 .row.prepend-top-default.append-bottom-default
-  .col-lg-3
+  .col-lg-4
     %h4.prepend-top-0
       = @service.title
       - [true, false].each do |value|
@@ -9,7 +9,7 @@
 
     - if @service.respond_to?(:detailed_description)
       %p= @service.detailed_description
-  .col-lg-9
+  .col-lg-8
     = form_for(@service, as: :service, url: scoped_integration_path(@service), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'can-test' => @service.can_test?, 'test-url' => test_project_service_path(@project, @service) } }) do |form|
       = render 'shared/service_settings', form: form, service: @service
       .footer-block.row-content-block

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

 - breadcrumb_title @service.title
 - add_to_breadcrumbs _('Integration Settings'), project_settings_integrations_path(@project)
 - page_title @service.title, _('Integrations')
+- @content_class = 'limit-container-width' unless fluid_layout
 
 = render 'form'
 - if @web_hook_logs

app/views/shared/_service_settings.html.haml

 = form_errors(@service)
+- trigger_events = Feature.enabled?(:integration_form_refactor) ? ServiceEventSerializer.new(service: @service).represent(@service.configurable_events).to_json : []
 
 - if lookup_context.template_exists?('help', "projects/services/#{@service.to_param}", true)
   = render "projects/services/#{@service.to_param}/help", subject: @service
@@ -9,9 +10,9 @@
 
 .service-settings
   .js-vue-integration-settings{ data: { show_active: @service.show_active_box?.to_s, activated: (@service.active || @service.new_record?).to_s, type: @service.to_param, merge_request_events: @service.merge_requests_events.to_s,
-commit_events: @service.commit_events.to_s, enable_comments: @service.comment_on_event_enabled.to_s, comment_detail: @service.comment_detail } }
+commit_events: @service.commit_events.to_s, enable_comments: @service.comment_on_event_enabled.to_s, comment_detail: @service.comment_detail, trigger_events: trigger_events } }
 
-  - if @service.configurable_events.present? && !@service.is_a?(JiraService)
+  - if @service.configurable_events.present? && !@service.is_a?(JiraService) && Feature.disabled?(:integration_form_refactor)
     .form-group.row
       %label.col-form-label.col-sm-2= _('Trigger')
 
@@ -33,15 +34,4 @@ commit_events: @service.commit_events.to_s, enable_comments: @service.comment_on
               = @service.class.event_description(event)
 
   - @service.global_fields.each do |field|
-    - type = field[:type]
-
-    - if type == 'fieldset'
-      - fields = field[:fields]
-      - legend = field[:legend]
-
-      %fieldset
-        %legend= legend
-        - fields.each do |subfield|
-          = render 'shared/field', form: form, field: subfield
-    - else
-      = render 'shared/field', form: form, field: field
+    = render 'shared/field', form: form, field: field

ee/spec/factories/services.rb

@@ -14,13 +14,6 @@ FactoryBot.define do
     type { 'GithubService' }
   end
 
-  factory :slack_service do
-    project
-    active { true }
-    webhook { 'https://slack.service.url' }
-    type { 'SlackService' }
-  end
-
   factory :slack_slash_commands_service do
     project
     active { true }

locale/gitlab.pot

@@ -3797,6 +3797,9 @@ msgstr ""
 msgid "Changing group path can have unintended side effects."
 msgstr ""
 
+msgid "Channel handle (e.g. town-square)"
+msgstr ""
+
 msgid "Charts"
 msgstr ""
 

spec/factories/services.rb

@@ -158,6 +158,13 @@ FactoryBot.define do
     token { 'test_token' }
   end
 
+  factory :slack_service do
+    project
+    active { true }
+    webhook { 'https://slack.service.url' }
+    type { 'SlackService' }
+  end
+
   # this is for testing storing values inside properties, which is deprecated and will be removed in
   # https://gitlab.com/gitlab-org/gitlab/issues/29404
   trait :without_properties_callback do

spec/features/admin/admin_settings_spec.rb

@@ -212,12 +212,12 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
         expect(current_settings.hide_third_party_offers).to be true
       end
 
-      it 'Change Slack Notifications Service template settings' do
+      it 'Change Slack Notifications Service template settings', :js do
         first(:link, 'Service Templates').click
         click_link 'Slack notifications'
         fill_in 'Webhook', with: 'http://localhost'
         fill_in 'Username', with: 'test_user'
-        fill_in 'service_push_channel', with: '#test_channel'
+        fill_in 'service[push_channel]', with: '#test_channel'
         page.check('Notify only broken pipelines')
         page.select 'All branches', from: 'Branches to be notified'
 
@@ -231,10 +231,10 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
         expect(page.all('input[type=checkbox]')).to all(be_checked)
         expect(find_field('Webhook').value).to eq 'http://localhost'
         expect(find_field('Username').value).to eq 'test_user'
-        expect(find('#service_push_channel').value).to eq '#test_channel'
+        expect(find('[name="service[push_channel]"]').value).to eq '#test_channel'
       end
 
-      it 'defaults Deployment events to false for chat notification template settings' do
+      it 'defaults Deployment events to false for chat notification template settings', :js do
         first(:link, 'Service Templates').click
         click_link 'Slack notifications'
 
@@ -500,13 +500,13 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
   def check_all_events
     page.check('Push')
     page.check('Issue')
-    page.check('Confidential issue')
-    page.check('Merge request')
+    page.check('Confidential Issue')
+    page.check('Merge Request')
     page.check('Note')
-    page.check('Confidential note')
-    page.check('Tag push')
+    page.check('Confidential Note')
+    page.check('Tag Push')
     page.check('Pipeline')
-    page.check('Wiki page')
+    page.check('Wiki Page')
     page.check('Deployment')
   end
 

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

@@ -18,7 +18,10 @@ describe('ActiveToggle', () => {
   };
 
   afterEach(() => {
-    if (wrapper) wrapper.destroy();
+    if (wrapper) {
+      wrapper.destroy();
+      wrapper = null;
+    }
   });
 
   const findGlToggle = () => wrapper.find(GlToggle);

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

@@ -2,6 +2,7 @@ import { shallowMount } from '@vue/test-utils';
 import IntegrationForm from '~/integrations/edit/components/integration_form.vue';
 import ActiveToggle from '~/integrations/edit/components/active_toggle.vue';
 import JiraTriggerFields from '~/integrations/edit/components/jira_trigger_fields.vue';
+import TriggerFields from '~/integrations/edit/components/trigger_fields.vue';
 
 describe('IntegrationForm', () => {
   let wrapper;
@@ -38,6 +39,7 @@ describe('IntegrationForm', () => {
 
   const findActiveToggle = () => wrapper.find(ActiveToggle);
   const findJiraTriggerFields = () => wrapper.find(JiraTriggerFields);
+  const findTriggerFields = () => wrapper.find(TriggerFields);
 
   describe('template', () => {
     describe('showActive is true', () => {
@@ -77,5 +79,21 @@ describe('IntegrationForm', () => {
         expect(findJiraTriggerFields().exists()).toBe(true);
       });
     });
+
+    describe('triggerEvents is present', () => {
+      it('renders TriggerFields', () => {
+        const events = [{ title: 'push' }];
+        const type = 'slack';
+
+        createComponent({
+          triggerEvents: events,
+          type,
+        });
+
+        expect(findTriggerFields().exists()).toBe(true);
+        expect(findTriggerFields().props('events')).toBe(events);
+        expect(findTriggerFields().props('type')).toBe(type);
+      });
+    });
   });
 });

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

@@ -18,7 +18,10 @@ describe('JiraTriggerFields', () => {
   };
 
   afterEach(() => {
-    if (wrapper) wrapper.destroy();
+    if (wrapper) {
+      wrapper.destroy();
+      wrapper = null;
+    }
   });
 
   const findCommentSettings = () => wrapper.find('[data-testid="comment-settings"]');

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

+import { mount } from '@vue/test-utils';
+import TriggerFields from '~/integrations/edit/components/trigger_fields.vue';
+import { GlFormGroup, GlFormCheckbox, GlFormInput } from '@gitlab/ui';
+
+describe('TriggerFields', () => {
+  let wrapper;
+
+  const defaultProps = {
+    type: 'slack',
+  };
+
+  const createComponent = props => {
+    wrapper = mount(TriggerFields, {
+      propsData: { ...defaultProps, ...props },
+    });
+  };
+
+  afterEach(() => {
+    if (wrapper) {
+      wrapper.destroy();
+      wrapper = null;
+    }
+  });
+
+  const findAllGlFormCheckboxes = () => wrapper.findAll(GlFormCheckbox);
+  const findAllGlFormInputs = () => wrapper.findAll(GlFormInput);
+
+  describe('template', () => {
+    it('renders a label with text "Trigger"', () => {
+      createComponent();
+
+      const triggerLabel = wrapper.find('[data-testid="trigger-fields-group"]').find('label');
+      expect(triggerLabel.exists()).toBe(true);
+      expect(triggerLabel.text()).toBe('Trigger');
+    });
+
+    describe('events without field property', () => {
+      const events = [
+        {
+          title: 'push',
+          name: 'push_event',
+          description: 'Event on push',
+          value: true,
+        },
+        {
+          title: 'merge_request',
+          name: 'merge_requests_event',
+          description: 'Event on merge_request',
+          value: false,
+        },
+      ];
+
+      beforeEach(() => {
+        createComponent({
+          events,
+        });
+      });
+
+      it('does not render GlFormInput for each event', () => {
+        expect(findAllGlFormInputs().exists()).toBe(false);
+      });
+
+      it('renders GlFormInput with description for each event', () => {
+        const groups = wrapper.find('#trigger-fields').findAll(GlFormGroup);
+
+        expect(groups).toHaveLength(2);
+        groups.wrappers.forEach((group, index) => {
+          expect(group.find('small').text()).toBe(events[index].description);
+        });
+      });
+
+      it('renders GlFormCheckbox for each event', () => {
+        const checkboxes = findAllGlFormCheckboxes();
+        const expectedResults = [
+          { labelText: 'Push', inputName: 'service[push_event]' },
+          { labelText: 'Merge Request', inputName: 'service[merge_requests_event]' },
+        ];
+        expect(checkboxes).toHaveLength(2);
+
+        checkboxes.wrappers.forEach((checkbox, index) => {
+          expect(checkbox.find('label').text()).toBe(expectedResults[index].labelText);
+          expect(checkbox.find('input').attributes('name')).toBe(expectedResults[index].inputName);
+          expect(checkbox.vm.$attrs.checked).toBe(events[index].value);
+        });
+      });
+    });
+
+    describe('events with field property', () => {
+      const events = [
+        {
+          field: {
+            name: 'push_channel',
+            value: '',
+          },
+        },
+        {
+          field: {
+            name: 'merge_request_channel',
+            value: 'gitlab-development',
+          },
+        },
+      ];
+
+      beforeEach(() => {
+        createComponent({
+          events,
+        });
+      });
+
+      it('renders GlFormCheckbox for each event', () => {
+        expect(findAllGlFormCheckboxes()).toHaveLength(2);
+      });
+
+      it('renders GlFormInput for each event', () => {
+        const fields = findAllGlFormInputs();
+        const expectedResults = [
+          {
+            name: 'service[push_channel]',
+            placeholder: 'Slack channels (e.g. general, development)',
+          },
+          {
+            name: 'service[merge_request_channel]',
+            placeholder: 'Slack channels (e.g. general, development)',
+          },
+        ];
+
+        expect(fields).toHaveLength(2);
+
+        fields.wrappers.forEach((field, index) => {
+          expect(field.attributes()).toMatchObject(expectedResults[index]);
+          expect(field.vm.$attrs.value).toBe(events[index].field.value);
+        });
+      });
+    });
+  });
+});

spec/serializers/service_event_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ServiceEventEntity do
+  let(:request) { double('request') }
+
+  subject { described_class.new(event, request: request, service: service).as_json }
+
+  before do
+    allow(request).to receive(:service).and_return(service)
+  end
+
+  describe '#as_json' do
+    context 'service without fields' do
+      let(:service) { create(:emails_on_push_service, push_events: true) }
+      let(:event) { 'push' }
+
+      it 'exposes correct attributes' do
+        expect(subject[:description]).to eq('Event will be triggered by a push to the repository')
+        expect(subject[:name]).to eq('push_events')
+        expect(subject[:title]).to eq('push')
+        expect(subject[:value]).to be(true)
+      end
+    end
+
+    context 'service with fields' do
+      let(:service) { create(:slack_service, note_events: false, note_channel: 'note-channel') }
+      let(:event) { 'note' }
+
+      it 'exposes correct attributes' do
+        expect(subject[:description]).to eq('Event will be triggered when someone adds a comment')
+        expect(subject[:name]).to eq('note_events')
+        expect(subject[:title]).to eq('note')
+        expect(subject[:value]).to eq(false)
+        expect(subject[:field][:name]).to eq('note_channel')
+        expect(subject[:field][:value]).to eq('note-channel')
+      end
+    end
+  end
+end

spec/support/shared_contexts/project_service_shared_context.rb

@@ -5,6 +5,7 @@ shared_context 'project service activation' do
   let(:user) { create(:user) }
 
   before do
+    stub_feature_flags(integration_form_refactor: false)
     project.add_maintainer(user)
     sign_in(user)
   end

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

@@ -7,6 +7,8 @@ describe 'projects/services/_form' do
   let(:user) { create(:admin) }
 
   before do
+    stub_feature_flags(integration_form_refactor: false)
+
     assign(:project, project)
 
     allow(controller).to receive(:current_user).and_return(user)