Merge branch '217686-system-note-for-alert-closing-issue' into 'master'

Create system note for alert when issue closing

Closes #217686

See merge request gitlab-org/gitlab!37039

app/models/issue.rb

@@ -152,10 +152,6 @@ class Issue < ApplicationRecord
       issue.closed_at = nil
       issue.closed_by = nil
     end
-
-    after_transition any => :closed do |issue|
-      issue.resolve_associated_alert_management_alert
-    end
   end
 
   # Alias to state machine .with_state_id method
@@ -369,18 +365,6 @@ class Issue < ApplicationRecord
     @design_collection ||= ::DesignManagement::DesignCollection.new(self)
   end
 
-  def resolve_associated_alert_management_alert
-    return unless alert_management_alert
-    return if alert_management_alert.resolve
-
-    Gitlab::AppLogger.warn(
-      message: 'Cannot resolve an associated Alert Management alert',
-      issue_id: id,
-      alert_id: alert_management_alert.id,
-      alert_errors: alert_management_alert.errors.messages
-    )
-  end
-
   def from_service_desk?
     author.id == User.support_bot.id
   end

app/services/issues/close_service.rb

@@ -33,6 +33,7 @@ module Issues
 
         notification_service.async.close_issue(issue, current_user, closed_via: closed_via) if notifications
         todo_service.close_issue(issue, current_user)
+        resolve_alert(issue)
         execute_hooks(issue, 'close')
         invalidate_cache_counts(issue, users: issue.assignees)
         issue.update_project_counter_caches
@@ -58,6 +59,22 @@ module Issues
       SystemNoteService.change_status(issue, issue.project, current_user, issue.state, current_commit)
     end
 
+    def resolve_alert(issue)
+      return unless alert = issue.alert_management_alert
+      return if alert.resolved?
+
+      if alert.resolve
+        SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: current_user).closed_alert_issue(issue)
+      else
+        Gitlab::AppLogger.warn(
+          message: 'Cannot resolve an associated Alert Management alert',
+          issue_id: issue.id,
+          alert_id: alert.id,
+          alert_errors: alert.errors.messages
+        )
+      end
+    end
+
     def store_first_mentioned_in_commit_at(issue, merge_request)
       metrics = issue.metrics
       return if metrics.nil? || metrics.first_mentioned_in_commit_at

app/services/system_note_service.rb

@@ -297,7 +297,7 @@ module SystemNoteService
   end
 
   def new_alert_issue(alert, issue, author)
-    ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: author).new_alert_issue(alert, issue)
+    ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: author).new_alert_issue(issue)
   end
 
   private

app/services/system_notes/alert_management_service.rb

@@ -12,7 +12,7 @@ module SystemNotes
     #
     # Returns the created Note object
     def change_alert_status(alert)
-      status = AlertManagement::Alert::STATUSES.key(alert.status).to_s.titleize
+      status = alert.state.to_s.titleize
       body = "changed the status to **#{status}**"
 
       create_note(NoteSummary.new(noteable, project, author, body, action: 'status'))
@@ -20,7 +20,6 @@ module SystemNotes
 
     # Called when an issue is created based on an AlertManagement::Alert
     #
-    # alert - AlertManagement::Alert object.
     # issue - Issue object.
     #
     # Example Note text:
@@ -28,10 +27,25 @@ module SystemNotes
     #   "created issue #17 for this alert"
     #
     # Returns the created Note object
-    def new_alert_issue(alert, issue)
+    def new_alert_issue(issue)
       body = "created issue #{issue.to_reference(project)} for this alert"
 
       create_note(NoteSummary.new(noteable, project, author, body, action: 'alert_issue_added'))
     end
+
+    # Called when an AlertManagement::Alert is resolved due to the associated issue being closed
+    #
+    # issue - Issue object.
+    #
+    # Example Note text:
+    #
+    #   "changed the status to Resolved by closing issue #17"
+    #
+    # Returns the created Note object
+    def closed_alert_issue(issue)
+      body = "changed the status to **Resolved** by closing issue #{issue.to_reference(project)}"
+
+      create_note(NoteSummary.new(noteable, project, author, body, action: 'status'))
+    end
   end
 end

changelogs/unreleased/217686-system-note-for-alert-closing-issue.yml

+---
+title: Add system note to alert when corresponding issue is closed
+merge_request: 37039
+author:
+type: added

spec/models/issue_spec.rb

@@ -5,6 +5,8 @@ require 'spec_helper'
 RSpec.describe Issue do
   include ExternalAuthorizationServiceHelpers
 
+  let_it_be(:user) { create(:user) }
+
   describe "Associations" do
     it { is_expected.to belong_to(:milestone) }
     it { is_expected.to belong_to(:iteration) }
@@ -166,39 +168,9 @@ RSpec.describe Issue do
 
       expect { issue.close }.to change { issue.state_id }.from(open_state).to(closed_state)
     end
-
-    context 'when there is an associated Alert Management Alert' do
-      context 'when alert can be resolved' do
-        let!(:alert) { create(:alert_management_alert, project: issue.project, issue: issue) }
-
-        it 'resolves an alert' do
-          expect { issue.close }.to change { alert.reload.resolved? }.to(true)
-        end
-      end
-
-      context 'when alert cannot be resolved' do
-        let!(:alert) { create(:alert_management_alert, :with_validation_errors, project: issue.project, issue: issue) }
-
-        before do
-          allow(Gitlab::AppLogger).to receive(:warn).and_call_original
-        end
-
-        it 'writes a warning into the log' do
-          issue.close
-
-          expect(Gitlab::AppLogger).to have_received(:warn).with(
-            message: 'Cannot resolve an associated Alert Management alert',
-            issue_id: issue.id,
-            alert_id: alert.id,
-            alert_errors: { hosts: ['hosts array is over 255 chars'] }
-          )
-        end
-      end
-    end
   end
 
   describe '#reopen' do
-    let(:user) { create(:user) }
     let(:issue) { create(:issue, state: 'closed', closed_at: Time.current, closed_by: user) }
 
     it 'sets closed_at to nil when an issue is reopend' do
@@ -282,7 +254,6 @@ RSpec.describe Issue do
   end
 
   describe '#assignee_or_author?' do
-    let(:user) { create(:user) }
     let(:issue) { create(:issue) }
 
     it 'returns true for a user that is assigned to an issue' do
@@ -303,7 +274,6 @@ RSpec.describe Issue do
   end
 
   describe '#can_move?' do
-    let(:user) { create(:user) }
     let(:issue) { create(:issue) }
 
     subject { issue.can_move?(user) }

spec/services/issues/close_service_spec.rb

@@ -252,6 +252,41 @@ RSpec.describe Issues::CloseService do
         expect(todo.reload).to be_done
       end
 
+      context 'when there is an associated Alert Management Alert' do
+        context 'when alert can be resolved' do
+          let!(:alert) { create(:alert_management_alert, issue: issue, project: project) }
+
+          it 'resolves an alert and sends a system note' do
+            expect_next_instance_of(SystemNotes::AlertManagementService) do |notes_service|
+              expect(notes_service).to receive(:closed_alert_issue).with(issue)
+            end
+
+            close_issue
+
+            expect(alert.reload.resolved?).to eq(true)
+          end
+        end
+
+        context 'when alert cannot be resolved' do
+          let!(:alert) { create(:alert_management_alert, :with_validation_errors, issue: issue, project: project) }
+
+          before do
+            allow(Gitlab::AppLogger).to receive(:warn).and_call_original
+          end
+
+          it 'writes a warning into the log' do
+            close_issue
+
+            expect(Gitlab::AppLogger).to have_received(:warn).with(
+              message: 'Cannot resolve an associated Alert Management alert',
+              issue_id: issue.id,
+              alert_id: alert.id,
+              alert_errors: { hosts: ['hosts array is over 255 chars'] }
+            )
+          end
+        end
+      end
+
       it 'deletes milestone issue counters cache' do
         issue.update(milestone: create(:milestone, project: project))
 

spec/services/system_note_service_spec.rb

@@ -699,7 +699,7 @@ RSpec.describe SystemNoteService do
 
     it 'calls AlertManagementService' do
       expect_next_instance_of(SystemNotes::AlertManagementService) do |service|
-        expect(service).to receive(:new_alert_issue).with(alert, alert.issue)
+        expect(service).to receive(:new_alert_issue).with(alert.issue)
       end
 
       described_class.new_alert_issue(alert, alert.issue, author)

spec/services/system_notes/alert_management_service_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe ::SystemNotes::AlertManagementService do
   describe '#new_alert_issue' do
     let_it_be(:issue) { noteable.issue }
 
-    subject { described_class.new(noteable: noteable, project: project, author: author).new_alert_issue(noteable, issue) }
+    subject { described_class.new(noteable: noteable, project: project, author: author).new_alert_issue(issue) }
 
     it_behaves_like 'a system note' do
       let(:action) { 'alert_issue_added' }
@@ -32,4 +32,18 @@ RSpec.describe ::SystemNotes::AlertManagementService do
       expect(subject.note).to eq("created issue #{issue.to_reference(project)} for this alert")
     end
   end
+
+  describe '#closed_alert_issue' do
+    let_it_be(:issue) { noteable.issue }
+
+    subject { described_class.new(noteable: noteable, project: project, author: author).closed_alert_issue(issue) }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'status' }
+    end
+
+    it 'has the appropriate message' do
+      expect(subject.note).to eq("changed the status to **Resolved** by closing issue #{issue.to_reference(project)}")
+    end
+  end
 end