Merge branch 'use-alert_id-instead-of-alert_payload-in-process-alert-worker' into 'master'

Pass only `alert_id` to `IncidentManagement::ProcessAlertWorker`

See merge request gitlab-org/gitlab!34948

app/services/projects/alerting/notify_service.rb

@@ -65,8 +65,7 @@ module Projects
       def process_incident_issues(alert)
         return if alert.issue
 
-        IncidentManagement::ProcessAlertWorker
-          .perform_async(project.id, parsed_payload, alert.id)
+        IncidentManagement::ProcessAlertWorker.perform_async(nil, nil, alert.id)
       end
 
       def send_alert_email

app/workers/incident_management/process_alert_worker.rb

@@ -7,39 +7,45 @@ module IncidentManagement
     queue_namespace :incident_management
     feature_category :incident_management
 
-    def perform(project_id, alert_payload, am_alert_id = nil)
-      project = find_project(project_id)
-      return unless project
+    # `project_id` and `alert_payload` are deprecated and can be removed
+    # starting from 14.0 release
+    # https://gitlab.com/gitlab-org/gitlab/-/issues/224500
+    def perform(_project_id = nil, _alert_payload = nil, alert_id = nil)
+      return unless alert_id
 
-      new_issue = create_issue(project, alert_payload)
-      return unless am_alert_id && new_issue&.persisted?
+      alert = find_alert(alert_id)
+      return unless alert
+
+      new_issue = create_issue_for(alert)
+      return unless new_issue&.persisted?
 
-      link_issue_with_alert(am_alert_id, new_issue.id)
+      link_issue_with_alert(alert, new_issue.id)
     end
 
     private
 
-    def find_project(project_id)
-      Project.find_by_id(project_id)
+    def find_alert(alert_id)
+      AlertManagement::Alert.find_by_id(alert_id)
+    end
+
+    def parsed_payload(alert)
+      Gitlab::Alerting::NotificationPayloadParser.call(alert.payload.to_h, alert.project)
     end
 
-    def create_issue(project, alert_payload)
+    def create_issue_for(alert)
       IncidentManagement::CreateIssueService
-        .new(project, alert_payload)
+        .new(alert.project, parsed_payload(alert))
         .execute
         .dig(:issue)
     end
 
-    def link_issue_with_alert(alert_id, issue_id)
-      alert = AlertManagement::Alert.find_by_id(alert_id)
-      return unless alert
-
+    def link_issue_with_alert(alert, issue_id)
       return if alert.update(issue_id: issue_id)
 
       Gitlab::AppLogger.warn(
         message: 'Cannot link an Issue with Alert',
         issue_id: issue_id,
-        alert_id: alert_id,
+        alert_id: alert.id,
         alert_errors: alert.errors.messages
       )
     end

spec/services/projects/alerting/notify_service_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe Projects::Alerting::NotifyService do
     it 'processes issues' do
       expect(IncidentManagement::ProcessAlertWorker)
         .to receive(:perform_async)
-        .with(project.id, kind_of(Hash), kind_of(Integer))
+        .with(nil, nil, kind_of(Integer))
         .once
 
       Sidekiq::Testing.inline! do

spec/workers/incident_management/process_alert_worker_spec.rb

@@ -7,35 +7,31 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
   let_it_be(:settings) { create(:project_incident_management_setting, project: project, create_issue: true) }
 
   describe '#perform' do
-    let(:alert_management_alert_id) { nil }
-    let(:alert_payload) do
-      {
-        'annotations' => { 'title' => 'title' },
-        'startsAt' => Time.now.rfc3339
-      }
-    end
-
-    let(:created_issue) { Issue.last }
+    let_it_be(:started_at) { Time.now.rfc3339 }
+    let_it_be(:payload) { { 'title' => 'title', 'start_time' => started_at } }
+    let_it_be(:parsed_payload) { Gitlab::Alerting::NotificationPayloadParser.call(payload, project) }
+    let_it_be(:alert) { create(:alert_management_alert, project: project, payload: payload, started_at: started_at) }
+    let(:created_issue) { Issue.last! }
 
-    subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) }
+    subject { described_class.new.perform(nil, nil, alert.id) }
 
     before do
       allow(IncidentManagement::CreateIssueService)
-        .to receive(:new).with(project, alert_payload)
+        .to receive(:new).with(alert.project, parsed_payload)
         .and_call_original
     end
 
     it 'creates an issue' do
       expect(IncidentManagement::CreateIssueService)
-        .to receive(:new).with(project, alert_payload)
+        .to receive(:new).with(alert.project, parsed_payload)
 
       expect { subject }.to change { Issue.count }.by(1)
     end
 
-    context 'with invalid project' do
-      let(:invalid_project_id) { non_existing_record_id }
+    context 'with invalid alert' do
+      let(:invalid_alert_id) { non_existing_record_id }
 
-      subject { described_class.new.perform(invalid_project_id, alert_payload) }
+      subject { described_class.new.perform(nil, nil, invalid_alert_id) }
 
       it 'does not create issues' do
         expect(IncidentManagement::CreateIssueService).not_to receive(:new)
@@ -44,16 +40,8 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
       end
     end
 
-    context 'when alert_management_alert_id is present' do
-      let!(:alert) { create(:alert_management_alert, project: project) }
-      let(:alert_management_alert_id) { alert.id }
-
+    context 'with valid alert' do
       before do
-        allow(AlertManagement::Alert)
-          .to receive(:find_by_id)
-          .with(alert_management_alert_id)
-          .and_return(alert)
-
         allow(Gitlab::AppLogger).to receive(:warn).and_call_original
       end
 
@@ -69,24 +57,24 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
 
           expect(Gitlab::AppLogger).not_to have_received(:warn)
         end
-      end
 
-      context 'when alert cannot be updated' do
-        let(:alert) { create(:alert_management_alert, :with_validation_errors, project: project) }
+        context 'when alert cannot be updated' do
+          let_it_be(:alert) { create(:alert_management_alert, :with_validation_errors, project: project, payload: payload) }
 
-        it 'updates AlertManagement::Alert#issue_id' do
-          expect { subject }.not_to change { alert.reload.issue_id }
-        end
+          it 'updates AlertManagement::Alert#issue_id' do
+            expect { subject }.not_to change { alert.reload.issue_id }
+          end
 
-        it 'logs a warning' do
-          subject
+          it 'logs a warning' do
+            subject
 
-          expect(Gitlab::AppLogger).to have_received(:warn).with(
-            message: 'Cannot link an Issue with Alert',
-            issue_id: created_issue.id,
-            alert_id: alert_management_alert_id,
-            alert_errors: { hosts: ['hosts array is over 255 chars'] }
-          )
+            expect(Gitlab::AppLogger).to have_received(:warn).with(
+              message: 'Cannot link an Issue with Alert',
+              issue_id: created_issue.id,
+              alert_id: alert.id,
+              alert_errors: { hosts: ['hosts array is over 255 chars'] }
+            )
+          end
         end
       end
     end