Shorten error messages for escalation rule count and fix edge case

If the number of rules provided exceeds 10, the error message will
no longer be collected for each rule.

Also circumvents some odd behavior around updating the rules which results
in unexpected numbers of rules being saved.

ee/app/models/incident_management/escalation_rule.rb

@@ -4,8 +4,6 @@ module IncidentManagement
   class EscalationRule < ApplicationRecord
     self.table_name = 'incident_management_escalation_rules'
 
-    MAX_RULE_PER_POLICY_COUNT = 10
-
     belongs_to :policy, class_name: 'EscalationPolicy', inverse_of: 'rules', foreign_key: 'policy_id'
     belongs_to :oncall_schedule, class_name: 'OncallSchedule', inverse_of: 'rotations', foreign_key: 'oncall_schedule_id'
 
@@ -18,16 +16,5 @@ module IncidentManagement
               numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 24.hours }
 
     validates :policy_id, uniqueness: { scope: [:oncall_schedule_id, :status, :elapsed_time_seconds], message: _('must have a unique schedule, status, and elapsed time') }
-
-    validate :rules_count_not_exceeded, on: :create, if: :policy
-
-    private
-
-    def rules_count_not_exceeded
-      # We need to add to the count if we aren't creating the rules at the same time as the policy.
-      rules_count = policy.new_record? ? policy.rules.size : policy.rules.size + 1
-
-      errors.add(:base, "cannot have more than #{MAX_RULE_PER_POLICY_COUNT} rules") if rules_count > MAX_RULE_PER_POLICY_COUNT
-    end
   end
 end

ee/app/services/incident_management/escalation_policies/base_service.rb

@@ -3,10 +3,16 @@
 module IncidentManagement
   module EscalationPolicies
     class BaseService
+      MAX_RULE_COUNT = 10
+
       def allowed?
         user&.can?(:admin_incident_management_escalation_policy, project)
       end
 
+      def too_many_rules?
+        params[:rules_attributes] && params[:rules_attributes].size > MAX_RULE_COUNT
+      end
+
       def invalid_schedules?
         params[:rules_attributes]&.any? { |attrs| attrs[:oncall_schedule]&.project != project }
       end
@@ -27,6 +33,10 @@ module IncidentManagement
         error(_('Escalation policies must have at least one rule'))
       end
 
+      def error_too_many_rules
+        error(_('Escalation policies may not have more than %{rule_count} rules') % { rule_count: MAX_RULE_COUNT })
+      end
+
       def error_bad_schedules
         error(_('All escalations rules must have a schedule in the same project as the policy'))
       end

ee/app/services/incident_management/escalation_policies/create_service.rb

@@ -21,6 +21,7 @@ module IncidentManagement
       def execute
         return error_no_permissions unless allowed?
         return error_no_rules if params[:rules_attributes].blank?
+        return error_too_many_rules if too_many_rules?
         return error_bad_schedules if invalid_schedules?
 
         escalation_policy = project.incident_management_escalation_policies.create(params)

ee/app/services/incident_management/escalation_policies/update_service.rb

@@ -26,6 +26,7 @@ module IncidentManagement
       def execute
         return error_no_permissions unless allowed?
         return error_no_rules if empty_rules?
+        return error_too_many_rules if too_many_rules?
         return error_bad_schedules if invalid_schedules?
 
         reconcile_rules!

ee/spec/models/incident_management/escalation_rule_spec.rb

@@ -20,13 +20,5 @@ RSpec.describe IncidentManagement::EscalationRule do
     it { is_expected.to validate_presence_of(:elapsed_time_seconds) }
     it { is_expected.to validate_numericality_of(:elapsed_time_seconds).is_greater_than_or_equal_to(0).is_less_than_or_equal_to(24.hours) }
     it { is_expected.to validate_uniqueness_of(:policy_id).scoped_to([:oncall_schedule_id, :status, :elapsed_time_seconds] ).with_message('must have a unique schedule, status, and elapsed time') }
-
-    it 'validates the number of rules' do
-      policy = create(:incident_management_escalation_policy, rule_count: 10)
-      rule = build(:incident_management_escalation_rule, policy: policy)
-
-      expect(rule).not_to be_valid
-      expect(rule.errors).to contain_exactly("cannot have more than #{described_class::MAX_RULE_PER_POLICY_COUNT} rules")
-    end
   end
 end

ee/spec/services/incident_management/escalation_policies/create_service_spec.rb

@@ -71,6 +71,20 @@ RSpec.describe IncidentManagement::EscalationPolicies::CreateService do
         it_behaves_like 'error response', 'Escalation policies must have at least one rule'
       end
 
+      context 'too many rules are given' do
+        let(:rule_params) do
+          (0..10).map do |idx|
+            {
+              oncall_schedule: oncall_schedule,
+              elapsed_time_seconds: idx,
+              status: :acknowledged
+            }
+          end
+        end
+
+        it_behaves_like 'error response', 'Escalation policies may not have more than 10 rules'
+      end
+
       context 'oncall schedule is blank' do
         before do
           rule_params[0][:oncall_schedule] = nil

ee/spec/services/incident_management/escalation_policies/update_service_spec.rb

@@ -148,6 +148,21 @@ RSpec.describe IncidentManagement::EscalationPolicies::UpdateService do
       it_behaves_like 'error response', 'Escalation policies must have at least one rule'
     end
 
+    context 'when too many rules are given' do
+      let(:rule_params) { [*existing_rules_params, *new_rule_params] }
+      let(:new_rule_params) do
+        (0..9).map do |idx|
+          {
+            oncall_schedule: oncall_schedule,
+            elapsed_time_seconds: idx,
+            status: :acknowledged
+          }
+        end
+      end
+
+      it_behaves_like 'error response', 'Escalation policies may not have more than 10 rules'
+    end
+
     context 'when the on-call schedule is not present on the rule' do
       let(:rule_params) { [new_rule_params.except(:oncall_schedule)] }
 

locale/gitlab.pot

@@ -12970,6 +12970,9 @@ msgstr ""
 msgid "Escalation policies"
 msgstr ""
 
+msgid "Escalation policies may not have more than %{rule_count} rules"
+msgstr ""
+
 msgid "Escalation policies must have at least one rule"
 msgstr ""