diff --git a/app/models/project_services/bugzilla_service.rb b/app/models/project_services/bugzilla_service.rb
index 8b79b5e9f0c17d972a611d2aac4959f79a375b90..0a498fde95a46e1a25fd36e9603f19784499bcdf 100644
--- a/app/models/project_services/bugzilla_service.rb
+++ b/app/models/project_services/bugzilla_service.rb
@@ -3,8 +3,6 @@
class BugzillaService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
- prop_accessor :project_url, :issues_url, :new_issue_url
-
def default_title
'Bugzilla'
end
diff --git a/app/models/project_services/custom_issue_tracker_service.rb b/app/models/project_services/custom_issue_tracker_service.rb
index 535fcf6b94e7dea38b55d58abad908cb7b86ca2a..dbc42b1b86d6f919e4baf431ac014a66a4a006cb 100644
--- a/app/models/project_services/custom_issue_tracker_service.rb
+++ b/app/models/project_services/custom_issue_tracker_service.rb
@@ -3,8 +3,6 @@
class CustomIssueTrackerService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
- prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url
-
def default_title
'Custom Issue Tracker'
end
diff --git a/app/models/project_services/data_fields.rb b/app/models/project_services/data_fields.rb
index 438d85098c8cc2e3b5e0f36a418fa4b92acc4512..0f5385f8ce2393f36d3e8653116748390dd68712 100644
--- a/app/models/project_services/data_fields.rb
+++ b/app/models/project_services/data_fields.rb
@@ -3,8 +3,56 @@
module DataFields
extend ActiveSupport::Concern
+ class_methods do
+ # Provide convenient accessor methods for data fields.
+ # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
+ def data_field(*args)
+ args.each do |arg|
+ self.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+ unless method_defined?(arg)
+ def #{arg}
+ data_fields.send('#{arg}') || (properties && properties['#{arg}'])
+ end
+ end
+
+ def #{arg}=(value)
+ @old_data_fields ||= {}
+ @old_data_fields['#{arg}'] ||= #{arg} # set only on the first assignment, IOW we remember the original value only
+ data_fields.send('#{arg}=', value)
+ end
+
+ def #{arg}_touched?
+ @old_data_fields ||= {}
+ @old_data_fields.has_key?('#{arg}')
+ end
+
+ def #{arg}_changed?
+ #{arg}_touched? && @old_data_fields['#{arg}'] != #{arg}
+ end
+
+ def #{arg}_was
+ return unless #{arg}_touched?
+ return if data_fields.persisted? # arg_was does not work for attr_encrypted
+
+ legacy_properties_data['#{arg}']
+ end
+ RUBY
+ end
+ end
+ end
+
included do
- has_one :issue_tracker_data
- has_one :jira_tracker_data
+ has_one :issue_tracker_data, autosave: true
+ has_one :jira_tracker_data, autosave: true
+
+ def data_fields
+ raise NotImplementedError
+ end
+
+ def data_fields_present?
+ data_fields.persisted?
+ rescue NotImplementedError
+ false
+ end
end
end
diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb
index 51032932eabed5cda89057bda846b9eea240ee56..ec28602b5e6ffaf55ec2ac13af0ecfcec97b70c7 100644
--- a/app/models/project_services/gitlab_issue_tracker_service.rb
+++ b/app/models/project_services/gitlab_issue_tracker_service.rb
@@ -5,8 +5,6 @@ class GitlabIssueTrackerService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
- prop_accessor :project_url, :issues_url, :new_issue_url
-
default_value_for :default, true
def default_title
diff --git a/app/models/project_services/issue_tracker_data.rb b/app/models/project_services/issue_tracker_data.rb
index 2c1d28ed421aad20f78bb3da7697b3dac259c858..b1d67657fe6fa0536bbe3cbb933eb3e192e4b415 100644
--- a/app/models/project_services/issue_tracker_data.rb
+++ b/app/models/project_services/issue_tracker_data.rb
@@ -6,9 +6,6 @@ class IssueTrackerData < ApplicationRecord
delegate :activated?, to: :service, allow_nil: true
validates :service, presence: true
- validates :project_url, presence: true, public_url: { enforce_sanitization: true }, if: :activated?
- validates :issues_url, presence: true, public_url: { enforce_sanitization: true }, if: :activated?
- validates :new_issue_url, public_url: { enforce_sanitization: true }, if: :activated?
def self.encryption_options
{
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index b6ad46513dbf8295a81835f61a5ae3c7bad36156..c201bd2ea185127adaee619839a459c5f8e8cb97 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -3,9 +3,14 @@
class IssueTrackerService < Service
validate :one_issue_tracker, if: :activated?, on: :manual_change
+ # TODO: we can probably just delegate as part of
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
+ data_field :project_url, :issues_url, :new_issue_url
+
default_value_for :category, 'issue_tracker'
- before_save :handle_properties
+ before_validation :handle_properties
+ before_validation :set_default_data, on: :create
# Pattern used to extract links from comments
# Override this method on services that uses different patterns
@@ -43,12 +48,31 @@ def description
end
def handle_properties
- properties.slice('title', 'description').each do |key, _|
+ # this has been moved from initialize_properties and should be improved
+ # as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
+ return unless properties
+
+ @legacy_properties_data = properties.dup
+ data_values = properties.slice!('title', 'description')
+ properties.each do |key, _|
current_value = self.properties.delete(key)
value = attribute_changed?(key) ? attribute_change(key).last : current_value
write_attribute(key, value)
end
+
+ data_values.reject! { |key| data_fields.changed.include?(key) }
+ data_fields.assign_attributes(data_values) if data_values.present?
+
+ self.properties = {}
+ end
+
+ def legacy_properties_data
+ @legacy_properties_data ||= {}
+ end
+
+ def data_fields
+ issue_tracker_data || self.build_issue_tracker_data
end
def default?
@@ -56,7 +80,7 @@ def default?
end
def issue_url(iid)
- self.issues_url.gsub(':id', iid.to_s)
+ issues_url.gsub(':id', iid.to_s)
end
def issue_tracker_path
@@ -80,25 +104,22 @@ def fields
]
end
+ def initialize_properties
+ {}
+ end
+
# Initialize with default properties values
- # or receive a block with custom properties
- def initialize_properties(&block)
- return unless properties.nil?
-
- if enabled_in_gitlab_config
- if block_given?
- yield
- else
- self.properties = {
- title: issues_tracker['title'],
- project_url: issues_tracker['project_url'],
- issues_url: issues_tracker['issues_url'],
- new_issue_url: issues_tracker['new_issue_url']
- }
- end
- else
- self.properties = {}
- end
+ def set_default_data
+ return unless issues_tracker.present?
+
+ self.title ||= issues_tracker['title']
+
+ # we don't want to override if we have set something
+ return if project_url || issues_url || new_issue_url
+
+ data_fields.project_url = issues_tracker['project_url']
+ data_fields.issues_url = issues_tracker['issues_url']
+ data_fields.new_issue_url = issues_tracker['new_issue_url']
end
def self.supported_events
diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index 0728c83005eee1a8f9d1edd5228c8a4aa3c35151..61ae78a0b95706c6c318b47cb3909fb27567b79f 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -17,7 +17,10 @@ class JiraService < IssueTrackerService
# Jira Cloud version is deprecating authentication via username and password.
# We should use username/password for Jira Server and email/api_token for Jira Cloud,
# for more information check: https://gitlab.com/gitlab-org/gitlab-ce/issues/49936.
- prop_accessor :username, :password, :url, :api_url, :jira_issue_transition_id
+
+ # TODO: we can probably just delegate as part of
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
+ data_field :username, :password, :url, :api_url, :jira_issue_transition_id
before_update :reset_password
@@ -35,24 +38,34 @@ def self.reference_pattern(only_long: true)
end
def initialize_properties
- super do
- self.properties = {
- url: issues_tracker['url'],
- api_url: issues_tracker['api_url']
- }
- end
+ {}
+ end
+
+ def data_fields
+ jira_tracker_data || self.build_jira_tracker_data
end
def reset_password
- self.password = nil if reset_password?
+ data_fields.password = nil if reset_password?
+ end
+
+ def set_default_data
+ return unless issues_tracker.present?
+
+ self.title ||= issues_tracker['title']
+
+ return if url
+
+ data_fields.url ||= issues_tracker['url']
+ data_fields.api_url ||= issues_tracker['api_url']
end
def options
url = URI.parse(client_url)
{
- username: self.username,
- password: self.password,
+ username: username,
+ password: password,
site: URI.join(url, '/').to_s, # Intended to find the root
context_path: url.path,
auth_type: :basic,
diff --git a/app/models/project_services/jira_tracker_data.rb b/app/models/project_services/jira_tracker_data.rb
index 4f528e3d81b36b666b956986a706357f51dcc2f2..e4e0f64150a23eb2749860cd96f21e21cf63d59f 100644
--- a/app/models/project_services/jira_tracker_data.rb
+++ b/app/models/project_services/jira_tracker_data.rb
@@ -6,13 +6,6 @@ class JiraTrackerData < ApplicationRecord
delegate :activated?, to: :service, allow_nil: true
validates :service, presence: true
- validates :url, public_url: { enforce_sanitization: true }, presence: true, if: :activated?
- validates :api_url, public_url: { enforce_sanitization: true }, allow_blank: true
- validates :username, presence: true, if: :activated?
- validates :password, presence: true, if: :activated?
- validates :jira_issue_transition_id,
- format: { with: Gitlab::Regex.jira_transition_id_regex, message: s_("JiraService|transition ids can have only numbers which can be split with , or ;") },
- allow_blank: true
def self.encryption_options
{
diff --git a/app/models/project_services/redmine_service.rb b/app/models/project_services/redmine_service.rb
index 5ca057ca833ab206eca7ca6cd0dce937e58fcda9..a4ca0d20669f89952fa44a7ae01bd4c71a844e2d 100644
--- a/app/models/project_services/redmine_service.rb
+++ b/app/models/project_services/redmine_service.rb
@@ -3,8 +3,6 @@
class RedmineService < IssueTrackerService
validates :project_url, :issues_url, :new_issue_url, presence: true, public_url: true, if: :activated?
- prop_accessor :project_url, :issues_url, :new_issue_url
-
def default_title
'Redmine'
end
diff --git a/app/models/project_services/youtrack_service.rb b/app/models/project_services/youtrack_service.rb
index f9de1f7dc496623458198b94bec28e560c269237..0416eaa5be0bb1d3f33f5e2e89a5c5408861f367 100644
--- a/app/models/project_services/youtrack_service.rb
+++ b/app/models/project_services/youtrack_service.rb
@@ -3,8 +3,6 @@
class YoutrackService < IssueTrackerService
validates :project_url, :issues_url, presence: true, public_url: true, if: :activated?
- prop_accessor :project_url, :issues_url
-
# {PROJECT-KEY}-{NUMBER} Examples: YT-1, PRJ-1, gl-030
def self.reference_pattern(only_long: false)
if only_long
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 6ca9cc40026136d3bbac2f12343e0af1b863d002..4018ce900e192b8bae9e5b76d7509f3e73332e3b 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1044,7 +1044,12 @@ class ProjectService < Grape::Entity
expose :job_events
# Expose serialized properties
expose :properties do |service, options|
- service.properties.slice(*service.api_field_names)
+ # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
+ if service.data_fields_present?
+ service.data_fields.as_json.slice(*service.api_field_names)
+ else
+ service.properties.slice(*service.api_field_names)
+ end
end
end
diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index 5cd54c302fcc4eda87b8ad98c112e1f74ad90af2..c6c2876033d2fad7cbfd8e48cca90ec94d6988d2 100644
--- a/lib/gitlab/usage_data.rb
+++ b/lib/gitlab/usage_data.rb
@@ -3,6 +3,7 @@
module Gitlab
class UsageData
APPROXIMATE_COUNT_MODELS = [Label, MergeRequest, Note, Todo].freeze
+ BATCH_SIZE = 100
class << self
def data(force_refresh: false)
@@ -13,10 +14,10 @@ def data(force_refresh: false)
def uncached_data
license_usage_data.merge(system_usage_data)
- .merge(features_usage_data)
- .merge(components_usage_data)
- .merge(cycle_analytics_usage_data)
- .merge(usage_counters)
+ .merge(features_usage_data)
+ .merge(components_usage_data)
+ .merge(cycle_analytics_usage_data)
+ .merge(usage_counters)
end
def to_json(force_refresh: false)
@@ -96,9 +97,8 @@ def system_usage_data
todos: count(Todo),
uploads: count(Upload),
web_hooks: count(WebHook)
- }
- .merge(services_usage)
- .merge(approximate_counts)
+ }.merge(services_usage)
+ .merge(approximate_counts)
}.tap do |data|
data[:counts][:user_preferences] = user_preferences_usage
end
@@ -173,17 +173,34 @@ def services_usage
def jira_usage
# Jira Cloud does not support custom domains as per https://jira.atlassian.com/browse/CLOUD-6999
# so we can just check for subdomains of atlassian.net
- services = count(
- Service.unscoped.where(type: :JiraService, active: true)
- .group("CASE WHEN properties LIKE '%.atlassian.net%' THEN 'cloud' ELSE 'server' END"),
- fallback: Hash.new(-1)
- )
- {
- projects_jira_server_active: services['server'] || 0,
- projects_jira_cloud_active: services['cloud'] || 0,
- projects_jira_active: services['server'] == -1 ? -1 : services.values.sum
+ results = {
+ projects_jira_server_active: 0,
+ projects_jira_cloud_active: 0,
+ projects_jira_active: -1
}
+
+ Service.unscoped
+ .where(type: :JiraService, active: true)
+ .includes(:jira_tracker_data)
+ .find_in_batches(batch_size: BATCH_SIZE) do |services|
+
+ counts = services.group_by do |service|
+ # TODO: Simplify as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
+ service_url = service.data_fields&.url || (service.properties && service.properties['url'])
+ service_url&.include?('.atlassian.net') ? :cloud : :server
+ end
+
+ results[:projects_jira_server_active] += counts[:server].count if counts[:server]
+ results[:projects_jira_cloud_active] += counts[:cloud].count if counts[:cloud]
+ if results[:projects_jira_active] == -1
+ results[:projects_jira_active] = count(services)
+ else
+ results[:projects_jira_active] += count(services)
+ end
+ end
+
+ results
end
def user_preferences_usage
diff --git a/spec/factories/services.rb b/spec/factories/services.rb
index b2d6ada91fa54be2f100ed70c0d9a649b71cc51b..c4cb3f07fe74de9511b659ca0a0a549a0f642b61 100644
--- a/spec/factories/services.rb
+++ b/spec/factories/services.rb
@@ -9,11 +9,7 @@
factory :custom_issue_tracker_service, class: CustomIssueTrackerService do
project
active true
- properties(
- project_url: 'https://project.url.com',
- issues_url: 'https://issues.url.com',
- new_issue_url: 'https://newissue.url.com'
- )
+ issue_tracker
end
factory :emails_on_push_service do
@@ -47,12 +43,24 @@
factory :jira_service do
project
active true
- properties(
- url: 'https://jira.example.com',
- username: 'jira_user',
- password: 'my-secret-password',
- project_key: 'jira-key'
- )
+
+ transient do
+ create_data true
+ url 'https://jira.example.com'
+ api_url nil
+ username 'jira_username'
+ password 'jira_password'
+ jira_issue_transition_id '56-1'
+ end
+
+ after(:build) do |service, evaluator|
+ if evaluator.create_data
+ create(:jira_tracker_data, service: service,
+ url: evaluator.url, api_url: evaluator.api_url, jira_issue_transition_id: evaluator.jira_issue_transition_id,
+ username: evaluator.username, password: evaluator.password
+ )
+ end
+ end
end
factory :bugzilla_service do
@@ -80,20 +88,26 @@
end
trait :issue_tracker do
- properties(
- project_url: 'http://issue-tracker.example.com',
- issues_url: 'http://issue-tracker.example.com/issues/:id',
- new_issue_url: 'http://issue-tracker.example.com'
- )
+ transient do
+ create_data true
+ project_url 'http://issuetracker.example.com'
+ issues_url 'http://issues.example.com/issues/:id'
+ new_issue_url 'http://new-issue.example.com'
+ end
+
+ after(:build) do |service, evaluator|
+ if evaluator.create_data
+ create(:issue_tracker_data, service: service,
+ project_url: evaluator.project_url, issues_url: evaluator.issues_url, new_issue_url: evaluator.new_issue_url
+ )
+ end
+ end
end
trait :jira_cloud_service do
- properties(
- url: 'https://mysite.atlassian.net',
- username: 'jira_user',
- password: 'my-secret-password',
- project_key: 'jira-key'
- )
+ url 'https://mysite.atlassian.net'
+ username 'jira_user'
+ password 'my-secret-password'
end
factory :hipchat_service do
@@ -102,15 +116,21 @@
token 'test_token'
end
+ # this is for testing storing values inside properties, which is deprecated and will be removed in
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
trait :without_properties_callback do
+ jira_tracker_data nil
+ issue_tracker_data nil
+ create_data false
+
after(:build) do |service|
- allow(service).to receive(:handle_properties)
+ IssueTrackerService.skip_callback(:validation, :before, :handle_properties)
end
- after(:create) do |service|
- # we have to remove the stub because the behaviour of
- # handle_properties method is tested after the creation
- allow(service).to receive(:handle_properties).and_call_original
+ to_create { |instance| instance.save(validate: false)}
+
+ after(:create) do
+ IssueTrackerService.set_callback(:validation, :before, :handle_properties)
end
end
end
diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
index 7eb63fea41349d23304b6c69943d2aa5a4d81acc..47ea273ef3afb0e6495c6543eaba5bb01ac6d907 100644
--- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
@@ -34,7 +34,7 @@
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
- expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
+ expect(link['href']).to eq 'http://issues.example.com/issues/12'
end
it 'parses cross-project references to regular issues' do
@@ -63,7 +63,7 @@
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
- expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
+ expect(link['href']).to eq 'http://issues.example.com/issues/12'
end
it 'allows to use long external reference syntax for Redmine' do
@@ -72,7 +72,7 @@
result = described_class.call(markdown, project: project)[:output]
link = result.css('a').first
- expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
+ expect(link['href']).to eq 'http://issues.example.com/issues/12'
end
it 'parses cross-project references to regular issues' do
diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb
index 8eb64b97d6a863405bddc92ad6ae15a6004f9359..62787c5abafa3f156f384671e7f2d02b744def1f 100644
--- a/spec/lib/gitlab/usage_data_spec.rb
+++ b/spec/lib/gitlab/usage_data_spec.rb
@@ -3,14 +3,16 @@
require 'spec_helper'
describe Gitlab::UsageData do
- let(:projects) { create_list(:project, 3) }
+ let(:projects) { create_list(:project, 4) }
let!(:board) { create(:board, project: projects[0]) }
describe '#data' do
before do
create(:jira_service, project: projects[0])
- create(:jira_service, project: projects[1])
+ create(:jira_service, :without_properties_callback, project: projects[1])
create(:jira_service, :jira_cloud_service, project: projects[2])
+ create(:jira_service, :without_properties_callback, project: projects[3],
+ properties: { url: 'https://mysite.atlassian.net' })
create(:prometheus_service, project: projects[1])
create(:service, project: projects[0], type: 'SlackSlashCommandsService', active: true)
create(:service, project: projects[1], type: 'SlackService', active: true)
@@ -156,7 +158,7 @@
count_data = subject[:counts]
expect(count_data[:boards]).to eq(1)
- expect(count_data[:projects]).to eq(3)
+ expect(count_data[:projects]).to eq(4)
expect(count_data.keys).to include(*expected_keys)
expect(expected_keys - count_data.keys).to be_empty
end
@@ -164,14 +166,14 @@
it 'gathers projects data correctly' do
count_data = subject[:counts]
- expect(count_data[:projects]).to eq(3)
+ expect(count_data[:projects]).to eq(4)
expect(count_data[:projects_prometheus_active]).to eq(1)
- expect(count_data[:projects_jira_active]).to eq(3)
+ expect(count_data[:projects_jira_active]).to eq(4)
expect(count_data[:projects_jira_server_active]).to eq(2)
- expect(count_data[:projects_jira_cloud_active]).to eq(1)
+ expect(count_data[:projects_jira_cloud_active]).to eq(2)
expect(count_data[:projects_slack_notifications_active]).to eq(2)
expect(count_data[:projects_slack_slash_active]).to eq(1)
- expect(count_data[:projects_with_repositories_enabled]).to eq(2)
+ expect(count_data[:projects_with_repositories_enabled]).to eq(3)
expect(count_data[:projects_with_error_tracking_enabled]).to eq(1)
expect(count_data[:clusters_enabled]).to eq(7)
diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb
index 74c85a13c88e7e25997b6bd8d6d6a07f537a0d26..e25d87f61d659755ac62fd5cfb1976a2d0807555 100644
--- a/spec/models/project_services/bugzilla_service_spec.rb
+++ b/spec/models/project_services/bugzilla_service_spec.rb
@@ -48,7 +48,7 @@
create(:bugzilla_service, :without_properties_callback, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in separated fields' do
@@ -56,7 +56,7 @@
create(:bugzilla_service, title: title, description: description, properties: access_params)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
@@ -65,7 +65,7 @@
create(:bugzilla_service, :without_properties_callback, title: title, description: description, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when no title & description are set' do
diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb
index 5259357a2540d8eb63b51894d853762085c07baa..8359bc6807adf9e5d7a03472dcd88398dbe179d0 100644
--- a/spec/models/project_services/custom_issue_tracker_service_spec.rb
+++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb
@@ -62,7 +62,7 @@
create(:custom_issue_tracker_service, :without_properties_callback, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in separated fields' do
@@ -70,7 +70,7 @@
create(:custom_issue_tracker_service, title: title, description: description, properties: access_params)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
@@ -79,7 +79,7 @@
create(:custom_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when no title & description are set' do
diff --git a/spec/models/project_services/data_fields_spec.rb b/spec/models/project_services/data_fields_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..146db0ae2278e5f7e11cefc3483ddfa9020dbb4e
--- /dev/null
+++ b/spec/models/project_services/data_fields_spec.rb
@@ -0,0 +1,138 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DataFields do
+ let(:url) { 'http://url.com' }
+ let(:username) { 'username_one' }
+ let(:properties) do
+ { url: url, username: username }
+ end
+
+ shared_examples 'data fields' do
+ describe '#arg' do
+ it 'returns an argument correctly' do
+ expect(service.url).to eq(url)
+ end
+ end
+
+ describe '{arg}_changed?' do
+ it 'returns false when the property has not been assigned a new value' do
+ service.username = 'new_username'
+ service.validate
+ expect(service.url_changed?).to be_falsy
+ end
+
+ it 'returns true when the property has been assigned a different value' do
+ service.url = "http://example.com"
+ service.validate
+ expect(service.url_changed?).to be_truthy
+ end
+
+ it 'returns true when the property has been assigned a different value twice' do
+ service.url = "http://example.com"
+ service.url = "http://example.com"
+ service.validate
+ expect(service.url_changed?).to be_truthy
+ end
+
+ it 'returns false when the property has been re-assigned the same value' do
+ service.url = 'http://url.com'
+ service.validate
+ expect(service.url_changed?).to be_falsy
+ end
+ end
+
+ describe '{arg}_touched?' do
+ it 'returns false when the property has not been assigned a new value' do
+ service.username = 'new_username'
+ service.validate
+ expect(service.url_changed?).to be_falsy
+ end
+
+ it 'returns true when the property has been assigned a different value' do
+ service.url = "http://example.com"
+ service.validate
+ expect(service.url_changed?).to be_truthy
+ end
+
+ it 'returns true when the property has been assigned a different value twice' do
+ service.url = "http://example.com"
+ service.url = "http://example.com"
+ service.validate
+ expect(service.url_changed?).to be_truthy
+ end
+
+ it 'returns true when the property has been re-assigned the same value' do
+ service.url = 'http://url.com'
+ expect(service.url_touched?).to be_truthy
+ end
+
+ it 'returns false when the property has been re-assigned the same value' do
+ service.url = 'http://url.com'
+ service.validate
+ expect(service.url_changed?).to be_falsy
+ end
+ end
+ end
+
+ context 'when data are stored in data_fields' do
+ let(:service) do
+ create(:jira_service, url: url, username: username)
+ end
+
+ it_behaves_like 'data fields'
+
+ describe '{arg}_was?' do
+ it 'returns nil' do
+ service.url = 'http://example.com'
+ service.validate
+ expect(service.url_was).to be_nil
+ end
+ end
+ end
+
+ context 'when data are stored in properties' do
+ let(:service) { create(:jira_service, :without_properties_callback, properties: properties) }
+
+ it_behaves_like 'data fields'
+
+ describe '{arg}_was?' do
+ it 'returns nil when the property has not been assigned a new value' do
+ service.username = 'new_username'
+ service.validate
+ expect(service.url_was).to be_nil
+ end
+
+ it 'returns initial value when the property has been assigned a different value' do
+ service.url = 'http://example.com'
+ service.validate
+ expect(service.url_was).to eq('http://url.com')
+ end
+
+ it 'returns initial value when the property has been re-assigned the same value' do
+ service.url = 'http://url.com'
+ service.validate
+ expect(service.url_was).to eq('http://url.com')
+ end
+ end
+ end
+
+ context 'when data are stored in both properties and data_fields' do
+ let(:service) do
+ create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service|
+ create(:jira_tracker_data, properties.merge(service: service))
+ end
+ end
+
+ it_behaves_like 'data fields'
+
+ describe '{arg}_was?' do
+ it 'returns nil' do
+ service.url = 'http://example.com'
+ service.validate
+ expect(service.url_was).to be_nil
+ end
+ end
+ end
+end
diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
index 0c4fc290a13001d5424087f0089ce6bc27b1b374..4f3736ca65b3f622a6e10cddd887d1d207c77093 100644
--- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
+++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
@@ -65,7 +65,7 @@
create(:gitlab_issue_tracker_service, :without_properties_callback, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in separated fields' do
@@ -73,7 +73,7 @@
create(:gitlab_issue_tracker_service, title: title, description: description, properties: access_params)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
@@ -82,7 +82,7 @@
create(:gitlab_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when no title & description are set' do
diff --git a/spec/models/project_services/issue_tracker_data_spec.rb b/spec/models/project_services/issue_tracker_data_spec.rb
index aaab654f874caf7bbddef3d714e8c0dc2eb95f75..db617cf0abbb1cc3f8f50da4bb96a2c57d9f9ee3 100644
--- a/spec/models/project_services/issue_tracker_data_spec.rb
+++ b/spec/models/project_services/issue_tracker_data_spec.rb
@@ -8,28 +8,4 @@
describe 'Associations' do
it { is_expected.to belong_to :service }
end
-
- describe 'Validations' do
- subject { described_class.new(service: service) }
-
- context 'url validations' do
- context 'when service is inactive' do
- it { is_expected.not_to validate_presence_of(:project_url) }
- it { is_expected.not_to validate_presence_of(:issues_url) }
- end
-
- context 'when service is active' do
- before do
- service.update(active: true)
- end
-
- it_behaves_like 'issue tracker service URL attribute', :project_url
- it_behaves_like 'issue tracker service URL attribute', :issues_url
- it_behaves_like 'issue tracker service URL attribute', :new_issue_url
-
- it { is_expected.to validate_presence_of(:project_url) }
- it { is_expected.to validate_presence_of(:issues_url) }
- end
- end
- end
end
diff --git a/spec/models/project_services/issue_tracker_service_spec.rb b/spec/models/project_services/issue_tracker_service_spec.rb
index 2fc4d69c2db168cb2228d568d3ab26102a2fec29..f1cdee5c4a3c1fcb101ba1e971992dfb66927edb 100644
--- a/spec/models/project_services/issue_tracker_service_spec.rb
+++ b/spec/models/project_services/issue_tracker_service_spec.rb
@@ -7,7 +7,7 @@
let(:project) { create :project }
describe 'only one issue tracker per project' do
- let(:service) { RedmineService.new(project: project, active: true) }
+ let(:service) { RedmineService.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) }
before do
create(:custom_issue_tracker_service, project: project)
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index 02060699e9a3a8ff3674bf21c1f2c294eb7dfd14..a976745023bb44c80dc2fe1cdfe51238f118346e 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -6,10 +6,18 @@
include Gitlab::Routing
include AssetsHelpers
+ 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(:transition_id) { 'test27' }
+
describe '#options' do
let(:service) do
- described_class.new(
- project: build_stubbed(:project),
+ described_class.create(
+ project: create(:project),
active: true,
username: 'username',
password: 'test',
@@ -32,78 +40,6 @@
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
- it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) }
- it { is_expected.to allow_value('1,2,3').for(:jira_issue_transition_id) }
- it { is_expected.to allow_value('1;2;3').for(:jira_issue_transition_id) }
- it { is_expected.not_to allow_value('a,b,cd').for(:jira_issue_transition_id) }
- end
-
- describe 'Validations' do
- context 'when service is active' do
- before do
- subject.active = true
- end
-
- it { is_expected.to validate_presence_of(:url) }
- it_behaves_like 'issue tracker service URL attribute', :url
- end
-
- context 'when service is inactive' do
- before do
- subject.active = false
- end
-
- it { is_expected.not_to validate_presence_of(:url) }
- it { is_expected.not_to validate_presence_of(:username) }
- it { is_expected.not_to validate_presence_of(:password) }
- end
-
- context 'validating urls' do
- let(:service) do
- described_class.new(
- project: create(:project),
- active: true,
- username: 'username',
- password: 'test',
- jira_issue_transition_id: 24,
- url: 'http://jira.test.com'
- )
- end
-
- it 'is valid when all fields have required values' do
- expect(service).to be_valid
- end
-
- it 'is not valid when url is not a valid url' do
- service.url = 'not valid'
-
- expect(service).not_to be_valid
- end
-
- it 'is not valid when api url is not a valid url' do
- service.api_url = 'not valid'
-
- expect(service).not_to be_valid
- end
-
- it 'is not valid when username is missing' do
- service.username = nil
-
- expect(service).not_to be_valid
- end
-
- it 'is not valid when password is missing' do
- service.password = nil
-
- expect(service).not_to be_valid
- end
-
- it 'is valid when api url is a valid url' do
- service.api_url = 'http://jira.test.com/api'
-
- expect(service).to be_valid
- end
- end
end
describe '.reference_pattern' do
@@ -118,55 +54,260 @@
describe '#create' do
let(:params) do
{
- project: create(:project), title: 'custom title', description: 'custom description'
+ project: create(:project),
+ title: 'custom title', description: 'custom description',
+ url: url, api_url: api_url,
+ username: username, password: password,
+ jira_issue_transition_id: transition_id
}
end
subject { described_class.create(params) }
- it 'does not store title & description into properties' do
- expect(subject.properties.keys).not_to include('title', 'description')
+ it 'does not store data into properties' do
+ expect(subject.properties).to be_nil
+ end
+
+ it 'sets title correctly' do
+ service = subject
+
+ expect(service.title).to eq('custom title')
end
- it 'sets title & description correctly' do
+ it 'sets service data correctly' do
service = subject
expect(service.title).to eq('custom title')
expect(service.description).to eq('custom description')
end
+
+ it 'stores data in data_fields correcty' do
+ service = subject
+
+ expect(service.jira_tracker_data.url).to eq(url)
+ expect(service.jira_tracker_data.api_url).to eq(api_url)
+ expect(service.jira_tracker_data.username).to eq(username)
+ expect(service.jira_tracker_data.password).to eq(password)
+ expect(service.jira_tracker_data.jira_issue_transition_id).to eq(transition_id)
+ end
end
+ # we need to make sure we are able to read both from properties and jira_tracker_data table
+ # TODO: change this as part of #63084
context 'overriding properties' do
- let(:url) { 'http://issue_tracker.example.com' }
let(:access_params) do
- { url: url, username: 'username', password: 'password' }
+ { url: url, api_url: api_url, username: username, password: password,
+ jira_issue_transition_id: transition_id }
+ end
+ let(:data_params) do
+ {
+ url: url, api_url: api_url,
+ username: username, password: password,
+ jira_issue_transition_id: transition_id
+ }
+ end
+
+ shared_examples 'handles jira fields' do
+ let(:data_params) do
+ {
+ url: url, api_url: api_url,
+ username: username, password: password,
+ jira_issue_transition_id: transition_id
+ }
+ end
+
+ context 'reading data' do
+ it 'reads data correctly' do
+ 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)
+ expect(service.jira_issue_transition_id).to eq(transition_id)
+ end
+ end
+
+ context '#update' do
+ context 'basic update' do
+ let(:new_username) { 'new_username' }
+ let(:new_url) { 'http://jira-new.example.com' }
+
+ before do
+ service.update(username: new_username, url: new_url)
+ end
+
+ it 'leaves properties field emtpy' do
+ # expect(service.reload.properties).to be_empty
+ end
+
+ it 'stores updated data in jira_tracker_data table' do
+ data = service.jira_tracker_data.reload
+
+ expect(data.url).to eq(new_url)
+ expect(data.api_url).to eq(api_url)
+ expect(data.username).to eq(new_username)
+ expect(data.password).to eq(password)
+ expect(data.jira_issue_transition_id).to eq(transition_id)
+ end
+ end
+
+ context 'stored password invalidation' do
+ context 'when a password was previously set' do
+ context 'when only web url present' do
+ let(:data_params) do
+ {
+ url: url, api_url: nil,
+ username: username, password: password,
+ jira_issue_transition_id: transition_id
+ }
+ end
+
+ it 'resets password if url changed' do
+ service
+ service.url = 'http://jira_edited.example.com'
+ service.save
+
+ expect(service.reload.url).to eq('http://jira_edited.example.com')
+ expect(service.password).to be_nil
+ end
+
+ it 'does not reset password if url "changed" to the same url as before' do
+ service.title = 'aaaaaa'
+ service.url = 'http://jira.example.com'
+ service.save
+
+ expect(service.reload.url).to eq('http://jira.example.com')
+ expect(service.password).not_to be_nil
+ end
+
+ it 'resets password if url not changed but api url added' do
+ service.api_url = 'http://jira_edited.example.com/rest/api/2'
+ service.save
+
+ expect(service.reload.api_url).to eq('http://jira_edited.example.com/rest/api/2')
+ expect(service.password).to be_nil
+ end
+
+ it 'does not reset password if new url is set together with password, even if it\'s the same password' do
+ service.url = 'http://jira_edited.example.com'
+ service.password = password
+ service.save
+
+ expect(service.password).to eq(password)
+ expect(service.url).to eq('http://jira_edited.example.com')
+ end
+
+ it 'resets password if url changed, even if setter called multiple times' do
+ service.url = 'http://jira1.example.com/rest/api/2'
+ service.url = 'http://jira1.example.com/rest/api/2'
+ service.save
+
+ expect(service.password).to be_nil
+ end
+
+ it 'does not reset password if username changed' do
+ service.username = 'some_name'
+ service.save
+
+ expect(service.reload.password).to eq(password)
+ end
+
+ it 'does not reset password if password changed' do
+ service.url = 'http://jira_edited.example.com'
+ service.password = 'new_password'
+ service.save
+
+ expect(service.reload.password).to eq('new_password')
+ end
+
+ it 'does not reset password if the password is touched and same as before' do
+ service.url = 'http://jira_edited.example.com'
+ service.password = password
+ service.save
+
+ expect(service.reload.password).to eq(password)
+ end
+ end
+
+ context 'when both web and api url present' do
+ let(:data_params) do
+ {
+ url: url, api_url: 'http://jira.example.com/rest/api/2',
+ username: username, password: password,
+ jira_issue_transition_id: transition_id
+ }
+ end
+
+ it 'resets password if api url changed' do
+ service.api_url = 'http://jira_edited.example.com/rest/api/2'
+ service.save
+
+ expect(service.password).to be_nil
+ end
+
+ it 'does not reset password if url changed' do
+ service.url = 'http://jira_edited.example.com'
+ service.save
+
+ expect(service.password).to eq(password)
+ end
+
+ it 'resets password if api url set to empty' do
+ service.update(api_url: '')
+
+ expect(service.reload.password).to be_nil
+ end
+ end
+ end
+
+ context 'when no password was previously set' do
+ let(:data_params) do
+ {
+ url: url, username: username
+ }
+ end
+
+ it 'saves password if new url is set together with password' do
+ service.url = 'http://jira_edited.example.com/rest/api/2'
+ service.password = 'password'
+ service.save
+ expect(service.reload.password).to eq('password')
+ expect(service.reload.url).to eq('http://jira_edited.example.com/rest/api/2')
+ end
+ end
+ end
+ end
end
# this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
context 'when data are stored in properties' do
- let(:properties) { access_params.merge(title: title, description: description) }
- let(:service) do
+ let(:properties) { data_params.merge(title: title, description: description) }
+ let!(:service) do
create(:jira_service, :without_properties_callback, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
+ it_behaves_like 'handles jira fields'
end
context 'when data are stored in separated fields' do
let(:service) do
- create(:jira_service, title: title, description: description, properties: access_params)
+ create(:jira_service, data_params.merge(properties: {}, title: title, description: description))
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
+ it_behaves_like 'handles jira fields'
end
context 'when data are stored in both properties and separated fields' do
- let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
+ let(:properties) { data_params.merge(title: title, description: description) }
let(:service) do
- create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties)
+ create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service|
+ create(:jira_tracker_data, data_params.merge(service: service))
+ end
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
+ it_behaves_like 'handles jira fields'
end
context 'when no title & description are set' do
@@ -410,111 +551,6 @@ def test_settings(api_url = nil)
end
end
- describe 'Stored password invalidation' do
- let(:project) { create(:project) }
-
- context 'when a password was previously set' do
- before do
- @jira_service = described_class.create!(
- project: project,
- properties: {
- url: 'http://jira.example.com/web',
- username: 'mic',
- password: 'password'
- }
- )
- end
-
- context 'when only web url present' do
- it 'reset password if url changed' do
- @jira_service.url = 'http://jira_edited.example.com/rest/api/2'
- @jira_service.save
-
- expect(@jira_service.password).to be_nil
- end
-
- it 'reset password if url not changed but api url added' do
- @jira_service.api_url = 'http://jira_edited.example.com/rest/api/2'
- @jira_service.save
-
- expect(@jira_service.password).to be_nil
- end
- end
-
- context 'when both web and api url present' do
- before do
- @jira_service.api_url = 'http://jira.example.com/rest/api/2'
- @jira_service.password = 'password'
-
- @jira_service.save
- end
- it 'reset password if api url changed' do
- @jira_service.api_url = 'http://jira_edited.example.com/rest/api/2'
- @jira_service.save
-
- expect(@jira_service.password).to be_nil
- end
-
- it 'does not reset password if url changed' do
- @jira_service.url = 'http://jira_edited.example.com/rweb'
- @jira_service.save
-
- expect(@jira_service.password).to eq('password')
- end
-
- it 'reset password if api url set to empty' do
- @jira_service.api_url = ''
- @jira_service.save
-
- expect(@jira_service.password).to be_nil
- end
- end
-
- it 'does not reset password if username changed' do
- @jira_service.username = 'some_name'
- @jira_service.save
-
- expect(@jira_service.password).to eq('password')
- end
-
- it 'does not reset password if new url is set together with password, even if it\'s the same password' do
- @jira_service.url = 'http://jira_edited.example.com/rest/api/2'
- @jira_service.password = 'password'
- @jira_service.save
-
- expect(@jira_service.password).to eq('password')
- expect(@jira_service.url).to eq('http://jira_edited.example.com/rest/api/2')
- end
-
- it 'resets password if url changed, even if setter called multiple times' do
- @jira_service.url = 'http://jira1.example.com/rest/api/2'
- @jira_service.url = 'http://jira1.example.com/rest/api/2'
- @jira_service.save
- expect(@jira_service.password).to be_nil
- end
- end
-
- context 'when no password was previously set' do
- before do
- @jira_service = described_class.create(
- project: project,
- properties: {
- url: 'http://jira.example.com/rest/api/2',
- username: 'mic'
- }
- )
- end
-
- it 'saves password if new url is set together with password' do
- @jira_service.url = 'http://jira_edited.example.com/rest/api/2'
- @jira_service.password = 'password'
- @jira_service.save
- expect(@jira_service.password).to eq('password')
- expect(@jira_service.url).to eq('http://jira_edited.example.com/rest/api/2')
- end
- end
- end
-
describe 'description and title' do
let(:title) { 'Jira One' }
let(:description) { 'Jira One issue tracker' }
@@ -539,7 +575,7 @@ def test_settings(api_url = nil)
context 'when it is set in properties' do
it 'values from properties are returned' do
- service = create(:jira_service, properties: properties)
+ service = create(:jira_service, :without_properties_callback, properties: properties)
expect(service.title).to eq(title)
expect(service.description).to eq(description)
@@ -602,8 +638,8 @@ def test_settings(api_url = nil)
project = create(:project)
service = project.create_jira_service(active: true)
- expect(service.properties['url']).to eq('http://jira.sample/projects/project_a')
- expect(service.properties['api_url']).to eq('http://jira.sample/api')
+ expect(service.url).to eq('http://jira.sample/projects/project_a')
+ expect(service.api_url).to eq('http://jira.sample/api')
end
end
diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/project_services/jira_tracker_data_spec.rb
index 1b6ece8531b5b747155572f282315473dff93316..6cd3eb33d9be7d13917d853c57192c3bb8b7f76f 100644
--- a/spec/models/project_services/jira_tracker_data_spec.rb
+++ b/spec/models/project_services/jira_tracker_data_spec.rb
@@ -8,35 +8,4 @@
describe 'Associations' do
it { is_expected.to belong_to(:service) }
end
-
- describe 'Validations' do
- subject { described_class.new(service: service) }
-
- context 'jira_issue_transition_id' do
- it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) }
- it { is_expected.to allow_value('1,2,3').for(:jira_issue_transition_id) }
- it { is_expected.to allow_value('1;2;3').for(:jira_issue_transition_id) }
- it { is_expected.not_to allow_value('a,b,cd').for(:jira_issue_transition_id) }
- end
-
- context 'url validations' do
- context 'when service is inactive' do
- it { is_expected.not_to validate_presence_of(:url) }
- it { is_expected.not_to validate_presence_of(:username) }
- it { is_expected.not_to validate_presence_of(:password) }
- end
-
- context 'when service is active' do
- before do
- service.update(active: true)
- end
-
- it_behaves_like 'issue tracker service URL attribute', :url
-
- it { is_expected.to validate_presence_of(:url) }
- it { is_expected.to validate_presence_of(:username) }
- it { is_expected.to validate_presence_of(:password) }
- end
- end
- end
end
diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb
index c1ee6546b12d735704c7834b777d10c3ade85ec4..4ef4064d0692d61e7760382b8089a641859a08b5 100644
--- a/spec/models/project_services/redmine_service_spec.rb
+++ b/spec/models/project_services/redmine_service_spec.rb
@@ -9,6 +9,15 @@
end
describe 'Validations' do
+ # if redmine is set in setting the urls are set to defaults
+ # therefore the validation passes as the values are not nil
+ before do
+ settings = {
+ 'redmine' => {}
+ }
+ allow(Gitlab.config).to receive(:issues_tracker).and_return(settings)
+ end
+
context 'when service is active' do
before do
subject.active = true
@@ -17,6 +26,7 @@
it { is_expected.to validate_presence_of(:project_url) }
it { is_expected.to validate_presence_of(:issues_url) }
it { is_expected.to validate_presence_of(:new_issue_url) }
+
it_behaves_like 'issue tracker service URL attribute', :project_url
it_behaves_like 'issue tracker service URL attribute', :issues_url
it_behaves_like 'issue tracker service URL attribute', :new_issue_url
@@ -54,7 +64,7 @@
create(:redmine_service, :without_properties_callback, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in separated fields' do
@@ -62,7 +72,7 @@
create(:redmine_service, title: title, description: description, properties: access_params)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
@@ -71,7 +81,7 @@
create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when no title & description are set' do
diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb
index c48bf487af0f9c4de61f61d9401e1df5dac45beb..eff9f451b1a2c8de78021b040d91e6ea4c1a639e 100644
--- a/spec/models/project_services/youtrack_service_spec.rb
+++ b/spec/models/project_services/youtrack_service_spec.rb
@@ -16,6 +16,7 @@
it { is_expected.to validate_presence_of(:project_url) }
it { is_expected.to validate_presence_of(:issues_url) }
+
it_behaves_like 'issue tracker service URL attribute', :project_url
it_behaves_like 'issue tracker service URL attribute', :issues_url
end
@@ -51,7 +52,7 @@
create(:youtrack_service, :without_properties_callback, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in separated fields' do
@@ -59,7 +60,7 @@
create(:youtrack_service, title: title, description: description, properties: access_params)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when data are stored in both properties and separated fields' do
@@ -68,7 +69,7 @@
create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties)
end
- include_examples 'issue tracker fields'
+ it_behaves_like 'issue tracker fields'
end
context 'when no title & description are set' do
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 0797b9a9d830863266ff011e7198f50bfd82a634..d96e139867791fa5fe555d40dce42e614a5a94ba 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -257,8 +257,8 @@
expect(service.title).to eq('random title')
end
- it 'creates the properties' do
- expect(service.properties).to eq({ "project_url" => "http://gitlab.example.com" })
+ it 'sets data correctly' do
+ expect(service.data_fields.project_url).to eq('http://gitlab.example.com')
end
end
diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb
index 76a70ab6e9e07787f1442936951a2a564c1208da..7153fcc99d7829a3ae361bda038d8c75230fb0c9 100644
--- a/spec/requests/api/services_spec.rb
+++ b/spec/requests/api/services_spec.rb
@@ -100,9 +100,15 @@
expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
end
- it "returns empty hash if properties are empty" do
+ it "returns empty hash if properties and data fields are empty" do
# deprecated services are not valid for update
initialized_service.update_attribute(:properties, {})
+
+ if initialized_service.data_fields_present?
+ initialized_service.data_fields.destroy
+ initialized_service.reload
+ end
+
get api("/projects/#{project.id}/services/#{dashed_service}", user)
expect(response).to have_gitlab_http_status(200)
diff --git a/spec/support/helpers/jira_service_helper.rb b/spec/support/helpers/jira_service_helper.rb
index 57c33c81ea34867476e7eac2f4e2341746146ba1..c23a8d52c842df449ed1d1a9dbbefbfc7132d679 100644
--- a/spec/support/helpers/jira_service_helper.rb
+++ b/spec/support/helpers/jira_service_helper.rb
@@ -5,16 +5,16 @@ module JiraServiceHelper
JIRA_API = JIRA_URL + "/rest/api/2"
def jira_service_settings
- properties = {
- title: "Jira tracker",
- url: JIRA_URL,
- username: 'jira-user',
- password: 'my-secret-password',
- project_key: "JIRA",
- jira_issue_transition_id: '1'
- }
+ title = "Jira tracker"
+ url = JIRA_URL
+ username = 'jira-user'
+ password = 'my-secret-password'
+ jira_issue_transition_id = '1'
- jira_tracker.update(properties: properties, active: true)
+ jira_tracker.update(
+ title: title, url: url, username: username, password: password,
+ jira_issue_transition_id: jira_issue_transition_id, active: true
+ )
end
def jira_issue_comments