From 7c89948c416fbc32b59e33a0ab454545b4f6fed7 Mon Sep 17 00:00:00 2001 From: Anton Khamets Date: Mon, 7 Aug 2017 17:38:51 +0300 Subject: [PATCH] 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 --- .../action_view/helpers/asset_tag_helper.rb | 25 +++++++++++- .../test/template/asset_tag_helper_test.rb | 3 -- activestorage/README.md | 2 +- activestorage/test/template/image_tag_test.rb | 40 +++++++++++++++++++ .../core_ext/module/delegation.rb | 2 + 5 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 activestorage/test/template/image_tag_test.rb diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index 66e31978c6..82b0fcbf2e 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/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") # # => Icon # image_tag("icon.png") @@ -235,12 +237,21 @@ def favicon_link_tag(source = "favicon.ico", options = {}) # # => # image_tag("pic.jpg", srcset: [["pic_1024.jpg", "1024w"], ["pic_1980.jpg", "1980w"]], sizes: "100vw") # # => + # + # Active Storage (images that are uploaded by the users of your app): + # + # image_tag(user.avatar) + # # => Tiger + # image_tag(user.avatar.variant(resize: "100x100")) + # # => Tiger + # image_tag(user.avatar.variant(resize: "100x100"), size: '100') + # # => 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) diff --git a/actionview/test/template/asset_tag_helper_test.rb b/actionview/test/template/asset_tag_helper_test.rb index 4d312d3b6d..5b8e50cc5e 100644 --- a/actionview/test/template/asset_tag_helper_test.rb +++ b/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 '', 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 diff --git a/activestorage/README.md b/activestorage/README.md index aad11325e3..957adc05c3 100644 --- a/activestorage/README.md +++ b/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 diff --git a/activestorage/test/template/image_tag_test.rb b/activestorage/test/template/image_tag_test.rb new file mode 100644 index 0000000000..83c95c01a1 --- /dev/null +++ b/activestorage/test/template/image_tag_test.rb @@ -0,0 +1,40 @@ +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 %(Racecar), image_tag(@blob) + end + + test "variant" do + variant = @blob.variant(resize: "100x100") + assert_dom_equal %(Racecar), image_tag(variant) + end + + test "attachment" do + attachment = ActiveStorage::Attachment.new(blob: @blob) + assert_dom_equal %(Racecar), 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 diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 6565d53a06..c979af9de3 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/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 -- GitLab