Merge branch 'sy-transition-presenter-to-parsed-payload' into 'master'

Swap AlertManagement presenter away from using Alerting::Alert

See merge request gitlab-org/gitlab!39390

app/graphql/types/alert_management/alert_type.rb

@@ -6,6 +6,8 @@ module Types
       graphql_name 'AlertManagementAlert'
       description "Describes an alert from the project's Alert Management"
 
+      present_using ::AlertManagement::AlertPresenter
+
       implements(Types::Notes::NoteableType)
 
       authorize :read_alert_management_alert
@@ -120,10 +122,6 @@ module Types
       def notes
         object.ordered_notes
       end
-
-      def runbook
-        object.parsed_payload.runbook
-      end
     end
   end
 end

app/models/alert_management/alert.rb

@@ -118,7 +118,7 @@ module AlertManagement
     end
 
     delegate :iid, to: :issue, prefix: true, allow_nil: true
-    delegate :metrics_dashboard_url, :details_url, :details, to: :present
+    delegate :details_url, to: :present
 
     scope :for_iid, -> (iid) { where(iid: iid) }
     scope :for_status, -> (status) { where(status: status) }
@@ -218,12 +218,6 @@ module AlertManagement
       end
     end
 
-    def present
-      return super(presenter_class: AlertManagement::PrometheusAlertPresenter) if prometheus?
-
-      super
-    end
-
     private
 
     def hook_data

app/presenters/alert_management/alert_presenter.rb

@@ -6,7 +6,10 @@ module AlertManagement
     include IncidentManagement::Settings
     include ActionView::Helpers::UrlHelper
 
-    MARKDOWN_LINE_BREAK = "  \n".freeze
+    MARKDOWN_LINE_BREAK = "  \n"
+    HORIZONTAL_LINE = "\n\n---\n\n"
+
+    delegate :metrics_dashboard_url, :runbook, to: :parsed_payload
 
     def initialize(alert, _attributes = {})
       super
@@ -16,28 +19,17 @@ module AlertManagement
     end
 
     def issue_description
-      horizontal_line = "\n\n---\n\n"
-
       [
         issue_summary_markdown,
         alert_markdown,
         incident_management_setting.issue_template_content
-      ].compact.join(horizontal_line)
+      ].compact.join(HORIZONTAL_LINE)
     end
 
     def start_time
       started_at&.strftime('%d %B %Y, %-l:%M%p (%Z)')
     end
 
-    def issue_summary_markdown
-      <<~MARKDOWN.chomp
-        #{metadata_list}
-        #{alert_details}#{metric_embed_for_alert}
-      MARKDOWN
-    end
-
-    def metrics_dashboard_url; end
-
     def details_url
       details_project_alert_management_url(project, alert.iid)
     end
@@ -49,15 +41,15 @@ module AlertManagement
     private
 
     attr_reader :alert, :project
+    delegate :alert_markdown, :full_query, to: :parsed_payload
 
-    def alerting_alert
-      strong_memoize(:alerting_alert) do
-        Gitlab::Alerting::Alert.new(project: project, payload: alert.payload).present
-      end
+    def issue_summary_markdown
+      <<~MARKDOWN.chomp
+        #{metadata_list}
+        #{alert_details}#{metric_embed_for_alert}
+      MARKDOWN
     end
 
-    def alert_markdown; end
-
     def metadata_list
       metadata = []
 
@@ -90,9 +82,9 @@ module AlertManagement
         .join(MARKDOWN_LINE_BREAK)
     end
 
-    def metric_embed_for_alert; end
-
-    def full_query; end
+    def metric_embed_for_alert
+      "\n[](#{metrics_dashboard_url})" if metrics_dashboard_url
+    end
 
     def list_item(key, value)
       "**#{key}:** #{value}".strip

app/presenters/alert_management/prometheus_alert_presenter.rb

-# frozen_string_literal: true
-
-module AlertManagement
-  class PrometheusAlertPresenter < AlertManagement::AlertPresenter
-    def metrics_dashboard_url
-      alerting_alert.metrics_dashboard_url
-    end
-
-    private
-
-    def alert_markdown
-      alerting_alert.alert_markdown
-    end
-
-    def metric_embed_for_alert
-      alerting_alert.metric_embed_for_alert
-    end
-
-    def full_query
-      alerting_alert.full_query
-    end
-  end
-end

spec/models/alert_management/alert_spec.rb

@@ -453,22 +453,4 @@ RSpec.describe AlertManagement::Alert do
       expect { subject }.to change { alert.events }.by(1)
     end
   end
-
-  describe '#present' do
-    context 'when alert is generic' do
-      let(:alert) { triggered_alert }
-
-      it 'uses generic alert presenter' do
-        expect(alert.present).to be_kind_of(AlertManagement::AlertPresenter)
-      end
-    end
-
-    context 'when alert is Prometheus specific' do
-      let(:alert) { build(:alert_management_alert, :prometheus, project: project) }
-
-      it 'uses Prometheus Alert presenter' do
-        expect(alert.present).to be_kind_of(AlertManagement::PrometheusAlertPresenter)
-      end
-    end
-  end
 end

spec/presenters/alert_management/alert_presenter_spec.rb

@@ -4,8 +4,7 @@ require 'spec_helper'
 
 RSpec.describe AlertManagement::AlertPresenter do
   let_it_be(:project) { create(:project) }
-
-  let_it_be(:generic_payload) do
+  let_it_be(:payload) do
     {
       'title' => 'Alert title',
       'start_time' => '2020-04-27T10:10:22.265949279Z',
@@ -20,42 +19,108 @@ RSpec.describe AlertManagement::AlertPresenter do
     }
   end
 
-  let_it_be(:alert) do
-    create(:alert_management_alert, :with_description, :with_host, :with_service, :with_monitoring_tool, project: project, payload: generic_payload)
-  end
-
+  let_it_be(:alert) { create(:alert_management_alert, project: project, payload: payload) }
   let(:alert_url) { "http://localhost/#{project.full_path}/-/alert_management/#{alert.iid}/details" }
 
   subject(:presenter) { described_class.new(alert) }
 
   describe '#issue_description' do
+    let_it_be(:alert) { create(:alert_management_alert, project: project, payload: {}) }
+
     let(:markdown_line_break) { '  ' }
 
-    it 'returns an alert issue description' do
-      expect(presenter.issue_description).to eq(
-        <<~MARKDOWN.chomp
-          **Start time:** #{presenter.start_time}#{markdown_line_break}
-          **Severity:** #{presenter.severity}#{markdown_line_break}
-          **Service:** #{alert.service}#{markdown_line_break}
-          **Monitoring tool:** #{alert.monitoring_tool}#{markdown_line_break}
-          **Hosts:** #{alert.hosts.join(' ')}#{markdown_line_break}
-          **Description:** #{alert.description}#{markdown_line_break}
-          **GitLab alert:** #{alert_url}
-
-          #### Alert Details
-
-          **title:** Alert title#{markdown_line_break}
-          **start_time:** 2020-04-27T10:10:22.265949279Z#{markdown_line_break}
-          **custom.alert.fields:** ["one", "two"]#{markdown_line_break}
-          **yet.another:** 73
-        MARKDOWN
-      )
+    subject { presenter.issue_description }
+
+    context 'with an empty payload' do
+      it do
+        is_expected.to eq(
+          <<~MARKDOWN.chomp
+            **Start time:** #{presenter.start_time}#{markdown_line_break}
+            **Severity:** #{presenter.severity}#{markdown_line_break}
+            **GitLab alert:** #{alert_url}
+
+          MARKDOWN
+        )
+      end
+    end
+
+    context 'with optional alert attributes' do
+      let_it_be(:alert) do
+        create(:alert_management_alert, :with_description, :with_host, :with_service, :with_monitoring_tool, project: project, payload: payload)
+      end
+
+      before do
+        allow(alert.parsed_payload).to receive(:full_query).and_return('metric > 1')
+      end
+
+      it do
+        is_expected.to eq(
+          <<~MARKDOWN.chomp
+            **Start time:** #{presenter.start_time}#{markdown_line_break}
+            **Severity:** #{presenter.severity}#{markdown_line_break}
+            **full_query:** `metric > 1`#{markdown_line_break}
+            **Service:** #{alert.service}#{markdown_line_break}
+            **Monitoring tool:** #{alert.monitoring_tool}#{markdown_line_break}
+            **Hosts:** #{alert.hosts.join(' ')}#{markdown_line_break}
+            **Description:** #{alert.description}#{markdown_line_break}
+            **GitLab alert:** #{alert_url}
+
+            #### Alert Details
+
+            **title:** Alert title#{markdown_line_break}
+            **start_time:** 2020-04-27T10:10:22.265949279Z#{markdown_line_break}
+            **custom.alert.fields:** ["one", "two"]#{markdown_line_break}
+            **yet.another:** 73
+          MARKDOWN
+        )
+      end
+    end
+
+    context 'with incident markdown' do
+      before do
+        allow(alert.parsed_payload).to receive(:alert_markdown).and_return('**`markdown example`**')
+      end
+
+      it do
+        is_expected.to eq(
+          <<~MARKDOWN.chomp
+            **Start time:** #{presenter.start_time}#{markdown_line_break}
+            **Severity:** #{presenter.severity}#{markdown_line_break}
+            **GitLab alert:** #{alert_url}
+
+
+            ---
+
+            **`markdown example`**
+          MARKDOWN
+        )
+      end
+    end
+
+    context 'with metrics_dashboard_url' do
+      before do
+        allow(alert.parsed_payload).to receive(:metrics_dashboard_url).and_return('https://gitlab.com/metrics')
+      end
+
+      it do
+        is_expected.to eq(
+          <<~MARKDOWN.chomp
+            **Start time:** #{presenter.start_time}#{markdown_line_break}
+            **Severity:** #{presenter.severity}#{markdown_line_break}
+            **GitLab alert:** #{alert_url}
+
+            [](https://gitlab.com/metrics)
+          MARKDOWN
+        )
+      end
     end
   end
 
-  describe '#metrics_dashboard_url' do
-    it 'is not defined' do
-      expect(presenter.metrics_dashboard_url).to be_nil
+  describe '#start_time' do
+    it 'formats the start time of the alert' do
+      alert.started_at = Time.utc(2019, 5, 5)
+
+      expect(presenter.start_time). to eq('05 May 2019, 12:00AM (UTC)')
     end
   end
 

spec/presenters/alert_management/prometheus_alert_presenter_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe AlertManagement::PrometheusAlertPresenter do
-  let_it_be(:project) { create(:project) }
-  let(:payload) do
-    {
-      'annotations' => {
-        'title' => 'Alert title',
-        'gitlab_incident_markdown' => '**`markdown example`**',
-        'custom annotation' => 'custom annotation value'
-      },
-      'startsAt' => '2020-04-27T10:10:22.265949279Z',
-      'generatorURL' => 'http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1'
-    }
-  end
-
-  let!(:alert) do
-    create(:alert_management_alert, :prometheus, project: project, payload: payload)
-  end
-
-  let(:alert_url) { "http://localhost/#{project.full_path}/-/alert_management/#{alert.iid}/details" }
-
-  subject(:presenter) { described_class.new(alert) }
-
-  describe '#issue_description' do
-    let(:markdown_line_break) { '  ' }
-
-    it 'returns an alert issue description' do
-      expect(presenter.issue_description).to eq(
-        <<~MARKDOWN.chomp
-          **Start time:** #{presenter.start_time}#{markdown_line_break}
-          **Severity:** #{presenter.severity}#{markdown_line_break}
-          **full_query:** `vector(1)`#{markdown_line_break}
-          **Monitoring tool:** Prometheus#{markdown_line_break}
-          **GitLab alert:** #{alert_url}
-
-          #### Alert Details
-
-          **annotations.custom annotation:** custom annotation value#{markdown_line_break}
-          **annotations.gitlab_incident_markdown:** **`markdown example`**#{markdown_line_break}
-          **annotations.title:** Alert title#{markdown_line_break}
-          **startsAt:** 2020-04-27T10:10:22.265949279Z#{markdown_line_break}
-          **generatorURL:** http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1
-
-          ---
-
-          **`markdown example`**
-        MARKDOWN
-      )
-    end
-  end
-
-  describe '#metrics_dashboard_url' do
-    subject { presenter.metrics_dashboard_url }
-
-    context 'for a non-prometheus alert' do
-      it { is_expected.to be_nil }
-    end
-
-    context 'for a self-managed prometheus alert' do
-      include_context 'self-managed prometheus alert attributes'
-
-      it { is_expected.to eq(dashboard_url_for_alert) }
-    end
-
-    context 'for a gitlab-managed prometheus alert' do
-      include_context 'gitlab-managed prometheus alert attributes'
-
-      it { is_expected.to eq(dashboard_url_for_alert) }
-    end
-  end
-end

spec/services/alert_management/create_alert_issue_service_spec.rb

@@ -66,7 +66,7 @@ RSpec.describe AlertManagement::CreateAlertIssueService do
       end
 
       it 'sets the issue description' do
-        expect(created_issue.description).to include(alert_presenter.issue_summary_markdown.strip)
+        expect(created_issue.description).to include(alert_presenter.send(:issue_summary_markdown).strip)
       end
     end