Merge branch '63082-specs-changes' into 'master'

Simplify factories for services

See merge request gitlab-org/gitlab-ce!30611

7 files changed, 21 insertions, 38 deletions

diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index a3b89b2543a..e571700fd02 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -250,7 +250,7 @@ class JiraService < IssueTrackerService
       end
 
       log_info("Successfully posted", client_url: client_url)
-      "SUCCESS: Successfully posted to http://jira.example.net."
+      "SUCCESS: Successfully posted to #{client_url}."
     end
   end
 
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 743ec322885..7d7738a30c8 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -306,34 +306,18 @@ FactoryBot.define do
   factory :redmine_project, parent: :project do
     has_external_issue_tracker true
 
-    after :create do |project|
-      project.create_redmine_service(
-        active: true,
-        properties: {
-          'project_url' => 'http://redmine/projects/project_name_in_redmine',
-          'issues_url' => 'http://redmine/projects/project_name_in_redmine/issues/:id',
-          'new_issue_url' => 'http://redmine/projects/project_name_in_redmine/issues/new'
-        }
-      )
-    end
+    redmine_service
   end
 
   factory :youtrack_project, parent: :project do
     has_external_issue_tracker true
 
-    after :create do |project|
-      project.create_youtrack_service(
-        active: true,
-        properties: {
-          'project_url' => 'http://youtrack/projects/project_guid_in_youtrack',
-          'issues_url' => 'http://youtrack/issues/:id'
-        }
-      )
-    end
+    youtrack_service
   end
 
   factory :jira_project, parent: :project do
     has_external_issue_tracker true
+
     jira_service
   end
 
diff --git a/spec/factories/services.rb b/spec/factories/services.rb
index cd1d2c33373..daf842e3075 100644
--- a/spec/factories/services.rb
+++ b/spec/factories/services.rb
@@ -79,14 +79,12 @@ FactoryBot.define do
   trait :issue_tracker do
     properties(
       project_url: 'http://issue-tracker.example.com',
-      issues_url: 'http://issue-tracker.example.com',
+      issues_url: 'http://issue-tracker.example.com/issues/:id',
       new_issue_url: 'http://issue-tracker.example.com'
     )
   end
 
-  factory :jira_cloud_service, class: JiraService do
-    project
-    active true
+  trait :jira_cloud_service do
     properties(
       url: 'https://mysite.atlassian.net',
       username: 'jira_user',
diff --git a/spec/factories/services_data.rb b/spec/factories/services_data.rb
index 387e130a743..5a3639895b6 100644
--- a/spec/factories/services_data.rb
+++ b/spec/factories/services_data.rb
@@ -1,18 +1,12 @@
 # frozen_string_literal: true
 
+# these factories should never be called directly, they are used when creating services
 FactoryBot.define do
   factory :jira_tracker_data do
     service
-    url 'http://jira.example.com'
-    api_url 'http://api-jira.example.com'
-    username 'jira_username'
-    password 'jira_password'
   end
 
   factory :issue_tracker_data do
     service
-    project_url 'http://issuetracker.example.com'
-    issues_url 'http://issues.example.com'
-    new_issue_url 'http://new-issue.example.com'
   end
 end
diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
index 7119c826bca..469692f7b5a 100644
--- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
@@ -32,7 +32,7 @@ describe Banzai::Pipeline::GfmPipeline do
         result = described_class.call(markdown, project: project)[:output]
         link = result.css('a').first
 
-        expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
+        expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
       end
 
       it 'parses cross-project references to regular issues' do
@@ -61,7 +61,7 @@ describe Banzai::Pipeline::GfmPipeline do
         result = described_class.call(markdown, project: project)[:output]
         link = result.css('a').first
 
-        expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
+        expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
       end
 
       it 'allows to use long external reference syntax for Redmine' do
@@ -70,7 +70,7 @@ describe Banzai::Pipeline::GfmPipeline do
         result = described_class.call(markdown, project: project)[:output]
         link = result.css('a').first
 
-        expect(link['href']).to eq 'http://redmine/projects/project_name_in_redmine/issues/12'
+        expect(link['href']).to eq 'http://issue-tracker.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 67d49a30825..90a534de202 100644
--- a/spec/lib/gitlab/usage_data_spec.rb
+++ b/spec/lib/gitlab/usage_data_spec.rb
@@ -8,7 +8,7 @@ describe Gitlab::UsageData do
     before do
       create(:jira_service, project: projects[0])
       create(:jira_service, project: projects[1])
-      create(:jira_cloud_service, project: projects[2])
+      create(:jira_service, :jira_cloud_service, project: projects[2])
       create(:prometheus_service, project: projects[1])
       create(:service, project: projects[0], type: 'SlackSlashCommandsService', active: true)
       create(:service, project: projects[1], type: 'SlackService', active: true)
diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb
index 3f79e332b90..91cb8760a04 100644
--- a/spec/requests/api/services_spec.rb
+++ b/spec/requests/api/services_spec.rb
@@ -85,9 +85,7 @@ describe API::Services do
       include_context service
 
       # inject some properties into the service
-      before do
-        initialize_service(service)
-      end
+      let!(:initialized_service) { initialize_service(service) }
 
       it 'returns authentication error when unauthenticated' do
         get api("/projects/#{project.id}/services/#{dashed_service}")
@@ -108,6 +106,15 @@ describe API::Services do
         expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
       end
 
+      it "returns empty hash if properties are empty" do
+        # deprecated services are not valid for update
+        initialized_service.update_attribute(:properties, {})
+        get api("/projects/#{project.id}/services/#{dashed_service}", user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['properties'].keys).to be_empty
+      end
+
       it "returns error when authenticated but not a project owner" do
         project.add_developer(user2)
         get api("/projects/#{project.id}/services/#{dashed_service}", user2)