Tweak validation to use conditions

- Test all statuses
- Add translation to validation message
- Adjust migration to hardcode status

app/models/alert_management/alert.rb

@@ -55,8 +55,12 @@ module AlertManagement
     validates :severity,        presence: true
     validates :status,          presence: true
     validates :started_at,      presence: true
-    validates :fingerprint,     uniqueness: { scope: :project }, allow_blank: true, unless: :resolved?
-    validate  :hosts_length
+    validates :fingerprint,     allow_blank: true, uniqueness: {
+      scope: :project,
+      conditions: -> { where.not(status: STATUSES[:resolved]) },
+      message: _('Cannot have multiple unresolved alerts')
+    }, unless: :resolved?
+    validate :hosts_length
 
     enum severity: {
       critical: 0,

db/migrate/20200706035141_adjust_unique_index_alert_management_alerts.rb

@@ -5,7 +5,7 @@ class AdjustUniqueIndexAlertManagementAlerts < ActiveRecord::Migration[6.0]
 
   DOWNTIME        = false
   INDEX_NAME      = 'index_alert_management_alerts_on_project_id_and_fingerprint'
-  RESOLVED_STATUS = AlertManagement::Alert::STATUSES[:resolved]
+  RESOLVED_STATUS = 2
 
   # rubocop:disable Migration/RemoveIndex
   # rubocop:disable Migration/AddIndex

locale/gitlab.pot

@@ -4137,6 +4137,9 @@ msgstr ""
 msgid "Cannot have multiple Jira imports running at the same time"
 msgstr ""
 
+msgid "Cannot have multiple unresolved alerts"
+msgstr ""
+
 msgid "Cannot import because issues are not available in this project."
 msgstr ""
 

spec/models/alert_management/alert_spec.rb

@@ -95,11 +95,35 @@ RSpec.describe AlertManagement::Alert do
 
           it { is_expected.not_to be_valid }
 
-          context 'resolved status' do
-            # We are only validating uniqueness for non-resolved alerts
-            let(:new_alert) { build(:alert_management_alert, :resolved, fingerprint: fingerprint, project: project) }
+          context 'various states' do
+            using RSpec::Parameterized::TableSyntax
 
-            it { is_expected.to be_valid }
+            # We are only validating uniqueness for non-resolved alerts
+            where(:existing_status, :new_status, :valid) do
+              :resolved      | :triggered    | true
+              :resolved      | :resolved     | true
+              :triggered     | :triggered    | false
+              :triggered     | :acknowledged | false
+              :triggered     | :ignored      | false
+              :triggered     | :resolved     | true
+              :acknowledged  | :triggered    | false
+              :acknowledged  | :acknowledged | false
+              :acknowledged  | :ignored      | false
+              :acknowledged  | :resolved     | true
+              :ignored       | :triggered    | false
+              :ignored       | :acknowledged | false
+              :ignored       | :ignored      | false
+              :ignored       | :resolved     | true
+            end
+
+            with_them do
+              let(:existing_alert) { create(:alert_management_alert, existing_status, fingerprint: fingerprint) }
+              let(:new_alert) { build(:alert_management_alert, new_status, fingerprint: fingerprint, project: project) }
+
+              it 'gives the correct validity' do
+                expect(new_alert.valid?).to eq(valid)
+              end
+            end
           end
         end