From: Go MAEDA <maeda@farend.jp>
Date: Fri, 23 Nov 2018 06:07:32 +0000 (+0000)
Subject: Fix: Attachments are added even if validation fails when updating an issue via email... 
X-Git-Tag: 4.0.0~57
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=f743d2356f1f94fc6bb32355824cc074f4896fb7;p=redmine.git

Fix: Attachments are added even if validation fails when updating an issue via email (#28576).

Patch by Takenori TAKAKI.


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

diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index bd0d81ba9..073e0dcc0 100755
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -250,8 +250,8 @@ class MailHandler < ActionMailer::Base
 
     # add To and Cc as watchers before saving so the watchers can reply to Redmine
     add_watchers(issue)
-    add_attachments(issue)
     issue.save!
+    add_attachments(issue)
     if logger
       logger.info "MailHandler: issue ##{issue.id} updated by #{user}"
     end
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index e63ab0958..5c3a570b7 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -890,6 +890,21 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert_equal 'Paella.jpg', detail.value
   end
 
+  def test_update_issue_should_discard_all_changes_on_validation_failure
+    Issue.any_instance.stubs(:valid?).returns(false)
+    assert_no_difference 'Journal.count' do
+      assert_no_difference 'JournalDetail.count' do
+        assert_no_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
+  end
+
   def test_update_issue_should_send_email_notification
     journal = submit_email('ticket_reply.eml')
     assert journal.is_a?(Journal)