-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Upgrading to Shrine 3.x #3257
Upgrading to Shrine 3.x #3257
Conversation
tested here, did not work when there is already an attached file, the attachment data is not loaded on a hidden field, and if the user do not upload a new file, it gives an error:
|
I was able to make it work. plugin :model, cache: true # false won't work |
So I fixed the Travis CI: I'm not sure why the tests randomly fail on the "nested form" part :( |
original: io, | ||
thumb: FakeIO.another_version(io, :thumb), | ||
} | ||
if Gem.loaded_specs['shrine'].version >= Gem::Version.create('3') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't there be an else
with the versions plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thumb
method will work only with derivatives
https://github.com/sferik/rails_admin/blob/e4d970e96a845f20ed898ebd6456062bb13a422c/lib/rails_admin/config/fields/types/shrine.rb#L10-L13
It won't work with version of shrine older than v3. So I just skipped tests about image versions:
https://github.com/sferik/rails_admin/blob/e4d970e96a845f20ed898ebd6456062bb13a422c/spec/rails_admin/config/fields/types/shrine_spec.rb#L14-L20
@prem-prakash, do you think we need to support thumbs for older versions of shrine too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akhsunna I would say that it would be a good practice at least to not break with older versions, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, maybe it's worth it :) Will add it soon
Merged in, since this is necessary in attempt to run Ruby 3.0 CI build. |
Considering that
derivatives
plugin replacedversions
plugin in shrine 3.x, some code should be updated.Also, I added
link_name
option toRailsAdmin::Config::Fields::Types::FileUpload
. I hope it's okay. I did it because currently links for shrine fields don't look nice: