Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not attempt to generate thumbs for svg files (fixes upload of svg files) #2090

Merged
merged 1 commit into from
May 13, 2021

Conversation

oneiros
Copy link
Contributor

@oneiros oneiros commented May 7, 2021

Hi everyone. First of all let me thank you for this fantastic project.

What is this pull request for?

While trying to set up a new alchemy based project a colleague of mine discovered a small problem: He was not able to upload any .svg files. I could reproduce this with current main branch. The exception we hit was:

NoMethodError undefined method `steps' for #<#<Class:0x00007fa2316963f8>:0x00007fa2316d4630> in /app/models/alchemy/picture_thumb/signature.rb:12:in `call'
/app/models/alchemy/picture_thumb.rb:47:in `block in generate_thumbs!'
/app/models/alchemy/picture_thumb.rb:42:in `each'
/app/models/alchemy/picture_thumb.rb:42:in `generate_thumbs!'
/app/models/alchemy/picture.rb:94:in `block in <class:Picture>'

I did a little digging and found that the missing #steps method is called on Alchemy::PictureVariant#image. The documentation of that method explicitly says that it can either return a Dragonfly::Attachment or a Dragonfly::Job. Only the latter seems to have this #steps method.

In this case I think a Dragonfly::Job is only returned for raster image formats for which an actual thumbnail is being generated. For other formats, any conversions are skipped and a Dragonfly::Attachment is being returned.

Notable changes

I was not 100% how to fix this. So please find my first clumsy attempt attached. I decided that the whole thumbnail (pre-)generation probably only makes sense for those formats that can actually be converted this way. So I tried to shut this off as early as possible.

Feel free to discard this if it does not make sense. Also, please let me know if I can improve on this in any way.

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. This makes total sense and is not at all clumsy, but a perfect solution 👍

Thumbnail (pre-)generation will not work and should
not be necessary for a vector format.
@tvdeyen
Copy link
Member

tvdeyen commented May 13, 2021

Rebased with latest main to fix CI issues

@tvdeyen tvdeyen linked an issue May 13, 2021 that may be closed by this pull request
@tvdeyen tvdeyen merged commit a6469f0 into AlchemyCMS:main May 13, 2021
@tvdeyen
Copy link
Member

tvdeyen commented May 13, 2021

Thank you ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uploading SVG in picture library fails at thumbnail generation
2 participants