Eagerly load preview images (N+1)

[tenderlove]

For non-image attachments (like videos), generated representations are
created as a preview_image_attachment instead of as normal variants, and
as a result aren't included in the with_attached_ scopes. This adds
those preview image attachments to the includes of these scopes to
avoid an N+1 when iterating over a collection of attachments and
fetching the key of their representation (variants).

Fixes #50560

[p8]

There is a similar scope that should probably be changed as well (maybe in another PR):

rails/activestorage/app/models/active_storage/attachment.rb

Line 45 in af2bbd5

scope :with_all_variant_records, -> { includes(blob: { variant_records: { image_attachment: :blob } }) }

[tenderlove]

There is a similar scope that should probably be changed as well (maybe in another PR):

rails/activestorage/app/models/active_storage/attachment.rb

Line 45 in af2bbd5

scope :with_all_variant_records, -> { includes(blob: { variant_records: { image_attachment: :blob } }) }

Good call. I added a test / fix for this in this PR since I think we should backport this.

[searls]

Thanks so much for helping with this!! 🙏

[boof]

This should be part of a bigger release IMHO as this breaks apps using non-UUID PKs in their blob tables whilst the FK actually uses UUIDs w/o a deprecation warning.
A deprecation warning would be nice, since this requires a bigger migration of data.

Another option could be a documented way to disable the preloading.

[p8]

@boof Could you create an issue?