WIP: Use Active Storage instead of Dragonfly #2288
Conversation
tvdeyen
force-pushed
the
active-storage
branch
3 times, most recently
from
8c7c2c8
to
b457134
Compare
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| } | ||
| end | ||
|
|
||
| def sharpen_option(options) |
There was a problem hiding this comment.
Method sharpen_option has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Helpful for local debugging in the browser
Necessary for active storage image transforms
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| scope :by_file_type, ->(file_type) { where(file_mime_type: file_type) } | ||
| scope :by_file_type, ->(file_type) { | ||
| with_attached_file.joins(:file_blob).where(active_storage_blobs: { content_type: file_type }) | ||
| } |
There was a problem hiding this comment.
Layout/BlockAlignment: } at 38, 6 is not aligned with ->(file_type) { at 36, 25 or scope :by_file_type, ->(file_type) { at 36, 4.
| scope :by_file_format, ->(format) { where(image_file_format: format) } | ||
| scope :by_file_format, ->(file_format) { | ||
| with_attached_image_file.joins(:image_file_blob).where(active_storage_blobs: { content_type: file_format }) | ||
| } |
There was a problem hiding this comment.
Layout/BlockAlignment: } at 96, 6 is not aligned with ->(file_format) { at 94, 27 or scope :by_file_format, ->(file_format) { at 94, 4.
| end | ||
|
|
||
| after(:build) do |picture, acc| |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
| end | ||
| end | ||
|
|
||
| after(:build) do |picture, acc| |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
ActiveStorage uses the image_processing gem to process images. This uses a hash to describe the conversion instead of a string
Instead of using the Dragonfly Model validations we write our own. That way we can use another image attachment library more easily
The vips image_processing processor sharpens images by default. This disables this behavior.
Active storage already knows if a file is convertible
This avoids N+1 in the image library
Using activestorage methods to read the values. Ideally we would cache these values, but active storage asynchronously sets these values.
We do not want to redirect to the service url, but use the proxy instead. This only works with Rails >= 6.1
Now that we use ActiveStorage we can remove our custom picture variant handling
This method is the only one left after the PictureVariant and PictureThumb removal.
Active storage has the mime type stored not the extension.
|
@tvdeyen let me know if i can help on this one. I've inherited some hacky ActiveStorage extensions that could be replaced by this :-) |
|
@pascalbetz thanks. feel free to review this PR and comment on the todos above. Currently I think ActiveStorage is not a perfect drop in replacement for Dragonfly. There are some blocking issues, especially the off band meta data update combined with the lack of callbacks. |
|
I am currently not working on this actively. Feel free to open PRs against my branch if you see something you can help with. There is no hurry with that, as well. |
|
I think we rather move our code over to dragonfly then, if AS will not replace Dragonfly anytime soon. |
No, this does not have a high priority and will not happen in short term. Dragonfly is a good choice for file attachments in Rails and it does not have the shortcomings ActiveStorage has. |
|
okay, thanks for the update. |
|
This pull request has not seen any activiy in a long time. |
|
This pull request has not seen any activiy in a long time. |
|
This pull request has not seen any activiy in a long time. |
|
This pull request has not seen any activiy in a long time. |
|
Re-opened #2544 to continue the work |
What is this pull request for?
Use ActiveStorage instead of Dragonfly for picture and file attachments.
Important changes
TODO
Alchemy::Attachmentas wellChecklist