Merge branch '39601-create-issuable-destroy-service' into 'master'

Create issue and merge request destroy services

Closes #39601

See merge request gitlab-org/gitlab-ce!15604

app/controllers/concerns/issuable_actions.rb

@@ -54,7 +54,7 @@ module IssuableActions
   end
 
   def destroy
-    issuable.destroy
+    Issuable::DestroyService.new(project, current_user).execute(issuable)
     TodoService.new.destroy_issuable(issuable, current_user)
 
     name = issuable.human_class_name

app/models/issue.rb

@@ -49,7 +49,6 @@ class Issue < ActiveRecord::Base
   scope :public_only, -> { where(confidential: false) }
 
   after_save :expire_etag_cache
-  after_commit :update_project_counter_caches, on: :destroy
 
   attr_spammable :title, spam_title: true
   attr_spammable :description, spam_description: true

app/models/merge_request.rb

@@ -52,7 +52,6 @@ class MergeRequest < ActiveRecord::Base
 
   after_create :ensure_merge_request_diff, unless: :importing?
   after_update :reload_diff_if_branch_changed
-  after_commit :update_project_counter_caches, on: :destroy
 
   # When this attribute is true some MR validation is ignored
   # It allows us to close or modify broken merge requests

app/services/issuable/destroy_service.rb

@@ -0,0 +1,9 @@
+module Issuable
+  class DestroyService < IssuableBaseService
+    def execute(issuable)
+      if issuable.destroy
+        issuable.update_project_counter_caches
+      end
+    end
+  end
+end

changelogs/unreleased/39601-create-issuable-destroy-service.yml

@@ -0,0 +1,5 @@
+---
+title: Create issuable destroy service
+merge_request: 15604
+author: George Andrinopoulos
+type: other

lib/api/issues.rb

@@ -255,7 +255,9 @@ module API
 
         authorize!(:destroy_issue, issue)
 
-        destroy_conditionally!(issue)
+        destroy_conditionally!(issue) do |issue|
+          Issuable::DestroyService.new(user_project, current_user).execute(issue)
+        end
       end
 
       desc 'List merge requests closing issue'  do

lib/api/merge_requests.rb

@@ -167,7 +167,9 @@ module API
 
         authorize!(:destroy_merge_request, merge_request)
 
-        destroy_conditionally!(merge_request)
+        destroy_conditionally!(merge_request) do |merge_request|
+          Issuable::DestroyService.new(user_project, current_user).execute(merge_request)
+        end
       end
 
       params do

spec/services/issuable/destroy_service_spec.rb

@@ -0,0 +1,38 @@
+require 'spec_helper'
+
+describe Issuable::DestroyService do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  subject(:service) { described_class.new(project, user) }
+
+  describe '#execute' do
+    context 'when issuable is an issue' do
+      let!(:issue) { create(:issue, project: project, author: user) }
+
+      it 'destroys the issue' do
+        expect { service.execute(issue) }.to change { project.issues.count }.by(-1)
+      end
+
+      it 'updates open issues count cache' do
+        expect_any_instance_of(Projects::OpenIssuesCountService).to receive(:refresh_cache)
+
+        service.execute(issue)
+      end
+    end
+
+    context 'when issuable is a merge request' do
+      let!(:merge_request) { create(:merge_request, target_project: project, source_project: project, author: user) }
+
+      it 'destroys the merge request' do
+        expect { service.execute(merge_request) }.to change { project.merge_requests.count }.by(-1)
+      end
+
+      it 'updates open merge requests count cache' do
+        expect_any_instance_of(Projects::OpenMergeRequestsCountService).to receive(:refresh_cache)
+
+        service.execute(merge_request)
+      end
+    end
+  end
+end