Moved journal details generation to Journal model.

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

app/models/issue.rb

@@ -687,14 +687,6 @@ class Issue < ActiveRecord::Base
 
   def init_journal(user, notes = "")
     @current_journal ||= Journal.new(:journalized => self, :user => user, :notes => notes)
-    if new_record?
-      @current_journal.notify = false
-    else
-      @attributes_before_change = attributes.dup
-      @custom_values_before_change = {}
-      self.custom_field_values.each {|c| @custom_values_before_change.store c.custom_field_id, c.value }
-    end
-    @current_journal
   end
 
   # Returns the current journal or nil if it's not initialized
@@ -702,6 +694,11 @@ class Issue < ActiveRecord::Base
     @current_journal
   end
 
+  # Returns the names of attributes that are journalized when updating the issue
+  def journalized_attribute_names
+    Issue.column_names - %w(id root_id lft rgt lock_version created_on updated_on closed_on)
+  end
+
   # Returns the id of the last journal or nil
   def last_journal_id
     if new_record?
@@ -1522,41 +1519,33 @@ class Issue < ActiveRecord::Base
   end
 
   # Callback on file attachment
-  def attachment_added(obj)
+  def attachment_added(attachment)
-    if @current_journal && !obj.new_record?
+    if current_journal && !attachment.new_record?
-      @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => obj.id, :value => obj.filename)
+      current_journal.journalize_attachment(attachment, :added)
     end
   end
 
   # Callback on attachment deletion
-  def attachment_removed(obj)
+  def attachment_removed(attachment)
-    if @current_journal && !obj.new_record?
+    if current_journal && !attachment.new_record?
-      @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => obj.id, :old_value => obj.filename)
+      current_journal.journalize_attachment(attachment, :removed)
-      @current_journal.save
+      current_journal.save
     end
   end
 
   # Called after a relation is added
   def relation_added(relation)
-    if @current_journal
+    if current_journal
-      @current_journal.details << JournalDetail.new(
+      current_journal.journalize_relation(relation, :added)
-        :property  => 'relation',
+      current_journal.save
-        :prop_key  => relation.relation_type_for(self),
-        :value => relation.other_issue(self).try(:id)
-      )
-      @current_journal.save
     end
   end
 
   # Called after a relation is removed
   def relation_removed(relation)
-    if @current_journal
+    if current_journal
-      @current_journal.details << JournalDetail.new(
+      current_journal.journalize_relation(relation, :removed)
-        :property  => 'relation',
+      current_journal.save
-        :prop_key  => relation.relation_type_for(self),
-        :old_value => relation.other_issue(self).try(:id)
-      )
-      @current_journal.save
     end
   end
 
@@ -1616,55 +1605,8 @@ class Issue < ActiveRecord::Base
   # Saves the changes in a Journal
   # Called after_save
   def create_journal
-    if @current_journal
+    if current_journal
-      # attributes changes
+      current_journal.save
-      if @attributes_before_change
-        (Issue.column_names - %w(id root_id lft rgt lock_version created_on updated_on closed_on)).each {|c|
-          before = @attributes_before_change[c]
-          after = send(c)
-          next if before == after || (before.blank? && after.blank?)
-          @current_journal.details << JournalDetail.new(:property => 'attr',
-                                                        :prop_key => c,
-                                                        :old_value => before,
-                                                        :value => after)
-        }
-      end
-      if @custom_values_before_change
-        # custom fields changes
-        custom_field_values.each {|c|
-          before = @custom_values_before_change[c.custom_field_id]
-          after = c.value
-          next if before == after || (before.blank? && after.blank?)
-
-          if before.is_a?(Array) || after.is_a?(Array)
-            before = [before] unless before.is_a?(Array)
-            after = [after] unless after.is_a?(Array)
-
-            # values removed
-            (before - after).reject(&:blank?).each do |value|
-              @current_journal.details << JournalDetail.new(:property => 'cf',
-                                                            :prop_key => c.custom_field_id,
-                                                            :old_value => value,
-                                                            :value => nil)
-            end
-            # values added
-            (after - before).reject(&:blank?).each do |value|
-              @current_journal.details << JournalDetail.new(:property => 'cf',
-                                                            :prop_key => c.custom_field_id,
-                                                            :old_value => nil,
-                                                            :value => value)
-            end
-          else
-            @current_journal.details << JournalDetail.new(:property => 'cf',
-                                                          :prop_key => c.custom_field_id,
-                                                          :old_value => before,
-                                                          :value => after)
-          end
-        }
-      end
-      @current_journal.save
-      # reset current journal
-      init_journal @current_journal.user, @current_journal.notes
     end
   end
 

app/models/journal.rb

@@ -50,7 +50,16 @@ class Journal < ActiveRecord::Base
       where("(#{Journal.table_name}.private_notes = ? OR (#{Project.allowed_to_condition(user, :view_private_notes, *args)}))", false)
   }
 
+  def initialize(*args)
+    super
+    if journalized && journalized.new_record?
+      self.notify = false
+    end
+    start
+  end
+
   def save(*args)
+    journalize_changes
     # Do not save an empty journal
     (details.empty? && notes.blank?) ? false : super
   end
@@ -166,8 +175,101 @@ class Journal < ActiveRecord::Base
     journals
   end
 
+  # Stores the values of the attributes and custom fields of the journalized object
+  def start
+    if journalized
+      @attributes_before_change = journalized.journalized_attribute_names.inject({}) do |h, attribute|
+        h[attribute] = journalized.send(attribute)
+        h
+      end
+      @custom_values_before_change = journalized.custom_field_values.inject({}) do |h, c|
+        h[c.custom_field_id] = c.value
+        h
+      end
+    end
+    self
+  end
+
+  # Adds a journal detail for an attachment that was added or removed
+  def journalize_attachment(attachment, added_or_removed)
+    key = (added_or_removed == :removed ? :old_value : :value)
+    details << JournalDetail.new(
+        :property => 'attachment',
+        :prop_key => attachment.id,
+        key => attachment.filename
+      )
+  end
+
+  # Adds a journal detail for an issue relation that was added or removed
+  def journalize_relation(relation, added_or_removed)
+    key = (added_or_removed == :removed ? :old_value : :value)
+    details << JournalDetail.new(
+        :property  => 'relation',
+        :prop_key  => relation.relation_type_for(journalized),
+        key => relation.other_issue(journalized).try(:id)
+      )
+  end
+
   private
 
+  # Generates journal details for attribute and custom field changes
+  def journalize_changes
+    # attributes changes
+    if @attributes_before_change
+      journalized.journalized_attribute_names.each {|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
+    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
+        next if before == after || (before.blank? && after.blank?)
+
+        if before.is_a?(Array) || after.is_a?(Array)
+          before = [before] unless before.is_a?(Array)
+          after = [after] unless after.is_a?(Array)
+
+          # values removed
+          (before - after).reject(&:blank?).each do |value|
+            add_custom_value_detail(c, value, nil)
+          end
+          # values added
+          (after - before).reject(&:blank?).each do |value|
+            add_custom_value_detail(c, nil, value)
+          end
+        else
+          add_custom_value_detail(c, before, after)
+        end
+      }
+    end
+    start
+  end
+
+  # Adds a journal detail for an attribute change
+  def add_attribute_detail(attribute, old_value, value)
+    add_detail('attr', attribute, old_value, value)
+  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)
+  end
+
+  # Adds a journal detail
+  def add_detail(property, prop_key, old_value, value)
+    details << JournalDetail.new(
+        :property => property,
+        :prop_key => prop_key,
+        :old_value => old_value,
+        :value => value
+      )
+  end
+
   def split_private_notes
     if private_notes?
       if notes.present?