Merge branch 'sy-cleanup-incident-management-system-notes' into 'master'

Refactor system notes for alerts and incidents for consistentcy

See merge request gitlab-org/gitlab!78243

app/models/state_note.rb

@@ -18,11 +18,11 @@ class StateNote < SyntheticNote
   def note_text(html: false)
     if event.state == 'closed'
       if event.close_after_error_tracking_resolve
-        return 'resolved the corresponding error and closed the issue.'
+        return 'resolved the corresponding error and closed the issue'
       end
 
       if event.close_auto_resolve_prometheus_alert
-        return 'automatically closed this issue because the alert resolved.'
+        return 'automatically closed this incident because the alert resolved'
       end
     end
 

app/services/issues/close_service.rb

@@ -81,7 +81,7 @@ module Issues
       return if alert.resolved?
 
       if alert.resolve
-        SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: current_user).closed_alert_issue(issue)
+        SystemNoteService.change_alert_status(alert, current_user, " by closing incident #{issue.to_reference(project)}")
       else
         Gitlab::AppLogger.warn(
           message: 'Cannot resolve an associated Alert Management alert',
@@ -97,7 +97,7 @@ module Issues
 
       status = issue.incident_management_issuable_escalation_status || issue.build_incident_management_issuable_escalation_status
 
-      SystemNoteService.resolve_incident_status(issue, current_user) if status.resolve
+      SystemNoteService.change_incident_status(issue, current_user, ' by closing the incident') if status.resolve
     end
 
     def store_first_mentioned_in_commit_at(issue, merge_request, max_commit_lookup: 100)

app/services/system_note_service.rb

@@ -335,10 +335,6 @@ module SystemNoteService
     ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_severity
   end
 
-  def resolve_incident_status(incident, author)
-    ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).resolve_incident_status
-  end
-
   def change_incident_status(incident, author, reason = nil)
     ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_status(reason)
   end

app/services/system_notes/alert_management_service.rb

@@ -40,30 +40,15 @@ module SystemNotes
     #
     # Example Note text:
     #
-    #   "created issue #17 for this alert"
+    #   "created incident #17 for this alert"
     #
     # Returns the created Note object
     def new_alert_issue(issue)
-      body = "created issue #{issue.to_reference(project)} for this alert"
+      body = "created incident #{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
-
     # Called when an alert is resolved due to received resolving alert payload
     #
     # alert - AlertManagement::Alert object.

app/services/system_notes/incident_service.rb

@@ -26,12 +26,6 @@ module SystemNotes
       end
     end
 
-    def resolve_incident_status
-      body = 'changed the status to **Resolved** by closing the incident'
-
-      create_note(NoteSummary.new(noteable, project, author, body, action: 'status'))
-    end
-
     # Called when the status of an IncidentManagement::IssuableEscalationStatus has changed
     #
     # reason - String.

spec/models/state_note_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe StateNote do
         it 'contains the expected values' do
           expect(subject.author).to eq(author)
           expect(subject.created_at).to eq(event.created_at)
-          expect(subject.note).to eq('resolved the corresponding error and closed the issue.')
+          expect(subject.note).to eq('resolved the corresponding error and closed the issue')
         end
       end
 
@@ -65,7 +65,7 @@ RSpec.describe StateNote do
         it 'contains the expected values' do
           expect(subject.author).to eq(author)
           expect(subject.created_at).to eq(event.created_at)
-          expect(subject.note).to eq('automatically closed this issue because the alert resolved.')
+          expect(subject.note).to eq('automatically closed this incident because the alert resolved')
         end
       end
     end

spec/services/issues/close_service_spec.rb

@@ -118,7 +118,7 @@ RSpec.describe Issues::CloseService do
           expect { service.execute(issue) }.to change { issue.notes.count }.by(1)
 
           new_note = issue.notes.last
-          expect(new_note.note).to eq('changed the status to **Resolved** by closing the incident')
+          expect(new_note.note).to eq('changed the incident status to **Resolved** by closing the incident')
           expect(new_note.author).to eq(user)
         end
 
@@ -334,8 +334,12 @@ RSpec.describe Issues::CloseService 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)
+            expect_any_instance_of(SystemNoteService) do |notes_service|
+              expect(notes_service).to receive(:change_alert_status).with(
+                alert,
+                current_user,
+                " by closing issue #{issue.to_reference(project)}"
+              )
             end
 
             close_issue

spec/services/system_note_service_spec.rb

@@ -632,18 +632,6 @@ RSpec.describe SystemNoteService do
     end
   end
 
-  describe '.resolve_incident_status' do
-    let(:incident) { build(:incident, :closed) }
-
-    it 'calls IncidentService' do
-      expect_next_instance_of(SystemNotes::IncidentService) do |service|
-        expect(service).to receive(:resolve_incident_status)
-      end
-
-      described_class.resolve_incident_status(incident, author)
-    end
-  end
-
   describe '.change_incident_status' do
     let(:incident) { instance_double('Issue', project: project) }
 

spec/services/system_notes/alert_management_service_spec.rb

@@ -54,21 +54,7 @@ RSpec.describe ::SystemNotes::AlertManagementService do
     end
 
     it 'has the appropriate message' 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)}")
+      expect(subject.note).to eq("created incident #{issue.to_reference(project)} for this alert")
     end
   end
 

spec/services/system_notes/incident_service_spec.rb

@@ -57,16 +57,6 @@ RSpec.describe ::SystemNotes::IncidentService do
     end
   end
 
-  describe '#resolve_incident_status' do
-    subject(:resolve_incident_status) { described_class.new(noteable: noteable, project: project, author: author).resolve_incident_status }
-
-    it 'creates a new note about resolved incident', :aggregate_failures do
-      expect { resolve_incident_status }.to change { noteable.notes.count }.by(1)
-
-      expect(noteable.notes.last.note).to eq('changed the status to **Resolved** by closing the incident')
-    end
-  end
-
   describe '#change_incident_status' do
     let_it_be(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: noteable) }