Fixed that deleting a project with subtasks may fail (#11185).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9858 e93f8b46-1217-0410-a6f0-8f06a7374b81

3 files changed, 58 insertions, 0 deletions

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