Merge branch '356215-save-incident-status-on-update' into 'master'

Persist issuable escalation statuses on issue update

See merge request gitlab-org/gitlab!83787

app/services/incident_management/issuable_escalation_statuses/build_service.rb

+# frozen_string_literal: true
+
+module IncidentManagement
+  module IssuableEscalationStatuses
+    class BuildService < ::BaseProjectService
+      def initialize(issue)
+        @issue = issue
+        @alert = issue.alert_management_alert
+
+        super(project: issue.project)
+      end
+
+      def execute
+        return issue.escalation_status if issue.escalation_status
+
+        issue.build_incident_management_issuable_escalation_status(alert_params)
+      end
+
+      private
+
+      attr_reader :issue, :alert
+
+      def alert_params
+        return {} unless alert
+
+        {
+          status_event: alert.status_event_for(alert.status_name)
+        }
+      end
+    end
+  end
+end
+
+IncidentManagement::IssuableEscalationStatuses::BuildService.prepend_mod

app/services/incident_management/issuable_escalation_statuses/create_service.rb

@@ -2,14 +2,15 @@
 
 module IncidentManagement
   module IssuableEscalationStatuses
-    class CreateService < BaseService
+    class CreateService < ::BaseProjectService
       def initialize(issue)
         @issue = issue
-        @alert = issue.alert_management_alert
+
+        super(project: issue.project)
       end
 
       def execute
-        escalation_status = ::IncidentManagement::IssuableEscalationStatus.new(issue: issue, **alert_params)
+        escalation_status = BuildService.new(issue).execute
 
         if escalation_status.save
           ServiceResponse.success(payload: { escalation_status: escalation_status })
@@ -20,17 +21,7 @@ module IncidentManagement
 
       private
 
-      attr_reader :issue, :alert
-
-      def alert_params
-        return {} unless alert
-
-        {
-          status_event: alert.status_event_for(alert.status_name)
-        }
-      end
+      attr_reader :issue
     end
   end
 end
-
-IncidentManagement::IssuableEscalationStatuses::CreateService.prepend_mod

app/services/incident_management/issuable_escalation_statuses/prepare_update_service.rb

@@ -31,9 +31,7 @@ module IncidentManagement
       attr_reader :issuable, :param_errors
 
       def available?
-        issuable.supports_escalation? &&
-          user_has_permissions? &&
-          escalation_status.present?
+        issuable.supports_escalation? && user_has_permissions?
       end
 
       def user_has_permissions?
@@ -42,7 +40,7 @@ module IncidentManagement
 
       def escalation_status
         strong_memoize(:escalation_status) do
-          issuable.escalation_status
+          issuable.escalation_status || BuildService.new(issuable).execute
         end
       end
 

ee/app/services/ee/incident_management/issuable_escalation_statuses/create_service.rb → ee/app/services/ee/incident_management/issuable_escalation_statuses/build_service.rb

@@ -3,7 +3,7 @@
 module EE
   module IncidentManagement
     module IssuableEscalationStatuses
-      module CreateService
+      module BuildService
         extend ::Gitlab::Utils::Override
 
         override :alert_params

ee/spec/services/ee/incident_management/issuable_escalation_statuses/build_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::IssuableEscalationStatuses::BuildService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:incident, reload: true) { create(:incident, project: project) }
+
+  let(:service) { described_class.new(incident) }
+
+  subject(:execute) { service.execute }
+
+  before do
+    stub_licensed_features(oncall_schedules: true, escalation_policies: true)
+  end
+
+  it_behaves_like 'initializes new escalation status with expected attributes'
+
+  context 'with associated alert' do
+    let_it_be(:alert) { create(:alert_management_alert, :acknowledged, project: project, issue: incident) }
+
+    it_behaves_like 'initializes new escalation status with expected attributes', { status_event: :acknowledge }
+
+    context 'with escalation policy' do
+      let_it_be(:policy) { create(:incident_management_escalation_policy, project: project) }
+
+      it_behaves_like 'initializes new escalation status with expected attributes' do
+        let(:expected_attributes) do
+          {
+            status_event: :acknowledge,
+            policy_id: policy.id,
+            escalations_started_at: alert.reload.created_at
+          }
+        end
+      end
+
+      context 'with escalation policies feature unavailable' do
+        before do
+          stub_licensed_features(oncall_schedules: false, escalation_policies: false)
+        end
+
+        it_behaves_like 'initializes new escalation status with expected attributes', { status_event: :acknowledge }
+      end
+    end
+  end
+end

ee/spec/services/ee/incident_management/issuable_escalation_statuses/create_service_spec.rb

@@ -13,7 +13,6 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::CreateService do
 
   before do
     stub_licensed_features(oncall_schedules: true, escalation_policies: true)
-    stub_feature_flags(incident_escalations: true)
   end
 
   shared_examples 'creates an escalation status for the incident with no policy set' do
@@ -52,9 +51,9 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::CreateService do
         expect(status.status_name).to eq(alert.status_name)
       end
 
-      context 'when feature flag is disabled' do
+      context 'when escalation policy features are disabled' do
         before do
-          stub_feature_flags(incident_escalations: false)
+          stub_licensed_features(oncall_schedules: false, escalation_policies: false)
         end
 
         it_behaves_like 'creates an escalation status for the incident with no policy set'

ee/spec/services/ee/issues/update_service_spec.rb

@@ -453,7 +453,6 @@ RSpec.describe Issues::UpdateService do
       let(:opts) { { escalation_status: { policy: policy } } }
       let(:escalation_update_class) { ::IncidentManagement::IssuableEscalationStatuses::AfterUpdateService }
 
-      let!(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: issue) }
       let!(:policy) { create(:incident_management_escalation_policy, project: project) }
 
       before do
@@ -484,7 +483,7 @@ RSpec.describe Issues::UpdateService do
 
       shared_examples 'does not change the status record' do
         specify do
-          expect { update_issue(opts) }.not_to change { issue.escalation_status.reload }
+          expect { update_issue(opts) }.not_to change { issue.reload.escalation_status }
         end
 
         it 'does not trigger side-effects' do
@@ -495,52 +494,87 @@ RSpec.describe Issues::UpdateService do
         end
       end
 
-      context 'when issue is an incident' do
-        let(:issue) { create(:incident, project: project) }
+      context 'with an existing escalation status record' do
+        let!(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: issue) }
 
-        context 'setting the escalation policy' do
-          include_examples 'updates the escalation status record' do
-            let(:expected_policy) { policy }
-            let(:expected_status) { :triggered }
-          end
+        context 'when issue is an incident' do
+          let(:issue) { create(:incident, project: project) }
 
-          context 'with the policy value defined but unchanged' do
-            let!(:escalation_status) { create(:incident_management_issuable_escalation_status, :paging, issue: issue, policy: policy) }
+          context 'setting the escalation policy' do
+            include_examples 'updates the escalation status record' do
+              let(:expected_policy) { policy }
+              let(:expected_status) { :triggered }
+            end
 
-            it_behaves_like 'does not change the status record'
-          end
-        end
+            context 'with the policy value defined but unchanged' do
+              let!(:escalation_status) { create(:incident_management_issuable_escalation_status, :paging, issue: issue, policy: policy) }
 
-        context 'unsetting the escalation policy' do
-          let(:policy) { nil }
+              it_behaves_like 'does not change the status record'
+            end
+          end
 
-          context 'when the policy is already set' do
-            let!(:escalation_status) { create(:incident_management_issuable_escalation_status, :paging, issue: issue) }
-            let(:expected_policy) { nil }
+          context 'unsetting the escalation policy' do
+            let(:policy) { nil }
 
-            include_examples 'updates the escalation status record' do
+            context 'when the policy is already set' do
+              let!(:escalation_status) { create(:incident_management_issuable_escalation_status, :paging, issue: issue) }
               let(:expected_policy) { nil }
-              let(:expected_status) { :triggered }
-            end
-
-            context 'in addition to other attributes' do
-              let(:opts) { { escalation_status: { policy: policy, status: 'acknowledged' } } }
 
               include_examples 'updates the escalation status record' do
                 let(:expected_policy) { nil }
-                let(:expected_status) { :acknowledged }
+                let(:expected_status) { :triggered }
+              end
+
+              context 'in addition to other attributes' do
+                let(:opts) { { escalation_status: { policy: policy, status: 'acknowledged' } } }
+
+                include_examples 'updates the escalation status record' do
+                  let(:expected_policy) { nil }
+                  let(:expected_status) { :acknowledged }
+                end
               end
             end
+
+            context 'with the policy value defined but unchanged' do
+              it_behaves_like 'does not change the status record'
+            end
           end
+        end
+
+        context 'when issue is not an incident' do
+          it_behaves_like 'does not change the status record'
+        end
+      end
+
+      context 'without an existing escalation status record' do
+        context 'when incident has an associated alert' do
+          let!(:alert) { create(:alert_management_alert, :acknowledged, :with_incident, project: project) }
+          let(:issue) { alert.issue }
+
+          context 'setting the status' do
+            let(:opts) { { escalation_status: { status: :resolved } } }
+
+            include_examples 'updates the escalation status record' do
+              let(:expected_policy) { policy }
+              let(:expected_status) { :resolved }
+            end
+          end
+
+          context 'unsetting the policy' do
+            let(:opts) { { escalation_status: { policy: nil } } }
 
-          context 'with the policy value defined but unchanged' do
             it_behaves_like 'does not change the status record'
           end
         end
-      end
 
-      context 'when issue is not an incident' do
-        it_behaves_like 'does not change the status record'
+        context 'when incident has no associated alert' do
+          let(:issue) { create(:incident, project: project) }
+
+          include_examples 'updates the escalation status record' do
+            let(:expected_policy) { policy }
+            let(:expected_status) { :triggered }
+          end
+        end
       end
     end
 

spec/services/incident_management/issuable_escalation_statuses/build_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::IssuableEscalationStatuses::BuildService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:incident, reload: true) { create(:incident, project: project) }
+
+  let(:service) { described_class.new(incident) }
+
+  subject(:execute) { service.execute }
+
+  it_behaves_like 'initializes new escalation status with expected attributes'
+
+  context 'with associated alert' do
+    let_it_be(:alert) { create(:alert_management_alert, :acknowledged, project: project, issue: incident) }
+
+    it_behaves_like 'initializes new escalation status with expected attributes', { status_event: :acknowledge }
+  end
+end

spec/services/incident_management/issuable_escalation_statuses/create_service_spec.rb

@@ -8,12 +8,12 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::CreateService do
   let(:incident) { create(:incident, project: project) }
   let(:service) { described_class.new(incident) }
 
-  subject(:execute) { service.execute}
+  subject(:execute) { service.execute }
 
   it 'creates an escalation status for the incident with no policy set' do
-    expect { execute }.to change { incident.reload.incident_management_issuable_escalation_status }.from(nil)
+    expect { execute }.to change { incident.reload.escalation_status }.from(nil)
 
-    status = incident.incident_management_issuable_escalation_status
+    status = incident.escalation_status
 
     expect(status.policy_id).to eq(nil)
     expect(status.escalations_started_at).to eq(nil)
@@ -24,7 +24,22 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::CreateService do
     let!(:existing_status) { create(:incident_management_issuable_escalation_status, issue: incident) }
 
     it 'exits without changing anything' do
-      expect { execute }.not_to change { incident.reload.incident_management_issuable_escalation_status }
+      expect { execute }.not_to change { incident.reload.escalation_status }
+    end
+  end
+
+  context 'with associated alert' do
+    before do
+      create(:alert_management_alert, :acknowledged, project: project, issue: incident)
+    end
+
+    it 'creates an escalation status matching the alert attributes' do
+      expect { execute }.to change { incident.reload.escalation_status }.from(nil)
+      expect(incident.escalation_status).to have_attributes(
+        status_name: :acknowledged,
+        policy_id: nil,
+        escalations_started_at: nil
+      )
     end
   end
 end

spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb

@@ -71,7 +71,12 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::PrepareUpdateServ
   context 'when an IssuableEscalationStatus record for the issue does not exist' do
     let(:issue) { create(:incident) }
 
-    it_behaves_like 'availability error response'
+    it_behaves_like 'successful response', { status_event: :acknowledge }
+
+    it 'initializes an issuable escalation status record' do
+      expect { result }.not_to change(::IncidentManagement::IssuableEscalationStatus, :count)
+      expect(issue.escalation_status).to be_present
+    end
   end
 
   context 'when called without params' do

spec/services/issues/update_service_spec.rb

@@ -1224,6 +1224,18 @@ RSpec.describe Issues::UpdateService, :mailer do
         end
 
         context 'without an escalation status record' do
+          it 'creates a new record' do
+            expect { update_issue(opts) }.to change(::IncidentManagement::IssuableEscalationStatus, :count).by(1)
+          end
+
+          it_behaves_like 'updates the escalation status record', :acknowledged
+        end
+
+        context 'with :incident_escalations feature flag disabled' do
+          before do
+            stub_feature_flags(incident_escalations: false)
+          end
+
           it_behaves_like 'does not change the status record'
         end
       end

spec/support/shared_examples/incident_management/issuable_escalation_statuses/build_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'initializes new escalation status with expected attributes' do |attributes = {}|
+  let(:expected_attributes) { attributes }
+
+  specify do
+    expect { execute }.to change { incident.escalation_status }
+      .from(nil)
+      .to(instance_of(::IncidentManagement::IssuableEscalationStatus))
+
+    expect(incident.escalation_status).to have_attributes(
+      id: nil,
+      issue_id: incident.id,
+      policy_id: nil,
+      escalations_started_at: nil,
+      status_event: nil,
+      **expected_attributes
+    )
+  end
+end