Merge branch 'remove-invalid-services' into 'master'

Remove invalid services with nil or empty type

See merge request !10847

7 files changed, 51 insertions, 24 deletions

diff --git a/app/models/service.rb b/app/models/service.rb
index dc76bf925d3..cbb75186206 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -26,6 +26,7 @@ class Service < ActiveRecord::Base
   has_one :service_hook
 
   validates :project_id, presence: true, unless: proc { |service| service.template? }
+  validates :type, presence: true
 
   scope :visible, -> { where.not(type: 'GitlabIssueTrackerService') }
   scope :issue_trackers, -> { where(category: 'issue_tracker') }
diff --git a/db/migrate/20170421102337_remove_nil_type_services.rb b/db/migrate/20170421102337_remove_nil_type_services.rb
new file mode 100644
index 00000000000..b835b9c6ed9
--- /dev/null
+++ b/db/migrate/20170421102337_remove_nil_type_services.rb
@@ -0,0 +1,12 @@
+class RemoveNilTypeServices < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def up
+    execute <<-SQL
+      DELETE FROM services WHERE type IS NULL OR type = '';
+    SQL
+  end
+
+  def down
+  end
+end
diff --git a/spec/factories/services.rb b/spec/factories/services.rb
index 88f6c265505..62aa71ae8d8 100644
--- a/spec/factories/services.rb
+++ b/spec/factories/services.rb
@@ -1,6 +1,19 @@
 FactoryGirl.define do
   factory :service do
     project factory: :empty_project
+    type 'Service'
+  end
+
+  factory :custom_issue_tracker_service, class: CustomIssueTrackerService do
+    project factory: :empty_project
+    type 'CustomIssueTrackerService'
+    category 'issue_tracker'
+    active true
+    properties(
+      project_url: 'https://project.url.com',
+      issues_url: 'https://issues.url.com',
+      new_issue_url: 'https://newissue.url.com'
+    )
   end
 
   factory :kubernetes_service do
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 7a0b0b06d4b..bfecfa28ed1 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -6981,28 +6981,6 @@
   ],
   "services": [
     {
-      "id": 164,
-      "title": null,
-      "project_id": 5,
-      "created_at": "2016-06-14T15:02:07.372Z",
-      "updated_at": "2016-06-14T15:02:07.372Z",
-      "active": false,
-      "properties": {
-
-      },
-      "template": false,
-      "push_events": true,
-      "issues_events": true,
-      "merge_requests_events": true,
-      "tag_push_events": true,
-      "note_events": true,
-      "build_events": true,
-      "category": "issue_tracker",
-      "type": "CustomIssueTrackerService",
-      "default": true,
-      "wiki_page_events": true
-    },
-    {
       "id": 100,
       "title": "JetBrains TeamCity CI",
       "project_id": 5,
@@ -7019,6 +6997,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "TeamcityService",
       "category": "ci",
       "default": false,
       "wiki_page_events": true
@@ -7040,6 +7019,7 @@
       "tag_push_events": true,
       "note_events": true,
       "pipeline_events": true,
+      "type": "SlackService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7061,6 +7041,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "RedmineService",
       "category": "issue_tracker",
       "default": false,
       "wiki_page_events": true
@@ -7082,6 +7063,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "PushoverService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7103,6 +7085,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "PivotalTrackerService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7125,6 +7108,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "JiraService",
       "category": "issue_tracker",
       "default": false,
       "wiki_page_events": true
@@ -7146,6 +7130,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "IrkerService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7167,6 +7152,7 @@
       "tag_push_events": true,
       "note_events": true,
       "pipeline_events": true,
+      "type": "HipchatService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7188,6 +7174,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "GemnasiumService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7209,6 +7196,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "FlowdockService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7230,6 +7218,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "ExternalWikiService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7251,6 +7240,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "EmailsOnPushService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7272,6 +7262,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "DroneCiService",
       "category": "ci",
       "default": false,
       "wiki_page_events": true
@@ -7293,6 +7284,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "CustomIssueTrackerService",
       "category": "issue_tracker",
       "default": false,
       "wiki_page_events": true
@@ -7314,6 +7306,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "CampfireService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7335,6 +7328,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "BuildkiteService",
       "category": "ci",
       "default": false,
       "wiki_page_events": true
@@ -7356,6 +7350,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "BambooService",
       "category": "ci",
       "default": false,
       "wiki_page_events": true
@@ -7377,6 +7372,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "AssemblaService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
@@ -7398,6 +7394,7 @@
       "tag_push_events": true,
       "note_events": true,
       "build_events": true,
+      "type": "AssemblaService",
       "category": "common",
       "default": false,
       "wiki_page_events": true
diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb
index fcc23a75ca1..06cd8ab87ed 100644
--- a/spec/lib/gitlab/import_export/relation_factory_spec.rb
+++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb
@@ -60,7 +60,7 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do
     end
 
     context 'original service exists' do
-      let(:service_id) { Service.create(project: project).id }
+      let(:service_id) { create(:service, project: project).id }
 
       it 'does not have the original service_id' do
         expect(created_object.service_id).not_to eq(service_id)
diff --git a/spec/models/project_services/issue_tracker_service_spec.rb b/spec/models/project_services/issue_tracker_service_spec.rb
index fbe6f344a98..869b25b933b 100644
--- a/spec/models/project_services/issue_tracker_service_spec.rb
+++ b/spec/models/project_services/issue_tracker_service_spec.rb
@@ -8,7 +8,7 @@ describe IssueTrackerService, models: true do
       let(:service) { RedmineService.new(project: project, active: true) }
 
       before do
-        create(:service, project: project, active: true, category: 'issue_tracker')  
+        create(:custom_issue_tracker_service, project: project)
       end
 
       context 'when service is changed manually by user' do
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 0e2f07e945f..4d0181ca297 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -6,6 +6,10 @@ describe Service, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    it { is_expected.to validate_presence_of(:type) }
+  end
+
   describe "Test Button" do
     before do
       @service = Service.new