Commit 2595a954 authored by Vitali Tatarintev's avatar Vitali Tatarintev

Pass only alert_id to ProcessAlertWorker

At the moment we call `ProcessAlertWorker`,
`AlertManagement::Alert` already exists.
Thus we can pass `alert_id` instead of whole payload.
parent f2fb3477
...@@ -65,8 +65,7 @@ module Projects ...@@ -65,8 +65,7 @@ module Projects
def process_incident_issues(alert) def process_incident_issues(alert)
return if alert.issue return if alert.issue
IncidentManagement::ProcessAlertWorker IncidentManagement::ProcessAlertWorker.perform_async(alert.id)
.perform_async(project.id, parsed_payload, alert.id)
end end
def send_alert_email def send_alert_email
......
...@@ -7,39 +7,40 @@ module IncidentManagement ...@@ -7,39 +7,40 @@ module IncidentManagement
queue_namespace :incident_management queue_namespace :incident_management
feature_category :incident_management feature_category :incident_management
def perform(project_id, alert_payload, am_alert_id = nil) def perform(alert_id)
project = find_project(project_id) alert = find_alert(alert_id)
return unless project return unless alert
new_issue = create_issue(project, alert_payload) new_issue = create_issue_for(alert)
return unless am_alert_id && new_issue&.persisted? return unless new_issue&.persisted?
link_issue_with_alert(am_alert_id, new_issue.id) link_issue_with_alert(alert, new_issue.id)
end end
private private
def find_project(project_id) def find_alert(alert_id)
Project.find_by_id(project_id) AlertManagement::Alert.find_by_id(alert_id)
end
def parsed_payload(alert)
Gitlab::Alerting::NotificationPayloadParser.call(alert.payload.to_h)
end end
def create_issue(project, alert_payload) def create_issue_for(alert)
IncidentManagement::CreateIssueService IncidentManagement::CreateIssueService
.new(project, alert_payload) .new(alert.project, parsed_payload(alert))
.execute .execute
.dig(:issue) .dig(:issue)
end end
def link_issue_with_alert(alert_id, issue_id) def link_issue_with_alert(alert, issue_id)
alert = AlertManagement::Alert.find_by_id(alert_id)
return unless alert
return if alert.update(issue_id: issue_id) return if alert.update(issue_id: issue_id)
Gitlab::AppLogger.warn( Gitlab::AppLogger.warn(
message: 'Cannot link an Issue with Alert', message: 'Cannot link an Issue with Alert',
issue_id: issue_id, issue_id: issue_id,
alert_id: alert_id, alert_id: alert.id,
alert_errors: alert.errors.messages alert_errors: alert.errors.messages
) )
end end
......
...@@ -21,7 +21,7 @@ RSpec.describe Projects::Alerting::NotifyService do ...@@ -21,7 +21,7 @@ RSpec.describe Projects::Alerting::NotifyService do
it 'processes issues' do it 'processes issues' do
expect(IncidentManagement::ProcessAlertWorker) expect(IncidentManagement::ProcessAlertWorker)
.to receive(:perform_async) .to receive(:perform_async)
.with(project.id, kind_of(Hash), kind_of(Integer)) .with(kind_of(Integer))
.once .once
Sidekiq::Testing.inline! do Sidekiq::Testing.inline! do
......
...@@ -7,35 +7,31 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do ...@@ -7,35 +7,31 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
let_it_be(:settings) { create(:project_incident_management_setting, project: project, create_issue: true) } let_it_be(:settings) { create(:project_incident_management_setting, project: project, create_issue: true) }
describe '#perform' do describe '#perform' do
let(:alert_management_alert_id) { nil } let_it_be(:started_at) { Time.now.rfc3339 }
let(:alert_payload) do let_it_be(:payload) { { 'title' => 'title', 'start_time' => started_at } }
{ let_it_be(:parsed_payload) { Gitlab::Alerting::NotificationPayloadParser.call(payload) }
'annotations' => { 'title' => 'title' }, let_it_be(:alert) { create(:alert_management_alert, project: project, payload: payload, started_at: started_at) }
'startsAt' => Time.now.rfc3339 let(:created_issue) { Issue.last! }
}
end
let(:created_issue) { Issue.last }
subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) } subject { described_class.new.perform(alert.id) }
before do before do
allow(IncidentManagement::CreateIssueService) allow(IncidentManagement::CreateIssueService)
.to receive(:new).with(project, alert_payload) .to receive(:new).with(alert.project, parsed_payload)
.and_call_original .and_call_original
end end
it 'creates an issue' do it 'creates an issue' do
expect(IncidentManagement::CreateIssueService) expect(IncidentManagement::CreateIssueService)
.to receive(:new).with(project, alert_payload) .to receive(:new).with(alert.project, parsed_payload)
expect { subject }.to change { Issue.count }.by(1) expect { subject }.to change { Issue.count }.by(1)
end end
context 'with invalid project' do context 'with invalid alert' do
let(:invalid_project_id) { non_existing_record_id } let(:invalid_alert_id) { non_existing_record_id }
subject { described_class.new.perform(invalid_project_id, alert_payload) } subject { described_class.new.perform(invalid_alert_id) }
it 'does not create issues' do it 'does not create issues' do
expect(IncidentManagement::CreateIssueService).not_to receive(:new) expect(IncidentManagement::CreateIssueService).not_to receive(:new)
...@@ -44,16 +40,8 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do ...@@ -44,16 +40,8 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
end end
end end
context 'when alert_management_alert_id is present' do context 'with valid alert' do
let!(:alert) { create(:alert_management_alert, project: project) }
let(:alert_management_alert_id) { alert.id }
before do before do
allow(AlertManagement::Alert)
.to receive(:find_by_id)
.with(alert_management_alert_id)
.and_return(alert)
allow(Gitlab::AppLogger).to receive(:warn).and_call_original allow(Gitlab::AppLogger).to receive(:warn).and_call_original
end end
...@@ -69,10 +57,9 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do ...@@ -69,10 +57,9 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
expect(Gitlab::AppLogger).not_to have_received(:warn) expect(Gitlab::AppLogger).not_to have_received(:warn)
end end
end
context 'when alert cannot be updated' do context 'when alert cannot be updated' do
let(:alert) { create(:alert_management_alert, :with_validation_errors, project: project) } let_it_be(:alert) { create(:alert_management_alert, :with_validation_errors, project: project, payload: payload) }
it 'updates AlertManagement::Alert#issue_id' do it 'updates AlertManagement::Alert#issue_id' do
expect { subject }.not_to change { alert.reload.issue_id } expect { subject }.not_to change { alert.reload.issue_id }
...@@ -84,11 +71,12 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do ...@@ -84,11 +71,12 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do
expect(Gitlab::AppLogger).to have_received(:warn).with( expect(Gitlab::AppLogger).to have_received(:warn).with(
message: 'Cannot link an Issue with Alert', message: 'Cannot link an Issue with Alert',
issue_id: created_issue.id, issue_id: created_issue.id,
alert_id: alert_management_alert_id, alert_id: alert.id,
alert_errors: { hosts: ['hosts array is over 255 chars'] } alert_errors: { hosts: ['hosts array is over 255 chars'] }
) )
end end
end end
end end
end end
end
end end
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