Commit f6654d13 authored by Jarka Košanová's avatar Jarka Košanová

Fix creating issue tracker services from templates

- fix problem when not expected proeprty is included
- fix data fileds not being copied
parent 93ba0ac6
...@@ -62,6 +62,7 @@ class IssueTrackerService < Service ...@@ -62,6 +62,7 @@ class IssueTrackerService < Service
end end
data_values.reject! { |key| data_fields.changed.include?(key) } 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? data_fields.assign_attributes(data_values) if data_values.present?
self.properties = {} self.properties = {}
...@@ -71,6 +72,10 @@ class IssueTrackerService < Service ...@@ -71,6 +72,10 @@ class IssueTrackerService < Service
@legacy_properties_data ||= {} @legacy_properties_data ||= {}
end end
def supports_data_fields?
true
end
def data_fields def data_fields
issue_tracker_data || self.build_issue_tracker_data issue_tracker_data || self.build_issue_tracker_data
end end
......
...@@ -291,6 +291,12 @@ class Service < ApplicationRecord ...@@ -291,6 +291,12 @@ class Service < ApplicationRecord
def self.build_from_template(project_id, template) def self.build_from_template(project_id, template)
service = template.dup service = template.dup
if template.supports_data_fields?
data_fields = template.data_fields.dup
data_fields.service = service
end
service.template = false service.template = false
service.project_id = project_id service.project_id = project_id
service.active = false if service.active? && !service.valid? service.active = false if service.active? && !service.valid?
...@@ -309,6 +315,11 @@ class Service < ApplicationRecord ...@@ -309,6 +315,11 @@ class Service < ApplicationRecord
find_by(template: true) find_by(template: true)
end end
# override if needed
def supports_data_fields?
false
end
private private
def cache_project_has_external_issue_tracker def cache_project_has_external_issue_tracker
......
...@@ -282,7 +282,7 @@ describe JiraService do ...@@ -282,7 +282,7 @@ describe JiraService do
context 'when data are stored in properties' do context 'when data are stored in properties' do
let(:properties) { data_params.merge(title: title, description: description) } let(:properties) { data_params.merge(title: title, description: description) }
let!(:service) do let!(:service) do
create(:jira_service, :without_properties_callback, properties: properties) create(:jira_service, :without_properties_callback, properties: properties.merge(additional: 'something'))
end end
it_behaves_like 'issue tracker fields' it_behaves_like 'issue tracker fields'
......
...@@ -78,10 +78,11 @@ describe Service do ...@@ -78,10 +78,11 @@ describe Service do
end end
describe "Template" do describe "Template" do
let(:project) { create(:project) }
describe '.build_from_template' do describe '.build_from_template' do
context 'when template is invalid' do context 'when template is invalid' do
it 'sets service template to inactive 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 = build(:prometheus_service, template: true, active: true, properties: {})
template.save(validate: false) template.save(validate: false)
...@@ -91,6 +92,64 @@ describe Service do ...@@ -91,6 +92,64 @@ describe Service do
expect(service.active).to be false expect(service.active).to be false
end end
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 end
describe "for pushover service" do describe "for pushover service" do
...@@ -104,7 +163,6 @@ describe Service do ...@@ -104,7 +163,6 @@ describe Service do
api_key: '123456789' api_key: '123456789'
}) })
end end
let(:project) { create(:project) }
describe 'is prefilled for projects pushover service' do describe 'is prefilled for projects pushover service' do
it "has all fields prefilled" do it "has all fields prefilled" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment