From 41fa2f1eb794ddc212bedf00ca6c57b6ee704b36 Mon Sep 17 00:00:00 2001
From: Eric Davis <edavis@littlestreamsoftware.com>
Date: Wed, 1 Jun 2011 20:53:18 -0700
Subject: [PATCH] [#441] Journals should touch their journaled to update updated_on/at fields

---
 app/models/issue.rb                                |   14 +++++
 app/models/journal.rb                              |    2 +-
 test/unit/issue_nested_set_test.rb                 |   62 ++++++++++----------
 test/unit/issue_test.rb                            |    4 +
 test/unit/journal_test.rb                          |   17 +++++-
 test/unit/mail_handler_test.rb                     |    6 +-
 .../lib/redmine/acts/journalized/reversion.rb      |    2 +-
 7 files changed, 72 insertions(+), 35 deletions(-)

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 311ddab..4d7e95d 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -529,6 +529,20 @@ class Issue < ActiveRecord::Base
     "#{tracker} ##{id}: #{subject}"
   end
 
+  # The number of "items" this issue spans in it's nested set
+  #
+  # A parent issue would span all of it's children + 1 left + 1 right (3)
+  #
+  #   |  parent |
+  #   || child ||
+  #
+  # A child would span only itself (1)
+  #
+  #   |child|
+  def nested_set_span
+    rgt - lft
+  end
+  
   # Returns a string of css classes that apply to the issue
   def css_classes
     s = "issue status-#{status.position} priority-#{priority.position}"
diff --git a/app/models/journal.rb b/app/models/journal.rb
index dec8388..8070d78 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -23,7 +23,7 @@ class Journal < ActiveRecord::Base
 
   # Make sure each journaled model instance only has unique version ids
   validates_uniqueness_of :version, :scope => [:journaled_id, :type]
-  belongs_to :journaled
+  belongs_to :journaled, :touch => true
   belongs_to :user
 
   # ActiveRecord::Base#changes is an existing method, so before serializing the +changes+ column,
diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb
index f78c8b7..28f4793 100644
--- a/test/unit/issue_nested_set_test.rb
+++ b/test/unit/issue_nested_set_test.rb
@@ -30,8 +30,10 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     issue1.reload
     issue2.reload
 
-    assert_equal [issue1.id, nil, 1, 2], [issue1.root_id, issue1.parent_id, issue1.lft, issue1.rgt]
-    assert_equal [issue2.id, nil, 1, 2], [issue2.root_id, issue2.parent_id, issue2.lft, issue2.rgt]
+    assert_equal issue1.id, issue1.root_id
+    assert issue1.leaf?
+    assert_equal issue2.id, issue2.root_id
+    assert issue2.leaf?
   end
 
   def test_create_child_issue
@@ -40,8 +42,8 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent.reload
     child.reload
 
-    assert_equal [parent.id, nil, 1, 4], [parent.root_id, parent.parent_id, parent.lft, parent.rgt]
-    assert_equal [parent.id, parent.id, 2, 3], [child.root_id, child.parent_id, child.lft, child.rgt]
+    assert_equal [parent.id, nil, 3], [parent.root_id, parent.parent_id, parent.rgt - parent.lft]
+    assert_equal [parent.id, parent.id, 1], [child.root_id, child.parent_id, child.rgt - child.lft]
   end
 
   def test_creating_a_child_in_different_project_should_not_validate
@@ -62,9 +64,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent1.reload
     parent2.reload
 
-    assert_equal [parent1.id, 1, 6], [parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [parent1.id, 4, 5], [parent2.root_id, parent2.lft, parent2.rgt]
-    assert_equal [parent1.id, 2, 3], [child.root_id, child.lft, child.rgt]
+    assert_equal [parent1.id, 5], [parent1.root_id, parent1.nested_set_span]
+    assert_equal [parent1.id, 1], [parent2.root_id, parent2.nested_set_span]
+    assert_equal [parent1.id, 1], [child.root_id, child.nested_set_span]
   end
 
   def test_move_a_child_to_root
@@ -78,9 +80,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent1.reload
     parent2.reload
 
-    assert_equal [parent1.id, 1, 2], [parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [parent2.id, 1, 2], [parent2.root_id, parent2.lft, parent2.rgt]
-    assert_equal [child.id, 1, 2], [child.root_id, child.lft, child.rgt]
+    assert_equal [parent1.id, 1], [parent1.root_id, parent1.nested_set_span]
+    assert_equal [parent2.id, 1], [parent2.root_id, parent2.nested_set_span]
+    assert_equal [child.id, 1], [child.root_id, child.nested_set_span]
   end
 
   def test_move_a_child_to_another_issue
@@ -94,9 +96,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent1.reload
     parent2.reload
 
-    assert_equal [parent1.id, 1, 2], [parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [parent2.id, 1, 4], [parent2.root_id, parent2.lft, parent2.rgt]
-    assert_equal [parent2.id, 2, 3], [child.root_id, child.lft, child.rgt]
+    assert_equal [parent1.id, 1], [parent1.root_id, parent1.nested_set_span]
+    assert_equal [parent2.id, 3], [parent2.root_id, parent2.nested_set_span]
+    assert_equal [parent2.id, 1], [child.root_id, child.nested_set_span]
   end
 
   def test_move_a_child_with_descendants_to_another_issue
@@ -110,10 +112,10 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     child.reload
     grandchild.reload
 
-    assert_equal [parent1.id, 1, 6], [parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [parent2.id, 1, 2], [parent2.root_id, parent2.lft, parent2.rgt]
-    assert_equal [parent1.id, 2, 5], [child.root_id, child.lft, child.rgt]
-    assert_equal [parent1.id, 3, 4], [grandchild.root_id, grandchild.lft, grandchild.rgt]
+    assert_equal [parent1.id, 5], [parent1.root_id, parent1.nested_set_span]
+    assert_equal [parent2.id, 1], [parent2.root_id, parent2.nested_set_span]
+    assert_equal [parent1.id, 3], [child.root_id, child.nested_set_span]
+    assert_equal [parent1.id, 1], [grandchild.root_id, grandchild.nested_set_span]
 
     child.reload.parent_issue_id = parent2.id
     child.save!
@@ -122,10 +124,10 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent1.reload
     parent2.reload
 
-    assert_equal [parent1.id, 1, 2], [parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [parent2.id, 1, 6], [parent2.root_id, parent2.lft, parent2.rgt]
-    assert_equal [parent2.id, 2, 5], [child.root_id, child.lft, child.rgt]
-    assert_equal [parent2.id, 3, 4], [grandchild.root_id, grandchild.lft, grandchild.rgt]
+    assert_equal [parent1.id, 1], [parent1.root_id, parent1.nested_set_span]
+    assert_equal [parent2.id, 5], [parent2.root_id, parent2.nested_set_span]
+    assert_equal [parent2.id, 3], [child.root_id, child.nested_set_span]
+    assert_equal [parent2.id, 1], [grandchild.root_id, grandchild.nested_set_span]
   end
 
   def test_move_a_child_with_descendants_to_another_project
@@ -138,9 +140,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     grandchild.reload
     parent1.reload
 
-    assert_equal [1, parent1.id, 1, 2], [parent1.project_id, parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [2, child.id, 1, 4], [child.project_id, child.root_id, child.lft, child.rgt]
-    assert_equal [2, child.id, 2, 3], [grandchild.project_id, grandchild.root_id, grandchild.lft, grandchild.rgt]
+    assert_equal [1, parent1.id, 1], [parent1.project_id, parent1.root_id, parent1.nested_set_span]
+    assert_equal [2, child.id, 3], [child.project_id, child.root_id, child.nested_set_span]
+    assert_equal [2, child.id, 1], [grandchild.project_id, grandchild.root_id, grandchild.nested_set_span]
   end
 
   def test_invalid_move_to_another_project
@@ -150,7 +152,7 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     Project.find(2).tracker_ids = [1]
 
     parent1.reload
-    assert_equal [1, parent1.id, 1, 6], [parent1.project_id, parent1.root_id, parent1.lft, parent1.rgt]
+    assert_equal [1, parent1.id, 5], [parent1.project_id, parent1.root_id, parent1.nested_set_span]
 
     # child can not be moved to Project 2 because its child is on a disabled tracker
     assert_equal false, Issue.find(child.id).move_to_project(Project.find(2))
@@ -159,9 +161,9 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     parent1.reload
 
     # no change
-    assert_equal [1, parent1.id, 1, 6], [parent1.project_id, parent1.root_id, parent1.lft, parent1.rgt]
-    assert_equal [1, parent1.id, 2, 5], [child.project_id, child.root_id, child.lft, child.rgt]
-    assert_equal [1, parent1.id, 3, 4], [grandchild.project_id, grandchild.root_id, grandchild.lft, grandchild.rgt]
+    assert_equal [1, parent1.id, 5], [parent1.project_id, parent1.root_id, parent1.nested_set_span]
+    assert_equal [1, parent1.id, 3], [child.project_id, child.root_id, child.nested_set_span]
+    assert_equal [1, parent1.id, 1], [grandchild.project_id, grandchild.root_id, grandchild.nested_set_span]
   end
 
   def test_moving_an_issue_to_a_descendant_should_not_validate
@@ -212,8 +214,8 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     issue4.reload
     assert !Issue.exists?(issue2.id)
     assert !Issue.exists?(issue3.id)
-    assert_equal [issue1.id, 1, 4], [issue1.root_id, issue1.lft, issue1.rgt]
-    assert_equal [issue1.id, 2, 3], [issue4.root_id, issue4.lft, issue4.rgt]
+    assert_equal [issue1.id, 3], [issue1.root_id, issue1.nested_set_span]
+    assert_equal [issue1.id, 1], [issue4.root_id, issue4.nested_set_span]
   end
 
   def test_destroy_parent_issue_updated_during_children_destroy
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 5f26b2c..b34e848 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -372,6 +372,7 @@ class IssueTest < ActiveSupport::TestCase
   def test_move_to_another_project_should_clear_fixed_version_when_not_shared
     issue = Issue.find(1)
     issue.update_attribute(:fixed_version_id, 1)
+    issue.reload
     assert issue.move_to_project(Project.find(2))
     issue.reload
     assert_equal 2, issue.project_id
@@ -382,6 +383,7 @@ class IssueTest < ActiveSupport::TestCase
   def test_move_to_another_project_should_keep_fixed_version_when_shared_with_the_target_project
     issue = Issue.find(1)
     issue.update_attribute(:fixed_version_id, 4)
+    issue.reload
     assert issue.move_to_project(Project.find(5))
     issue.reload
     assert_equal 5, issue.project_id
@@ -392,6 +394,7 @@ class IssueTest < ActiveSupport::TestCase
   def test_move_to_another_project_should_clear_fixed_version_when_not_shared_with_the_target_project
     issue = Issue.find(1)
     issue.update_attribute(:fixed_version_id, 1)
+    issue.reload
     assert issue.move_to_project(Project.find(5))
     issue.reload
     assert_equal 5, issue.project_id
@@ -402,6 +405,7 @@ class IssueTest < ActiveSupport::TestCase
   def test_move_to_another_project_should_keep_fixed_version_when_shared_systemwide
     issue = Issue.find(1)
     issue.update_attribute(:fixed_version_id, 7)
+    issue.reload
     assert issue.move_to_project(Project.find(2))
     issue.reload
     assert_equal 2, issue.project_id
diff --git a/test/unit/journal_test.rb b/test/unit/journal_test.rb
index 6c3de0d..6274a2d 100644
--- a/test/unit/journal_test.rb
+++ b/test/unit/journal_test.rb
@@ -36,12 +36,13 @@ class JournalTest < ActiveSupport::TestCase
     ActionMailer::Base.deliveries.clear
     issue = Issue.find(:first)
     if issue.journals.empty?
-      issue.init_journal(User.current, "This journal represents the creational journal version 1")
+      issue.init_journal(User.current, "This journal represents the creationa of journal version 1")
       issue.save
     end
     user = User.find(:first)
 
     assert_equal 0, ActionMailer::Base.deliveries.size
+    issue.reload
     issue.update_attribute(:subject, "New subject to trigger automatic journal entry")
     assert_equal 1, ActionMailer::Base.deliveries.size
   end
@@ -58,4 +59,18 @@ class JournalTest < ActiveSupport::TestCase
     end
     assert_equal 0, ActionMailer::Base.deliveries.size
   end
+
+  test "creating a journal should update the updated_on value of the parent record (touch)" do
+    @user = User.generate!
+    @project = Project.generate!
+    @issue = Issue.generate_for_project!(@project).reload
+    start = @issue.updated_on
+
+    assert_difference("Journal.count") do
+      @issue.init_journal(@user, "A note")
+      @issue.save
+    end
+
+    assert_not_equal start, @issue.reload.updated_on
+  end
 end
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index 6f56567..f3bd0fc 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -60,7 +60,8 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert_equal Version.find_by_name('alpha'), issue.fixed_version
     assert_equal 2.5, issue.estimated_hours
     assert_equal 30, issue.done_ratio
-    assert_equal [issue.id, 1, 2], [issue.root_id, issue.lft, issue.rgt]
+    assert_equal issue.id, issue.root_id
+    assert issue.leaf?
     # keywords should be removed from the email body
     assert !issue.description.match(/^Project:/i)
     assert !issue.description.match(/^Status:/i)
@@ -208,7 +209,8 @@ class MailHandlerTest < ActiveSupport::TestCase
         assert issue.is_a?(Issue)
         assert issue.author.anonymous?
         assert !issue.project.is_public?
-        assert_equal [issue.id, 1, 2], [issue.root_id, issue.lft, issue.rgt]
+        assert_equal issue.id, issue.root_id
+        assert issue.leaf?
       end
     end
   end
diff --git a/vendor/plugins/acts_as_journalized/lib/redmine/acts/journalized/reversion.rb b/vendor/plugins/acts_as_journalized/lib/redmine/acts/journalized/reversion.rb
index 44fc9a5..1add479 100644
--- a/vendor/plugins/acts_as_journalized/lib/redmine/acts/journalized/reversion.rb
+++ b/vendor/plugins/acts_as_journalized/lib/redmine/acts/journalized/reversion.rb
@@ -107,4 +107,4 @@ module Redmine::Acts::Journalized
         end
     end
   end
-end
\ No newline at end of file
+end
-- 
1.7.4.1

