From 7dc59eb4bf4cb44d87e30f64a0216da9903a3a68 Mon Sep 17 00:00:00 2001 From: Arit Amana <32520970+msarit@users.noreply.github.com> Date: Tue, 11 Jan 2022 14:55:06 -0500 Subject: [PATCH] Raise Error with Invalid UnifiedEmbed URL (#16051) * raise error with invalid embed url * fix spec; update docs --- app/liquid_tags/unified_embed/tag.rb | 28 ++++++++++------------ spec/liquid_tags/unified_embed/tag_spec.rb | 8 ++++--- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/app/liquid_tags/unified_embed/tag.rb b/app/liquid_tags/unified_embed/tag.rb index 34372b8bf..8f893714d 100644 --- a/app/liquid_tags/unified_embed/tag.rb +++ b/app/liquid_tags/unified_embed/tag.rb @@ -21,22 +21,20 @@ module UnifiedEmbed # @return [LiquidTagBase] def self.new(tag_name, link, parse_context) klass = UnifiedEmbed::Registry.find_liquid_tag_for(link: link) + # If we can't find a registered "embed" tag, let's raise an exception. + # This exception will give the user an opportunity to adjust their approach. + # + # In a prior implementation, we chose to render an A-tag using the given URL. + # With that prior implementation, a user expecting a "rich embed" might not + # notice that they didn't have a rich embed and instead published a basic + # A-tag. In addition, said A-tag would goes nowhere; which may confuse + # users and/or Forem readers. + raise StandardError, "Embed URL not valid" unless klass - if klass - # Why the __send__? Because a LiquidTagBase class "privatizes" - # the `.new` method. And we want to instantiate the specific - # liquid tag for the given link. - klass.__send__(:new, tag_name, link, parse_context) - else - # If we don't know how to handle the embed, let's just give the - # user an A-tag. - super - end - end - - def render(_context) - link, _options = strip_tags(@markup) - %(#{link}) + # Why the __send__? Because a LiquidTagBase class "privatizes" + # the `.new` method. And we want to instantiate the specific + # liquid tag for the given link. + klass.__send__(:new, tag_name, link, parse_context) end end end diff --git a/spec/liquid_tags/unified_embed/tag_spec.rb b/spec/liquid_tags/unified_embed/tag_spec.rb index cb97084b9..e56b199b0 100644 --- a/spec/liquid_tags/unified_embed/tag_spec.rb +++ b/spec/liquid_tags/unified_embed/tag_spec.rb @@ -10,9 +10,11 @@ RSpec.describe UnifiedEmbed::Tag, type: :liquid_tag do expect(GistTag).to have_received(:new) end - it "renders an A-tag when no link-matching class is found" do + it "raises an error when no link-matching class is found" do link = "https://takeonrules.com/about" - parsed_tag = Liquid::Template.parse("{% embed #{link} %}") - expect(parsed_tag.render).to eq(%(#{link})) + + expect do + Liquid::Template.parse("{% embed #{link} %}") + end.to raise_error(StandardError, "Embed URL not valid") end end