Merge branch 'sy-revert-alert-attribute-sync' into 'master'

Revert "Always sync alert escalation attributes to incidents"

See merge request gitlab-org/gitlab!84633

app/models/concerns/issuable.rb

@@ -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

app/services/alert_management/alerts/update_service.rb

@@ -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

app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb

@@ -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
 

app/services/issues/create_service.rb

@@ -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

spec/models/concerns/issuable_spec.rb

@@ -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

spec/services/alert_management/alerts/update_service_spec.rb

@@ -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

spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb

@@ -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

spec/services/issues/create_service_spec.rb

@@ -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'
+          it 'calls IncidentManagement::Incidents::CreateEscalationStatusService' do
+            expect_next(::IncidentManagement::IssuableEscalationStatuses::CreateService, a_kind_of(Issue))
+              .to receive(:execute)
 
-            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
+            issue
           end
 
           context 'when invalid' do

spec/support/shared_examples/services/incident_management/escalation_status_shared_examples.rb

-# 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