Background job for project deletion (#36691).

Due to the deletion of dependent objects (issues etc), project deletion may take a long time.

This patch moves the actual project deletion into an ActiveJob job. It also introduces a new project status (SCHEDULED_FOR_DELETION) that is used to effectively hide the project that is about to be deleted (and any potential descendant projects) from the system immediately.

A security notification is sent out to the user that deleted the project, informing about success / failure.

The projects list is extended to be able to filter for the new status, so in case of a failure, the project can still be accessed for examination.

Patch by Jens Krämer.


git-svn-id: https://svn.redmine.org/redmine/trunk@21591 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/projects_controller.rb

@@ -300,7 +300,8 @@ class ProjectsController < ApplicationController
 
     @project_to_destroy = @project
     if api_request? || params[:confirm] == @project_to_destroy.identifier
-      @project_to_destroy.destroy
+      DestroyProjectJob.schedule(@project_to_destroy)
+      flash[:notice] = l(:notice_successful_delete)
       respond_to do |format|
         format.html do
           redirect_to(

app/helpers/admin_helper.rb

@@ -22,7 +22,8 @@ module AdminHelper
     options_for_select([[l(:label_all), ''],
                         [l(:project_status_active), '1'],
                         [l(:project_status_closed), '5'],
-                        [l(:project_status_archived), '9']], selected.to_s)
+                        [l(:project_status_archived), '9'],
+                        [l(:project_status_scheduled_for_deletion), '10']], selected.to_s)
   end
 
   def plugin_data_for_updates(plugins)

app/jobs/application_job.rb

@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+
+class ApplicationJob < ActiveJob::Base
+end

app/jobs/destroy_project_job.rb

@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+class DestroyProjectJob < ApplicationJob
+  include Redmine::I18n
+
+  def self.schedule(project, user: User.current)
+    # make the project (and any children) disappear immediately
+    project.self_and_descendants.update_all status: Project::STATUS_SCHEDULED_FOR_DELETION
+    perform_later project.id, user.id, user.remote_ip
+  end
+
+  def perform(project_id, user_id, remote_ip)
+    user_current_was = User.current
+
+    unless @user = User.active.find_by_id(user_id)
+      info "User check failed: User #{user_id} triggering project destroy does not exist anymore or isn't active."
+      return
+    end
+    @user.remote_ip = remote_ip
+    User.current = @user
+    set_language_if_valid @user.language || Setting.default_language
+
+    unless @project = Project.find_by_id(project_id)
+      info "Project check failed: Project has already been deleted."
+      return
+    end
+
+    unless @project.deletable?
+      info "Project check failed: User #{user_id} lacks permissions."
+      return
+    end
+
+    message = if @project.descendants.any?
+                :mail_destroy_project_with_subprojects_successful
+              else
+                :mail_destroy_project_successful
+              end
+    delete_project ? success(message) : failure
+  ensure
+    User.current = user_current_was
+    info "End destroy project"
+  end
+
+  private
+
+  def delete_project
+    info "Starting with project deletion"
+    return !!@project.destroy
+  rescue
+    info "Error while deleting project: #{$!}"
+    false
+  end
+
+  def success(message)
+    Mailer.deliver_security_notification(
+      @user, @user,
+      message: message,
+      value: @project.name,
+      url: {controller: 'admin', action: 'projects'},
+      title: :label_project_plural
+    )
+  end
+
+  def failure
+    Mailer.deliver_security_notification(
+      @user, @user,
+      message: :mail_destroy_project_failed,
+      value: @project.name,
+      url: {controller: 'admin', action: 'projects'},
+      title: :label_project_plural
+    )
+  end
+
+  def info(msg)
+    Rails.logger.info("[DestroyProjectJob] --- #{msg}")
+  end
+end

app/models/project.rb

@@ -25,6 +25,7 @@ class Project < ActiveRecord::Base
   STATUS_ACTIVE     = 1
   STATUS_CLOSED     = 5
   STATUS_ARCHIVED   = 9
+  STATUS_SCHEDULED_FOR_DELETION = 10
 
   # Maximum length for project identifiers
   IDENTIFIER_MAX_LENGTH = 100
@@ -182,7 +183,7 @@ class Project < ActiveRecord::Base
     perm = Redmine::AccessControl.permission(permission)
     base_statement =
       if perm && perm.read?
-        "#{Project.table_name}.status <> #{Project::STATUS_ARCHIVED}"
+        "#{Project.table_name}.status <> #{Project::STATUS_ARCHIVED} AND #{Project.table_name}.status <> #{Project::STATUS_SCHEDULED_FOR_DELETION}"
       else
         "#{Project.table_name}.status = #{Project::STATUS_ACTIVE}"
       end
@@ -399,6 +400,10 @@ class Project < ActiveRecord::Base
     self.status == STATUS_ARCHIVED
   end
 
+  def scheduled_for_deletion?
+    self.status == STATUS_SCHEDULED_FOR_DELETION
+  end
+
   # Archives the project and its descendants
   def archive
     # Check that there is no issue of a non descendant project that is assigned

app/models/project_query.rb

@@ -111,6 +111,7 @@ class ProjectQuery < Query
     values = super
     if self.admin_projects
       values << [l(:project_status_archived), Project::STATUS_ARCHIVED.to_s]
+      values << [l(:project_status_scheduled_for_deletion), Project::STATUS_SCHEDULED_FOR_DELETION.to_s]
     end
     values
   end

app/views/context_menus/projects.html.erb

@@ -1,5 +1,5 @@
 <ul>
-  <% if @project %>
+  <% if @project && !@project.scheduled_for_deletion? %>
     <% if @project.archived? %>
       <li><%= context_menu_link l(:button_unarchive), unarchive_project_path(@project), method: :post, class: 'icon icon-unlock' %></li>
     <% else %>

app/views/projects/_list.html.erb

@@ -40,13 +40,13 @@
     </tr>
   <% end %>
   <tr id="project-<%= entry.id %>" class="<%= cycle('odd', 'even') %> hascontextmenu <%= entry.css_classes %> <%= level > 0 ? "idnt idnt-#{level}" : nil %>">
-    <% if @admin_list %>
+    <% if @admin_list && !entry.scheduled_for_deletion? %>
       <td class="checkbox hide-when-print"><%= check_box_tag("ids[]", entry.id, false, :id => nil) %></td>
     <% end %>
     <% @query.inline_columns.each do |column| %>
     <%= content_tag('td', column_content(column, entry), :class => column.css_classes) %>
     <% end %>
-    <% if @admin_list %>
+    <% if @admin_list && !entry.scheduled_for_deletion? %>
       <td class="buttons"><%= link_to_context_menu %></td>
     <% end %>
   </tr>

config/locales/en.yml

@@ -270,6 +270,10 @@ en:
   mail_body_security_notification_notify_disabled: "Email address %{value} no longer receives notifications."
   mail_body_settings_updated: "The following settings were changed:"
   mail_body_password_updated: "Your password has been changed."
+  mail_destroy_project_failed: Project %{value} could not be deleted.
+  mail_destroy_project_successful: Project %{value} was deleted successfully.
+  mail_destroy_project_with_subprojects_successful: Project %{value} and its subprojects were deleted successfully.
+
 
   field_name: Name
   field_description: Description
@@ -1197,6 +1201,7 @@ en:
   project_status_active: active
   project_status_closed: closed
   project_status_archived: archived
+  project_status_scheduled_for_deletion: scheduled for deletion
 
   version_status_open: open
   version_status_locked: locked

test/functional/projects_controller_test.rb

@@ -33,6 +33,7 @@ class ProjectsControllerTest < Redmine::ControllerTest
   def setup
     @request.session[:user_id] = nil
     Setting.default_language = 'en'
+    ActiveJob::Base.queue_adapter = :inline
   end
 
   def test_index_by_anonymous_should_not_show_private_projects
@@ -1118,6 +1119,25 @@ class ProjectsControllerTest < Redmine::ControllerTest
                             'eCookbook Subproject 2'].join(', ')
   end
 
+  def test_destroy_should_mark_project_and_subprojects_for_deletion
+    queue_adapter_was = ActiveJob::Base.queue_adapter
+    ActiveJob::Base.queue_adapter = :test
+    set_tmp_attachments_directory
+    @request.session[:user_id] = 1 # admin
+
+    assert_no_difference 'Project.count' do
+      delete(:destroy, :params => {:id => 1, :confirm => 'ecookbook'})
+      assert_redirected_to '/admin/projects'
+    end
+    assert p = Project.find_by_id(1)
+    assert_equal Project::STATUS_SCHEDULED_FOR_DELETION, p.status
+    p.descendants.each do |child|
+      assert_equal Project::STATUS_SCHEDULED_FOR_DELETION, child.status
+    end
+  ensure
+    ActiveJob::Base.queue_adapter = queue_adapter_was
+  end
+
   def test_destroy_with_confirmation_should_destroy_the_project_and_subprojects
     set_tmp_attachments_directory
     @request.session[:user_id] = 1 # admin

test/integration/api_test/projects_test.rb

@@ -20,6 +20,7 @@
 require File.expand_path('../../../test_helper', __FILE__)
 
 class Redmine::ApiTest::ProjectsTest < Redmine::ApiTest::Base
+  include ActiveJob::TestHelper
   fixtures :projects, :versions, :users, :roles, :members, :member_roles, :issues, :journals, :journal_details,
            :trackers, :projects_trackers, :issue_statuses, :enabled_modules, :enumerations, :boards, :messages,
            :attachments, :custom_fields, :custom_values, :custom_fields_projects, :time_entries, :issue_categories,
@@ -361,13 +362,16 @@ class Redmine::ApiTest::ProjectsTest < Redmine::ApiTest::Base
     assert_select 'errors error', :text => "Name cannot be blank"
   end
 
-  test "DELETE /projects/:id.xml should delete the project" do
-    assert_difference('Project.count', -1) do
+  test "DELETE /projects/:id.xml should schedule deletion of the project" do
+    assert_no_difference('Project.count') do
       delete '/projects/2.xml', :headers => credentials('admin')
     end
+    assert_enqueued_with(job: DestroyProjectJob,
+                         args: ->(job_args){ job_args[0] == 2})
     assert_response :no_content
     assert_equal '', @response.body
-    assert_nil Project.find_by_id(2)
+    assert p = Project.find_by_id(2)
+    assert_equal Project::STATUS_SCHEDULED_FOR_DELETION, p.status
   end
 
   test "PUT /projects/:id/archive.xml should archive project" do

test/unit/jobs/destroy_project_job_test.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-2022  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require File.expand_path('../../../test_helper', __FILE__)
+
+class DestroyProjectJobTest < ActiveJob::TestCase
+  fixtures :users, :projects, :email_addresses
+
+  setup do
+    @project = Project.find 1
+    @user = User.find_by_admin true
+  end
+
+  test "schedule should mark project and children for deletion" do
+    assert @project.descendants.any?
+    DestroyProjectJob.schedule @project, user: @user
+    @project.reload
+    assert_equal Project::STATUS_SCHEDULED_FOR_DELETION, @project.status
+    @project.descendants.each do |child|
+      assert_equal Project::STATUS_SCHEDULED_FOR_DELETION, child.status
+    end
+  end
+
+  test "schedule should enqueue job" do
+    DestroyProjectJob.schedule @project, user: @user
+    assert_enqueued_with(
+      job: DestroyProjectJob,
+      args: ->(job_args){
+        job_args[0] == @project.id &&
+        job_args[1] == @user.id
+      }
+    )
+  end
+
+  test "should destroy project and send email" do
+    assert_difference 'Project.count', -5 do
+      DestroyProjectJob.perform_now @project.id, @user.id, '127.0.0.1'
+    end
+    assert_enqueued_with(
+      job: ActionMailer::MailDeliveryJob,
+      args: ->(job_args){
+        job_args[1] == 'security_notification' &&
+        job_args[3].to_s.include?("mail_destroy_project_with_subprojects_successful")
+      }
+    )
+  end
+end