diff options
| -rw-r--r-- | app/controllers/concerns/issuable_actions.rb | 2 | ||||
| -rw-r--r-- | app/models/issue.rb | 1 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 1 | ||||
| -rw-r--r-- | app/services/issuable/destroy_service.rb | 9 | ||||
| -rw-r--r-- | changelogs/unreleased/39601-create-issuable-destroy-service.yml | 5 | ||||
| -rw-r--r-- | lib/api/issues.rb | 4 | ||||
| -rw-r--r-- | lib/api/merge_requests.rb | 4 | ||||
| -rw-r--r-- | spec/services/issuable/destroy_service_spec.rb | 38 |
8 files changed, 59 insertions, 5 deletions
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 072dffaff7a..f6d9f88032f 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/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 diff --git a/app/models/issue.rb b/app/models/issue.rb index a9863a50d84..d6ef58d150b 100644 --- a/app/models/issue.rb +++ b/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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2753e4b16e5..e4d8f486c77 100644 --- a/app/models/merge_request.rb +++ b/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 diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb new file mode 100644 index 00000000000..0610b401213 --- /dev/null +++ b/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 diff --git a/changelogs/unreleased/39601-create-issuable-destroy-service.yml b/changelogs/unreleased/39601-create-issuable-destroy-service.yml new file mode 100644 index 00000000000..b0463f02eba --- /dev/null +++ b/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 diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 74dfd9f96de..e60e00d7956 100644 --- a/lib/api/issues.rb +++ b/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 diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 5b4642a2f57..d34886fca2e 100644 --- a/lib/api/merge_requests.rb +++ b/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 diff --git a/spec/services/issuable/destroy_service_spec.rb b/spec/services/issuable/destroy_service_spec.rb new file mode 100644 index 00000000000..d74d98c6079 --- /dev/null +++ b/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 |