Merge branch 'justin_ho-refine-ui-of-integration-form' into 'master'

Refine UI of integration form

See merge request gitlab-org/gitlab!34707

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

@@ -135,8 +135,11 @@ export default {
     :label-for="fieldId"
     :invalid-feedback="__('This field is required.')"
     :state="valid"
-    :description="help"
   >
+    <template #description>
+      <span v-html="help"></span>
+    </template>
+
     <template v-if="isCheckbox">
       <input :name="fieldName" type="hidden" value="false" />
       <gl-form-checkbox v-model="model" v-bind="sharedProps">

app/controllers/concerns/integrations_actions.rb

@@ -54,9 +54,8 @@ module IntegrationsActions
   end
 
   def integration
-    # Using instance variable `@service` still required as it's used in ServiceParams
-    # and app/views/shared/_service_settings.html.haml. Should be removed once
-    # those 2 are refactored to use `@integration`.
+    # Using instance variable `@service` still required as it's used in ServiceParams.
+    # Should be removed once that is refactored to use `@integration`.
     @integration = @service ||= find_or_initialize_integration(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
   end
 

app/helpers/services_helper.rb

@@ -44,13 +44,6 @@ module ServicesHelper
     end
   end
 
-  def event_action_description(action)
-    case action
-    when "comment"
-      s_("ProjectService|Comment will be posted on each event")
-    end
-  end
-
   def service_save_button
     button_tag(class: 'btn btn-success', type: 'submit', data: { qa_selector: 'save_changes_button' }) do
       icon('spinner spin', class: 'hidden js-btn-spinner') +
@@ -90,7 +83,7 @@ module ServicesHelper
 
   def scoped_test_integration_path(integration)
     if @project.present?
-      test_project_settings_integration_path(@project, integration)
+      test_project_service_path(@project, integration)
     elsif @group.present?
       test_group_settings_integration_path(@group, integration)
     else
@@ -102,22 +95,36 @@ module ServicesHelper
     Feature.enabled?(:integration_form_refactor, @project)
   end
 
-  def trigger_events_for_service
+  def integration_form_data(integration)
+    {
+      show_active: integration.show_active_box?.to_s,
+      activated: (integration.active || integration.new_record?).to_s,
+      type: integration.to_param,
+      merge_request_events: integration.merge_requests_events.to_s,
+      commit_events: integration.commit_events.to_s,
+      enable_comments: integration.comment_on_event_enabled.to_s,
+      comment_detail: integration.comment_detail,
+      trigger_events: trigger_events_for_service(integration),
+      fields: fields_for_service(integration)
+    }
+  end
+
+  def trigger_events_for_service(integration)
     return [] unless integration_form_refactor?
 
-    ServiceEventSerializer.new(service: @service).represent(@service.configurable_events).to_json
+    ServiceEventSerializer.new(service: integration).represent(integration.configurable_events).to_json
   end
 
-  def fields_for_service
+  def fields_for_service(integration)
     return [] unless integration_form_refactor?
 
-    ServiceFieldSerializer.new(service: @service).represent(@service.global_fields).to_json
+    ServiceFieldSerializer.new(service: integration).represent(integration.global_fields).to_json
   end
 
-  def show_service_trigger_events?
-    return false if @service.is_a?(JiraService) || integration_form_refactor?
+  def show_service_trigger_events?(integration)
+    return false if integration.is_a?(JiraService) || integration_form_refactor?
 
-    @service.configurable_events.present?
+    integration.configurable_events.present?
   end
 
   extend self

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

@@ -4,7 +4,7 @@
 %p #{@service.description} template.
 
 = form_for :service, url: admin_application_settings_service_path, method: :put, html: { class: 'fieldset-form' } do |form|
-  = render 'shared/service_settings', form: form, service: @service
+  = render 'shared/service_settings', form: form, integration: @service
 
   .footer-block.row-content-block
     = form.submit 'Save', class: 'btn btn-success'

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

 - add_to_breadcrumbs "Service Templates", admin_application_settings_services_path
 - breadcrumb_title @service.title
 - page_title @service.title, "Service Templates"
+- @content_class = 'limit-container-width' unless fluid_layout
 
 = render 'form'

app/views/layouts/nav/sidebar/_admin.html.haml

@@ -216,7 +216,7 @@
               %strong.fly-out-top-item-name
                 = _('Appearance')
 
-      = nav_link(controller: :application_settings) do
+      = nav_link(controller: [:application_settings, :integrations]) do
         = link_to general_admin_application_settings_path do
           .nav-icon-container
             = sprite_icon('settings')
@@ -224,7 +224,7 @@
             = _('Settings')
 
         %ul.sidebar-sub-level-items.qa-admin-sidebar-settings-submenu
-          = nav_link(controller: :application_settings, html_options: { class: "fly-out-top-item" } ) do
+          = nav_link(controller: [:application_settings, :integrations], html_options: { class: "fly-out-top-item" } ) do
             = link_to general_admin_application_settings_path do
               %strong.fly-out-top-item-name
                 = _('Settings')
@@ -233,7 +233,7 @@
               = link_to general_admin_application_settings_path, title: _('General'), class: 'qa-admin-settings-general-item' do
                 %span
                   = _('General')
-            = nav_link(path: 'application_settings#integrations') do
+            = nav_link(path: ['application_settings#integrations', 'integrations#edit']) do
               = link_to integrations_admin_application_settings_path, title: _('Integrations'), data: { qa_selector: 'integration_settings_link' } do
                 %span
                   = _('Integrations')

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

@@ -11,7 +11,7 @@
       %p= @service.detailed_description
   .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
+      = render 'shared/service_settings', form: form, integration: @service
       .footer-block.row-content-block
         %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referrer }
         = service_save_button

app/views/shared/_service_settings.html.haml

-= form_errors(@service)
+= form_errors(integration)
 
-- if lookup_context.template_exists?('help', "projects/services/#{@service.to_param}", true)
-  = render "projects/services/#{@service.to_param}/help", subject: @service
-- elsif @service.help.present?
+- if lookup_context.template_exists?('help', "projects/services/#{integration.to_param}", true)
+  = render "projects/services/#{integration.to_param}/help", subject: integration
+- elsif integration.help.present?
   .info-well
     .well-segment
-      = markdown @service.help
+      = markdown integration.help
 
 .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, trigger_events: trigger_events_for_service, fields: fields_for_service } }
+  .js-vue-integration-settings{ data: integration_form_data(integration) }
 
-  - if show_service_trigger_events?
+  - if show_service_trigger_events?(integration)
     .form-group.row
       %label.col-form-label.col-sm-2= _('Trigger')
 
       .col-sm-10
-        - @service.configurable_events.each do |event|
+        - integration.configurable_events.each do |event|
           .form-group
             .form-check
               = form.check_box service_event_field_name(event), class: 'form-check-input'
@@ -24,14 +23,14 @@ commit_events: @service.commit_events.to_s, enable_comments: @service.comment_on
                 %strong
                   = event.humanize
 
-            - field = @service.event_field(event)
+            - field = integration.event_field(event)
 
             - if field
               = form.text_field field[:name], class: "form-control", placeholder: field[:placeholder]
 
             %p.text-muted
-              = @service.class.event_description(event)
+              = integration.class.event_description(event)
 
   - unless integration_form_refactor?
-    - @service.global_fields.each do |field|
+    - integration.global_fields.each do |field|
       = render 'shared/field', form: form, field: field

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

 - add_to_breadcrumbs _('Integrations'), scoped_integrations_path
 - breadcrumb_title @integration.title
 - page_title @integration.title, _('Integrations')
+- @content_class = 'limit-container-width' unless fluid_layout
 
 = render 'shared/integrations/form', integration: @integration

changelogs/unreleased/justin_ho-refine-ui-of-integration-form.yml

+---
+title: Refine UI of integration form
+merge_request: 34707
+author:
+type: changed

locale/gitlab.pot

@@ -17449,9 +17449,6 @@ msgstr ""
 msgid "ProjectService|Comment"
 msgstr ""
 
-msgid "ProjectService|Comment will be posted on each event"
-msgstr ""
-
 msgid "ProjectService|Perform common operations on GitLab project: %{project_name}"
 msgstr ""
 

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

@@ -147,6 +147,20 @@ describe('DynamicField', () => {
             .text(),
         ).toBe(defaultProps.help);
       });
+
+      it('renders description with help text as HTML', () => {
+        const helpHTML = 'The <strong>URL</strong> of the project';
+
+        createComponent({
+          help: helpHTML,
+        });
+
+        expect(
+          findGlFormGroup()
+            .find('small')
+            .html(),
+        ).toContain(helpHTML);
+      });
     });
 
     describe('label text', () => {

spec/helpers/services_helper_spec.rb

@@ -7,9 +7,4 @@ describe ServicesHelper do
     it { expect(event_action_title('comment')).to eq 'Comment' }
     it { expect(event_action_title('something')).to eq 'Something' }
   end
-
-  describe 'event_action_description' do
-    it { expect(event_action_description('comment')).to eq 'Comment will be posted on each event' }
-    it { expect(event_action_description('something')).to eq nil }
-  end
 end