Merge pull request rails#50043 from jonathanhefner/active_storage-strict-loading-preview-url

Fix strict loading for Active Storage previews

Author: jonathanhefner

activestorage/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Prevent `ActiveRecord::StrictLoadingViolationError` when strict loading is
+    enabled and the variant of an Active Storage preview has already been
+    processed (for example, by calling `ActiveStorage::Preview#url`).
+
+    *Jonathan Hefner*
+
 *   Fix `preprocessed: true` option for named variants of previewable files.
 
     *Nico Wenterodt*

activestorage/app/models/active_storage/blob.rb

@@ -134,7 +134,10 @@ def signed_id_verifier # :nodoc:
 
     def scope_for_strict_loading # :nodoc:
       if strict_loading_by_default? && ActiveStorage.track_variants
-        includes(variant_records: { image_attachment: :blob }, preview_image_attachment: :blob)
+        includes(
+          variant_records: { image_attachment: :blob },
+          preview_image_attachment: { blob: { variant_records: { image_attachment: :blob } } }
+        )
       else
         all
       end

activestorage/test/controllers/representations/redirect_controller_test.rb

@@ -110,7 +110,7 @@ class ActiveStorage::Representations::RedirectControllerWithPreviewsTest < Actio
 class ActiveStorage::Representations::RedirectControllerWithPreviewsWithStrictLoadingTest < ActionDispatch::IntegrationTest
   setup do
     @blob = create_file_blob filename: "report.pdf", content_type: "application/pdf"
-    @blob.preview(resize_to_limit: [100, 100]).processed
+    @blob.preview(resize_to_limit: [100, 100]).processed.send(:variant).processed
   end
 
   test "showing existing preview record inline" do

activestorage/test/models/blob_test.rb

@@ -328,28 +328,13 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
   end
 
   test "scope_for_strict_loading adds includes only when track_variants and strict_loading_by_default" do
-    assert_empty(
-      ActiveStorage::Blob.scope_for_strict_loading.includes_values,
-      "Expected ActiveStorage::Blob.scope_for_strict_loading have no includes"
-    )
+    assert_empty ActiveStorage::Blob.scope_for_strict_loading.includes_values
 
     with_strict_loading_by_default do
-      includes_values = ActiveStorage::Blob.scope_for_strict_loading.includes_values
-
-      assert(
-        includes_values.any? { |values| values[:variant_records] == { image_attachment: :blob } },
-        "Expected ActiveStorage::Blob.scope_for_strict_loading to have variant_records included"
-      )
-      assert(
-        includes_values.any? { |values| values[:preview_image_attachment] == :blob },
-        "Expected ActiveStorage::Blob.scope_for_strict_loading to have preview_image_attachment included"
-      )
+      assert_not_empty ActiveStorage::Blob.scope_for_strict_loading.includes_values
 
       without_variant_tracking do
-        assert_empty(
-          ActiveStorage::Blob.scope_for_strict_loading.includes_values,
-          "Expected ActiveStorage::Blob.scope_for_strict_loading have no includes"
-        )
+        assert_empty ActiveStorage::Blob.scope_for_strict_loading.includes_values
       end
     end
   end