diff options
| -rw-r--r-- | app/controllers/issues_controller.rb | 15 | ||||
| -rw-r--r-- | app/models/issue.rb | 9 | ||||
| -rw-r--r-- | config/locales/en.yml | 1 | ||||
| -rw-r--r-- | config/locales/fr.yml | 1 | ||||
| -rw-r--r-- | test/functional/issues_controller_test.rb | 48 |
5 files changed, 68 insertions, 6 deletions
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index a4a3a34de..8686544ef 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -343,21 +343,28 @@ class IssuesController < ApplicationController def destroy raise Unauthorized unless @issues.all?(&:deletable?) - @hours = TimeEntry.where(:issue_id => @issues.map(&:id)).sum(:hours).to_f + + # all issues and their descendants are about to be deleted + issues_and_descendants_ids = Issue.self_and_descendants(@issues).pluck(:id) + time_entries = TimeEntry.where(:issue_id => issues_and_descendants_ids) + @hours = time_entries.sum(:hours).to_f + if @hours > 0 case params[:todo] when 'destroy' # nothing to do when 'nullify' - TimeEntry.where(['issue_id IN (?)', @issues]).update_all('issue_id = NULL') + time_entries.update_all(:issue_id => nil) when 'reassign' reassign_to = @project.issues.find_by_id(params[:reassign_to_id]) if reassign_to.nil? flash.now[:error] = l(:error_issue_not_found_in_project) return + elsif issues_and_descendants_ids.include?(reassign_to.id) + flash.now[:error] = l(:error_cannot_reassign_time_entries_to_an_issue_about_to_be_deleted) + return else - TimeEntry.where(['issue_id IN (?)', @issues]). - update_all("issue_id = #{reassign_to.id}") + time_entries.update_all(:issue_id => reassign_to.id) end else # display the destroy form if it's a user request diff --git a/app/models/issue.rb b/app/models/issue.rb index 65d0784ad..3e1988244 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1102,6 +1102,15 @@ class Issue < ActiveRecord::Base end end + # Returns a scope of the given issues and their descendants + def self.self_and_descendants(issues) + issue_ids = Issue.joins("JOIN #{Issue.table_name} ancestors" + + " ON ancestors.root_id = #{Issue.table_name}.root_id" + + " AND ancestors.lft <= #{Issue.table_name}.lft AND ancestors.rgt >= #{Issue.table_name}.rgt" + ). + where(:ancestors => {:id => issues.map(&:id)}) + end + # Finds an issue relation given its id. def find_relation(relation_id) IssueRelation.where("issue_to_id = ? OR issue_from_id = ?", id, id).find(relation_id) diff --git a/config/locales/en.yml b/config/locales/en.yml index b5f771d4b..deece3a17 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -218,6 +218,7 @@ en: error_no_tracker_allowed_for_new_issue_in_project: "The project doesn't have any trackers for which you can create an issue" error_no_projects_with_tracker_allowed_for_new_issue: "There are no projects with trackers for which you can create an issue" error_move_of_child_not_possible: "Subtask %{child} could not be moved to the new project: %{errors}" + error_cannot_reassign_time_entries_to_an_issue_about_to_be_deleted: "Spent time cannot be reassigned to an issue that is about to be deleted" mail_subject_lost_password: "Your %{value} password" mail_body_lost_password: 'To change your password, click on the following link:' diff --git a/config/locales/fr.yml b/config/locales/fr.yml index e23ab86da..3996eccac 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -238,6 +238,7 @@ fr: error_no_tracker_allowed_for_new_issue_in_project: "Le projet ne dispose d'aucun tracker sur lequel vous pouvez créer une demande" error_no_projects_with_tracker_allowed_for_new_issue: "Aucun projet ne dispose d'un tracker sur lequel vous pouvez créer une demande" error_move_of_child_not_possible: "La sous-tâche %{child} n'a pas pu être déplacée dans le nouveau projet : %{errors}" + error_cannot_reassign_time_entries_to_an_issue_about_to_be_deleted: "Le temps passé ne peut pas être réaffecté à une demande qui va être supprimée" mail_subject_lost_password: "Votre mot de passe %{value}" mail_body_lost_password: 'Pour changer votre mot de passe, cliquez sur le lien suivant :' diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 3e1bfa234..f51a890d0 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -4609,7 +4609,7 @@ class IssuesControllerTest < Redmine::ControllerTest assert_response :success end - def test_destroy_issue_with_no_time_entries + def test_destroy_issue_with_no_time_entries_should_delete_the_issues assert_nil TimeEntry.find_by_issue_id(2) @request.session[:user_id] = 2 @@ -4620,7 +4620,7 @@ class IssuesControllerTest < Redmine::ControllerTest assert_nil Issue.find_by_id(2) end - def test_destroy_issues_with_time_entries + def test_destroy_issues_with_time_entries_should_show_the_reassign_form @request.session[:user_id] = 2 assert_no_difference 'Issue.count' do @@ -4633,6 +4633,20 @@ class IssuesControllerTest < Redmine::ControllerTest end end + def test_destroy_issues_with_time_entries_should_show_hours_on_issues_and_descendants + parent = Issue.generate_with_child! + TimeEntry.generate!(:issue => parent) + TimeEntry.generate!(:issue => parent.children.first) + leaf = Issue.generate! + TimeEntry.generate!(:issue => leaf) + @request.session[:user_id] = 2 + + delete :destroy, :ids => [parent.id, leaf.id] + assert_response :success + + assert_select 'p', :text => /3\.00 hours were reported/ + end + def test_destroy_issues_and_destroy_time_entries @request.session[:user_id] = 2 @@ -4674,6 +4688,24 @@ class IssuesControllerTest < Redmine::ControllerTest assert_equal 2, TimeEntry.find(2).issue_id end + def test_destroy_issues_with_time_entries_should_reassign_time_entries_of_issues_and_descendants + parent = Issue.generate_with_child! + TimeEntry.generate!(:issue => parent) + TimeEntry.generate!(:issue => parent.children.first) + leaf = Issue.generate! + TimeEntry.generate!(:issue => leaf) + target = Issue.generate! + @request.session[:user_id] = 2 + + assert_difference 'Issue.count', -3 do + assert_no_difference 'TimeEntry.count' do + delete :destroy, :ids => [parent.id, leaf.id], :todo => 'reassign', :reassign_to_id => target.id + assert_response 302 + end + end + assert_equal 3, target.time_entries.count + end + def test_destroy_issues_and_reassign_time_entries_to_an_invalid_issue_should_fail @request.session[:user_id] = 2 @@ -4686,6 +4718,18 @@ class IssuesControllerTest < Redmine::ControllerTest assert_response :success end + def test_destroy_issues_and_reassign_time_entries_to_an_issue_to_delete_should_fail + @request.session[:user_id] = 2 + + assert_no_difference 'Issue.count' do + assert_no_difference 'TimeEntry.count' do + delete :destroy, :ids => [1, 3], :todo => 'reassign', :reassign_to_id => 3 + end + end + assert_response :success + assert_select '#flash_error', :text => I18n.t(:error_cannot_reassign_time_entries_to_an_issue_about_to_be_deleted) + end + def test_destroy_issues_from_different_projects @request.session[:user_id] = 2 |