Fixed: MailHandler does not include JournalDetail for attached files (#7966).

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

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 92eb9157a45ed742f17cadbb3ed5dcc8e2ac07fd..7e3a915815830cc2cdf76822e12cb1e32d2fae17 100644 (file)
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -149,7 +149,8 @@ class Attachment < ActiveRecord::Base
                               :file => file,
                               :description => attachment['description'].to_s.strip,
                               :author => User.current)
-
+        obj.attachments << a
+        
         if a.new_record?
           obj.unsaved_attachments ||= []
           obj.unsaved_attachments << a

diff --git a/app/models/issue.rb b/app/models/issue.rb
index b4aac4a36d5abaf64d80af6bc0e9b871b3cc5488..48a0afb2a23e27a0f88ea86151c1af63806d9d07 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -35,7 +35,7 @@ class Issue < ActiveRecord::Base
   has_many :relations_to, :class_name => 'IssueRelation', :foreign_key => 'issue_to_id', :dependent => :delete_all
 
   acts_as_nested_set :scope => 'root_id', :dependent => :destroy
-  acts_as_attachable :after_remove => :attachment_removed
+  acts_as_attachable :after_add => :attachment_added, :after_remove => :attachment_removed
   acts_as_customizable
   acts_as_watchable
   acts_as_searchable :columns => ['subject', "#{table_name}.description", "#{Journal.table_name}.notes"],
@@ -612,8 +612,6 @@ class Issue < ActiveRecord::Base
 
       if valid?
         attachments = Attachment.attach_files(self, params[:attachments])
-
-        attachments[:files].each {|a| @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
         # TODO: Rename hook
         Redmine::Hook.call_hook(:controller_issues_edit_before_save, { :params => params, :issue => self, :time_entry => @time_entry, :journal => @current_journal})
         begin
@@ -842,6 +840,13 @@ class Issue < ActiveRecord::Base
       end
     end
   end
+  
+  # Callback on attachment deletion
+  def attachment_added(obj)
+    if @current_journal && !obj.new_record?
+      @current_journal.details << JournalDetail.new(:property => 'attachment', :prop_key => obj.id, :value => obj.filename)
+    end
+  end
 
   # Callback on attachment deletion
   def attachment_removed(obj)

diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index 0ba8d3fd2b8961b50582c2beebd6d7bda3d535cf..2cfb116984feeef2b1ff9934ecc1cb97ec5443d1 100644 (file)
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -200,7 +200,7 @@ class MailHandler < ActionMailer::Base
   def add_attachments(obj)
     if email.has_attachments?
       email.attachments.each do |attachment|
-        Attachment.create(:container => obj,
+        obj.attachments << Attachment.create(:container => obj,
                           :file => attachment,
                           :author => user,
                           :content_type => attachment.content_type)

diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index d272e6090f0ddbb7b7275ac8cd5c4ebbac4a2b62..a80f1ec1606dfb509a8b7892b138e617fab86227 100644 (file)
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -307,7 +307,7 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert_equal 1, ActionMailer::Base.deliveries.size
   end
 
-  def test_add_issue_note
+  def test_update_issue
     journal = submit_email('ticket_reply.eml')
     assert journal.is_a?(Journal)
     assert_equal User.find_by_login('jsmith'), journal.user
@@ -316,7 +316,7 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert_equal 'Feature request', journal.issue.tracker.name
   end
 
-  def test_add_issue_note_with_attribute_changes
+  def test_update_issue_with_attribute_changes
     # This email contains: 'Status: Resolved'
     journal = submit_email('ticket_reply_with_status.eml')
     assert journal.is_a?(Journal)
@@ -334,15 +334,36 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert !journal.notes.match(/^Status:/i)
     assert !journal.notes.match(/^Start Date:/i)
   end
+  
+  def test_update_issue_with_attachment
+    assert_difference 'Journal.count' do
+      assert_difference 'JournalDetail.count' do
+        assert_difference 'Attachment.count' do
+          assert_no_difference 'Issue.count' do
+            journal = submit_email('ticket_with_attachment.eml') do |raw|
+              raw.gsub! /^Subject: .*$/, 'Subject: Re: [Cookbook - Feature #2] (New) Add ingredients categories'
+            end
+          end
+        end
+      end
+    end
+    journal = Journal.first(:order => 'id DESC')
+    assert_equal Issue.find(2), journal.journalized
+    assert_equal 1, journal.details.size
+    
+    detail = journal.details.first
+    assert_equal 'attachment', detail.property
+    assert_equal 'Paella.jpg', detail.value
+  end
 
-  def test_add_issue_note_should_send_email_notification
+  def test_update_issue_should_send_email_notification
     ActionMailer::Base.deliveries.clear
     journal = submit_email('ticket_reply.eml')
     assert journal.is_a?(Journal)
     assert_equal 1, ActionMailer::Base.deliveries.size
   end
 
-  def test_add_issue_note_should_not_set_defaults
+  def test_update_issue_should_not_set_defaults
     journal = submit_email('ticket_reply.eml', :issue => {:tracker => 'Support request', :priority => 'High'})
     assert journal.is_a?(Journal)
     assert_match /This is reply/, journal.notes