Code Climate has analyzed commit 6289e72 and detected 0 issues on this pull request.

View more on Code Climate.

Hi @Olli thanks for submitting this contribution.

Could you please give some details about the issue that this is solving and how to reproduce it? It will help the review process

@Paul-Bob well if there is a ActionText field, AVO utilize it but it's not possible to perform uploads on this field because the upload button is not available (even if include ActionText::Attachable is in the model).
This little patch makes uploads possible which means you can embed images to a certain ActionText field (if include ActionText::Attachable is in the model).

@Paul-Bob well if there is a ActionText field, AVO utilize it but it's not possible to perform uploads on this field because the upload button is not available (even if include ActionText::Attachable is in the model).

In order to upload files, right now, it is necessary to configure the attachment_key and assign it a has_many_attached attribute.

This little patch makes uploads possible which means you can embed images to a certain ActionText field (if include ActionText::Attachable is in the model).

I think we can make attachment_key optional instead of mandatory, but we still need to make sure authorization is enforced. Using the rails_direct_uploads_url directly skips the authorization, which could be an issue.

@Olli I made some changes to this PR

It removes the requirement for attachment_key when the field is an ActionText.

Let me know if it works as expected on your use-case.

This enables uploads to ActionText by default keeping the authorization in place.

Thx ... does work here. 👍
My basic intention for this (and using ActionText) was that I don't have control over the output of the images if I use trix with a textfield and in addition the url of the app image is written in the text field. So the data isn't easy moveable from one domain to another.
With ActionText together with ActiveStorage it's more controllable and it can moved from one domain to another (in case the Rails app changes domains).

I didn't know that about domain portability @Olli.
Is there any documentation on that? Something that we can read and take care of in the future?

@adrianthedev Tbh actually I do not fully understand how ActionText works but if I understand it right the attachments are referenced by the Signed GlobalId.
So for example it's saved like that in the database

<action-text-attachment sgid="eyJfcmFpbHMiOnsiZGF0YSI6ImdpZDovL3Rlc3RyYWlscy9BY3RpdmVTdG9yYWdlOjpCbG9iLzM_ZXhwaXJlc19pbiIsInB1ciI6ImF0dGFjaGFibGUifX0=--e768571bdc79861a8477a610a21e139d768e3cf0" content-type="image/jpeg" url="http://localhost:3001/rails/active_storage/blobs/redirect/eyJfcmFpbHMiOnsiZGF0YSI6MywicHVyIjoiYmxvYl9pZCJ9fQ==--765d0b7ae074f9c73493a3ff89d7b3bd2e209b5a/image.jpg" filename="image.jpg" filesize="647852" width="1200" height="987" previewable="true" presentation="gallery">
</action-text-attachment></div>

even if the blob url is embedded changing the port or the url doesn't matter because it's referenced by the sgid.
AVO embeds a attachment as far as I know in the text with a "hardcoded" url.
Do you understand what I mean or am I on a wrong track?

Thx ... does work here. 👍

Awesome! Let's merge this PR as it is since it already fixes a problem, however let's continue the domain portability discussion which can lead to a new issue / PR

Yeah it makes sense.
I guess we can return a signed ID as well and replicate that markup.
Let's merge this one because it fixes the issue and tackle the sgid in another PR if we need it.

Thanks for explaining that