Commit 9bc1f71f authored by Sarah Yasonik's avatar Sarah Yasonik Committed by Alper Akgun

Revert "Return only booleans"

This reverts commit 8eb5307cc65c23e592f7a36a55ce7c39e068c3b2.
parent e9f85a9f
......@@ -200,18 +200,6 @@ module Issuable
incident?
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?
is_a?(Issue) && super
end
......
......@@ -151,25 +151,14 @@ module AlertManagement
status_change_reason: " by changing the status of #{alert.to_reference(project)}"
}
}
).execute(issue)
end
def issue
strong_memoize(:issue) { alert.issue }
).execute(alert.issue)
end
def should_sync_to_incident?
return false unless sync_available?
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?
alert.issue &&
alert.issue.supports_escalation? &&
alert.issue.escalation_status &&
alert.issue.escalation_status.status != alert.status
end
def filter_duplicate
......
......@@ -31,10 +31,7 @@ module IncidentManagement
attr_reader :issuable, :param_errors
def available?
(
issuable.supports_escalation? ||
issuable.sync_escalation_attributes_from_alert? # Remove with https://gitlab.com/gitlab-org/gitlab/-/issues/345769
) && user_has_permissions?
issuable.supports_escalation? && user_has_permissions?
end
def user_has_permissions?
......@@ -63,14 +60,6 @@ module IncidentManagement
status = params.delete(: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)
add_param_error(:status) && return unless status_event
......
......@@ -87,10 +87,7 @@ module Issues
attr_reader :spam_params
def create_escalation_status(issue)
# 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
::IncidentManagement::IssuableEscalationStatuses::CreateService.new(issue).execute if issue.supports_escalation?
end
def user_agent_detail_service
......
......@@ -993,33 +993,6 @@ RSpec.describe Issuable do
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
where(:issuable_type, :incident) do
:issue | false
......
......@@ -253,51 +253,25 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
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'
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
before do
issue.update!(issue_type: Issue.issue_types[:incident])
end
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 feature flag is disabled' do
before do
stub_feature_flags(incident_escalations: false)
end
it_behaves_like 'updates the incident escalation status with the new alert status'
end
def escalation_status
issue.reload.escalation_status
end
end
it_behaves_like 'does not sync with the incident status'
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'
it 'updates the incident escalation status with the new alert status' 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
context 'when the statuses match' do
before do
......@@ -312,7 +286,7 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
stub_feature_flags(incident_escalations: false)
end
it_behaves_like 'updates the incident escalation status with the new alert status'
it_behaves_like 'does not sync with the incident status'
end
end
end
......
......@@ -48,29 +48,6 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::PrepareUpdateServ
end
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
context 'when user is anonymous' do
......
......@@ -98,7 +98,6 @@ RSpec.describe Issues::CreateService do
end
it_behaves_like 'not an incident issue'
include_examples 'does not call the escalation status CreateService'
context 'when issue is incident type' do
before do
......@@ -119,22 +118,12 @@ RSpec.describe Issues::CreateService do
end
it_behaves_like 'incident issue'
include_examples 'calls the escalation status CreateService'
context 'when :incident_escalations feature flag is disabled' do
before do
stub_feature_flags(incident_escalations: false)
end
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
it 'calls IncidentManagement::Incidents::CreateEscalationStatusService' do
expect_next(::IncidentManagement::IssuableEscalationStatuses::CreateService, a_kind_of(Issue))
.to receive(:execute)
include_examples 'calls the escalation status CreateService'
end
issue
end
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