From 672107a4157c762d5694d61a0e84346c11673541 Mon Sep 17 00:00:00 2001
From: Karl Entwistle <karl@simpleweb.co.uk>
Date: Thu, 6 Jul 2017 16:14:36 +0100
Subject: [PATCH] Fix Paperclip::Errors::NotIdentifiedByImageMagickError on
 invalid images

In the paperclip README it says NOTE: Post processing will not even start if
the attachment is not valid according to the validations. Your callbacks and
processors will only be called with valid attachments.

However find_dimensions is still being called and unless a valid attachment is
attached to Image the error Paperclip::Errors::NotIdentifiedByImageMagickError
is raised.

This pull requests adds a guard clause to after_post_process so it only calls
find_dimensions if the image is valid. There wasn't any unit tests for image
prior to this PR so it also introduces a very simple happy path test for save
and the inverse to prevent future regression of the aforementioned issue.
---
 core/app/models/spree/image.rb       |  2 +-
 core/spec/models/spree/image_spec.rb | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 core/spec/models/spree/image_spec.rb

diff --git a/core/app/models/spree/image.rb b/core/app/models/spree/image.rb
index 9bbf5cef4eb..ae1787d21cd 100644
--- a/core/app/models/spree/image.rb
+++ b/core/app/models/spree/image.rb
@@ -15,7 +15,7 @@ class Image < Asset
 
     # save the w,h of the original image (from which others can be calculated)
     # we need to look at the write-queue for images which have not been saved yet
-    after_post_process :find_dimensions
+    after_post_process :find_dimensions, if: :valid?
 
     # used by admin products autocomplete
     def mini_url
diff --git a/core/spec/models/spree/image_spec.rb b/core/spec/models/spree/image_spec.rb
new file mode 100644
index 00000000000..014652cd0e0
--- /dev/null
+++ b/core/spec/models/spree/image_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Spree::Image, type: :model do
+  context '#save' do
+    context 'invalid attachment' do
+      let(:invalid_image) { File.open(__FILE__) }
+      subject { described_class.new(attachment: invalid_image) }
+
+      it 'returns false' do
+        expect(subject.save).to be false
+      end
+    end
+
+    context 'valid attachment' do
+      let(:valid_image) { File.open(File.join('spec', 'fixtures', 'thinking-cat.jpg')) }
+      subject { described_class.new(attachment: valid_image) }
+
+      it 'returns true' do
+        expect(subject.save).to be true
+      end
+    end
+  end
+end