Return only available service templates

Before, the scenario of removing one service type implies a migration
script to delete the service from the database, otherwise, we will have
an ActiveRecord::SubclassNotFound error.

The new code doesn't assume that the previous available service type
records are removed from the database.

app/models/service.rb

@@ -46,7 +46,7 @@ class Service < ApplicationRecord
   scope :active, -> { where(active: true) }
   scope :without_defaults, -> { where(default: false) }
   scope :by_type, -> (type) { where(type: type) }
-  scope :templates, -> { where(template: true) }
+  scope :templates, -> { where(template: true, type: available_services_types) }
 
   scope :push_hooks, -> { where(push_events: true, active: true) }
   scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) }
@@ -263,16 +263,13 @@ class Service < ApplicationRecord
   # Find all service templates; if some of them do not exist, create them
   # within a transaction to perform the lowest possible SQL queries.
   def self.find_or_create_templates
-    if templates.size == available_services_types.size
-      templates
-    else
-      create_nonexistent_templates
-      templates.reset
-    end
+    create_nonexistent_templates
+    templates
   end
 
   private_class_method def self.create_nonexistent_templates
-    nonexistent_services = available_services_types - templates.map(&:type)
+    nonexistent_services = available_services_types - templates.pluck(:type)
+    return if nonexistent_services.empty?
 
     transaction do
       nonexistent_services.each do |service_type|

spec/controllers/admin/services_controller_spec.rb

@@ -13,7 +13,7 @@ describe Admin::ServicesController do
     it 'avoids N+1 queries' do
       query_count = ActiveRecord::QueryRecorder.new { get :index }.count
 
-      expect(query_count).to be <= 80
+      expect(query_count).to be <= 79
     end
 
     context 'with all existing templates' do

spec/models/service_spec.rb

@@ -157,6 +157,10 @@ describe Service do
         expect { Service.find_or_create_templates }.to change { Service.count }.from(0).to(Service.available_services_names.size)
       end
 
+      it 'returns the available service templates' do
+        expect(Service.find_or_create_templates.map(&:type)).to match_array(Service.available_services_types)
+      end
+
       context 'with all existing templates' do
         before do
           Service.insert_all(
@@ -167,6 +171,21 @@ describe Service do
         it 'does not create service templates' do
           expect { Service.find_or_create_templates }.to change { Service.count }.by(0)
         end
+
+        it 'returns the available service templates' do
+          expect(Service.find_or_create_templates.map(&:type)).to match_array(Service.available_services_types)
+        end
+
+        context 'with a previous existing service (Previous) and a new service (Asana)' do
+          before do
+            Service.insert(type: 'PreviousService', template: true)
+            Service.delete_by(type: 'AsanaService', template: true)
+          end
+
+          it 'returns the available service templates' do
+            expect(Service.find_or_create_templates.map(&:type)).to match_array(Service.available_services_types)
+          end
+        end
       end
 
       context 'with a few existing templates' do
@@ -177,6 +196,10 @@ describe Service do
         it 'creates the rest of the service templates' do
           expect { Service.find_or_create_templates }.to change { Service.count }.from(1).to(Service.available_services_names.size)
         end
+
+        it 'returns the available service templates' do
+          expect(Service.find_or_create_templates.map(&:type)).to match_array(Service.available_services_types)
+        end
       end
     end