FEATURE: Include optimized thumbnails for topics (PR #9215)

This ensures that all thumbnail images are real uploads, and not hotlinked to another site. It also paves the way for optimising post/topic icons

A migration attempts to match existing urls to upload records. If a match cannot be found then the posts will be queued for rebake.

This has a couple of negative side effects

  1. Hotlinked images are now never used as thumbnails
  2. All thumbnails are now full-resolution original images. Previously, the URL was extracted from cooked, which contained optimized URLs for large images.

Because of (2), I am opening this as a draft PR until we build a solution

GitHub

The title of this pull request changed from “REFACTOR: Replace image_url column with image_upload_id” to "FEATURE: Include optimized thumbnails for topics

@SamSaffron would you mind reviewing this if you have some time?

typo… line is here twice.

A slight concern here I have is that we are adding DB consistency foreign keys in some spots when a whole bunch of columns on the same table do not have them … like user_id post_id and the list goes on.

Feels a bit inconsistent as a rule to apply.

I think we should just stick to adding the column and indexes we want by hand.

feels like a post migration is what we want here, no need to hold off the deploy for this, just run it after all app servers have the new code.

use of super is a bit surprising here… read_attribute(:topic_thumbnail_sizes) is a bit less surprising

write_attribute

should we leave image_url around and print a deprecation when it is accessed? how many plugins are we going to break?

Thanks for reviewing @SamSaffron

should we leave image_url around and print a deprecation when it is accessed

IMO we should keep it, and we don’t need a deprecation. There is zero cost in keeping it, and it is a much simpler API than the thumbnails array. It’s still used in core for things like opengraph/sharing images, and I have seen people use it for things like IFTTT/Zapier integrations.

I set it to prefer a 1024x1024 thumbnail, and fall back to the original image. It doesn’t use the old image_url database column any more.

If we run this as a post-migration, then sites will stop serving opengraph images on all topics for a few minutes. I guess that’s ok… but if we keep it as a pre-deploy migration then the change should be seamless. I think it should only take a few seconds.

1 Like

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/blog-post-styling/110841/86