From 5631ece67fad35d85fc87d08b76345a44062da2a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 29 Jun 2020 00:08:58 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../projects/alerting/notify_service.rb | 3 +- .../process_alert_worker.rb | 36 ++++++----- .../projects/alerting/notify_service_spec.rb | 2 +- .../process_alert_worker_spec.rb | 64 ++++++++----------- 4 files changed, 49 insertions(+), 56 deletions(-) diff --git a/app/services/projects/alerting/notify_service.rb b/app/services/projects/alerting/notify_service.rb index ca520a43af5..4af3eba5df0 100644 --- a/app/services/projects/alerting/notify_service.rb +++ b/app/services/projects/alerting/notify_service.rb @@ -65,8 +65,7 @@ module Projects def process_incident_issues(alert) return if alert.issue - IncidentManagement::ProcessAlertWorker - .perform_async(project.id, parsed_payload, alert.id) + IncidentManagement::ProcessAlertWorker.perform_async(nil, nil, alert.id) end def send_alert_email diff --git a/app/workers/incident_management/process_alert_worker.rb b/app/workers/incident_management/process_alert_worker.rb index 0af34fa35d5..bc23dbda693 100644 --- a/app/workers/incident_management/process_alert_worker.rb +++ b/app/workers/incident_management/process_alert_worker.rb @@ -7,39 +7,45 @@ module IncidentManagement queue_namespace :incident_management feature_category :incident_management - def perform(project_id, alert_payload, am_alert_id = nil) - project = find_project(project_id) - return unless project + # `project_id` and `alert_payload` are deprecated and can be removed + # starting from 14.0 release + # https://gitlab.com/gitlab-org/gitlab/-/issues/224500 + def perform(_project_id = nil, _alert_payload = nil, alert_id = nil) + return unless alert_id - new_issue = create_issue(project, alert_payload) - return unless am_alert_id && new_issue&.persisted? + alert = find_alert(alert_id) + return unless alert - link_issue_with_alert(am_alert_id, new_issue.id) + new_issue = create_issue_for(alert) + return unless new_issue&.persisted? + + link_issue_with_alert(alert, new_issue.id) end private - def find_project(project_id) - Project.find_by_id(project_id) + def find_alert(alert_id) + AlertManagement::Alert.find_by_id(alert_id) end - def create_issue(project, alert_payload) + def parsed_payload(alert) + Gitlab::Alerting::NotificationPayloadParser.call(alert.payload.to_h, alert.project) + end + + def create_issue_for(alert) IncidentManagement::CreateIssueService - .new(project, alert_payload) + .new(alert.project, parsed_payload(alert)) .execute .dig(:issue) end - def link_issue_with_alert(alert_id, issue_id) - alert = AlertManagement::Alert.find_by_id(alert_id) - return unless alert - + def link_issue_with_alert(alert, issue_id) return if alert.update(issue_id: issue_id) Gitlab::AppLogger.warn( message: 'Cannot link an Issue with Alert', issue_id: issue_id, - alert_id: alert_id, + alert_id: alert.id, alert_errors: alert.errors.messages ) end diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index 045075daf63..7afad925cdd 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Projects::Alerting::NotifyService do it 'processes issues' do expect(IncidentManagement::ProcessAlertWorker) .to receive(:perform_async) - .with(project.id, kind_of(Hash), kind_of(Integer)) + .with(nil, nil, kind_of(Integer)) .once Sidekiq::Testing.inline! do diff --git a/spec/workers/incident_management/process_alert_worker_spec.rb b/spec/workers/incident_management/process_alert_worker_spec.rb index ff138dd6885..75696d15ab8 100644 --- a/spec/workers/incident_management/process_alert_worker_spec.rb +++ b/spec/workers/incident_management/process_alert_worker_spec.rb @@ -7,35 +7,31 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do let_it_be(:settings) { create(:project_incident_management_setting, project: project, create_issue: true) } describe '#perform' do - let(:alert_management_alert_id) { nil } - let(:alert_payload) do - { - 'annotations' => { 'title' => 'title' }, - 'startsAt' => Time.now.rfc3339 - } - end + let_it_be(:started_at) { Time.now.rfc3339 } + let_it_be(:payload) { { 'title' => 'title', 'start_time' => started_at } } + let_it_be(:parsed_payload) { Gitlab::Alerting::NotificationPayloadParser.call(payload, project) } + let_it_be(:alert) { create(:alert_management_alert, project: project, payload: payload, started_at: started_at) } + let(:created_issue) { Issue.last! } - let(:created_issue) { Issue.last } - - subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) } + subject { described_class.new.perform(nil, nil, alert.id) } before do allow(IncidentManagement::CreateIssueService) - .to receive(:new).with(project, alert_payload) + .to receive(:new).with(alert.project, parsed_payload) .and_call_original end it 'creates an issue' do 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) end - context 'with invalid project' do - let(:invalid_project_id) { non_existing_record_id } + context 'with invalid alert' do + let(:invalid_alert_id) { non_existing_record_id } - subject { described_class.new.perform(invalid_project_id, alert_payload) } + subject { described_class.new.perform(nil, nil, invalid_alert_id) } it 'does not create issues' do expect(IncidentManagement::CreateIssueService).not_to receive(:new) @@ -44,16 +40,8 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do end end - context 'when alert_management_alert_id is present' do - let!(:alert) { create(:alert_management_alert, project: project) } - let(:alert_management_alert_id) { alert.id } - + context 'with valid alert' 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 end @@ -69,24 +57,24 @@ RSpec.describe IncidentManagement::ProcessAlertWorker do expect(Gitlab::AppLogger).not_to have_received(:warn) end - end - context 'when alert cannot be updated' do - let(:alert) { create(:alert_management_alert, :with_validation_errors, project: project) } + context 'when alert cannot be updated' do + let_it_be(:alert) { create(:alert_management_alert, :with_validation_errors, project: project, payload: payload) } - it 'updates AlertManagement::Alert#issue_id' do - expect { subject }.not_to change { alert.reload.issue_id } - end + it 'updates AlertManagement::Alert#issue_id' do + expect { subject }.not_to change { alert.reload.issue_id } + end - it 'logs a warning' do - subject + it 'logs a warning' do + subject - expect(Gitlab::AppLogger).to have_received(:warn).with( - message: 'Cannot link an Issue with Alert', - issue_id: created_issue.id, - alert_id: alert_management_alert_id, - alert_errors: { hosts: ['hosts array is over 255 chars'] } - ) + expect(Gitlab::AppLogger).to have_received(:warn).with( + message: 'Cannot link an Issue with Alert', + issue_id: created_issue.id, + alert_id: alert.id, + alert_errors: { hosts: ['hosts array is over 255 chars'] } + ) + end end end end