Notify the user of missing attachments (#22401).

under certain (rare) circumstances it may happen that, when an issue or other
container is saved, added attachments have already been removed (i.e. by the
attachments:prune rake task). This patch adds a validation error to the
container in this case.

Patch by Jens Kraemer.

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

2 files changed, 23 insertions, 1 deletions

diff --git a/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb b/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
index 31403d7dc..bc4242564 100644
--- a/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
+++ b/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
@@ -34,6 +34,7 @@ module Redmine
                                  options.merge(:as => :container, :dependent => :destroy, :inverse_of => :container)
           send :include, Redmine::Acts::Attachable::InstanceMethods
           before_save :attach_saved_attachments
+          validate :warn_about_failed_attachments
         end
       end
 
@@ -82,6 +83,7 @@ module Redmine
             attachments = attachments.map(&:last)
           end
           if attachments.is_a?(Array)
+            @failed_attachment_count = 0
             attachments.each do |attachment|
               next unless attachment.is_a?(Hash)
               a = nil
@@ -90,7 +92,10 @@ module Redmine
                 a = Attachment.create(:file => file, :author => author)
               elsif token = attachment['token']
                 a = Attachment.find_by_token(token)
-                next unless a
+                unless a
+                  @failed_attachment_count += 1
+                  next
+                end
                 a.filename = attachment['filename'] unless attachment['filename'].blank?
                 a.content_type = attachment['content_type'] unless attachment['content_type'].blank?
               end
@@ -112,6 +117,12 @@ module Redmine
           end
         end
 
+        def warn_about_failed_attachments
+          if @failed_attachment_count && @failed_attachment_count > 0
+            errors.add :base, ::I18n.t('warning_attachments_not_saved', count: @failed_attachment_count)
+          end
+        end
+
         module ClassMethods
         end
       end
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index a07b18ff5..03dd57229 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -2527,6 +2527,17 @@ class IssueTest < ActiveSupport::TestCase
     assert_equal %w(upload foo bar), issue.attachments.map(&:filename)
   end
 
+  def test_save_attachments_with_array_should_warn_about_missing_tokens
+    set_tmp_attachments_directory
+    issue = Issue.generate!
+    issue.save_attachments([
+      {'token' => 'missing'}
+    ])
+    assert !issue.save
+    assert issue.errors[:base].present?
+    assert_equal 0, issue.reload.attachments.count
+  end
+
   def test_closed_on_should_be_nil_when_creating_an_open_issue
     issue = Issue.generate!(:status_id => 1).reload
     assert !issue.closed?