Merge branch 'jk-delete-epic-backport' into 'master'

Refactor issuable destroy action

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

app/controllers/concerns/issuable_actions.rb

@@ -57,12 +57,11 @@ module IssuableActions
 
   def destroy
     issuable.destroy
-    destroy_method = "destroy_#{issuable.class.name.underscore}".to_sym
-    TodoService.new.public_send(destroy_method, issuable, current_user) # rubocop:disable GitlabSecurity/PublicSend
+    TodoService.new.destroy_issuable(issuable, current_user)
 
     name = issuable.human_class_name
     flash[:notice] = "The #{name} was successfully deleted."
-    index_path = polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable.class])
+    index_path = polymorphic_path([parent, issuable.class])
 
     respond_to do |format|
       format.html { redirect_to index_path }
@@ -164,4 +163,8 @@ module IssuableActions
   def update_service
     raise NotImplementedError
   end
+
+  def parent
+    @project || @group
+  end
 end

app/services/todo_service.rb

@@ -31,12 +31,12 @@ class TodoService
     mark_pending_todos_as_done(issue, current_user)
   end
 
-  # When we destroy an issue we should:
+  # When we destroy an issuable we should:
   #
   #  * refresh the todos count cache for the current user
   #
-  def destroy_issue(issue, current_user)
-    destroy_issuable(issue, current_user)
+  def destroy_issuable(issuable, user)
+    user.update_todos_count_cache
   end
 
   # When we reassign an issue we should:
@@ -72,14 +72,6 @@ class TodoService
     mark_pending_todos_as_done(merge_request, current_user)
   end
 
-  # When we destroy a merge request we should:
-  #
-  #  * refresh the todos count cache for the current user
-  #
-  def destroy_merge_request(merge_request, current_user)
-    destroy_issuable(merge_request, current_user)
-  end
-
   # When we reassign a merge request we should:
   #
   #  * creates a pending todo for new assignee if merge request is assigned
@@ -234,10 +226,6 @@ class TodoService
     create_mention_todos(issuable.project, issuable, author, nil, skip_users)
   end
 
-  def destroy_issuable(issuable, user)
-    user.update_todos_count_cache
-  end
-
   def toggling_tasks?(issuable)
     issuable.previous_changes.include?('description') &&
       issuable.tasks? && issuable.updated_tasks.any?

app/services/users/migrate_to_ghost_user_service.rb

@@ -25,7 +25,7 @@ module Users
         user.block
 
         # Reverse the user block if record migration fails
-        if !migrate_records && transition
+        if !migrate_records_in_transaction && transition
           transition.rollback
           user.save!
         end
@@ -36,17 +36,21 @@ module Users
 
     private
 
-    def migrate_records
+    def migrate_records_in_transaction
       user.transaction(requires_new: true) do
         @ghost_user = User.ghost
 
+        migrate_records
+      end
+    end
+
+    def migrate_records
       migrate_issues
       migrate_merge_requests
       migrate_notes
       migrate_abuse_reports
       migrate_award_emojis
     end
-    end
 
     def migrate_issues
       user.issues.update_all(author_id: ghost_user.id)

spec/controllers/projects/issues_controller_spec.rb

@@ -861,7 +861,7 @@ describe Projects::IssuesController do
       end
 
       it 'delegates the update of the todos count cache to TodoService' do
-        expect_any_instance_of(TodoService).to receive(:destroy_issue).with(issue, owner).once
+        expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(issue, owner).once
 
         delete :destroy, namespace_id: project.namespace, project_id: project, id: issue.iid
       end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -456,7 +456,7 @@ describe Projects::MergeRequestsController do
       end
 
       it 'delegates the update of the todos count cache to TodoService' do
-        expect_any_instance_of(TodoService).to receive(:destroy_merge_request).with(merge_request, owner).once
+        expect_any_instance_of(TodoService).to receive(:destroy_issuable).with(merge_request, owner).once
 
         delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid
       end

spec/services/todo_service_spec.rb

@@ -248,11 +248,11 @@ describe TodoService do
       end
     end
 
-    describe '#destroy_issue' do
+    describe '#destroy_issuable' do
       it 'refresh the todos count cache for the user' do
         expect(john_doe).to receive(:update_todos_count_cache).and_call_original
 
-        service.destroy_issue(issue, john_doe)
+        service.destroy_issuable(issue, john_doe)
       end
     end
 
@@ -643,14 +643,6 @@ describe TodoService do
       end
     end
 
-    describe '#destroy_merge_request' do
-      it 'refresh the todos count cache for the user' do
-        expect(john_doe).to receive(:update_todos_count_cache).and_call_original
-
-        service.destroy_merge_request(mr_assigned, john_doe)
-      end
-    end
-
     describe '#reassigned_merge_request' do
       it 'creates a pending todo for new assignee' do
         mr_unassigned.update_attribute(:assignee, john_doe)