Merge branch '9552-alert-notification-emails-are-not-sent' into 'master'

Resolve "Alert notification emails are not sent"

See merge request gitlab-org/gitlab-ee!9393

ee/app/controllers/projects/prometheus/alerts_controller.rb

@@ -25,7 +25,8 @@ module Projects
 
       def notify
         token = extract_alert_manager_token(request)
-        notify = Projects::Prometheus::Alerts::NotifyService.new(project, current_user, params)
+        notify = Projects::Prometheus::Alerts::NotifyService
+          .new(project, current_user, params.permit!)
 
         if notify.execute(token)
           head :ok

ee/app/services/projects/prometheus/alerts/notify_service.rb

@@ -8,8 +8,7 @@ module Projects
           return false unless valid_version?
           return false unless valid_alert_manager_token?(token)
 
-          notification_service.async.prometheus_alerts_fired(project, firings) if firings.any?
-
+          send_alert_email(project, firings) if firings.any?
           persist_events(project, current_user, params)
 
           true
@@ -83,6 +82,12 @@ module Projects
           ActiveSupport::SecurityUtils.variable_size_secure_compare(expected, actual)
         end
 
+        def send_alert_email(projects, firing_alerts)
+          notification_service
+            .async
+            .prometheus_alerts_fired(project, firings)
+        end
+
         def persist_events(project, current_user, params)
           CreateEventsService.new(project, current_user, params).execute
         end

ee/changelogs/unreleased/9552-alert-notification-emails-are-not-sent.yml

+---
+title: Fix alert notification emails are not being sent
+merge_request: 9393
+author:
+type: fixed

ee/spec/controllers/projects/prometheus/alerts_controller_spec.rb

@@ -86,16 +86,18 @@ describe Projects::Prometheus::AlertsController do
 
   describe 'POST #notify' do
     let(:notify_service) { spy }
-    let(:payload) { {} }
 
     before do
-      allow(Projects::Prometheus::Alerts::NotifyService).to receive(:new).and_return(notify_service)
+      expect(Projects::Prometheus::Alerts::NotifyService)
+        .to receive(:new)
+        .and_return(notify_service)
+        .with(project, user, duck_type(:permitted?))
     end
 
-    it 'sends a notification for firing alerts only' do
+    it 'renders ok if notification succeeds' do
       expect(notify_service).to receive(:execute).and_return(true)
 
-      post :notify, params: project_params(payload), session: { as: :json }
+      post :notify, params: project_params, session: { as: :json }
 
       expect(response).to have_gitlab_http_status(200)
     end

ee/spec/services/projects/prometheus/alerts/notify_service_spec.rb

@@ -14,22 +14,24 @@ describe Projects::Prometheus::Alerts::NotifyService do
     let(:notification_service) { spy }
     let(:create_events_service) { spy }
 
-    before do
-      allow(NotificationService).to receive(:new).and_return(notification_service)
-      allow(Projects::Prometheus::Alerts::CreateEventsService)
-        .to receive(:new).and_return(create_events_service)
-    end
-
     it 'sends a notification for firing alerts only' do
+      expect(NotificationService)
+        .to receive(:new)
+        .and_return(notification_service)
+
       expect(notification_service)
         .to receive_message_chain(:async, :prometheus_alerts_fired)
-        .with(project, [payload_alert_firing])
 
       expect(subject).to eq(true)
     end
 
     it 'persists events' do
-      expect(create_events_service).to receive(:execute)
+      expect(Projects::Prometheus::Alerts::CreateEventsService)
+        .to receive(:new)
+        .and_return(create_events_service)
+
+      expect(create_events_service)
+        .to receive(:execute)
 
       expect(subject).to eq(true)
     end
@@ -50,8 +52,9 @@ describe Projects::Prometheus::Alerts::NotifyService do
   context 'with valid payload' do
     let(:alert_firing) { create(:prometheus_alert, project: project) }
     let(:alert_resolved) { create(:prometheus_alert, project: project) }
-    let(:payload) { payload_for(firing: [alert_firing], resolved: [alert_resolved]) }
-    let(:payload_alert_firing) { payload['alerts'].first }
+    let(:payload_raw) { payload_for(firing: [alert_firing], resolved: [alert_resolved]) }
+    let(:payload) { ActionController::Parameters.new(payload_raw).permit! }
+    let(:payload_alert_firing) { payload_raw['alerts'].first }
     let(:token) { 'token' }
 
     context 'with project specific cluster' do