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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix editor image routing #12683

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

ahukkanen
Copy link
Contributor

馃帺 What? Why?

While implementing #12576, there was one problem regarding the blob URLs that are persisted to the database which is that they cannot be created dynamically at render time because the HTML is persisted to the database as-is.

This fixes the issue by replacing references to any blob URLs with the blob's global ID and then swapping it back to the actual URL when these values are rendered.

This fixes two issues:

  1. The blob URLs can be represented in more performant way referencing the direct storage URLs as described at Improve ActiveStorage asset linking performance聽#12576
  2. The blob URLs should not expire if they are stored to the database in a timely manner (i.e. within 7 days from creation)

This also changes the signed blob URL expiry period to 7 days which is the maximum set by Amazon S3 and Google Cloud Files. This has to be used together with a sensible caching strategy as the expiry periods of these 3rd party services cannot be extended. In other words, when using Amazon S3 or Google Cloud Files, the cache has to be cleared at least once every 7 days because e.g. the content block cache is set to expire never.

馃搶 Related Issues

Link your PR to an issue

Testing

  • Edit some content that has rich text editor, e.g. a static page
  • Embed an image to the content
  • Save the form
  • See the rich text content value in the database, you should now see the image src attribute replaced with gid://decidim-development-app/ActiveStorage::Blob/XYZ (where XYZ is the blob ID in the database)
  • Go to see the page at the participant-facing view
  • See that the image is rendered correctly

Anything that uses the SettingsManifest can also have rich text
content where the blob replacement logic has to be applied at.
github-actions[bot]
github-actions bot previously approved these changes Apr 10, 2024
github-actions[bot]
github-actions bot previously approved these changes Apr 10, 2024
@ahukkanen
Copy link
Contributor Author

Note that this PR depends on #12576 which needs to be merged first.

This PR needs one RSpec matcher added in #12576 which would also fix the Codeclimate issues (telling to remove TODO messages).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expiration time in URL attachments
1 participant