Extract creating incidents into a separate class

Extract `Incidents::CreateService` from create incident issue classes

app/services/alert_management/create_alert_issue_service.rb

@@ -16,9 +16,9 @@ module AlertManagement
       return error_issue_already_exists if alert.issue
 
       result = create_issue
-      issue = result.payload[:issue]
+      return result unless result.success?
 
-      return error(result.message, issue) if result.error?
+      issue = result.payload[:issue]
       return error(object_errors(alert), issue) unless associate_alert_with_issue(issue)
 
       SystemNoteService.new_alert_issue(alert, issue, user)
@@ -37,29 +37,18 @@ module AlertManagement
     end
 
     def create_issue
-      label_result = find_or_create_incident_label
-
-      issue = Issues::CreateService.new(
+      ::IncidentManagement::Incidents::CreateService.new(
         project,
         user,
         title: alert_presenter.title,
-        description: alert_presenter.issue_description,
-        label_ids: [label_result.payload[:label].id]
+        description: alert_presenter.issue_description
       ).execute
-
-      return error(object_errors(issue), issue) unless issue.valid?
-
-      success(issue)
     end
 
     def associate_alert_with_issue(issue)
       alert.update(issue_id: issue.id)
     end
 
-    def success(issue)
-      ServiceResponse.success(payload: { issue: issue })
-    end
-
     def error(message, issue = nil)
       ServiceResponse.error(payload: { issue: issue }, message: message)
     end
@@ -78,10 +67,6 @@ module AlertManagement
       end
     end
 
-    def find_or_create_incident_label
-      IncidentManagement::CreateIncidentLabelService.new(project, user).execute
-    end
-
     def object_errors(object)
       object.errors.full_messages.to_sentence
     end

app/services/incident_management/create_issue_service.rb

@@ -3,6 +3,7 @@
 module IncidentManagement
   class CreateIssueService < BaseService
     include Gitlab::Utils::StrongMemoize
+    include IncidentManagement::Settings
 
     def initialize(project, params)
       super(project, User.alert_bot, params)
@@ -12,23 +13,20 @@ module IncidentManagement
       return error_with('setting disabled') unless incident_management_setting.create_issue?
       return error_with('invalid alert') unless alert.valid?
 
-      issue = create_issue
-      return error_with(issue_errors(issue)) unless issue.valid?
+      result = create_issue
+      return error_with(result.message) unless result.success?
 
-      success(issue: issue)
+      success(issue: result.payload[:issue])
     end
 
     private
 
     def create_issue
-      label_result = find_or_create_incident_label
-
-      Issues::CreateService.new(
+      ::IncidentManagement::Incidents::CreateService.new(
         project,
         current_user,
         title: issue_title,
-        description: issue_description,
-        label_ids: [label_result.payload[:label].id]
+        description: issue_description
       ).execute
     end
 
@@ -46,10 +44,6 @@ module IncidentManagement
       ].compact.join(horizontal_line)
     end
 
-    def find_or_create_incident_label
-      IncidentManagement::CreateIncidentLabelService.new(project, current_user).execute
-    end
-
     def alert_summary
       alert.issue_summary_markdown
     end
@@ -68,17 +62,6 @@ module IncidentManagement
       incident_management_setting.issue_template_content
     end
 
-    def incident_management_setting
-      strong_memoize(:incident_management_setting) do
-        project.incident_management_setting ||
-          project.build_incident_management_setting
-      end
-    end
-
-    def issue_errors(issue)
-      issue.errors.full_messages.to_sentence
-    end
-
     def error_with(message)
       log_error(%{Cannot create incident issue for "#{project.full_name}": #{message}})
 

app/services/incident_management/incidents/create_service.rb

+# frozen_string_literal: true
+
+module IncidentManagement
+  module Incidents
+    class CreateService < BaseService
+      def initialize(project, current_user, title:, description:)
+        super(project, current_user)
+
+        @title = title
+        @description = description
+      end
+
+      def execute
+        issue = Issues::CreateService.new(
+          project,
+          current_user,
+          title: title,
+          description: description,
+          label_ids: [find_or_create_incident_label.id]
+        ).execute
+
+        return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid?
+
+        success(issue)
+      end
+
+      private
+
+      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
+
+      def error(message, issue = nil)
+        ServiceResponse.error(payload: { issue: issue }, message: message)
+      end
+    end
+  end
+end

app/services/incident_management/pager_duty/create_incident_issue_service.rb

@@ -12,10 +12,7 @@ module IncidentManagement
       def execute
         return forbidden unless webhook_available?
 
-        issue = create_issue
-        return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid?
-
-        success(issue)
+        create_issue
       end
 
       private
@@ -23,14 +20,11 @@ module IncidentManagement
       alias_method :incident_payload, :params
 
       def create_issue
-        label_result = find_or_create_incident_label
-
-        Issues::CreateService.new(
+        ::IncidentManagement::Incidents::CreateService.new(
           project,
           current_user,
           title: issue_title,
-          description: issue_description,
-          label_ids: [label_result.payload[:label].id]
+          description: issue_description
         ).execute
       end
 
@@ -42,10 +36,6 @@ module IncidentManagement
         ServiceResponse.error(message: 'Forbidden', http_status: :forbidden)
       end
 
-      def find_or_create_incident_label
-        ::IncidentManagement::CreateIncidentLabelService.new(project, current_user).execute
-      end
-
       def issue_title
         incident_payload['title']
       end
@@ -53,14 +43,6 @@ module IncidentManagement
       def issue_description
         Gitlab::IncidentManagement::PagerDuty::IncidentIssueDescription.new(incident_payload).to_s
       end
-
-      def success(issue)
-        ServiceResponse.success(payload: { issue: issue })
-      end
-
-      def error(message, issue = nil)
-        ServiceResponse.error(payload: { issue: issue }, message: message)
-      end
     end
   end
 end

spec/services/alert_management/create_alert_issue_service_spec.rb

@@ -88,7 +88,6 @@ RSpec.describe AlertManagement::CreateAlertIssueService do
 
         it_behaves_like 'creating an alert issue'
         it_behaves_like 'setting an issue attributes'
-        it_behaves_like 'create alert issue sets issue labels'
       end
 
       context 'when the alert is generic' do
@@ -97,7 +96,6 @@ RSpec.describe AlertManagement::CreateAlertIssueService do
 
         it_behaves_like 'creating an alert issue'
         it_behaves_like 'setting an issue attributes'
-        it_behaves_like 'create alert issue sets issue labels'
       end
 
       context 'when issue cannot be created' do

spec/services/incident_management/create_issue_service_spec.rb

@@ -197,10 +197,6 @@ RSpec.describe IncidentManagement::CreateIssueService do
         it_behaves_like 'invalid alert'
       end
     end
-
-    describe "label `incident`" do
-      it_behaves_like 'create alert issue sets issue labels'
-    end
   end
 
   context 'when create_issue disabled' do

spec/services/incident_management/incidents/create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::Incidents::CreateService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { User.alert_bot }
+  let(:description) { 'Incident description' }
+
+  describe '#execute' do
+    subject(:create_incident) { described_class.new(project, user, title: title, description: description).execute }
+
+    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] }
+
+      it 'responds with success' do
+        expect(create_incident).to be_success
+      end
+
+      it 'creates an incident issue' do
+        expect { create_incident }.to change(Issue, :count).by(1)
+      end
+
+      it 'created issue has correct attributes' do
+        create_incident
+
+        expect(new_issue.title).to eq(title)
+        expect(new_issue.description).to eq(description)
+        expect(new_issue.author).to eq(user)
+        expect(new_issue.labels.map(&:title)).to eq([label_title])
+      end
+
+      context 'when incident label does not exists' do
+        it 'creates incident label' do
+          expect { create_incident }.to change { project.labels.where(title: label_title).count }.by(1)
+        end
+      end
+
+      context 'when incident label already exists' do
+        let!(:label) { create(:label, project: project, title: label_title) }
+
+        it 'does not create new labels' do
+          expect { create_incident }.not_to change(Label, :count)
+        end
+      end
+    end
+
+    context 'when incident has no title' do
+      let(:title) { '' }
+
+      it 'does not create an issue' do
+        expect { create_incident }.not_to change(Issue, :count)
+      end
+
+      it 'responds with errors' do
+        expect(create_incident).to be_error
+        expect(create_incident.message).to eq("Title can't be blank")
+      end
+
+      it 'result payload contains an Issue object' do
+        expect(create_incident.payload[:issue]).to be_kind_of(Issue)
+      end
+    end
+  end
+end

spec/support/shared_examples/create_alert_issue_shared_examples.rb

-# frozen_string_literal: true
-
-RSpec.shared_examples 'create alert issue sets issue labels' do
-  let(:title) { IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title] }
-  let!(:label) { create(:label, project: project, title: title) }
-  let(:label_service) { instance_double(IncidentManagement::CreateIncidentLabelService, execute: label_service_response) }
-
-  before do
-    allow(IncidentManagement::CreateIncidentLabelService).to receive(:new).with(project, user).and_return(label_service)
-  end
-
-  context 'when create incident label responds with success' do
-    let(:label_service_response) { ServiceResponse.success(payload: { label: label }) }
-
-    it 'adds label to issue' do
-      expect(issue.labels).to eq([label])
-    end
-  end
-end