From 10cbdf5d96515ecfc74f84a7475327bcca69f28b Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Wed, 8 Apr 2009 19:11:30 +0000
Subject: [PATCH] Create the journal after issue save (#3140).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2669 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/issue.rb     | 50 +++++++++++++++++++++--------------------
 test/unit/issue_test.rb | 19 ++++++++++++++++
 2 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/app/models/issue.rb b/app/models/issue.rb
index e7360d3d7..c3627ddb2 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -56,6 +56,8 @@ class Issue < ActiveRecord::Base
   
   named_scope :open, :conditions => ["#{IssueStatus.table_name}.is_closed = ?", false], :include => :status
   
+  after_save :create_journal
+  
   # Returns true if usr or current user is allowed to view the issue
   def visible?(usr=nil)
     (usr || User.current).allowed_to?(:view_issues, self.project)
@@ -154,30 +156,6 @@ class Issue < ActiveRecord::Base
     end
   end
   
-  def before_save  
-    if @current_journal
-      # attributes changes
-      (Issue.column_names - %w(id description lock_version created_on updated_on)).each {|c|
-        @current_journal.details << JournalDetail.new(:property => 'attr',
-                                                      :prop_key => c,
-                                                      :old_value => @issue_before_change.send(c),
-                                                      :value => send(c)) unless send(c)==@issue_before_change.send(c)
-      }
-      # custom fields changes
-      custom_values.each {|c|
-        next if (@custom_values_before_change[c.custom_field_id]==c.value ||
-                  (@custom_values_before_change[c.custom_field_id].blank? && c.value.blank?))
-        @current_journal.details << JournalDetail.new(:property => 'cf', 
-                                                      :prop_key => c.custom_field_id,
-                                                      :old_value => @custom_values_before_change[c.custom_field_id],
-                                                      :value => c.value)
-      }      
-      @current_journal.save
-    end
-    # Save the issue even if the journal is not saved (because empty)
-    true
-  end
-  
   def after_save
     # Reload is needed in order to get the right status
     reload
@@ -301,4 +279,28 @@ class Issue < ActiveRecord::Base
                                          :old_value => obj.filename)
     journal.save
   end
+  
+  # Saves the changes in a Journal
+  # Called after_save
+  def create_journal
+    if @current_journal
+      # attributes changes
+      (Issue.column_names - %w(id description lock_version created_on updated_on)).each {|c|
+        @current_journal.details << JournalDetail.new(:property => 'attr',
+                                                      :prop_key => c,
+                                                      :old_value => @issue_before_change.send(c),
+                                                      :value => send(c)) unless send(c)==@issue_before_change.send(c)
+      }
+      # custom fields changes
+      custom_values.each {|c|
+        next if (@custom_values_before_change[c.custom_field_id]==c.value ||
+                  (@custom_values_before_change[c.custom_field_id].blank? && c.value.blank?))
+        @current_journal.details << JournalDetail.new(:property => 'cf', 
+                                                      :prop_key => c.custom_field_id,
+                                                      :old_value => @custom_values_before_change[c.custom_field_id],
+                                                      :value => c.value)
+      }      
+      @current_journal.save
+    end
+  end
 end
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index ae3fa5bda..5052f7a0c 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -249,4 +249,23 @@ class IssueTest < Test::Unit::TestCase
     assert issue.save
     assert_equal 1, ActionMailer::Base.deliveries.size
   end
+  
+  def test_stale_issue_should_not_send_email_notification
+    ActionMailer::Base.deliveries.clear
+    issue = Issue.find(1)
+    stale = Issue.find(1)
+    
+    issue.init_journal(User.find(1))
+    issue.subject = 'Subjet update'
+    assert issue.save
+    assert_equal 1, ActionMailer::Base.deliveries.size
+    ActionMailer::Base.deliveries.clear
+    
+    stale.init_journal(User.find(1))
+    stale.subject = 'Another subjet update'
+    assert_raise ActiveRecord::StaleObjectError do
+      stale.save
+    end
+    assert ActionMailer::Base.deliveries.empty?
+  end
 end