SpinaCMS / Spina

Spina CMS
http://www.spinacms.com
Other
2.19k stars 405 forks source link

Rails 6 ActiveStorage switch from mini_magick to image_processing breaks image upload #642

Closed wakproductions closed 3 years ago

wakproductions commented 3 years ago

Rails changed a dependency that breaks Spina if you're using Rails 6. They changed the image processing gem within ActiveStorage.

https://edgeguides.rubyonrails.org/active_storage_overview.html http://railsdiff.org/5.2.4.4/6.0.0 A nice overview of the changes in a blog post

I would like to make a pull request to fix this, but not sure of the architectural approach you'd like to handle it. I'm considering two options:

1) Make a change to Spina that switches dependency from mini_magick to image_processing, and make the appropriate changes to the resizing method calls. The disadvantage of this is that Spina will no longer be compatible with Rails 5.

2) Try to abstract the image processing routines so that both mini_magick and image_processing are kept as dependencies, and somewhere in the background it will change the image handling appropriately depending on whether you are using Rails 5 or 6. This approach is going to take a lot more work, but one of the benefits is that by abstracting the image processor to a generic internal interface, we might be able to support other image hosting platforms like CarrierWave (yes, I saw there's another open issue requesting that).

If you give me some guidance on what you think the best approach is, I'd be happy to try taking that approach in a PR.

Bramjetten commented 3 years ago

This is already fixed in the latest 2.0.0.alpha :)

wakproductions commented 3 years ago

I'm working off of the master branch, which I think is more current than v2.0.0.alpha.... looks like it's still using mini_magick as a dependency and making the old style method calls. I got this going on when trying to load an image:

image

wakproductions commented 3 years ago

I am able to resolve this issue by adding s.add_dependency 'image_processing' to the spina.gemspec file. But, I'm having another issue when using the image upload form on the Page editor. The ActiveStorage Direct Upload feature seems to be creating the blobs alright, but the ImagesController fails because it doesn't recognize the request as being an AJAX request and raises a template not found error because there's no create.haml file to render, only a create.js.erb, which it doesn't reach because it doesn't think this is AJAX. image

image

I'm still trying to figure out the magic going on that gets from the direct upload to the ImagesController. Not sure if this is a bug or a feature in ActiveStorage.

wakproductions commented 3 years ago

I figured out what's broken. You need to add local: false to get this to submit as XHR

image

Still doing some further testing and polishing to make sure I got this right and didn't miss anything. Something that really stumped me for a few hours was figuring out why the media upload form was submitting automatically immediately after selecting a file. For a while I was thinking that's a feature of ActiveStorage Direct Upload. I finally found the code that submits the form in upload.coffee.

What is this code invoking? Is this a JQuery feature? I can't find any "dropZone" or fileupload related feature in JQuery so I'm a little puzzled as to how this gets triggered.


$.fn.uploadPhoto = ->
  $(this).fileupload
    dataType: "script"
    singleFileUploads: true
    replaceFileInput: true
    dropZone: $(this).find('.photo-field')
    add: (e, data) ->
      types = /(\.|\/)(gif|jpe?g|png)$/i
      file = data.files[0]
      if types.test(file.type) || types.test(file.name)
        $(this).find('.customfile').addClass('loading')
        data.submit()
      else
        alert("#{file.name} is not gif-, jpeg- or png- file")
    done: (e, data) ->
      $(this).find('.customfile').removeClass('loading')
``
Bramjetten commented 3 years ago

It's a jQuery plugin called fileupload that does that. Since the latest Rails release, form_with doesn't always generate a remote form anymore.

I'm working on a total refactor of all js and css, including the image library. We're switching to Hotwire using Turbo and Stimulus. That should fix all of these issues!

wakproductions commented 3 years ago

I opened a PR that will resolve this issue for now until the refactor is complete.

Bramjetten commented 3 years ago

This is fixed by #667