diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index b1ed973d178..9c222549cdc 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/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 diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index b6125cafa83..e694c5761da 100644 --- a/app/services/todo_service.rb +++ b/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? diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb index 3a9c151cf9b..976017dfa82 100644 --- a/app/services/users/migrate_to_ghost_user_service.rb +++ b/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,18 +36,22 @@ module Users private - def migrate_records + def migrate_records_in_transaction user.transaction(requires_new: true) do @ghost_user = User.ghost - migrate_issues - migrate_merge_requests - migrate_notes - migrate_abuse_reports - migrate_award_emojis + migrate_records end end + def migrate_records + migrate_issues + migrate_merge_requests + migrate_notes + migrate_abuse_reports + migrate_award_emojis + end + def migrate_issues user.issues.update_all(author_id: ghost_user.id) Issue.where(last_edited_by_id: user.id).update_all(last_edited_by_id: ghost_user.id) diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 8016176110e..4dbbaecdd6d 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/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 diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 14021b8ca50..bfdad85c082 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/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 diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index a9b34a5258a..dc2673abc73 100644 --- a/spec/services/todo_service_spec.rb +++ b/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)