DEV: Replace custom Onebox blank implementation with ActiveSupport (#23827)

We have a custom implementation of #blank? in our Onebox helpers. This is likely a legacy from when Onebox was a standalone gem. This change replaces all usages with respective incarnations of #blank?, #present?, and #presence from ActiveSupport. It changes a bunch of "unless blank" to "if present" as well.
This commit is contained in:
Ted Johansson 2023-10-07 19:54:26 +02:00 committed by GitHub
parent 85c6914f80
commit 60e624e768
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 42 additions and 77 deletions

View File

@ -94,18 +94,18 @@ module Onebox
html_entities = HTMLEntities.new
d = { link: link }.merge(raw)
if !Onebox::Helpers.blank?(d[:title])
if d[:title].present?
d[:title] = html_entities.decode(Onebox::Helpers.truncate(d[:title], 80))
end
d[:description] ||= d[:summary]
if !Onebox::Helpers.blank?(d[:description])
if d[:description].present?
d[:description] = html_entities.decode(Onebox::Helpers.truncate(d[:description], 250))
end
if !Onebox::Helpers.blank?(d[:site_name])
if d[:site_name].present?
d[:domain] = html_entities.decode(Onebox::Helpers.truncate(d[:site_name], 80))
elsif !Onebox::Helpers.blank?(d[:domain])
elsif d[:domain].present?
d[:domain] = "http://#{d[:domain]}" unless d[:domain] =~ %r{^https?://}
d[:domain] = begin
URI(d[:domain]).host.to_s.sub(/^www\./, "")
@ -118,15 +118,14 @@ module Onebox
d[:image] = d[:image_secure_url] || d[:image_url] || d[:thumbnail_url] || d[:image]
d[:image] = Onebox::Helpers.get_absolute_image_url(d[:image], @url)
d[:image] = Onebox::Helpers.normalize_url_for_output(html_entities.decode(d[:image]))
d[:image] = nil if Onebox::Helpers.blank?(d[:image])
d[:image] = nil if d[:image].blank?
d[:video] = d[:video_secure_url] || d[:video_url] || d[:video]
d[:video] = nil if Onebox::Helpers.blank?(d[:video])
d[:video] = nil if d[:video].blank?
d[:published_time] = d[:article_published_time] unless Onebox::Helpers.blank?(
d[:article_published_time],
)
if !Onebox::Helpers.blank?(d[:published_time])
d[:published_time] = d[:article_published_time] if d[:article_published_time].present?
if d[:published_time].present?
d[:article_published_time] = Time.parse(d[:published_time]).strftime("%-d %b %y")
d[:article_published_time_title] = Time.parse(d[:published_time]).strftime(
"%I:%M%p - %d %B %Y",
@ -134,18 +133,18 @@ module Onebox
end
# Twitter labels
if !Onebox::Helpers.blank?(d[:label1]) && !Onebox::Helpers.blank?(d[:data1]) &&
if d[:label1].present? && d[:data1].present? &&
!!AllowlistedGenericOnebox.allowed_twitter_labels.find { |l|
d[:label1] =~ /#{l}/i
}
d[:label_1] = Onebox::Helpers.truncate(d[:label1])
d[:data_1] = Onebox::Helpers.truncate(d[:data1])
end
if !Onebox::Helpers.blank?(d[:label2]) && !Onebox::Helpers.blank?(d[:data2]) &&
if d[:label2].present? && d[:data2].present? &&
!!AllowlistedGenericOnebox.allowed_twitter_labels.find { |l|
d[:label2] =~ /#{l}/i
}
if Onebox::Helpers.blank?(d[:label_1])
if d[:label_1].blank?
d[:label_1] = Onebox::Helpers.truncate(d[:label2])
d[:data_1] = Onebox::Helpers.truncate(d[:data2])
else
@ -154,8 +153,7 @@ module Onebox
end
end
if Onebox::Helpers.blank?(d[:label_1]) && !Onebox::Helpers.blank?(d[:price_amount]) &&
!Onebox::Helpers.blank?(d[:price_currency])
if d[:label_1].blank? && d[:price_amount].present? && d[:price_currency].present?
d[:label_1] = "Price"
d[:data_1] = Onebox::Helpers.truncate(
"#{d[:price_currency].strip} #{d[:price_amount].strip}",
@ -205,11 +203,11 @@ module Onebox
end
def has_text?
has_title? && !Onebox::Helpers.blank?(data[:description])
has_title? && data[:description].present?
end
def has_title?
!Onebox::Helpers.blank?(data[:title])
data[:title].present?
end
def is_image_article?
@ -221,12 +219,11 @@ module Onebox
end
def has_image?
!Onebox::Helpers.blank?(data[:image])
data[:image].present?
end
def is_video?
data[:type] =~ %r{^video[/\.]} && data[:video_type] == "video/mp4" && # Many sites include 'videos' with text/html types (i.e. iframes)
!Onebox::Helpers.blank?(data[:video])
data[:type] =~ %r{^video[/\.]} && data[:video_type] == "video/mp4" && data[:video].present? # Many sites include 'videos' with text/html types (i.e. iframes)
end
def is_embedded?
@ -267,7 +264,7 @@ module Onebox
end
def image_html
return if Onebox::Helpers.blank?(data[:image])
return if data[:image].blank?
escaped_src = ::Onebox::Helpers.normalize_url_for_output(data[:image])

View File

@ -17,7 +17,7 @@ module Onebox
def placeholder_html
oembed = get_oembed
return if Onebox::Helpers.blank?(oembed.thumbnail_url)
return if oembed.thumbnail_url.blank?
"<img src='#{oembed.thumbnail_url}' #{oembed.title_attr}>"
end

View File

@ -25,8 +25,7 @@ module Onebox
HTML
else
html = Onebox::Engine::AllowlistedGenericOnebox.new(@url, @timeout).to_html
return if Onebox::Helpers.blank?(html)
html
html.presence
end
end
end

View File

@ -22,7 +22,7 @@ module Onebox
short_type = SHORT_TYPES[match[:endpoint].to_sym]
description =
if Onebox::Helpers.blank?(og_data.description)
if og_data.description.blank?
"This #{short_type.to_s.chop.capitalize} is private"
else
Onebox::Helpers.truncate(og_data.description, 250)

View File

@ -46,8 +46,7 @@ module Onebox
HTML
else
html = Onebox::Engine::AllowlistedGenericOnebox.new(@url, @timeout).to_html
return if Onebox::Helpers.blank?(html)
html
html.presence
end
end
end

View File

@ -16,7 +16,7 @@ module Onebox
def placeholder_html
oembed = get_oembed
return if Onebox::Helpers.blank?(oembed.thumbnail_url)
return if oembed.thumbnail_url.blank?
"<img src='#{oembed.thumbnail_url}' #{oembed.title_attr}>"
end

View File

@ -17,7 +17,7 @@ module Onebox
def placeholder_html
oembed = get_oembed
return if Onebox::Helpers.blank?(oembed.thumbnail_url)
return if oembed.thumbnail_url.blank?
"<img src='#{oembed.thumbnail_url}' #{oembed.title_attr}>"
end

View File

@ -82,9 +82,7 @@ module Onebox
if (m["property"] && m["property"][/^twitter:(.+)$/i]) ||
(m["name"] && m["name"][/^twitter:(.+)$/i])
value = (m["content"] || m["value"]).to_s
twitter[$1.tr("-:", "_").to_sym] ||= value unless (
Onebox::Helpers.blank?(value) || value == "0 minutes"
)
twitter[$1.tr("-:", "_").to_sym] ||= value if (value.present? && value != "0 minutes")
end
end
@ -115,7 +113,7 @@ module Onebox
def get_json_response
oembed_url = get_oembed_url
return "{}" if Onebox::Helpers.blank?(oembed_url)
return "{}" if oembed_url.blank?
begin
Onebox::Helpers.fetch_response(oembed_url)
@ -137,12 +135,12 @@ module Onebox
end
if html_doc
if Onebox::Helpers.blank?(oembed_url)
if oembed_url.blank?
application_json = html_doc.at("//link[@type='application/json+oembed']/@href")
oembed_url = application_json.value if application_json
end
if Onebox::Helpers.blank?(oembed_url)
if oembed_url.blank?
text_json = html_doc.at("//link[@type='text/json+oembed']/@href")
oembed_url ||= text_json.value if text_json
end
@ -170,7 +168,7 @@ module Onebox
def set_twitter_data_on_raw
twitter = get_twitter
twitter.each { |k, v| @raw[k] ||= v unless Onebox::Helpers.blank?(v) }
twitter.each { |k, v| @raw[k] ||= v if v.present? }
end
def set_oembed_data_on_raw
@ -185,13 +183,13 @@ module Onebox
def set_favicon_data_on_raw
favicon = get_favicon
@raw[:favicon] = favicon unless Onebox::Helpers.blank?(favicon)
@raw[:favicon] = favicon if favicon.present?
end
def set_description_on_raw
unless @raw[:description]
description = get_description
@raw[:description] = description unless Onebox::Helpers.blank?(description)
@raw[:description] = description if description.present?
end
end
end

View File

@ -34,7 +34,7 @@ module Onebox
def placeholder_html
oembed = get_oembed
return if Onebox::Helpers.blank?(oembed.thumbnail_url)
return if oembed.thumbnail_url.blank?
"<img src='#{oembed.thumbnail_url}' #{oembed.title_attr}>"
end

View File

@ -78,7 +78,7 @@ module Onebox
else
# for channel pages
html = Onebox::Engine::AllowlistedGenericOnebox.new(@url, @timeout).to_html
return if Onebox::Helpers.blank?(html)
return if html.blank?
html.gsub!(%r{['"]//}, "https://")
html
end

View File

@ -165,9 +165,7 @@ module Onebox
end
http.request_head([uri.path, uri.query].join("?")) do |response|
code = response.code.to_i
return nil unless code === 200 || Onebox::Helpers.blank?(response.content_length)
return response.content_length
return response.code.to_i == 200 ? response.content_length.presence : nil
end
end
end
@ -190,27 +188,17 @@ module Onebox
"<div style=\"background:transparent;position:relative;width:#{width}px;height:#{height}px;top:#{height}px;margin-top:-#{height}px;\" onClick=\"style.pointerEvents='none'\"></div>"
end
def self.blank?(value)
if value.nil?
true
elsif String === value
value.empty? || !(/[[:^space:]]/ === value)
else
value.respond_to?(:empty?) ? !!value.empty? : !value
end
end
def self.truncate(string, length = 50)
return string if string.nil?
string.size > length ? string[0...(string.rindex(" ", length) || length)] + "..." : string
end
def self.get(meta, attr)
(meta && !blank?(meta[attr])) ? sanitize(meta[attr]) : nil
(meta && meta[attr].present?) ? sanitize(meta[attr]) : nil
end
def self.sanitize(value, length = 50)
return nil if blank?(value)
return nil if value.blank?
Sanitize.fragment(value).strip
end

View File

@ -13,7 +13,7 @@ module Onebox
private
def extract(doc)
return {} if Onebox::Helpers.blank?(doc)
return {} if doc.blank?
doc
.css('script[type="application/ld+json"]')

View File

@ -14,14 +14,13 @@ module Onebox
def method_missing(attr, *args, &block)
value = get(attr, *args)
return nil if Onebox::Helpers.blank?(value)
return nil if value.blank?
method_name = attr.to_s
if method_name.end_with?(*integer_suffixes)
value.to_i
elsif method_name.end_with?(*url_suffixes)
result = Onebox::Helpers.normalize_url_for_output(value)
result unless Onebox::Helpers.blank?(result)
Onebox::Helpers.normalize_url_for_output(value).presence
else
value
end

View File

@ -25,7 +25,7 @@ module Onebox
COLLECTIONS = %i[article_section article_section_color article_tag]
def extract(doc)
return {} if Onebox::Helpers.blank?(doc)
return {} if doc.blank?
data = {}
@ -35,7 +35,7 @@ module Onebox
if (m["property"] && m["property"][/\A(?:og|article|product):(.+)\z/i]) ||
(m["name"] && m["name"][/\A(?:og|article|product):(.+)\z/i])
value = (m["content"] || m["value"]).to_s
next if Onebox::Helpers.blank?(value)
next if value.blank?
key = $1.tr("-:", "_").to_sym
data[key] ||= value
if key.in?(COLLECTIONS)
@ -48,9 +48,7 @@ module Onebox
# Attempt to retrieve the title from the meta tag
title_element = doc.at_css("title")
if title_element && title_element.text
data[:title] ||= title_element.text unless Onebox::Helpers.blank?(title_element.text)
end
data[:title] ||= title_element.text if title_element && title_element.text.present?
data
end

View File

@ -1,19 +1,6 @@
# frozen_string_literal: true
RSpec.describe Onebox::Helpers do
describe ".blank?" do
it { expect(described_class.blank?("")).to be(true) }
it { expect(described_class.blank?(" ")).to be(true) }
it { expect(described_class.blank?("test")).to be(false) }
it { expect(described_class.blank?(%w[test testing])).to be(false) }
it { expect(described_class.blank?([])).to be(true) }
it { expect(described_class.blank?({})).to be(true) }
it { expect(described_class.blank?(nil)).to be(true) }
it { expect(described_class.blank?(true)).to be(false) }
it { expect(described_class.blank?(false)).to be(true) }
it { expect(described_class.blank?(a: "test")).to be(false) }
end
describe ".truncate" do
let(:test_string) { "Chops off on spaces" }
it { expect(described_class.truncate(test_string)).to eq(test_string) }