diff --git a/app/jobs/scheduled/badge_grant.rb b/app/jobs/scheduled/badge_grant.rb index cbc591fb2dc..bf124ed72d9 100644 --- a/app/jobs/scheduled/badge_grant.rb +++ b/app/jobs/scheduled/badge_grant.rb @@ -1,6 +1,10 @@ module Jobs class BadgeGrant < Jobs::Scheduled + def self.run + self.new.execute(nil) + end + every 1.day def execute(args) diff --git a/app/models/badge.rb b/app/models/badge.rb index b94e24e9bff..f8b7d600d14 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -11,12 +11,26 @@ class Badge < ActiveRecord::Base FirstShare = 12 FirstFlag = 13 FirstLink = 14 + FirstQuote = 15 # other consts AutobiographerMinBioLength = 10 module Queries + FirstQuote = < p2.topic_id + WHERE NOT reflection AND p1.topic_id <> p2.topic_id AND not quote GROUP BY l1.user_id ) ids JOIN topic_links l ON l.id = ids.id diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index ef27432a8cd..50f9253c0eb 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -104,12 +104,13 @@ class TopicLink < ActiveRecord::Base PrettyText .extract_links(post.cooked) - .map{|u| [u, URI.parse(u)] rescue nil} + .map{|u| [u, URI.parse(u.url)] rescue nil} .reject{|u,p| p.nil?} - .uniq{|u,p| u} - .each do |url, parsed| + .uniq{|u,p| p} + .each do |link, parsed| begin + url = link.url internal = false topic_id = nil post_number = nil @@ -157,7 +158,9 @@ class TopicLink < ActiveRecord::Base domain: parsed.host || Discourse.current_hostname, internal: internal, link_topic_id: topic_id, - link_post_id: reflected_post.try(:id)) + link_post_id: reflected_post.try(:id), + quote: link.is_quote + ) # Create the reflection if we can if topic_id.present? diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 5e5a0999877..f95c2a79124 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1993,3 +1993,6 @@ en: first_link: name: First Link description: Added an internal link to another topic + first_quote: + name: First Quote + description: Quoted a user diff --git a/db/fixtures/006_badges.rb b/db/fixtures/006_badges.rb index 929913a46a8..a8cb34eccf0 100644 --- a/db/fixtures/006_badges.rb +++ b/db/fixtures/006_badges.rb @@ -27,6 +27,15 @@ Badge.seed do |b| b.query = Badge::Queries::FirstLink end +Badge.seed do |b| + b.id = Badge::FirstQuote + b.name = "First Quote" + b.badge_type_id = BadgeType::Bronze + b.multiple_grant = false + b.target_posts = true + b.query = Badge::Queries::FirstQuote +end + Badge.seed do |b| b.id = Badge::FirstLike b.name = "First Like" diff --git a/db/migrate/20140710224658_add_is_quote_to_topic_links.rb b/db/migrate/20140710224658_add_is_quote_to_topic_links.rb new file mode 100644 index 00000000000..0abe4d3c8e1 --- /dev/null +++ b/db/migrate/20140710224658_add_is_quote_to_topic_links.rb @@ -0,0 +1,23 @@ +class AddIsQuoteToTopicLinks < ActiveRecord::Migration + def up + add_column :topic_links, :quote, :boolean, default: false, null: false + + # a primitive backfill, eventual rebake will catch missing + execute " + UPDATE topic_links + SET quote = true + WHERE id IN ( + SELECT l.id + FROM topic_links l + JOIN posts p ON p.id = l.post_id + JOIN posts lp ON l.link_post_id = lp.id + WHERE p.raw LIKE '%\[quote=%post:' || + lp.post_number::varchar || ',%topic:' || + lp.topic_id::varchar || '%\]%\[/quote]%' + )" + end + + def down + remove_column :topic_links, :quote + end +end diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index e116f37f929..83a3f5f7117 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -203,13 +203,29 @@ module PrettyText doc.to_html end + class DetectedLink + attr_accessor :is_quote, :url + + def initialize(url, is_quote=false) + @url = url + @is_quote = is_quote + end + end + + def self.extract_links(html) links = [] doc = Nokogiri::HTML.fragment(html) # remove href inside quotes doc.css("aside.quote a").each { |l| l["href"] = "" } + # extract all links from the post - doc.css("a").each { |l| links << l["href"] unless l["href"].blank? } + doc.css("a").each { |l| + unless l["href"].blank? + links << DetectedLink.new(l["href"]) + end + } + # extract links to quotes doc.css("aside.quote[data-topic]").each do |a| topic_id = a['data-topic'] @@ -219,7 +235,7 @@ module PrettyText url << "/#{post_number}" end - links << url + links << DetectedLink.new(url, true) end links diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb index 19b242de442..682649077a0 100644 --- a/spec/components/pretty_text_spec.rb +++ b/spec/components/pretty_text_spec.rb @@ -139,27 +139,37 @@ describe PrettyText do PrettyText.extract_links("\n").to_a.should be_empty end + def extract_urls(text) + PrettyText.extract_links(text).map(&:url).to_a + end + it "should be able to extract links" do - PrettyText.extract_links("http://bla.com").to_a.should == ["http://cnn.com"] + extract_urls("http://bla.com").should == ["http://cnn.com"] end it "should extract links to topics" do - PrettyText.extract_links("").to_a.should == ["/t/topic/321"] + extract_urls("").should == ["/t/topic/321"] end it "should extract links to posts" do - PrettyText.extract_links("").to_a.should == ["/t/topic/1234/4567"] + extract_urls("").should == ["/t/topic/1234/4567"] end it "should not extract links inside quotes" do - PrettyText.extract_links(" + links = PrettyText.extract_links(" http://useless1.com http://useless2.com - ").to_a.should == ["http://body_only.com", "http://body_and_quote.com", "/t/topic/1234"] + ") + + links.map{|l| [l.url, l.is_quote]}.to_a.sort.should == + [["http://body_only.com",false], + ["http://body_and_quote.com", false], + ["/t/topic/1234",true] + ].sort end it "should not preserve tags in code blocks" do diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index a50d15ebeb3..871ad64304e 100644 --- a/spec/models/topic_link_spec.rb +++ b/spec/models/topic_link_spec.rb @@ -2,194 +2,187 @@ require 'spec_helper' describe TopicLink do - it { should belong_to :topic } - it { should belong_to :post } - it { should belong_to :user } - it { should have_many :topic_link_clicks } it { should validate_presence_of :url } def test_uri URI.parse(Discourse.base_url) end - before do - @topic = Fabricate(:topic, title: 'unique topic name') - @user = @topic.user + let(:topic) do + Fabricate(:topic, title: 'unique topic name') + end + + let(:user) do + topic.user end it "can't link to the same topic" do - ftl = TopicLink.new(url: "/t/#{@topic.id}", - topic_id: @topic.id, - link_topic_id: @topic.id) + ftl = TopicLink.new(url: "/t/#{topic.id}", + topic_id: topic.id, + link_topic_id: topic.id) ftl.valid?.should be_false end describe 'external links' do before do - @post = Fabricate(:post, raw: " + post = Fabricate(:post, raw: " http://a.com/ http://b.com/b http://#{'a'*200}.com/invalid http://b.com/#{'a'*500} - ", user: @user, topic: @topic) + ", user: user, topic: topic) - TopicLink.extract_from(@post) + TopicLink.extract_from(post) end it 'works' do # has the forum topic links - @topic.topic_links.count.should == 2 + topic.topic_links.count.should == 2 # works with markdown links - @topic.topic_links.exists?(url: "http://a.com/").should be_true + topic.topic_links.exists?(url: "http://a.com/").should be_true #works with markdown links followed by a period - @topic.topic_links.exists?(url: "http://b.com/b").should be_true + topic.topic_links.exists?(url: "http://b.com/b").should be_true end end describe 'internal links' do - context "rendered onebox" do + it "extracts onebox" do + other_topic = Fabricate(:topic, user: user) + other_topic.posts.create(user: user, raw: "some content for the first post") + other_post = other_topic.posts.create(user: user, raw: "some content for the second post") - before do - @other_topic = Fabricate(:topic, user: @user) - @other_topic.posts.create(user: @user, raw: "some content for the first post") - @other_post = @other_topic.posts.create(user: @user, raw: "some content for the second post") + url = "http://#{test_uri.host}/t/#{other_topic.slug}/#{other_topic.id}/#{other_post.post_number}" + invalid_url = "http://#{test_uri.host}/t/#{other_topic.slug}/9999999999999999999999999999999" - @url = "http://#{test_uri.host}/t/#{@other_topic.slug}/#{@other_topic.id}/#{@other_post.post_number}" - @invalid_url = "http://#{test_uri.host}/t/#{@other_topic.slug}/9999999999999999999999999999999" + topic.posts.create(user: user, raw: 'initial post') + post = topic.posts.create(user: user, raw: "Link to another topic:\n\n#{url}\n\n#{invalid_url}") + post.reload - @topic.posts.create(user: @user, raw: 'initial post') - @post = @topic.posts.create(user: @user, raw: "Link to another topic:\n\n#{@url}\n\n#{@invalid_url}") - @post.reload - - TopicLink.extract_from(@post) - - @link = @topic.topic_links.first - end - - it 'works' do - # should have a link - @link.should be_present - # should be the canonical URL - @link.url.should == @url - end + TopicLink.extract_from(post) + link = topic.topic_links.first + # should have a link + link.should be_present + # should be the canonical URL + link.url.should == url end + context 'topic link' do - before do - @other_topic = Fabricate(:topic, user: @user) - @other_post = @other_topic.posts.create(user: @user, raw: "some content") - @url = "http://#{test_uri.host}/t/#{@other_topic.slug}/#{@other_topic.id}" + let(:other_topic) do + Fabricate(:topic, user: user) + end - @topic.posts.create(user: @user, raw: 'initial post') - @post = @topic.posts.create(user: @user, raw: "Link to another topic: #{@url}") - - TopicLink.extract_from(@post) - - @link = @topic.topic_links.first + let(:post) do + other_topic.posts.create(user: user, raw: "some content") end it 'works' do - # extracted the link - @link.should be_present - # is set to internal - @link.should be_internal + # ensure other_topic has a post + post - # has the correct url - @link.url.should == @url + url = "http://#{test_uri.host}/t/#{other_topic.slug}/#{other_topic.id}" - # has the extracted domain - @link.domain.should == test_uri.host + topic.posts.create(user: user, raw: 'initial post') + linked_post = topic.posts.create(user: user, raw: "Link to another topic: #{url}") - # should have the id of the linked forum - @link.link_topic_id == @other_topic.id + TopicLink.extract_from(linked_post) - # should not be the reflection - @link.should_not be_reflection - end + link = topic.topic_links.first + link.should be_present + link.should be_internal + link.url.should == url + link.domain.should == test_uri.host + link.link_topic_id == other_topic.id + link.should_not be_reflection - describe 'reflection in the other topic' do + reflection = other_topic.topic_links.first - before do - @reflection = @other_topic.topic_links.first - end + reflection.should be_present + reflection.should be_reflection + reflection.post_id.should be_present + reflection.domain.should == test_uri.host + reflection.url.should == "http://#{test_uri.host}/t/unique-topic-name/#{topic.id}/#{linked_post.post_number}" + reflection.link_topic_id.should == topic.id + reflection.link_post_id.should == linked_post.id - it 'works' do - # exists - @reflection.should be_present - @reflection.should be_reflection - @reflection.post_id.should be_present - @reflection.domain.should == test_uri.host - @reflection.url.should == "http://#{test_uri.host}/t/unique-topic-name/#{@topic.id}/#{@post.post_number}" - @reflection.link_topic_id.should == @topic.id - @reflection.link_post_id.should == @post.id - - #has the user id of the original link - @reflection.user_id.should == @link.user_id - end + reflection.user_id.should == link.user_id end context 'removing a link' do before do - @post.revise(@post.user, "no more linkies") - TopicLink.extract_from(@post) + post.revise(post.user, "no more linkies") + TopicLink.extract_from(post) end it 'should remove the link' do - @topic.topic_links.where(post_id: @post.id).should be_blank + topic.topic_links.where(post_id: post.id).should be_blank # should remove the reflected link - @reflection = @other_topic.topic_links.should be_blank + other_topic.topic_links.should be_blank end end end context "link to a user on discourse" do - let(:post) { @topic.posts.create(user: @user, raw: "user") } + let(:post) { topic.posts.create(user: user, raw: "user") } before do TopicLink.extract_from(post) end it 'does not extract a link' do - @topic.topic_links.should be_blank + topic.topic_links.should be_blank end end context "link to a discourse resource like a FAQ" do - let(:post) { @topic.posts.create(user: @user, raw: "faq link here") } + let(:post) { topic.posts.create(user: user, raw: "faq link here") } before do TopicLink.extract_from(post) end it 'does not extract a link' do - @topic.topic_links.should be_present + topic.topic_links.should be_present end end - context "@mention links" do - let(:post) { @topic.posts.create(user: @user, raw: "Hey @#{@user.username_lower}") } + context "mention links" do + let(:post) { topic.posts.create(user: user, raw: "Hey #{user.username_lower}") } before do TopicLink.extract_from(post) end it 'does not extract a link' do - @topic.topic_links.should be_blank + topic.topic_links.should be_blank + end + end + + context "quote links" do + it "sets quote correctly" do + linked_post = topic.posts.create(user: user, raw: "my test post") + quoting_post = Fabricate(:post, raw: "[quote=\"#{user.username}, post: #{linked_post.post_number}, topic: #{topic.id}\"]\nquote\n[/quote]") + + TopicLink.extract_from(quoting_post) + link = quoting_post.topic.topic_links.first + + link.link_post_id.should == linked_post.id + link.quote.should == true end end context "link to a local attachments" do - let(:post) { @topic.posts.create(user: @user, raw: 'ruby.rb') } + let(:post) { topic.posts.create(user: user, raw: 'ruby.rb') } it "extracts the link" do TopicLink.extract_from(post) - link = @topic.topic_links.first + link = topic.topic_links.first # extracted the link link.should be_present # is set to internal @@ -203,11 +196,11 @@ http://b.com/#{'a'*500} end context "link to an attachments uploaded on S3" do - let(:post) { @topic.posts.create(user: @user, raw: 'ruby.rb') } + let(:post) { topic.posts.create(user: user, raw: 'ruby.rb') } it "extracts the link" do TopicLink.extract_from(post) - link = @topic.topic_links.first + link = topic.topic_links.first # extracted the link link.should be_present # is not internal @@ -223,40 +216,36 @@ http://b.com/#{'a'*500} end describe 'internal link from pm' do - before do - @pm = Fabricate(:topic, user: @user, archetype: 'private_message') - @other_post = @pm.posts.create(user: @user, raw: "some content") + it 'works' do + pm = Fabricate(:topic, user: user, archetype: 'private_message') + pm.posts.create(user: user, raw: "some content") - @url = "http://#{test_uri.host}/t/topic-slug/#{@topic.id}" + url = "http://#{test_uri.host}/t/topic-slug/#{topic.id}" - @pm.posts.create(user: @user, raw: 'initial post') - @linked_post = @pm.posts.create(user: @user, raw: "Link to another topic: #{@url}") + pm.posts.create(user: user, raw: 'initial post') + linked_post = pm.posts.create(user: user, raw: "Link to another topic: #{url}") - TopicLink.extract_from(@linked_post) + TopicLink.extract_from(linked_post) - @link = @topic.topic_links.first + topic.topic_links.first.should be_nil + pm.topic_links.first.should_not be_nil end - it 'should not create a reflection' do - @topic.topic_links.first.should be_nil - end - - it 'should not create a normal link' do - @pm.topic_links.first.should_not be_nil - end end describe 'internal link with non-standard port' do it 'includes the non standard port if present' do - @other_topic = Fabricate(:topic, user: @user) - SiteSetting.stubs(:port).returns(5678) + other_topic = Fabricate(:topic, user: user) + SiteSetting.port = 5678 alternate_uri = URI.parse(Discourse.base_url) - @url = "http://#{alternate_uri.host}:5678/t/topic-slug/#{@other_topic.id}" - @post = @topic.posts.create(user: @user, - raw: "Link to another topic: #{@url}") - TopicLink.extract_from(@post) - @reflection = @other_topic.topic_links.first - @reflection.url.should == "http://#{alternate_uri.host}:5678/t/unique-topic-name/#{@topic.id}" + + url = "http://#{alternate_uri.host}:5678/t/topic-slug/#{other_topic.id}" + post = topic.posts.create(user: user, + raw: "Link to another topic: #{url}") + TopicLink.extract_from(post) + reflection = other_topic.topic_links.first + + reflection.url.should == "http://#{alternate_uri.host}:5678/t/unique-topic-name/#{topic.id}" end end