Commit 300da376 authored by Sarah Yasonik's avatar Sarah Yasonik Committed by Aleksei Lipniagov

Always sync alert escalation attributes to incidents

Swaps syncing behavior between alerts and incidents
to happen from alerts to incidents, indepdent of the
:incident_escalations feature flag. This ensures
accurate incident escalation data at feature rollout.
parent 0e9ea31e
...@@ -200,6 +200,18 @@ module Issuable ...@@ -200,6 +200,18 @@ module Issuable
incident? incident?
end end
# When :incident_escalations feature flag is disabled, new
# incidents should not have a status record unless the incident
# is associated with the alert. However, escalation attributes
# are synced with Alert/IssuableEscalationStatus, so we want to
# ensure that parity is kept prior to rollout.
# Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/345769.
def sync_escalation_attributes_from_alert?
incident? &&
::Feature.disabled?(:incident_escalations, project) &&
alert_management_alert.present?
end
def incident? def incident?
is_a?(Issue) && super is_a?(Issue) && super
end end
......
...@@ -151,14 +151,25 @@ module AlertManagement ...@@ -151,14 +151,25 @@ module AlertManagement
status_change_reason: " by changing the status of #{alert.to_reference(project)}" status_change_reason: " by changing the status of #{alert.to_reference(project)}"
} }
} }
).execute(alert.issue) ).execute(issue)
end
def issue
strong_memoize(:issue) { alert.issue }
end end
def should_sync_to_incident? def should_sync_to_incident?
alert.issue && return false unless sync_available?
alert.issue.supports_escalation? &&
alert.issue.escalation_status && issue.escalation_status&.status != alert.status
alert.issue.escalation_status.status != alert.status end
def sync_available?
return false unless issue.present?
# Remove sync check with https://gitlab.com/gitlab-org/gitlab/-/issues/345769
issue.supports_escalation? ||
issue.sync_escalation_attributes_from_alert?
end end
def filter_duplicate def filter_duplicate
......
...@@ -31,7 +31,10 @@ module IncidentManagement ...@@ -31,7 +31,10 @@ module IncidentManagement
attr_reader :issuable, :param_errors attr_reader :issuable, :param_errors
def available? def available?
issuable.supports_escalation? && user_has_permissions? (
issuable.supports_escalation? ||
issuable.sync_escalation_attributes_from_alert? # Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/345769
) && user_has_permissions?
end end
def user_has_permissions? def user_has_permissions?
...@@ -60,6 +63,14 @@ module IncidentManagement ...@@ -60,6 +63,14 @@ module IncidentManagement
status = params.delete(:status) status = params.delete(:status)
return unless status return unless status
# If we're updating the escalation status because the
# alert was updated & the feature flag is disabled, then
# we should not allow the status to be different from the alert's.
# Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/345769
if issuable.sync_escalation_attributes_from_alert? && status != issuable.alert_management_alert.status_name
add_param_error(:status) && return
end
status_event = escalation_status.status_event_for(status) status_event = escalation_status.status_event_for(status)
add_param_error(:status) && return unless status_event add_param_error(:status) && return unless status_event
......
...@@ -87,7 +87,10 @@ module Issues ...@@ -87,7 +87,10 @@ module Issues
attr_reader :spam_params attr_reader :spam_params
def create_escalation_status(issue) def create_escalation_status(issue)
::IncidentManagement::IssuableEscalationStatuses::CreateService.new(issue).execute if issue.supports_escalation? # Remove sync check with https://gitlab.com/gitlab-org/gitlab/-/issues/345769
return unless issue.supports_escalation? || issue.sync_escalation_attributes_from_alert?
::IncidentManagement::IssuableEscalationStatuses::CreateService.new(issue).execute
end end
def user_agent_detail_service def user_agent_detail_service
......
...@@ -993,6 +993,33 @@ RSpec.describe Issuable do ...@@ -993,6 +993,33 @@ RSpec.describe Issuable do
end end
end end
describe '#sync_escalation_attributes_from_alert?' do
where(:issuable_type, :args, :sync_escalation_attributes_from_alert) do
:issue | {} | false
:issue | ref(:alert_args) | false
:incident | {} | false
:incident | ref(:alert_args) | true
:merge_request | {} | false
end
with_them do
let(:alert_args) { { alert_management_alert: build_stubbed(:alert_management_alert) } }
let(:issuable) { build_stubbed(issuable_type, **args) }
subject { issuable.sync_escalation_attributes_from_alert? }
it { is_expected.to eq(false) }
context 'with feature disabled' do
before do
stub_feature_flags(incident_escalations: false)
end
it { is_expected.to eq(sync_escalation_attributes_from_alert) }
end
end
end
describe '#incident?' do describe '#incident?' do
where(:issuable_type, :incident) do where(:issuable_type, :incident) do
:issue | false :issue | false
......
...@@ -253,25 +253,51 @@ RSpec.describe AlertManagement::Alerts::UpdateService do ...@@ -253,25 +253,51 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
end end
end end
shared_examples 'updates the incident escalation status with the new alert status' do
specify do
expect(::Issues::UpdateService).to receive(:new).once.and_call_original
expect(described_class).to receive(:new).once.and_call_original
expect { response }.to change { escalation_status&.reload&.acknowledged? }.to(true)
.and change { alert.reload.acknowledged? }.to(true)
end
end
it_behaves_like 'does not sync with the incident status' it_behaves_like 'does not sync with the incident status'
context 'when feature flag is disabled' do
before do
stub_feature_flags(incident_escalations: false)
end
it_behaves_like 'does not sync with the incident status'
end
context 'when the issue is an incident' do context 'when the issue is an incident' do
before do before do
issue.update!(issue_type: Issue.issue_types[:incident]) issue.update!(issue_type: Issue.issue_types[:incident])
end end
it_behaves_like 'does not sync with the incident status' context 'when the incident does not have an escalation status' do
it_behaves_like 'updates the incident escalation status with the new alert status'
context 'when the incident has an escalation status' do context 'when feature flag is disabled' do
let_it_be(:escalation_status, reload: true) { create(:incident_management_issuable_escalation_status, issue: issue) } before do
stub_feature_flags(incident_escalations: false)
end
it 'updates the incident escalation status with the new alert status' do it_behaves_like 'updates the incident escalation status with the new alert status'
expect(::Issues::UpdateService).to receive(:new).once.and_call_original end
expect(described_class).to receive(:new).once.and_call_original
expect { response }.to change { escalation_status.reload.acknowledged? }.to(true) def escalation_status
.and change { alert.reload.acknowledged? }.to(true) issue.reload.escalation_status
end end
end
context 'when the incident has an escalation status' do
let_it_be(:escalation_status, reload: true) { create(:incident_management_issuable_escalation_status, issue: issue) }
it_behaves_like 'updates the incident escalation status with the new alert status'
context 'when the statuses match' do context 'when the statuses match' do
before do before do
...@@ -286,7 +312,7 @@ RSpec.describe AlertManagement::Alerts::UpdateService do ...@@ -286,7 +312,7 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
stub_feature_flags(incident_escalations: false) stub_feature_flags(incident_escalations: false)
end end
it_behaves_like 'does not sync with the incident status' it_behaves_like 'updates the incident escalation status with the new alert status'
end end
end end
end end
......
...@@ -48,6 +48,29 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::PrepareUpdateServ ...@@ -48,6 +48,29 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::PrepareUpdateServ
end end
it_behaves_like 'availability error response' it_behaves_like 'availability error response'
context 'with incident associated with alert' do
let(:alert_status) { :acknowledged }
before do
create(:alert_management_alert, alert_status, project: issue.project, issue: issue)
issue.reload
end
it_behaves_like 'successful response', { status_event: :acknowledge }
context 'when alert status does not match incident status' do
let(:alert_status) { :triggered }
include_examples 'error response', 'Invalid value was provided for parameters: status'
end
context 'with a standard issue' do
let(:issue) { create(:issue) }
it_behaves_like 'availability error response'
end
end
end end
context 'when user is anonymous' do context 'when user is anonymous' do
......
...@@ -98,6 +98,7 @@ RSpec.describe Issues::CreateService do ...@@ -98,6 +98,7 @@ RSpec.describe Issues::CreateService do
end end
it_behaves_like 'not an incident issue' it_behaves_like 'not an incident issue'
include_examples 'does not call the escalation status CreateService'
context 'when issue is incident type' do context 'when issue is incident type' do
before do before do
...@@ -118,12 +119,22 @@ RSpec.describe Issues::CreateService do ...@@ -118,12 +119,22 @@ RSpec.describe Issues::CreateService do
end end
it_behaves_like 'incident issue' it_behaves_like 'incident issue'
include_examples 'calls the escalation status CreateService'
it 'calls IncidentManagement::Incidents::CreateEscalationStatusService' do context 'when :incident_escalations feature flag is disabled' do
expect_next(::IncidentManagement::IssuableEscalationStatuses::CreateService, a_kind_of(Issue)) before do
.to receive(:execute) stub_feature_flags(incident_escalations: false)
end
issue include_examples 'does not call the escalation status CreateService'
context 'with associated alert' do
before do
opts.merge!(alert_management_alert: build(:alert_management_alert, project: project))
end
include_examples 'calls the escalation status CreateService'
end
end end
context 'when invalid' do context 'when invalid' do
......
# frozen_string_literal: true
# This shared_example requires the following variables:
# - issue (required)
RSpec.shared_examples 'calls the escalation status CreateService' do
it 'calls IncidentManagement::Incidents::CreateEscalationStatusService' do
expect_next(::IncidentManagement::IssuableEscalationStatuses::CreateService, a_kind_of(Issue))
.to receive(:execute)
issue
end
end
# This shared_example requires the following variables:
# - issue (required)
RSpec.shared_examples 'does not call the escalation status CreateService' do
it 'does not call the ::IncidentManagement::IssuableEscalationStatuses::CreateService' do
expect(::IncidentManagement::IssuableEscalationStatuses::CreateService).not_to receive(:new)
issue
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