Merge branch 'bw-fix-checking-disabled-services' into 'master'51761-create-new-project-auto-populate-project-slug-string-to-project-name-if-name-is-empty34758-split-create-gcp-controller

Check disabled_services when finding a service

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

2 files changed, 31 insertions, 11 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 3fa47c1cda3..0cdd876dc20 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1088,9 +1088,7 @@ class Project < ActiveRecord::Base
       find_or_initialize_service(service_name)
     end
 
-    available_services.reject do |service|
-      disabled_services.include?(service.to_param)
-    end
+    available_services.compact
   end
 
   def disabled_services
@@ -1098,6 +1096,8 @@ class Project < ActiveRecord::Base
   end
 
   def find_or_initialize_service(name)
+    return if disabled_services.include?(name)
+
     service = find_service(services, name)
     return service if service
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index ff259dc12b3..31c69e5bd2c 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4037,19 +4037,39 @@ describe Project do
         expect(result).to be_empty
       end
     end
+  end
 
-    describe "#find_or_initialize_service" do
-      subject { build(:project) }
+  describe "#find_or_initialize_services" do
+    subject { build(:project) }
 
-      it 'avoids N+1 database queries' do
-        allow(Service).to receive(:available_services_names).and_return(%w(prometheus pushover))
+    it 'returns only enabled services' do
+      allow(Service).to receive(:available_services_names).and_return(%w(prometheus pushover))
+      allow(subject).to receive(:disabled_services).and_return(%w(prometheus))
 
-        control_count = ActiveRecord::QueryRecorder.new { project.find_or_initialize_service('prometheus') }.count
+      services = subject.find_or_initialize_services
 
-        allow(Service).to receive(:available_services_names).and_call_original
+      expect(services.count).to eq 1
+      expect(services).to include(PushoverService)
+    end
+  end
 
-        expect { project.find_or_initialize_service('prometheus') }.not_to exceed_query_limit(control_count)
-      end
+  describe "#find_or_initialize_service" do
+    subject { build(:project) }
+
+    it 'avoids N+1 database queries' do
+      allow(Service).to receive(:available_services_names).and_return(%w(prometheus pushover))
+
+      control_count = ActiveRecord::QueryRecorder.new { subject.find_or_initialize_service('prometheus') }.count
+
+      allow(Service).to receive(:available_services_names).and_call_original
+
+      expect { subject.find_or_initialize_service('prometheus') }.not_to exceed_query_limit(control_count)
+    end
+
+    it 'returns nil if service is disabled' do
+      allow(subject).to receive(:disabled_services).and_return(%w(prometheus))
+
+      expect(subject.find_or_initialize_service('prometheus')).to be_nil
     end
   end