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

"Undefined method purge_later" error when attaching file to file fragment in Rails 6 #898

Closed
morgant opened this issue Sep 10, 2019 · 11 comments

Comments

@morgant
Copy link
Contributor

morgant commented Sep 10, 2019

Note: For general questions and feature requests please leave a message
on Gitter: https://gitter.im/comfy/comfortable-mexican-sofa

Expected behavior

Upload file via page file fragment, pages#update should result in file being attached to fragment without error.

Actual behavior

Under Ruby 2.6.3 & Rails 6.0.0, during pages#update the following error is generated:

A NoMethodError occurred in pages#update:

undefined method `purge_later' for # <ActiveStorage::Attached::Many:0x00007f71be809260>

Steps to reproduce

  1. Edit a page with a layout containing a file tag such as:

    {{ cms:file slide_image, namespace: slide, render: false }}

  2. Select an image file for the the file field (e.g. slide_image field)

  3. Click "Update Page" button

This does not occur if the image is attached inline within a WYSIWYG content fragment.

System configuration

Rails version: 6.0.0

CMS version: 2.0.18

Ruby version: 2.6.3

@morgant
Copy link
Contributor Author

morgant commented Sep 11, 2019

I'm not sure why this isn't working, the Rails 6 documentation shows that ActiveStorage::Attached::Many should have a purge_later method (see https://api.rubyonrails.org/classes/ActiveStorage/Attached/Many.html#method-i-purge_later). If I debug the code I can successfully perform attachments.first.purge_later.

@morgant
Copy link
Contributor Author

morgant commented Sep 16, 2019

It seems that the default action for Rails 6 is to replace existing attachments, instead of append, so the #purge_later call might not even be required. See rails/rails#36716

@morgant
Copy link
Contributor Author

morgant commented Sep 16, 2019

It appears that the aforementioned change in Rails 6's default action is for #update, #attach retains its append functionality.

That said, Rails 6 did have a change to the implementation of ActiveStorage::Attached::Many#purge & #purge_later: rails/rails@f9a5839.

@nitsujri
Copy link
Contributor

nitsujri commented Sep 17, 2019

@morgant I am unable to attach through the WYSIWYG as you mention above. I get an infinite loop when attaching a file:

https://gist.github.com/nitsujri/7b0a815d1702f159860a9a679f70abb4

It appears to be trying to add and then immediately purge the blob? I'm not 100% sure/understand.

I am running a similar setup:

Ruby 2.6.4
Rails 6.0.0

Let me know if I can help. I have downgraded my Rails version (easy) since we cannot upload images to blog posts/pages.

[Edit] I created a blank project example w/ steps showing what I'm seeing above.

https://github.com/nitsujri/comfy-activestorage-example

Thanks guys for this great gem!

@morgant
Copy link
Contributor Author

morgant commented Sep 17, 2019

@nitsujri Thanks, upon further investigation, I'm getting an infinite loop if I comment out the purge_later call. In fact, the purge_later works fine on the first iteration and fails on the second.

@morgant
Copy link
Contributor Author

morgant commented Sep 17, 2019

@nitsujri It looks like this may be a bug in Rails 6.0 related to #attach and after_save callbacks: rails/rails#37022

@morgant
Copy link
Contributor Author

morgant commented Sep 18, 2019

@nitsujri Changing the after_save callbacks in app/models/comfy/cms/fragment.rb seems to resolve the issue in my testing. I can't yet explain precisely why and it doesn't fix rails/rails#37022, but I'm curious if it has a positive effect in your environment.

@morgant
Copy link
Contributor Author

morgant commented Sep 18, 2019

@nitsujri This is essentially the same fix as PR #892, but for Comfy::Cms::Fragment as well as Comfy::Cms::File.

@morgant morgant mentioned this issue Sep 18, 2019
2 tasks
@nitsujri
Copy link
Contributor

@morgant Ah yep, that worked!

https://github.com/nitsujri/comfy-activestorage-example/tree/fixed

The example has a fixed branch now working, and I tested it on our actual app which works great too.

Love to see this merged in asap =)

@morgant
Copy link
Contributor Author

morgant commented Sep 19, 2019

Now that PR #892 applies the fix for Comfy::Cms::Fragment as well, this is working for me. Will close when PR #892 is merged.

@morgant
Copy link
Contributor Author

morgant commented Dec 9, 2019

PR #899—a duplicate of PR #892—was merged, so this can be closed.

@morgant morgant closed this as completed Dec 9, 2019
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

No branches or pull requests

2 participants