Fix Paperclip::Errors::NotIdentifiedByImageMagickError on invalid images #2064
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
jhawthorn
reviewed
Jul 6, 2017
| describe Spree::Image, type: :model do | ||
| context '#save' do | ||
| context 'invalid attachment' do | ||
| let(:invalid_image) { File.open(__FILE__) } |
There was a problem hiding this comment.
Same here!
And congrats on the first tests on the Image model : )
|
This should probably also be reported upstream to paperclip EDIT: Done in thoughtbot/paperclip#2462 |
tvdeyen
approved these changes
Jul 6, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.