-
Notifications
You must be signed in to change notification settings - Fork 65
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
Remove carrierwave dependency from Spotlight, relying on ActiveStorage #2739
base: main
Are you sure you want to change the base?
Conversation
mejackreed
commented
Mar 29, 2021
•
edited
Loading
edited
- determine if we need/want to update existing SirTrevor blocks with uploaded item urls
- if we do ^^ , allow for the migration task to cleanup old carrierwave files
raise Riiif::ImageNotFoundError, "unable to find file for #{id}" if uploaded_file.nil? | ||
|
||
uploaded_file.file | ||
ActiveStorage::Blob.service.path_for(uploaded_file.key) |
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.
I'm not sure how this will work with other types of ActiveStorage::Service, but should those people just write their own file resolver?
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.
Yeah.. I think we might have to adapt RIIIF to handle some of this better. I think ActiveStorage::Blob.service.path_for
won't work with some of the remote storage options, right?
(but... the whole download-with-a-block API won't play nice with RIIIF)
filename: attachment.attributes['file'], | ||
content_type: Mime::Type.lookup_by_extension(File.extname(attachment.attributes['file'])[1..]) | ||
) | ||
end |
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.
Should we clean up the old files?
|
||
def as_json(options = nil) | ||
file.as_json(options).merge(name: name, uid: uid, attachment: to_global_id) | ||
def as_json(_options = nil) |
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.
This is super weird, but seems like we need to provide the accessible url at this point for our uploader forms to work correctly.
75d24b6
to
3fce96c
Compare
3fce96c
to
a12caa5
Compare
Putting this on ice as it seems like it may cause some additional complexities. |