Journalize values that are cleared after project or tracker change (#21623).

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

3 files changed, 41 insertions, 15 deletions

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 3f1a34952..c43c4da9c 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -226,18 +226,26 @@ class Journal < ActiveRecord::Base
   def journalize_changes
     # attributes changes
     if @attributes_before_change
-      journalized.journalized_attribute_names.each {|attribute|
+      attrs = (journalized.journalized_attribute_names + @attributes_before_change.keys).uniq
+      attrs.each do |attribute|
         before = @attributes_before_change[attribute]
         after = journalized.send(attribute)
         next if before == after || (before.blank? && after.blank?)
         add_attribute_detail(attribute, before, after)
-      }
+      end
     end
+    # custom fields changes
     if @custom_values_before_change
-      # custom fields changes
-      journalized.custom_field_values.each {|c|
-        before = @custom_values_before_change[c.custom_field_id]
-        after = c.value
+      values_by_custom_field_id = {}
+      @custom_values_before_change.each do |custom_field_id, value|
+        values_by_custom_field_id[custom_field_id] = nil
+      end
+      journalized.custom_field_values.each do |c|
+        values_by_custom_field_id[c.custom_field_id] = c.value
+      end
+
+      values_by_custom_field_id.each do |custom_field_id, after|
+        before = @custom_values_before_change[custom_field_id]
         next if before == after || (before.blank? && after.blank?)
 
         if before.is_a?(Array) || after.is_a?(Array)
@@ -246,16 +254,16 @@ class Journal < ActiveRecord::Base
 
           # values removed
           (before - after).reject(&:blank?).each do |value|
-            add_custom_value_detail(c, value, nil)
+            add_custom_field_detail(custom_field_id, value, nil)
           end
           # values added
           (after - before).reject(&:blank?).each do |value|
-            add_custom_value_detail(c, nil, value)
+            add_custom_field_detail(custom_field_id, nil, value)
           end
         else
-          add_custom_value_detail(c, before, after)
+          add_custom_field_detail(custom_field_id, before, after)
         end
-      }
+      end
     end
     start
   end
@@ -266,8 +274,8 @@ class Journal < ActiveRecord::Base
   end
 
   # Adds a journal detail for a custom field value change
-  def add_custom_value_detail(custom_value, old_value, value)
-    add_detail('cf', custom_value.custom_field_id, old_value, value)
+  def add_custom_field_detail(custom_field_id, old_value, value)
+    add_detail('cf', custom_field_id, old_value, value)
   end
 
   # Adds a journal detail
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 61c03af60..d4c693a5a 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -3342,7 +3342,7 @@ class IssuesControllerTest < Redmine::ControllerTest
 
     with_settings :notified_events => %w(issue_updated) do
       assert_difference('Journal.count') do
-        assert_difference('JournalDetail.count', 2) do
+        assert_difference('JournalDetail.count', 3) do
           put :update, :id => 1, :issue => {:project_id => '1',
                                            :tracker_id => '2',
                                            :priority_id => '6'
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index a532001dc..fa6063cfd 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -643,7 +643,8 @@ class IssueTest < ActiveSupport::TestCase
     assert_nil issue.due_date
   end
 
-  def test_changing_tracker_should_not_add_cleared_fields_to_journal
+  def test_attribute_cleared_on_tracker_change_should_be_journalized
+    CustomField.delete_all
     tracker = Tracker.find(2)
     tracker.core_fields = tracker.core_fields - %w(due_date)
     tracker.save!
@@ -658,7 +659,8 @@ class IssueTest < ActiveSupport::TestCase
       assert_nil issue.due_date
     end
     journal = Journal.order('id DESC').first
-    assert_equal 1, journal.details.count
+    details = journal.details.select {|d| d.prop_key == 'due_date'}
+    assert_equal 1, details.count
   end
 
   def test_reload_should_reload_custom_field_values
@@ -2473,6 +2475,22 @@ class IssueTest < ActiveSupport::TestCase
     end
   end
 
+  def test_custom_value_cleared_on_tracker_change_should_be_journalized
+    a = IssueCustomField.generate!(:tracker_ids => [1])
+    issue = Issue.generate!(:project_id => 1, :tracker_id => 1, :custom_field_values => {a.id.to_s => "foo"})
+    assert_equal "foo", issue.custom_field_value(a)
+
+    journal = new_record(Journal) do
+      issue.init_journal(User.first)
+      issue.tracker_id = 2
+      issue.save!
+    end
+    details = journal.details.select {|d| d.property == 'cf' && d.prop_key == a.id.to_s}
+    assert_equal 1, details.size
+    assert_equal 'foo', details.first.old_value
+    assert_nil details.first.value
+  end
+
   def test_description_eol_should_be_normalized
     i = Issue.new(:description => "CR \r LF \n CRLF \r\n")
     assert_equal "CR \r\n LF \r\n CRLF \r\n", i.description