From: Jean-Philippe Lang Date: Mon, 18 Jun 2012 18:17:49 +0000 (+0000) Subject: Merged r9858 from trunk. X-Git-Tag: 2.0.3~3 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=8d2d46bd8ea446a93f423c20f43f8d401223cf1d;p=redmine.git Merged r9858 from trunk. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/branches/2.0-stable@9859 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/models/issue.rb b/app/models/issue.rb index 118b8acf1..96c3f8526 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -124,6 +124,28 @@ class Issue < ActiveRecord::Base end end + # AR#Persistence#destroy would raise and RecordNotFound exception + # if the issue was already deleted or updated (non matching lock_version). + # This is a problem when bulk deleting issues or deleting a project + # (because an issue may already be deleted if its parent was deleted + # first). + # The issue is reloaded by the nested_set before being deleted so + # the lock_version condition should not be an issue but we handle it. + def destroy + super + rescue ActiveRecord::RecordNotFound + # Stale or already deleted + begin + reload + rescue ActiveRecord::RecordNotFound + # The issue was actually already deleted + @destroyed = true + return freeze + end + # The issue was stale, retry to destroy + super + end + # Overrides Redmine::Acts::Customizable::InstanceMethods#available_custom_fields def available_custom_fields (project && tracker) ? (project.all_issue_custom_fields & tracker.custom_fields.all) : [] diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 1e228cd38..b5799f3fc 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -761,6 +761,30 @@ class IssueTest < ActiveSupport::TestCase assert_nil TimeEntry.find_by_issue_id(1) end + def test_destroying_a_deleted_issue_should_not_raise_an_error + issue = Issue.find(1) + Issue.find(1).destroy + + assert_nothing_raised do + assert_no_difference 'Issue.count' do + issue.destroy + end + assert issue.destroyed? + end + end + + def test_destroying_a_stale_issue_should_not_raise_an_error + issue = Issue.find(1) + Issue.find(1).update_attribute :subject, "Updated" + + assert_nothing_raised do + assert_difference 'Issue.count', -1 do + issue.destroy + end + assert issue.destroyed? + end + end + def test_blocked blocked_issue = Issue.find(9) blocking_issue = Issue.find(10) diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb index 39ed5d058..64a2631ea 100644 --- a/test/unit/project_test.rb +++ b/test/unit/project_test.rb @@ -190,6 +190,18 @@ class ProjectTest < ActiveSupport::TestCase assert_nil Issue.first(:conditions => {:project_id => @ecookbook.id}) end + def test_destroy_should_destroy_subtasks + issues = (0..2).to_a.map {Issue.create!(:project_id => 1, :tracker_id => 1, :author_id => 1, :subject => 'test')} + issues[0].update_attribute :parent_issue_id, issues[1].id + issues[2].update_attribute :parent_issue_id, issues[1].id + assert_equal 2, issues[1].children.count + + assert_nothing_raised do + Project.find(1).destroy + end + assert Issue.find_all_by_id(issues.map(&:id)).empty? + end + def test_destroying_root_projects_should_clear_data Project.roots.each do |root| root.destroy