Commit 3b3f92ba authored by Sarah Yasonik's avatar Sarah Yasonik Committed by Aleksei Lipniagov

Refactor system notes for alerts and incidents for consistentcy

Differences are minor, such as removing extra periods or clarifying
why a particular action occured.

Changelog: changed
parent c56fa1bb
...@@ -18,11 +18,11 @@ class StateNote < SyntheticNote ...@@ -18,11 +18,11 @@ class StateNote < SyntheticNote
def note_text(html: false) def note_text(html: false)
if event.state == 'closed' if event.state == 'closed'
if event.close_after_error_tracking_resolve 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 end
if event.close_auto_resolve_prometheus_alert 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
end end
......
...@@ -81,7 +81,7 @@ module Issues ...@@ -81,7 +81,7 @@ module Issues
return if alert.resolved? return if alert.resolved?
if alert.resolve 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 else
Gitlab::AppLogger.warn( Gitlab::AppLogger.warn(
message: 'Cannot resolve an associated Alert Management alert', message: 'Cannot resolve an associated Alert Management alert',
...@@ -97,7 +97,7 @@ module Issues ...@@ -97,7 +97,7 @@ module Issues
status = issue.incident_management_issuable_escalation_status || issue.build_incident_management_issuable_escalation_status 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 end
def store_first_mentioned_in_commit_at(issue, merge_request, max_commit_lookup: 100) def store_first_mentioned_in_commit_at(issue, merge_request, max_commit_lookup: 100)
......
...@@ -335,10 +335,6 @@ module SystemNoteService ...@@ -335,10 +335,6 @@ module SystemNoteService
::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_severity ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_severity
end 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) def change_incident_status(incident, author, reason = nil)
::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_status(reason) ::SystemNotes::IncidentService.new(noteable: incident, project: incident.project, author: author).change_incident_status(reason)
end end
......
...@@ -40,30 +40,15 @@ module SystemNotes ...@@ -40,30 +40,15 @@ module SystemNotes
# #
# Example Note text: # Example Note text:
# #
# "created issue #17 for this alert" # "created incident #17 for this alert"
# #
# Returns the created Note object # Returns the created Note object
def new_alert_issue(issue) 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')) create_note(NoteSummary.new(noteable, project, author, body, action: 'alert_issue_added'))
end 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 # Called when an alert is resolved due to received resolving alert payload
# #
# alert - AlertManagement::Alert object. # alert - AlertManagement::Alert object.
......
...@@ -26,12 +26,6 @@ module SystemNotes ...@@ -26,12 +26,6 @@ module SystemNotes
end end
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 # Called when the status of an IncidentManagement::IssuableEscalationStatus has changed
# #
# reason - String. # reason - String.
......
...@@ -55,7 +55,7 @@ RSpec.describe StateNote do ...@@ -55,7 +55,7 @@ RSpec.describe StateNote do
it 'contains the expected values' do it 'contains the expected values' do
expect(subject.author).to eq(author) expect(subject.author).to eq(author)
expect(subject.created_at).to eq(event.created_at) 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
end end
...@@ -65,7 +65,7 @@ RSpec.describe StateNote do ...@@ -65,7 +65,7 @@ RSpec.describe StateNote do
it 'contains the expected values' do it 'contains the expected values' do
expect(subject.author).to eq(author) expect(subject.author).to eq(author)
expect(subject.created_at).to eq(event.created_at) 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 end
end end
......
...@@ -118,7 +118,7 @@ RSpec.describe Issues::CloseService do ...@@ -118,7 +118,7 @@ RSpec.describe Issues::CloseService do
expect { service.execute(issue) }.to change { issue.notes.count }.by(1) expect { service.execute(issue) }.to change { issue.notes.count }.by(1)
new_note = issue.notes.last 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) expect(new_note.author).to eq(user)
end end
...@@ -334,8 +334,12 @@ RSpec.describe Issues::CloseService do ...@@ -334,8 +334,12 @@ RSpec.describe Issues::CloseService do
let!(:alert) { create(:alert_management_alert, issue: issue, project: project) } let!(:alert) { create(:alert_management_alert, issue: issue, project: project) }
it 'resolves an alert and sends a system note' do it 'resolves an alert and sends a system note' do
expect_next_instance_of(SystemNotes::AlertManagementService) do |notes_service| expect_any_instance_of(SystemNoteService) do |notes_service|
expect(notes_service).to receive(:closed_alert_issue).with(issue) expect(notes_service).to receive(:change_alert_status).with(
alert,
current_user,
" by closing issue #{issue.to_reference(project)}"
)
end end
close_issue close_issue
......
...@@ -632,18 +632,6 @@ RSpec.describe SystemNoteService do ...@@ -632,18 +632,6 @@ RSpec.describe SystemNoteService do
end end
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 describe '.change_incident_status' do
let(:incident) { instance_double('Issue', project: project) } let(:incident) { instance_double('Issue', project: project) }
......
...@@ -54,21 +54,7 @@ RSpec.describe ::SystemNotes::AlertManagementService do ...@@ -54,21 +54,7 @@ RSpec.describe ::SystemNotes::AlertManagementService do
end end
it 'has the appropriate message' do it 'has the appropriate message' do
expect(subject.note).to eq("created issue #{issue.to_reference(project)} for this alert") expect(subject.note).to eq("created incident #{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 end
......
...@@ -57,16 +57,6 @@ RSpec.describe ::SystemNotes::IncidentService do ...@@ -57,16 +57,6 @@ RSpec.describe ::SystemNotes::IncidentService do
end end
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 describe '#change_incident_status' do
let_it_be(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: noteable) } let_it_be(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: noteable) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment