Add comment on public_send and refactor specs

- Use a shared method for the feature toggle
- Since field[:name] is definied in each service's fields
method, we can assume it's safe to use public_send.

app/helpers/services_helper.rb

@@ -98,20 +98,24 @@ module ServicesHelper
     end
   end
 
+  def integration_form_refactor?
+    Feature.enabled?(:integration_form_refactor, @project)
+  end
+
   def trigger_events_for_service
-    return [] unless Feature.enabled?(:integration_form_refactor, @project)
+    return [] unless integration_form_refactor?
 
     ServiceEventSerializer.new(service: @service).represent(@service.configurable_events).to_json
   end
 
   def fields_for_service
-    return [] unless Feature.enabled?(:integration_form_refactor, @project)
+    return [] unless integration_form_refactor?
 
     ServiceFieldSerializer.new(service: @service).represent(@service.global_fields).to_json
   end
 
-  def show_service_trigger_events
-    return false if @service.is_a?(JiraService) || Feature.enabled?(:integration_form_refactor, @project)
+  def show_service_trigger_events?
+    return false if @service.is_a?(JiraService) || integration_form_refactor?
 
     @service.configurable_events.present?
   end

app/serializers/service_field_entity.rb

@@ -6,6 +6,7 @@ class ServiceFieldEntity < Grape::Entity
   expose :type, :name, :title, :placeholder, :required, :choices, :help
 
   expose :value do |field|
+    # field[:name] is not user input and so can assume is safe
     value = service.public_send(field[:name]) # rubocop:disable GitlabSecurity/PublicSend
 
     if field[:type] == 'password' && value.present?

app/views/shared/_service_settings.html.haml

@@ -11,7 +11,7 @@
   .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 } }
 
-  - if show_service_trigger_events
+  - if show_service_trigger_events?
     .form-group.row
       %label.col-form-label.col-sm-2= _('Trigger')
 
@@ -32,6 +32,6 @@ commit_events: @service.commit_events.to_s, enable_comments: @service.comment_on
             %p.text-muted
               = @service.class.event_description(event)
 
-  - if Feature.disabled?(:integration_form_refactor, @project)
+  - unless integration_form_refactor?
     - @service.global_fields.each do |field|
       = render 'shared/field', form: form, field: field

spec/serializers/service_field_entity_spec.rb

@@ -17,8 +17,9 @@ describe ServiceFieldEntity do
 
       context 'field with type text' do
         let(:field) { service.global_fields.find { |field| field[:name] == 'username' } }
-        let(:expected_hash) do
-          {
+
+        it 'exposes correct attributes' do
+          expected_hash = {
             type: 'text',
             name: 'username',
             title: 'Username or Email',
@@ -28,9 +29,7 @@ describe ServiceFieldEntity do
             help: nil,
             value: 'jira_username'
           }
-        end
 
-        it 'exposes correct attributes' do
           is_expected.to eq(expected_hash)
         end
       end
@@ -38,8 +37,8 @@ describe ServiceFieldEntity do
       context 'field with type password' do
         let(:field) { service.global_fields.find { |field| field[:name] == 'password' } }
 
-        let(:expected_hash) do
-          {
+        it 'exposes correct attributes but hides password' do
+          expected_hash = {
             type: 'password',
             name: 'password',
             title: 'Password or API token',
@@ -49,9 +48,7 @@ describe ServiceFieldEntity do
             help: nil,
             value: 'true'
           }
-        end
 
-        it 'exposes correct attributes but hides password' do
           is_expected.to eq(expected_hash)
         end
       end
@@ -62,8 +59,9 @@ describe ServiceFieldEntity do
 
       context 'field with type checkbox' do
         let(:field) { service.global_fields.find { |field| field[:name] == 'send_from_committer_email' } }
-        let(:expected_hash) do
-          {
+
+        it 'exposes correct attributes' do
+          expected_hash = {
             type: 'checkbox',
             name: 'send_from_committer_email',
             title: 'Send from committer',
@@ -72,9 +70,7 @@ describe ServiceFieldEntity do
             choices: nil,
             value: true
           }
-        end
 
-        it 'exposes correct attributes' do
           is_expected.to include(expected_hash)
           expect(subject[:help]).to include("Send notifications from the committer's email address if the domain is part of the domain GitLab is running on")
         end
@@ -82,8 +78,9 @@ describe ServiceFieldEntity do
 
       context 'field with type select' do
         let(:field) { service.global_fields.find { |field| field[:name] == 'branches_to_be_notified' } }
-        let(:expected_hash) do
-          {
+
+        it 'exposes correct attributes' do
+          expected_hash = {
             type: 'select',
             name: 'branches_to_be_notified',
             title: nil,
@@ -93,9 +90,7 @@ describe ServiceFieldEntity do
             help: nil,
             value: nil
           }
-        end
 
-        it 'exposes correct attributes' do
           is_expected.to eq(expected_hash)
         end
       end