Improve Ruby style

Use as_json with except

app/services/projects/propagate_service_template.rb

@@ -4,8 +4,10 @@ module Projects
   class PropagateServiceTemplate
     BATCH_SIZE = 100
 
-    def self.propagate(*args)
-      new(*args).propagate
+    delegate :data_fields_present?, to: :template
+
+    def self.propagate(template)
+      new(template).propagate
     end
 
     def initialize(template)
@@ -13,15 +15,15 @@ module Projects
     end
 
     def propagate
-      return unless @template.active?
-
-      Rails.logger.info("Propagating services for template #{@template.id}") # rubocop:disable Gitlab/RailsLogger
+      return unless template.active?
 
       propagate_projects_with_template
     end
 
     private
 
+    attr_reader :template
+
     def propagate_projects_with_template
       loop do
         batch = Project.uncached { project_ids_batch }
@@ -38,7 +40,14 @@ module Projects
       end
 
       Project.transaction do
-        bulk_insert_services(service_hash.keys << 'project_id', service_list)
+        results = bulk_insert(Service, service_hash.keys << 'project_id', service_list)
+
+        if data_fields_present?
+          data_list = results.map { |row| data_hash.values << row['id'] }
+
+          bulk_insert(template.data_fields.class, data_hash.keys << 'service_id', data_list)
+        end
+
         run_callbacks(batch)
       end
     end
@@ -52,36 +61,27 @@ module Projects
             SELECT true
             FROM services
             WHERE services.project_id = projects.id
-            AND services.type = '#{@template.type}'
+            AND services.type = #{ActiveRecord::Base.connection.quote(template.type)}
           )
           AND projects.pending_delete = false
           AND projects.archived = false
           LIMIT #{BATCH_SIZE}
-      SQL
+        SQL
       )
     end
 
-    def bulk_insert_services(columns, values_array)
-      ActiveRecord::Base.connection.execute(
-        <<-SQL.strip_heredoc
-          INSERT INTO services (#{columns.join(', ')})
-          VALUES #{values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
-      SQL
-      )
+    def bulk_insert(klass, columns, values_array)
+      items_to_insert = values_array.map { |array| Hash[columns.zip(array)] }
+
+      klass.insert_all(items_to_insert, returning: [:id])
     end
 
     def service_hash
-      @service_hash ||=
-        begin
-          template_hash = @template.as_json(methods: :type).except('id', 'template', 'project_id')
-
-          template_hash.each_with_object({}) do |(key, value), service_hash|
-            value = value.is_a?(Hash) ? value.to_json : value
+      @service_hash ||= template.as_json(methods: :type, except: %w[id template project_id])
+    end
 
-            service_hash[ActiveRecord::Base.connection.quote_column_name(key)] =
-              ActiveRecord::Base.connection.quote(value)
-          end
-        end
+    def data_hash
+      @data_hash ||= template.data_fields.as_json(only: template.data_fields.class.column_names).except('id', 'service_id')
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
@@ -97,11 +97,11 @@ module Projects
     # rubocop: enable CodeReuse/ActiveRecord
 
     def active_external_issue_tracker?
-      @template.issue_tracker? && !@template.default
+      template.issue_tracker? && !template.default
     end
 
     def active_external_wiki?
-      @template.type == 'ExternalWikiService'
+      template.type == 'ExternalWikiService'
     end
   end
 end

changelogs/unreleased/214834-propagate-service-tempate.yml

+---
+title: Propagation of service templates also covers services with separate data tables.
+merge_request: 29805
+author:
+type: fixed

spec/services/projects/propagate_service_template_spec.rb

@@ -8,16 +8,19 @@ describe Projects::PropagateServiceTemplate do
       PushoverService.create(
         template: true,
         active: true,
+        push_events: false,
         properties: {
           device: 'MyDevice',
           sound: 'mic',
           priority: 4,
           user_key: 'asdf',
           api_key: '123456789'
-        })
+        }
+      )
     end
 
     let!(:project) { create(:project) }
+    let(:excluded_attributes) { %w[id project_id template created_at updated_at title description] }
 
     it 'creates services for projects' do
       expect(project.pushover_service).to be_nil
@@ -35,7 +38,7 @@ describe Projects::PropagateServiceTemplate do
         properties: {
           bamboo_url: 'http://gitlab.com',
           username: 'mic',
-          password: "password",
+          password: 'password',
           build_key: 'build'
         }
       )
@@ -54,7 +57,7 @@ describe Projects::PropagateServiceTemplate do
         properties: {
           bamboo_url: 'http://gitlab.com',
           username: 'mic',
-          password: "password",
+          password: 'password',
           build_key: 'build'
         }
       )
@@ -70,6 +73,33 @@ describe Projects::PropagateServiceTemplate do
       described_class.propagate(service_template)
 
       expect(project.pushover_service.properties).to eq(service_template.properties)
+
+      expect(project.pushover_service.attributes.except(*excluded_attributes))
+        .to eq(service_template.attributes.except(*excluded_attributes))
+    end
+
+    context 'service with data fields' do
+      let(:service_template) do
+        JiraService.create!(
+          template: true,
+          active: true,
+          push_events: false,
+          url: 'http://jira.instance.com',
+          username: 'user',
+          password: 'secret'
+        )
+      end
+
+      it 'creates the service containing the template attributes' do
+        described_class.propagate(service_template)
+
+        expect(project.jira_service.attributes.except(*excluded_attributes))
+          .to eq(service_template.attributes.except(*excluded_attributes))
+
+        excluded_attributes = %w[id service_id created_at updated_at]
+        expect(project.jira_service.data_fields.attributes.except(*excluded_attributes))
+          .to eq(service_template.data_fields.attributes.except(*excluded_attributes))
+      end
     end
 
     describe 'bulk update', :use_sql_query_cache do