Merge branch 'pl-add-incident-label-for-incidents' into 'master'

Add incident label for manually created incident issues

See merge request gitlab-org/gitlab!41598

app/services/incident_management/incidents/create_service.rb

@@ -18,7 +18,6 @@ module IncidentManagement
           current_user,
           title: title,
           description: description,
-          label_ids: [find_or_create_incident_label.id],
           issue_type: ISSUE_TYPE
         ).execute
 
@@ -31,13 +30,6 @@ module IncidentManagement
 
       attr_reader :title, :description
 
-      def find_or_create_incident_label
-        IncidentManagement::CreateIncidentLabelService
-          .new(project, current_user)
-          .execute
-          .payload[:label]
-      end
-
       def success(issue)
         ServiceResponse.success(payload: { issue: issue })
       end

app/services/issues/base_service.rb

@@ -61,6 +61,22 @@ module Issues
 
       Milestones::IssuesCountService.new(milestone).delete_cache
     end
+
+    # Applies label "incident" (creates it if missing) to incident issues.
+    # Please use in "after" hooks only to ensure we are not appyling
+    # labels prematurely.
+    def add_incident_label(issue)
+      return unless issue.incident?
+
+      label = ::IncidentManagement::CreateIncidentLabelService
+        .new(project, current_user)
+        .execute
+        .payload[:label]
+
+      return if issue.label_ids.include?(label.id)
+
+      issue.labels << label
+    end
   end
 end
 

app/services/issues/create_service.rb

@@ -25,12 +25,13 @@ module Issues
       end
     end
 
-    def after_create(issuable)
-      todo_service.new_issue(issuable, current_user)
+    def after_create(issue)
+      add_incident_label(issue)
+      todo_service.new_issue(issue, current_user)
       user_agent_detail_service.create
-      resolve_discussions_with_issue(issuable)
-      delete_milestone_total_issue_counter_cache(issuable.milestone)
-      track_incident_action(current_user, issuable, :incident_created)
+      resolve_discussions_with_issue(issue)
+      delete_milestone_total_issue_counter_cache(issue.milestone)
+      track_incident_action(current_user, issue, :incident_created)
 
       super
     end

app/services/issues/update_service.rb

@@ -22,6 +22,7 @@ module Issues
     end
 
     def after_update(issue)
+      add_incident_label(issue)
       IssuesChannel.broadcast_to(issue, event: 'updated') if Gitlab::ActionCable::Config.in_app? || Feature.enabled?(:broadcast_issue_updates, issue.project)
     end
 

changelogs/unreleased/pl-add-incident-label-for-incidents.yml

+---
+title: Add incident label for manually created incident issues
+merge_request: 41598
+author:
+type: fixed

spec/factories/labels.rb

@@ -18,6 +18,13 @@ FactoryBot.define do
     title { "#{prefix}::#{generate(:label_title)}" }
   end
 
+  trait :incident do
+    properties = IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES
+    title { properties.fetch(:title) }
+    description { properties.fetch(:description) }
+    color { properties.fetch(:color) }
+  end
+
   factory :label, traits: [:base_label], class: 'ProjectLabel' do
     project
 

spec/factories/usage_data.rb

@@ -51,12 +51,11 @@ FactoryBot.define do
       create(:protected_branch, name: 'main', project: projects[0])
 
       # Incident Labeled Issues
-      incident_label_attrs = IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES
-      incident_label = create(:label, project: projects[0], **incident_label_attrs)
+      incident_label = create(:label, :incident, project: projects[0])
       create(:labeled_issue, project: projects[0], labels: [incident_label])
       incident_group = create(:group)
-      incident_label_scoped_to_project = create(:label, project: projects[1], **incident_label_attrs)
-      incident_label_scoped_to_group = create(:group_label, group: incident_group, **incident_label_attrs)
+      incident_label_scoped_to_project = create(:label, :incident, project: projects[1])
+      incident_label_scoped_to_group = create(:group_label, :incident, group: incident_group)
       create(:labeled_issue, project: projects[1], labels: [incident_label_scoped_to_project])
       create(:labeled_issue, project: projects[1], labels: [incident_label_scoped_to_group])
 

spec/services/incident_management/create_incident_label_service_spec.rb

@@ -10,9 +10,10 @@ RSpec.describe IncidentManagement::CreateIncidentLabelService do
   subject(:execute) { service.execute }
 
   describe 'execute' do
-    let(:title) { described_class::LABEL_PROPERTIES[:title] }
-    let(:color) { described_class::LABEL_PROPERTIES[:color] }
-    let(:description) { described_class::LABEL_PROPERTIES[:description] }
+    let(:incident_label_attributes) { attributes_for(:label, :incident) }
+    let(:title) { incident_label_attributes[:title] }
+    let(:color) { incident_label_attributes[:color] }
+    let(:description) { incident_label_attributes[:description] }
 
     shared_examples 'existing label' do
       it 'returns the existing label' do

spec/services/incident_management/incidents/create_service_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe IncidentManagement::Incidents::CreateService do
     context 'when incident has title and description' do
       let(:title) { 'Incident title' }
       let(:new_issue) { Issue.last! }
-      let(:label_title) { IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title] }
+      let(:label_title) { attributes_for(:label, :incident)[:title] }
 
       it 'responds with success' do
         expect(create_incident).to be_success
@@ -23,15 +23,20 @@ RSpec.describe IncidentManagement::Incidents::CreateService do
         expect { create_incident }.to change(Issue, :count).by(1)
       end
 
-      it 'created issue has correct attributes' do
+      it 'created issue has correct attributes', :aggregate_failures do
         create_incident
-        aggregate_failures do
-          expect(new_issue.title).to eq(title)
-          expect(new_issue.description).to eq(description)
-          expect(new_issue.author).to eq(user)
-          expect(new_issue.issue_type).to eq('incident')
-          expect(new_issue.labels.map(&:title)).to eq([label_title])
+
+        expect(new_issue.title).to eq(title)
+        expect(new_issue.description).to eq(description)
+        expect(new_issue.author).to eq(user)
+      end
+
+      it_behaves_like 'incident issue' do
+        before do
+          create_incident
         end
+
+        let(:issue) { new_issue }
       end
 
       context 'when incident label does not exists' do

spec/services/issues/create_service_spec.rb

@@ -3,18 +3,18 @@
 require 'spec_helper'
 
 RSpec.describe Issues::CreateService do
-  let(:project) { create(:project) }
-  let(:user) { create(:user) }
+  let_it_be_with_reload(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
 
   describe '#execute' do
+    let_it_be(:assignee) { create(:user) }
+    let_it_be(:milestone) { create(:milestone, project: project) }
     let(:issue) { described_class.new(project, user, opts).execute }
-    let(:assignee) { create(:user) }
-    let(:milestone) { create(:milestone, project: project) }
 
     context 'when params are valid' do
-      let(:labels) { create_pair(:label, project: project) }
+      let_it_be(:labels) { create_pair(:label, project: project) }
 
-      before do
+      before_all do
         project.add_maintainer(user)
         project.add_maintainer(assignee)
       end
@@ -49,16 +49,35 @@ RSpec.describe Issues::CreateService do
         end
       end
 
+      it_behaves_like 'not an incident issue'
+
       context 'issue is incident type' do
         before do
-          opts[:issue_type] = 'incident'
+          opts.merge!(issue_type: 'incident')
         end
 
         let(:current_user) { user }
+        let(:incident_label_attributes) { attributes_for(:label, :incident) }
 
         subject { issue }
 
+        it_behaves_like 'incident issue'
         it_behaves_like 'an incident management tracked event', :incident_management_incident_created
+
+        it 'does create an incident label' do
+          expect { subject }
+            .to change { Label.where(incident_label_attributes).count }.by(1)
+        end
+
+        context 'when invalid' do
+          before do
+            opts.merge!(title: '')
+          end
+
+          it 'does not create an incident label prematurely' do
+            expect { subject }.not_to change(Label, :count)
+          end
+        end
       end
 
       it 'refreshes the number of open issues', :use_clean_rails_memory_store_caching do
@@ -66,9 +85,9 @@ RSpec.describe Issues::CreateService do
       end
 
       context 'when current user cannot admin issues in the project' do
-        let(:guest) { create(:user) }
+        let_it_be(:guest) { create(:user) }
 
-        before do
+        before_all do
           project.add_guest(guest)
         end
 
@@ -263,7 +282,7 @@ RSpec.describe Issues::CreateService do
 
     context 'issue create service' do
       context 'assignees' do
-        before do
+        before_all do
           project.add_maintainer(user)
         end
 
@@ -329,7 +348,7 @@ RSpec.describe Issues::CreateService do
           }
         end
 
-        before do
+        before_all do
           project.add_maintainer(user)
           project.add_maintainer(assignee)
         end
@@ -343,11 +362,11 @@ RSpec.describe Issues::CreateService do
     end
 
     context 'resolving discussions' do
-      let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
-      let(:merge_request) { discussion.noteable }
-      let(:project) { merge_request.source_project }
+      let_it_be(:discussion) { create(:diff_note_on_merge_request).to_discussion }
+      let_it_be(:merge_request) { discussion.noteable }
+      let_it_be(:project) { merge_request.source_project }
 
-      before do
+      before_all do
         project.add_maintainer(user)
       end
 

spec/services/issues/update_service_spec.rb

@@ -78,6 +78,12 @@ RSpec.describe Issues::UpdateService, :mailer do
 
           expect(issue.severity).to eq(IssuableSeverity::DEFAULT)
         end
+
+        it_behaves_like 'not an incident issue' do
+          before do
+            update_issue(opts)
+          end
+        end
       end
 
       context 'when issue type is incident' do
@@ -88,6 +94,27 @@ RSpec.describe Issues::UpdateService, :mailer do
 
           expect(issue.severity).to eq('low')
         end
+
+        it_behaves_like 'incident issue' do
+          before do
+            update_issue(opts)
+          end
+        end
+
+        context 'with existing incident label' do
+          let_it_be(:incident_label) { create(:label, :incident, project: project) }
+
+          before do
+            opts.delete(:label_ids) # don't override but retain existing labels
+            issue.labels << incident_label
+          end
+
+          it_behaves_like 'incident issue' do
+            before do
+              update_issue(opts)
+            end
+          end
+        end
       end
 
       it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do
@@ -113,15 +140,37 @@ RSpec.describe Issues::UpdateService, :mailer do
       end
 
       context 'issue in incident type' do
+        let(:current_user) { user }
+        let(:incident_label_attributes) { attributes_for(:label, :incident) }
+
         before do
-          opts[:issue_type] = 'incident'
+          opts.merge!(issue_type: 'incident', confidential: true)
         end
 
-        let(:current_user) { user }
-
-        subject { update_issue(confidential: true) }
+        subject { update_issue(opts) }
 
         it_behaves_like 'an incident management tracked event', :incident_management_incident_change_confidential
+
+        it_behaves_like 'incident issue' do
+          before do
+            subject
+          end
+        end
+
+        it 'does create an incident label' do
+          expect { subject }
+            .to change { Label.where(incident_label_attributes).count }.by(1)
+        end
+
+        context 'when invalid' do
+          before do
+            opts.merge!(title: '')
+          end
+
+          it 'does not create an incident label prematurely' do
+            expect { subject }.not_to change(Label, :count)
+          end
+        end
       end
 
       it 'updates open issue counter for assignees when issue is reassigned' do

spec/support/shared_examples/services/incident_shared_examples.rb

+# frozen_string_literal: true
+
+# This shared_example requires the following variables:
+# - issue (required)
+#
+# Usage:
+#
+#   it_behaves_like 'incident issue' do
+#     let(:issue) { ... }
+#   end
+#
+#   include_examples 'incident issue'
+RSpec.shared_examples 'incident issue' do
+  let(:label_properties) { attributes_for(:label, :incident) }
+
+  it 'has incident as issue type' do
+    expect(issue.issue_type).to eq('incident')
+  end
+
+  it 'has exactly one incident label' do
+    expect(issue.labels).to be_one do |label|
+      label.slice(*label_properties.keys).symbolize_keys == label_properties
+    end
+  end
+end
+
+# This shared_example requires the following variables:
+# - issue (required)
+#
+# Usage:
+#
+#   it_behaves_like 'not an incident issue' do
+#     let(:issue) { ... }
+#   end
+#
+#   include_examples 'not an incident issue'
+RSpec.shared_examples 'not an incident issue' do
+  let(:label_properties) { attributes_for(:label, :incident) }
+
+  it 'has not incident as issue type' do
+    expect(issue.issue_type).not_to eq('incident')
+  end
+
+  it 'has not an incident label' do
+    expect(issue.labels).not_to include(have_attributes(label_properties))
+  end
+end