Merge branch '32544-fix-service-templates' into 'master'

Fix creating issue tracker services from templates

Closes #32544

See merge request gitlab-org/gitlab!17708

app/models/project_services/issue_tracker_service.rb

@@ -62,6 +62,7 @@ class IssueTrackerService < Service
     end
 
     data_values.reject! { |key| data_fields.changed.include?(key) }
+    data_values.slice!(*data_fields.attributes.keys)
     data_fields.assign_attributes(data_values) if data_values.present?
 
     self.properties = {}
@@ -71,6 +72,10 @@ class IssueTrackerService < Service
     @legacy_properties_data ||= {}
   end
 
+  def supports_data_fields?
+    true
+  end
+
   def data_fields
     issue_tracker_data || self.build_issue_tracker_data
   end

app/models/service.rb

@@ -291,6 +291,12 @@ class Service < ApplicationRecord
 
   def self.build_from_template(project_id, template)
     service = template.dup
+
+    if template.supports_data_fields?
+      data_fields = template.data_fields.dup
+      data_fields.service = service
+    end
+
     service.template = false
     service.project_id = project_id
     service.active = false if service.active? && !service.valid?
@@ -309,6 +315,11 @@ class Service < ApplicationRecord
     find_by(template: true)
   end
 
+  # override if needed
+  def supports_data_fields?
+    false
+  end
+
   private
 
   def cache_project_has_external_issue_tracker

spec/models/project_services/jira_service_spec.rb

@@ -282,7 +282,7 @@ describe JiraService do
     context 'when data are stored in properties' do
       let(:properties) { data_params.merge(title: title, description: description) }
       let!(:service) do
-        create(:jira_service, :without_properties_callback, properties: properties)
+        create(:jira_service, :without_properties_callback, properties: properties.merge(additional: 'something'))
       end
 
       it_behaves_like 'issue tracker fields'

spec/models/service_spec.rb

@@ -78,10 +78,11 @@ describe Service do
   end
 
   describe "Template" do
+    let(:project) { create(:project) }
+
     describe '.build_from_template' do
       context 'when template is invalid' do
         it 'sets service template to inactive when template is invalid' do
-          project = create(:project)
           template = build(:prometheus_service, template: true, active: true, properties: {})
           template.save(validate: false)
 
@@ -91,6 +92,64 @@ describe Service do
           expect(service.active).to be false
         end
       end
+
+      describe 'build issue tracker from a template' do
+        let(:title) { 'custom title' }
+        let(:description) { 'custom description' }
+        let(:url) { 'http://jira.example.com' }
+        let(:api_url) { 'http://api-jira.example.com' }
+        let(:username) { 'jira-username' }
+        let(:password) { 'jira-password' }
+        let(:data_params) do
+          {
+            url: url, api_url: api_url,
+            username: username, password: password
+          }
+        end
+
+        shared_examples 'service creation from a template' do
+          it 'creates a correct service' do
+            service = described_class.build_from_template(project.id, template)
+
+            expect(service).to be_active
+            expect(service.title).to eq(title)
+            expect(service.description).to eq(description)
+            expect(service.url).to eq(url)
+            expect(service.api_url).to eq(api_url)
+            expect(service.username).to eq(username)
+            expect(service.password).to eq(password)
+          end
+        end
+
+        # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-foss/issues/63084
+        context 'when data are stored in properties' do
+          let(:properties) { data_params.merge(title: title, description: description) }
+          let!(:template) do
+            create(:jira_service, :without_properties_callback, template: true, properties: properties.merge(additional: 'something'))
+          end
+
+          it_behaves_like 'service creation from a template'
+        end
+
+        context 'when data are stored in separated fields' do
+          let(:template) do
+            create(:jira_service, data_params.merge(properties: {}, title: title, description: description, template: true))
+          end
+
+          it_behaves_like 'service creation from a template'
+        end
+
+        context 'when data are stored in both properties and separated fields' do
+          let(:properties) { data_params.merge(title: title, description: description) }
+          let(:template) do
+            create(:jira_service, :without_properties_callback, active: true, template: true, properties: properties).tap do |service|
+              create(:jira_tracker_data, data_params.merge(service: service))
+            end
+          end
+
+          it_behaves_like 'service creation from a template'
+        end
+      end
     end
 
     describe "for pushover service" do
@@ -104,7 +163,6 @@ describe Service do
             api_key: '123456789'
           })
       end
-      let(:project) { create(:project) }
 
       describe 'is prefilled for projects pushover service' do
         it "has all fields prefilled" do