Ensuring that embed fallback renders URL (#17680)
Prior to this commit, our fallback HREF was using only the URL's domain (e.g. `guides.rubyonrails.org/routing.html`) instead of the provided URL (e.g. `https://guides.rubyonrails.org/routing.html`). This resulted in the link resolving to `<url-of-article>/guides.rubyonrails.org/routing.html`) With this commit, we're using the given URL for the HREF. An interesting side note, the Ruby on Rails guides includes an `og:title` but not an `og:url` so we hit the fallback condition. Could we use the given URL if `og:url` does not exist? Fixes forem/forem#17679
This commit is contained in:
parent
127e3ac248
commit
0c6b6b9e62
5 changed files with 56 additions and 1 deletions
|
|
@ -6,6 +6,7 @@ class OpenGraphTag < LiquidTagBase
|
|||
def initialize(_tag_name, url, _parse_context)
|
||||
super
|
||||
|
||||
@url = url
|
||||
@page = OpenGraph.new url
|
||||
@url_domain = URI.parse(url).host.delete_prefix("www.")
|
||||
end
|
||||
|
|
@ -15,6 +16,7 @@ class OpenGraphTag < LiquidTagBase
|
|||
partial: PARTIAL,
|
||||
locals: {
|
||||
page: @page,
|
||||
url: @url,
|
||||
url_domain: @url_domain
|
||||
},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ class OpenGraph
|
|||
end
|
||||
|
||||
def main_properties
|
||||
# QUESTION: If we don't have `og:url` could we infer the url based on what was passed?
|
||||
%w[og:title og:url]
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@
|
|||
</div>
|
||||
</div>
|
||||
<% else %>
|
||||
<a href="<%= url_domain %>" target="_blank" rel="noopener noreferrer">
|
||||
<a href="<%= url %>" target="_blank" rel="noopener noreferrer">
|
||||
<%= url_domain %>
|
||||
</a>
|
||||
<% end %>
|
||||
|
|
|
|||
18
spec/fixtures/files/guides.rubyonrails.org-routing.html
vendored
Normal file
18
spec/fixtures/files/guides.rubyonrails.org-routing.html
vendored
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
<!DOCTYPE html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="utf-8">
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>Rails Routing from the Outside In — Ruby on Rails Guides</title>
|
||||
<meta property="og:title" content="Rails Routing from the Outside In — Ruby on Rails Guides" />
|
||||
<meta name="description" content="Rails Routing from the Outside InThis guide covers the user-facing features of Rails routing.After reading this guide, you will know: How to interpret the code in config/routes.rb. How to construct your own routes, using either the preferred resourceful style or the match method. How to declare route parameters, which are passed onto controller actions. How to automatically create paths and URLs using route helpers. Advanced techniques such as creating constraints and mounting Rack endpoints." />
|
||||
<meta property="og:description" content="Rails Routing from the Outside InThis guide covers the user-facing features of Rails routing.After reading this guide, you will know: How to interpret the code in config/routes.rb. How to construct your own routes, using either the preferred resourceful style or the match method. How to declare route parameters, which are passed onto controller actions. How to automatically create paths and URLs using route helpers. Advanced techniques such as creating constraints and mounting Rack endpoints." />
|
||||
<meta property="og:locale" content="en_US" />
|
||||
<meta property="og:site_name" content="Ruby on Rails Guides" />
|
||||
<meta property="og:image" content="https://avatars.githubusercontent.com/u/4223" />
|
||||
<meta property="og:type" content="website" />
|
||||
</head>
|
||||
<body class="guide">
|
||||
<h1>The Guide</h1>
|
||||
</body>
|
||||
</html>
|
||||
|
|
@ -3,6 +3,40 @@ require "rails_helper"
|
|||
RSpec.describe UnifiedEmbed::Tag, type: :liquid_tag do
|
||||
let(:listing) { create(:listing) }
|
||||
|
||||
# See https://github.com/forem/forem/issues/17679; Note the document has `og:title` but not
|
||||
# `og:url`; should we fallback to the given URL instead?
|
||||
# rubocop:disable RSpec/ExampleLength
|
||||
it "handles https://guides.rubyonrails.org" do
|
||||
link = "https://guides.rubyonrails.org/routing.html"
|
||||
stub_request(:head, link)
|
||||
.with(
|
||||
headers: {
|
||||
"Accept" => "*/*",
|
||||
"User-Agent" => "DEV(local) (#{URL.url})"
|
||||
},
|
||||
)
|
||||
.to_return(status: 200, body: "", headers: {})
|
||||
|
||||
stub_request(:get, link)
|
||||
.with(
|
||||
headers: {
|
||||
"Accept" => "*/*",
|
||||
"Accept-Encoding" => "gzip;q=1.0,deflate;q=0.6,identity;q=0.3",
|
||||
"Host" => "guides.rubyonrails.org",
|
||||
"User-Agent" => "Ruby"
|
||||
},
|
||||
)
|
||||
.to_return(
|
||||
status: 200,
|
||||
body: Rails.root.join("spec/fixtures/files/guides.rubyonrails.org-routing.html").read,
|
||||
headers: {},
|
||||
)
|
||||
|
||||
parsed_tag = Liquid::Template.parse("{% embed #{link} %}")
|
||||
expect(parsed_tag.render).to include("<a href=\"#{link}\"")
|
||||
end
|
||||
# rubocop:enable RSpec/ExampleLength
|
||||
|
||||
it "delegates parsing to the link-matching class" do
|
||||
link = "https://gist.github.com/jeremyf/662585f5c4d22184a6ae133a71bf891a"
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue