Merge branch '356215-always-sync-alert-info' into 'master'

Always sync alert escalation attributes to incidents

See merge request gitlab-org/gitlab!84139

app/models/concerns/issuable.rb

@@ -200,6 +200,18 @@ 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,14 +151,25 @@ module AlertManagement
               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
 
       def should_sync_to_incident?
-        alert.issue &&
-          alert.issue.supports_escalation? &&
-          alert.issue.escalation_status &&
-          alert.issue.escalation_status.status != alert.status
+        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?
       end
 
       def filter_duplicate

app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb

@@ -31,7 +31,10 @@ module IncidentManagement
       attr_reader :issuable, :param_errors
 
       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
 
       def user_has_permissions?
@@ -60,6 +63,14 @@ 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,7 +87,10 @@ module Issues
     attr_reader :spam_params
 
     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
 
     def user_agent_detail_service

spec/models/concerns/issuable_spec.rb

@@ -993,6 +993,33 @@ 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,25 +253,51 @@ 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
 
-          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
-            let_it_be(:escalation_status, reload: true) { create(:incident_management_issuable_escalation_status, issue: issue) }
+            context 'when feature flag is disabled' do
+              before do
+                stub_feature_flags(incident_escalations: false)
+              end
 
-            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
+              it_behaves_like 'updates the incident escalation status with the new alert status'
+            end
 
-              expect { response }.to change { escalation_status.reload.acknowledged? }.to(true)
-                                 .and change { alert.reload.acknowledged? }.to(true)
+            def escalation_status
+              issue.reload.escalation_status
             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
               before do
@@ -286,7 +312,7 @@ RSpec.describe AlertManagement::Alerts::UpdateService do
                 stub_feature_flags(incident_escalations: false)
               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

spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb

@@ -48,6 +48,29 @@ 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,6 +98,7 @@ 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
@@ -118,12 +119,22 @@ RSpec.describe Issues::CreateService do
           end
 
           it_behaves_like 'incident issue'
+          include_examples 'calls the escalation status CreateService'
 
-          it 'calls IncidentManagement::Incidents::CreateEscalationStatusService' do
-            expect_next(::IncidentManagement::IssuableEscalationStatuses::CreateService, a_kind_of(Issue))
-              .to receive(:execute)
+          context 'when :incident_escalations feature flag is disabled' do
+            before do
+              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
 
           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