diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 5f06261c1b7..cd563f12eda 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -300,10 +300,10 @@ class CookedPostProcessor } # apply oneboxes - Oneboxer.apply(@doc, topic_id: @post.topic_id) { |url| + Oneboxer.apply(@doc, topic_id: @post.topic_id) do |url| @has_oneboxes = true Oneboxer.onebox(url, args) - } + end # make sure we grab dimensions for oneboxed images oneboxed_images.each { |img| limit_size!(img) } diff --git a/lib/onebox/engine/discourse_local_onebox.rb b/lib/onebox/engine/discourse_local_onebox.rb index 3e022e05263..37f7a487d75 100644 --- a/lib/onebox/engine/discourse_local_onebox.rb +++ b/lib/onebox/engine/discourse_local_onebox.rb @@ -3,118 +3,84 @@ module Onebox class DiscourseLocalOnebox include Engine - # we need to allow for multisite here - def self.is_on_site?(url) - Regexp.new("^#{Discourse.base_url.gsub(".","\\.")}.*$", true) === url.to_s - end - # Use this onebox before others def self.priority 1 end def self.===(other) - if other.kind_of?(URI) - uri = other - begin - route = Rails.application.routes.recognize_path(uri.path.sub(Discourse.base_uri, "")) - case route[:controller] - when 'uploads' - is_on_site?(other) - when 'topics' - # super will use matches_regexp to match the domain name - is_on_site?(other) - else - false - end - rescue ActionController::RoutingError - false - end - else - is_on_site?(other) - end + url = other.to_s + return false unless url[Discourse.base_url] + + path = url.sub(Discourse.base_url, "") + route = Rails.application.routes.recognize_path(path) + + !!(route[:controller] =~ /topics|uploads/) + rescue ActionController::RoutingError + false end def to_html - uri = URI::parse(@url) - route = Rails.application.routes.recognize_path(uri.path.sub(Discourse.base_uri, "")) - url = @url.sub(/[&?]source_topic_id=(\d+)/, "") - source_topic_id = $1.to_i + path = @url.sub(Discourse.base_url, "") + route = Rails.application.routes.recognize_path(path) - # Figure out what kind of onebox to show based on the URL case route[:controller] - when 'uploads' + when "uploads" then upload_html(path) + when "topics" then topic_html(path, route) + end + end - url.gsub!("http:", "https:") if SiteSetting.force_https - if File.extname(uri.path) =~ /^.(mov|mp4|webm|ogv)$/ - return "<video width='100%' height='100%' controls><source src='#{url}'><a href='#{url}'>#{url}</a></video>" - elsif File.extname(uri.path) =~ /^.(mp3|ogg|wav)$/ - return "<audio controls><source src='#{url}'><a href='#{url}'>#{url}</a></audio>" - else - return false + private + + def upload_html(path) + case File.extname(path) + when /^\.(mov|mp4|webm|ogv)$/ + "<video width='100%' height='100%' controls><source src='#{@url}'><a href='#{@url}'>#{@url}</a></video>" + when /^\.(mp3|ogg|wav)$/ + "<audio controls><source src='#{@url}'><a href='#{@url}'>#{@url}</a></audio>" end - when 'topics' + end + + def topic_html(path, route) + link = "<a href='#{@url}'>#{@url}</a>" + source_topic_id = @url[/[&?]source_topic_id=(\d+)/, 1].to_i - linked = "<a href='#{url}'>#{url}</a>" if route[:post_number].present? && route[:post_number].to_i > 1 - # Post Link - post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number].to_i) - return linked unless post - return linked if post.hidden - return linked unless Guardian.new.can_see?(post) + post = Post.find_by(topic_id: route[:topic_id], post_number: route[:post_number]) + return link if post.nil? || post.hidden || !Guardian.new.can_see?(post) topic = post.topic slug = Slug.for(topic.title) - excerpt = post.excerpt(SiteSetting.post_onebox_maxlength) - excerpt.gsub!("\n"," ") - # hack to make it render for now - excerpt.gsub!("[/quote]", "[quote]") + excerpt.gsub!(/[\r\n]+/, " ") + excerpt.gsub!("[/quote]", "[quote]") # don't break my quote + quote = "[quote=\"#{post.user.username}, topic:#{topic.id}, slug:#{slug}, post:#{post.post_number}\"]#{excerpt}[/quote]" args = {} args[:topic_id] = source_topic_id if source_topic_id > 0 - cooked = PrettyText.cook(quote, args) - return cooked + PrettyText.cook(quote, args) else - # Topic Link - topic = Topic.where(id: route[:topic_id].to_i).includes(:user).first - return linked unless topic - return linked unless Guardian.new.can_see?(topic) + topic = Topic.find_by(id: route[:topic_id]) + return link if topic.nil? || !Guardian.new.can_see?(topic) - post = topic.posts.first + first_post = topic.ordered_posts.first - posters = topic.posters_summary.map do |p| - { - username: p[:user].username, - avatar: PrettyText.avatar_img(p[:user].avatar_template, 'tiny'), - description: p[:description], - extras: p[:extras] - } - end + args = { + topic: topic.id, + avatar: PrettyText.avatar_img(topic.user.avatar_template, "tiny"), + original_url: @url, + title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)), + category_html: CategoryBadge.html_for(topic.category), + quote: first_post.excerpt(SiteSetting.post_onebox_maxlength), + } - quote = post.excerpt(SiteSetting.post_onebox_maxlength) - args = { original_url: url, - title: PrettyText.unescape_emoji(CGI::escapeHTML(topic.title)), - avatar: PrettyText.avatar_img(topic.user.avatar_template, 'tiny'), - posts_count: topic.posts_count, - last_post: FreedomPatches::Rails4.time_ago_in_words(topic.last_posted_at, false, scope: :'datetime.distance_in_words_verbose'), - age: FreedomPatches::Rails4.time_ago_in_words(topic.created_at, false, scope: :'datetime.distance_in_words_verbose'), - views: topic.views, - posters: posters, - quote: quote, - category_html: CategoryBadge.html_for(topic.category), - topic: topic.id } - - return Mustache.render(File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs"), args) + template = File.read("#{Rails.root}/lib/onebox/templates/discourse_topic_onebox.hbs") + Mustache.render(template, args) end end - rescue ActionController::RoutingError - nil - end - end end end diff --git a/lib/onebox/templates/discourse_topic_onebox.hbs b/lib/onebox/templates/discourse_topic_onebox.hbs index b9ca0aff51b..41a075db7f3 100644 --- a/lib/onebox/templates/discourse_topic_onebox.hbs +++ b/lib/onebox/templates/discourse_topic_onebox.hbs @@ -1,11 +1,9 @@ <aside class='quote' data-post="1" data-topic="{{topic}}"> <div class='title'> - <div class='quote-controls'></div> {{{avatar}}} <a href="{{original_url}}">{{{title}}}</a> {{{category_html}}} </div> - <blockquote>{{{quote}}} - <div class='topic-info'> - </div> + <blockquote> + {{{quote}}} </blockquote> </aside> diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb index db6f184901f..13d81522025 100644 --- a/lib/oneboxer.rb +++ b/lib/oneboxer.rb @@ -62,9 +62,7 @@ module Oneboxer onebox_links = doc.search("a.onebox") if onebox_links.present? onebox_links.each do |link| - if link['href'].present? - yield link['href'], link - end + yield(link['href'], link) if link['href'].present? end end diff --git a/spec/components/onebox/engine/discourse_local_onebox_spec.rb b/spec/components/onebox/engine/discourse_local_onebox_spec.rb index 9cb7e84c0dd..90f9bf4534d 100644 --- a/spec/components/onebox/engine/discourse_local_onebox_spec.rb +++ b/spec/components/onebox/engine/discourse_local_onebox_spec.rb @@ -1,17 +1,8 @@ require 'rails_helper' describe Onebox::Engine::DiscourseLocalOnebox do - it "matches for a topic url" do - url = "#{Discourse.base_url}/t/hot-topic" - expect(Onebox.has_matcher?(url)).to eq(true) - expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class) - end - it "matches for a post url" do - url = "#{Discourse.base_url}/t/hot-topic/23/2" - expect(Onebox.has_matcher?(url)).to eq(true) - expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class) - end + before { SiteSetting.external_system_avatars_enabled = false } context "for a link to a post" do let(:post) { Fabricate(:post) } @@ -36,13 +27,11 @@ describe Onebox::Engine::DiscourseLocalOnebox do it "returns some onebox goodness if post exists and can be seen" do url = "#{Discourse.base_url}#{post2.url}?source_topic_id=#{post2.topic_id+1}" - Guardian.any_instance.stubs(:can_see?).returns(true) html = Onebox.preview(url).to_s expect(html).to include(post2.excerpt) expect(html).to include(post2.topic.title) - url = "#{Discourse.base_url}#{post2.url}" - html = Onebox.preview(url).to_s + html = Onebox.preview("#{Discourse.base_url}#{post2.url}").to_s expect(html).to include(post2.user.username) expect(html).to include(post2.excerpt) end @@ -52,8 +41,6 @@ describe Onebox::Engine::DiscourseLocalOnebox do let(:post) { Fabricate(:post) } let(:topic) { post.topic } - before { topic.last_posted_at = Time.zone.now; topic.save; } # otherwise errors - it "returns a link if topic isn't found" do url = "#{Discourse.base_url}/t/not-found/123" expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>") @@ -67,16 +54,14 @@ describe Onebox::Engine::DiscourseLocalOnebox do it "replaces emoji in the title" do topic.update_column(:title, "Who wants to eat a :hamburger:") - expect(Onebox.preview(topic.url).to_s).to match(/hamburger.png/) + expect(Onebox.preview(topic.url).to_s).to match(/hamburger\.png/) end it "returns some onebox goodness if post exists and can be seen" do - SiteSetting.external_system_avatars_enabled = false - url = "#{topic.url}" Guardian.any_instance.stubs(:can_see?).returns(true) - html = Onebox.preview(url).to_s - expect(html).to include(topic.posts.first.user.username) - expect(html).to include("topic-info") + html = Onebox.preview(topic.url).to_s + expect(html).to include(topic.ordered_posts.first.user.username) + expect(html).to include("<blockquote>") end end @@ -102,59 +87,42 @@ describe Onebox::Engine::DiscourseLocalOnebox do end context "When deployed to a subfolder" do - let(:base_url) { "http://test.localhost/subfolder" } let(:base_uri) { "/subfolder" } + let(:base_url) { "http://test.localhost#{base_uri}" } before do Discourse.stubs(:base_url).returns(base_url) Discourse.stubs(:base_uri).returns(base_uri) end - it "matches for a topic url" do - url = "#{Discourse.base_url}/t/hot-topic" - expect(Onebox.has_matcher?(url)).to eq(true) - expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class) - end - - it "matches for a post url" do - url = "#{Discourse.base_url}/t/hot-topic/23/2" - expect(Onebox.has_matcher?(url)).to eq(true) - expect(Onebox::Matcher.new(url).oneboxed).to eq(described_class) - end - context "for a link to a post" do let(:post) { Fabricate(:post) } let(:post2) { Fabricate(:post, topic: post.topic, post_number: 2) } - it "returns a link if post isn't found" do - url = "#{Discourse.base_url}/t/not-exist/3/2" - expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>") - end - - it "returns a link if not allowed to see the post" do - url = "#{Discourse.base_url}#{post2.url}" - Guardian.any_instance.stubs(:can_see?).returns(false) - expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>") - end - - it "returns a link if post is hidden" do - hidden_post = Fabricate(:post, topic: post.topic, post_number: 2, hidden: true, hidden_reason_id: Post.hidden_reasons[:flag_threshold_reached]) - url = "#{Discourse.base_url}#{hidden_post.url}" - expect(Onebox.preview(url).to_s).to eq("<a href='#{url}'>#{url}</a>") - end - it "returns some onebox goodness if post exists and can be seen" do url = "#{Discourse.base_url}#{post2.url}?source_topic_id=#{post2.topic_id+1}" Guardian.any_instance.stubs(:can_see?).returns(true) html = Onebox.preview(url).to_s expect(html).to include(post2.excerpt) expect(html).to include(post2.topic.title) - - url = "#{Discourse.base_url}#{post2.url}" - html = Onebox.preview(url).to_s - expect(html).to include(post2.user.username) - expect(html).to include(post2.excerpt) end end end + + context "When login_required is enabled" do + before { SiteSetting.login_required = true } + + context "for a link to a topic" do + let(:post) { Fabricate(:post) } + let(:topic) { post.topic } + + it "returns some onebox goodness if post exists and can be seen" do + html = Onebox.preview(topic.url).to_s + expect(html).to include(topic.ordered_posts.first.user.username) + expect(html).to include("<blockquote>") + end + end + + end + end