Extend image_tag to accept ActiveStorage Attachments and Variants (#30084)

* Extend image_tag to accept ActiveStorage's Attachments and Variants

* Flip resolve_image_source around

* Add tests for the new use-cases of image_tag

* Remove the higher-level test

* Update image_tag documentation

* Add error states into the test suite

* Re-raise polymorhic_url's NoMethodError as ArgumentError

* delegate_missing_to will raise DelegationError instead of NoMethodError

actionview/lib/action_view/helpers/asset_tag_helper.rb

@@ -200,7 +200,7 @@ def favicon_link_tag(source = "favicon.ico", options = {})
       end
 
       # Returns an HTML image tag for the +source+. The +source+ can be a full
-      # path or a file.
+      # path, a file or an Active Storage attachment.
       #
       # ==== Options
       #
@@ -217,6 +217,8 @@ def favicon_link_tag(source = "favicon.ico", options = {})
       #
       # ==== Examples
       #
+      # Assets (images that are part of your app):
+      #
       #   image_tag("icon")
       #   # => <img alt="Icon" src="/assets/icon" />
       #   image_tag("icon.png")
@@ -235,12 +237,21 @@ def favicon_link_tag(source = "favicon.ico", options = {})
       #   # => <img src="/assets/icon.png" srcset="/assets/icon_2x.png 2x, /assets/icon_4x.png 4x">
       #   image_tag("pic.jpg", srcset: [["pic_1024.jpg", "1024w"], ["pic_1980.jpg", "1980w"]], sizes: "100vw")
       #   # => <img src="/assets/pic.jpg" srcset="/assets/pic_1024.jpg 1024w, /assets/pic_1980.jpg 1980w" sizes="100vw">
+      #
+      # Active Storage (images that are uploaded by the users of your app):
+      #
+      #   image_tag(user.avatar)
+      #   # => <img src="/rails/active_storage/blobs/.../tiger.jpg" alt="Tiger" />
+      #   image_tag(user.avatar.variant(resize: "100x100"))
+      #   # => <img src="/rails/active_storage/variants/.../tiger.jpg" alt="Tiger" />
+      #   image_tag(user.avatar.variant(resize: "100x100"), size: '100')
+      #   # => <img width="100" height="100" src="/rails/active_storage/variants/.../tiger.jpg" alt="Tiger" />
       def image_tag(source, options = {})
         options = options.symbolize_keys
         check_for_image_tag_errors(options)
         skip_pipeline = options.delete(:skip_pipeline)
 
-        src = options[:src] = path_to_image(source, skip_pipeline: skip_pipeline)
+        src = options[:src] = resolve_image_source(source, skip_pipeline)
 
         unless src.start_with?("cid:") || src.start_with?("data:") || src.blank?
           options[:alt] = options.fetch(:alt) { image_alt(src) }
@@ -364,6 +375,16 @@ def multiple_sources_tag_builder(type, sources)
           end
         end
 
+        def resolve_image_source(source, skip_pipeline)
+          if source.is_a?(Symbol) || source.is_a?(String)
+            path_to_image(source, skip_pipeline: skip_pipeline)
+          else
+            polymorphic_url(source)
+          end
+        rescue NoMethodError => e
+          raise ArgumentError, "Can't resolve image into URL: #{e}"
+        end
+
         def extract_dimensions(size)
           size = size.to_s
           if /\A\d+x\d+\z/.match?(size)

actionview/test/template/asset_tag_helper_test.rb

@@ -565,9 +565,6 @@ class PlaceholderImage
     def blank?; true; end
     def to_s; "no-image-yet.png"; end
   end
-  def test_image_tag_with_blank_placeholder
-    assert_equal '<img alt="" src="/images/no-image-yet.png" />', image_tag(PlaceholderImage.new, alt: "")
-  end
   def test_image_path_with_blank_placeholder
     assert_equal "/images/no-image-yet.png", image_path(PlaceholderImage.new)
   end

activestorage/README.md

@@ -80,7 +80,7 @@ Variation of image attachment:
 
 ```erb
 <%# Hitting the variant URL will lazy transform the original blob and then redirect to its new service location %>
-<%= image_tag url_for(user.avatar.variant(resize: "100x100")) %>
+<%= image_tag user.avatar.variant(resize: "100x100") %>
 ```
 
 ## Installation

activestorage/test/template/image_tag_test.rb

+require "test_helper"
+require "database/setup"
+
+class User < ActiveRecord::Base
+  has_one_attached :avatar
+end
+
+class ActiveStorage::ImageTagTest < ActionView::TestCase
+  tests ActionView::Helpers::AssetTagHelper
+
+  setup do
+    @blob = create_image_blob filename: "racecar.jpg"
+  end
+
+  test "blob" do
+    assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url @blob}" />), image_tag(@blob)
+  end
+
+  test "variant" do
+    variant = @blob.variant(resize: "100x100")
+    assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url variant}" />), image_tag(variant)
+  end
+
+  test "attachment" do
+    attachment = ActiveStorage::Attachment.new(blob: @blob)
+    assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url attachment}" />), image_tag(attachment)
+  end
+
+  test "error when attachment's empty" do
+    @user = User.create!(name: "DHH")
+
+    assert_not @user.avatar.attached?
+    assert_raises(ArgumentError) { image_tag(@user.avatar) }
+  end
+
+  test "error when object can't be resolved into url" do
+    unresolvable_object = ActionView::Helpers::AssetTagHelper
+    assert_raises(ArgumentError) { image_tag(unresolvable_object) }
+  end
+end

activesupport/lib/active_support/core_ext/module/delegation.rb

@@ -273,6 +273,8 @@ def respond_to_missing?(name, include_private = false)
       def method_missing(method, *args, &block)
         if #{target}.respond_to?(method)
           #{target}.public_send(method, *args, &block)
+        elsif #{target}.nil?
+          raise DelegationError, "\#{method} delegated to #{target}, but #{target} is nil"
         else
           super
         end