Clean up uniqueness validations for service type

- Remove `on: :create` for project services, this can now always be
  validated after we've removed all duplicates.

- Unify the validation syntax for all service types.

app/models/service.rb

@@ -54,11 +54,11 @@ class Service < ApplicationRecord
   validates :project_id, presence: true, unless: -> { template? || instance? || group_id }
   validates :group_id, presence: true, unless: -> { template? || instance? || project_id }
   validates :project_id, :group_id, absence: true, if: -> { template? || instance? }
-  validates :type, uniqueness: { scope: :project_id }, unless: -> { template? || instance? || group_id }, on: :create
-  validates :type, uniqueness: { scope: :group_id }, unless: -> { template? || instance? || project_id }
   validates :type, presence: true
-  validates :template, uniqueness: { scope: :type }, if: -> { template? }
-  validates :instance, uniqueness: { scope: :type }, if: -> { instance? }
+  validates :type, uniqueness: { scope: :template }, if: :template?
+  validates :type, uniqueness: { scope: :instance }, if: :instance?
+  validates :type, uniqueness: { scope: :project_id }, if: :project_id?
+  validates :type, uniqueness: { scope: :group_id }, if: :group_id?
   validate :validate_is_instance_or_template
   validate :validate_belongs_to_project_or_group
 

changelogs/unreleased/207385-simplify-service-uniqueness-validations.yml

+---
+title: Clean up uniqueness validations for service type
+merge_request: 52565
+author:
+type: changed

spec/models/service_spec.rb

@@ -39,35 +39,29 @@ RSpec.describe Service do
       end
     end
 
-    context 'with an existing service template' do
-      before do
+    context 'with existing services' do
+      before_all do
         create(:service, :template)
+        create(:service, :instance)
+        create(:service, project: project)
+        create(:service, group: group, project: nil)
       end
 
-      it 'validates only one service template per type' do
+      it 'allows only one service template per type' do
         expect(build(:service, :template)).to be_invalid
       end
-    end
 
-    context 'with an existing instance service' do
-      before do
-        create(:service, :instance)
-      end
-
-      it 'validates only one service instance per type' do
+      it 'allows only one instance service per type' do
         expect(build(:service, :instance)).to be_invalid
       end
-    end
 
-    it 'validates uniqueness of type and project_id on create' do
-      expect(create(:service, project: project, type: 'Service')).to be_valid
-      expect(build(:service, project: project, type: 'Service').valid?(:create)).to eq(false)
-      expect(build(:service, project: project, type: 'Service').valid?(:update)).to eq(true)
-    end
+      it 'allows only one project service per type' do
+        expect(build(:service, project: project)).to be_invalid
+      end
 
-    it 'validates uniqueness of type and group_id' do
-      expect(create(:service, group_id: group.id, project_id: nil, type: 'Service')).to be_valid
-      expect(build(:service, group_id: group.id, project_id: nil, type: 'Service')).to be_invalid
+      it 'allows only one group service per type' do
+        expect(build(:service, group: group, project: nil)).to be_invalid
+      end
     end
   end