Merge branch 'pl-present-alerts-in-mail' into 'master'

Use alert presenter in notification mails

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

ee/app/mailers/ee/emails/projects.rb

@@ -20,19 +20,16 @@ module EE
              subject: subject('Mirror user changed'))
       end
 
-      def prometheus_alert_fired_email(project_id, user_id, alert_params)
-        alert_metric_id = alert_params.dig('labels', 'gitlab_alert_id')
-
+      def prometheus_alert_fired_email(project_id, user_id, alert_payload)
         @project = ::Project.find(project_id)
         user = ::User.find(user_id)
 
-        @alert = @project.prometheus_alerts.for_metric(alert_metric_id).first
-        return unless @alert
-
-        @environment = @alert.environment
-
-        subject_text = "Alert: #{@environment.name} - #{@alert.title} #{@alert.computed_operator} #{@alert.threshold} for 5 minutes"
+        @alert = ::Gitlab::Alerting::Alert
+          .new(project: @project, payload: alert_payload)
+          .present
+        return unless @alert.valid?
 
+        subject_text = "Alert: #{@alert.email_subject}"
         mail(to: user.notification_email, subject: subject(subject_text))
       end
     end

ee/app/presenters/projects/prometheus/alert_presenter.rb

+# frozen_string_literal: true
+
+module Projects
+  module Prometheus
+    class AlertPresenter < Gitlab::View::Presenter::Delegated
+      def email_subject
+        [environment_name, query_title].compact.join(' ')
+      end
+
+      def project_full_path
+        project.full_path
+      end
+
+      def metric_query
+        gitlab_alert&.full_query
+      end
+
+      def environment_name
+        environment&.name
+      end
+
+      def performance_dashboard_link
+        if environment
+          metrics_project_environment_url(project, environment)
+        else
+          metrics_project_environments_url(project)
+        end
+      end
+
+      private
+
+      def query_title
+        return unless gitlab_alert
+
+        "#{gitlab_alert.title} #{gitlab_alert.computed_operator} #{gitlab_alert.threshold} for 5 minutes"
+      end
+    end
+  end
+end

ee/app/views/notify/prometheus_alert_fired_email.html.haml

 %p
-  An alert has been triggered in #{@project.full_path}.
+  An alert has been triggered in #{@alert.project_full_path}.
 
-%p
-  Environment: #{@environment.name}
+- if env_name = @alert.environment_name
+  %p
+    Environment: #{env_name}
 
-%p
-  Metric:
+- if metric_query = @alert.metric_query
+  %p
+    Metric:
 
-  %pre
-    = @alert.full_query
+    %pre
+      = metric_query
 
 %p
-  = link_to("View #{@environment.name} performance dashboard.", metrics_project_environment_url(@environment.project, @environment))
+  = link_to('View performance dashboard.', @alert.performance_dashboard_link)

ee/app/views/notify/prometheus_alert_fired_email.text.erb

-An alert has been triggered in <%= @project.full_path %>.
+An alert has been triggered in <%= @alert.project_full_path %>.
 
-Environment: <%= @environment.name %>
+<% if env_name = @alert.environment_name %>
+Environment: <%= env_name %>
+<% end %>
 
-Metric: <%= @alert.full_query %>
+<% if metric_query = @alert.metric_query %>
+Metric: <%= metric_query %>
+<% end %>
 
-You can view the <%= @environment.name %> performance dashboard at <%= metrics_project_environment_url(@environment.project, @environment) %>.
+View the performance dashboard at <%= @alert.performance_dashboard_link %>

ee/lib/gitlab/alerting/alert.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Alerting
+    class Alert
+      include ActiveModel::Model
+      include Gitlab::Utils::StrongMemoize
+      include Presentable
+
+      attr_accessor :project, :payload
+
+      def gitlab_alert
+        strong_memoize(:gitlab_alert) do
+          parse_gitlab_alert_from_payload
+        end
+      end
+
+      def environment
+        gitlab_alert&.environment
+      end
+
+      def valid?
+        project && gitlab_alert
+      end
+
+      def present
+        super(presenter_class: Projects::Prometheus::AlertPresenter)
+      end
+
+      private
+
+      def parse_gitlab_alert_from_payload
+        metric_id = payload&.dig('labels', 'gitlab_alert_id')
+        return unless metric_id
+
+        project.prometheus_alerts.for_metric(metric_id).first
+      end
+    end
+  end
+end

ee/spec/factories/alerting/alert.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :alerting_alert, class: Gitlab::Alerting::Alert do
+    project
+    payload { {} }
+
+    transient do
+      metric_id nil
+
+      after(:build) do |alert, evaluator|
+        if metric_id = evaluator.metric_id
+          alert.payload['labels'] ||= {}
+          alert.payload['labels']['gitlab_alert_id'] = metric_id.to_s
+        end
+      end
+    end
+
+    skip_create
+  end
+end

ee/spec/lib/gitlab/alerting/alert_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Alerting::Alert do
+  set(:project) { create(:project) }
+
+  let(:alert) { build(:alerting_alert, project: project, payload: payload) }
+  let(:payload) { {} }
+
+  context 'with gitlab alert' do
+    let!(:gitlab_alert) { create(:prometheus_alert, project: project) }
+
+    before do
+      payload['labels'] = {
+        'gitlab_alert_id' => gitlab_alert_id
+      }
+    end
+
+    context 'with matching gitlab_alert_id' do
+      let(:gitlab_alert_id) { gitlab_alert.prometheus_metric_id.to_s }
+
+      it 'loads gitlab_alert' do
+        expect(alert.gitlab_alert).to eq(gitlab_alert)
+      end
+
+      it 'delegates environment to gitlab_alert' do
+        expect(alert.environment).to eq(gitlab_alert.environment)
+      end
+
+      it 'is valid' do
+        expect(alert).to be_valid
+      end
+    end
+
+    context 'with unknown gitlab_alert_id' do
+      let(:gitlab_alert_id) { 'unknown' }
+
+      it 'cannot load gitlab_alert' do
+        expect(alert.gitlab_alert).to be_nil
+      end
+
+      it 'is invalid' do
+        expect(alert).not_to be_valid
+      end
+    end
+  end
+
+  context 'with empty payload' do
+    it 'cannot load gitlab_alert' do
+      expect(alert.gitlab_alert).to be_nil
+    end
+  end
+end

ee/spec/mailers/ee/emails/projects_spec.rb

@@ -29,12 +29,16 @@ describe EE::Emails::Projects do
         }
       end
 
-      let(:environment) { alert.environment }
+      let(:title) do
+        "#{alert.title} #{alert.computed_operator} #{alert.threshold}"
+      end
 
       let(:metrics_url) do
         metrics_project_environment_url(project, environment)
       end
 
+      let(:environment) { alert.environment }
+
       let!(:alert) { create(:prometheus_alert, project: project) }
 
       it_behaves_like 'an email sent from GitLab'
@@ -42,19 +46,15 @@ describe EE::Emails::Projects do
       it_behaves_like 'a user cannot unsubscribe through footer link'
 
       it 'has expected subject' do
-        aggregate_failures do
-          is_expected.to have_subject(/Alert:/)
-          is_expected.to have_subject(/#{environment.name}/)
-
-          title = "#{alert.title} #{alert.computed_operator} #{alert.threshold}"
-          is_expected.to have_subject(/#{title}/)
-        end
+        is_expected.to have_subject(/Alert: #{environment.name} #{title}/)
       end
 
       it 'has expected content' do
         is_expected.to have_body_text('An alert has been triggered')
         is_expected.to have_body_text(project.full_path)
+        is_expected.to have_body_text('Environment:')
         is_expected.to have_body_text(environment.name)
+        is_expected.to have_body_text('Metric:')
         is_expected.to have_body_text(alert.full_query)
         is_expected.to have_body_text(metrics_url)
       end

ee/spec/presenters/projects/prometheus/alert_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Prometheus::AlertPresenter do
+  set(:project) { create(:project) }
+
+  let(:presenter) { described_class.new(alert) }
+  let(:alert) { create(:alerting_alert, project: project) }
+
+  describe '#project_full_path' do
+    subject { presenter.project_full_path }
+
+    it { is_expected.to eq(project.full_path) }
+  end
+
+  context 'with gitlab alert' do
+    let(:gitlab_alert) { create(:prometheus_alert, project: project) }
+    let(:metric_id) { gitlab_alert.prometheus_metric_id }
+
+    let(:alert) do
+      create(:alerting_alert, project: project, metric_id: metric_id)
+    end
+
+    describe '#email_subject' do
+      let(:query_title) do
+        "#{gitlab_alert.title} #{gitlab_alert.computed_operator} #{gitlab_alert.threshold} for 5 minutes"
+      end
+
+      let(:expected_subject) do
+        "#{alert.environment.name} #{query_title}"
+      end
+
+      subject { presenter.email_subject }
+
+      it { is_expected.to eq(expected_subject) }
+    end
+
+    describe '#metric_query' do
+      subject { presenter.metric_query }
+
+      it { is_expected.to eq(gitlab_alert.full_query) }
+    end
+
+    describe '#environment_name' do
+      subject { presenter.environment_name }
+
+      it { is_expected.to eq(alert.environment.name) }
+    end
+
+    describe '#performance_dashboard_link' do
+      before do
+        gitlab_alert.save!
+      end
+
+      let(:expected_link) do
+        Gitlab::Routing.url_helpers
+          .metrics_project_environment_url(project, alert.environment)
+      end
+
+      subject { presenter.performance_dashboard_link }
+
+      it { is_expected.to eq(expected_link) }
+    end
+  end
+
+  context 'without gitlab alert' do
+    describe '#email_subject' do
+      subject { presenter.email_subject }
+
+      it { is_expected.to eq('') }
+    end
+
+    describe '#metric_query' do
+      subject { presenter.metric_query }
+
+      it { is_expected.to be_nil }
+    end
+
+    describe '#environment_name' do
+      subject { presenter.environment_name }
+
+      it { is_expected.to be_nil }
+    end
+
+    describe '#performance_dashboard_link' do
+      let(:expected_link) do
+        Gitlab::Routing.url_helpers.metrics_project_environments_url(project)
+      end
+
+      subject { presenter.performance_dashboard_link }
+
+      it { is_expected.to eq(expected_link) }
+    end
+  end
+end