diff --git a/lib/diaspora/mentionable.rb b/lib/diaspora/mentionable.rb index de9cfb5f3..72a0ffcf5 100644 --- a/lib/diaspora/mentionable.rb +++ b/lib/diaspora/mentionable.rb @@ -14,7 +14,7 @@ module Diaspora::Mentionable def self.mention_attrs(mention_str) name, diaspora_id = mention_str.match(REGEX).captures - [name.try(:strip), diaspora_id.strip] + [name.try(:strip).presence, diaspora_id.strip] end # takes a message text and returns the text with mentions in (html escaped) @@ -32,7 +32,7 @@ module Diaspora::Mentionable name, diaspora_id = mention_attrs(match_str) person = people.find {|p| p.diaspora_handle == diaspora_id } - ERB::Util.h(MentionsInternal.mention_link(person, name, opts)) + ERB::Util.h(MentionsInternal.mention_link(person, name, diaspora_id, opts)) } end @@ -60,9 +60,12 @@ module Diaspora::Mentionable msg_text.to_s.gsub(REGEX) {|match_str| name, diaspora_id = mention_attrs(match_str) person = mentioned_ppl.find {|p| p.diaspora_handle == diaspora_id } - mention = MentionsInternal.profile_link(person, name) unless allowed_people.include?(person.id) - mention || match_str + if person && allowed_people.include?(person.id) + match_str + else + MentionsInternal.profile_link(person, name, diaspora_id) + end } end @@ -97,11 +100,11 @@ module Diaspora::Mentionable # @param [Person] AR Person # @param [String] display name # @param [Hash] formatting options - def self.mention_link(person, display_name, opts) - return display_name unless person.present? + def self.mention_link(person, display_name, diaspora_id, opts) + return display_name || diaspora_id unless person.present? if opts[:plain_text] - display_name.presence || person.name + display_name || person.name else person_link(person, class: PERSON_HREF_CLASS, display_name: display_name) end @@ -113,10 +116,10 @@ module Diaspora::Mentionable # @param [Person] AR Person # @param [String] display name # @return [String] markdown person link - def self.profile_link(person, display_name) - return display_name unless person.present? + def self.profile_link(person, display_name, diaspora_id) + return display_name || diaspora_id unless person.present? - "[#{display_name.presence || person.name}](#{local_or_remote_person_path(person)})" + "[#{display_name || person.name}](#{local_or_remote_person_path(person)})" end end end diff --git a/spec/lib/diaspora/mentionable_spec.rb b/spec/lib/diaspora/mentionable_spec.rb index f48e495fa..58b453c17 100644 --- a/spec/lib/diaspora/mentionable_spec.rb +++ b/spec/lib/diaspora/mentionable_spec.rb @@ -1,66 +1,74 @@ describe Diaspora::Mentionable do include PeopleHelper - before do - @people = [alice, bob, eve].map(&:person) - @names = %w(Alice\ A Bob\ B "Eve>\ E) - @test_txt = <<-STR + let(:people) { [alice, bob, eve].map(&:person) } + let(:names) { %w(Alice\ A Bob\ B "Eve>\ E) } + + let(:test_text_with_names) { <<-STR } This post contains a lot of mentions -one @{#{@names[0]}; #{@people[0].diaspora_handle}}, -two @{#{@names[1]}; #{@people[1].diaspora_handle}} and finally -three @{#{@names[2]}; #{@people[2].diaspora_handle}}. +one @{#{names[0]}; #{people[0].diaspora_handle}}, +two @{#{names[1]}; #{people[1].diaspora_handle}} and finally +three @{#{names[2]}; #{people[2].diaspora_handle}}. STR - @test_txt_plain = <<-STR + + let(:test_text_without_names) { <<-STR } This post contains a lot of mentions -one Alice A, -two Bob B and finally -three "Eve> E. +one @{#{people[0].diaspora_handle}}, +two @{#{people[1].diaspora_handle}} and finally +three @{#{people[2].diaspora_handle}}. STR - end describe ".mention_attrs" do it "returns name and diaspora ID" do - name, diaspora_id = Diaspora::Mentionable.mention_attrs("@{#{@names[0]}; #{@people[0].diaspora_handle}}") - expect(name).to eq(@names[0]) - expect(diaspora_id).to eq(@people[0].diaspora_handle) + name, diaspora_id = Diaspora::Mentionable.mention_attrs("@{#{names[0]}; #{people[0].diaspora_handle}}") + expect(name).to eq(names[0]) + expect(diaspora_id).to eq(people[0].diaspora_handle) end it "returns only diaspora-ID when no name is included" do - name, diaspora_id = Diaspora::Mentionable.mention_attrs("@{#{@people[0].diaspora_handle}}") - expect(diaspora_id).to eq(@people[0].diaspora_handle) + name, diaspora_id = Diaspora::Mentionable.mention_attrs("@{#{people[0].diaspora_handle}}") + expect(diaspora_id).to eq(people[0].diaspora_handle) expect(name).to be_nil end it "trims the name if available" do - name, diaspora_id = Diaspora::Mentionable.mention_attrs("@{#{@names[0]} ; #{@people[0].diaspora_handle}}") - expect(name).to eq(@names[0]) - expect(diaspora_id).to eq(@people[0].diaspora_handle) + name, diaspora_id = Diaspora::Mentionable.mention_attrs("@{#{names[0]} ; #{people[0].diaspora_handle}}") + expect(name).to eq(names[0]) + expect(diaspora_id).to eq(people[0].diaspora_handle) end end describe ".format" do context "html output" do it "adds the links to the formatted message" do - fmt_msg = Diaspora::Mentionable.format(@test_txt, @people) + fmt_msg = Diaspora::Mentionable.format(test_text_with_names, people) - [@people, @names].transpose.each do |person, name| + [people, names].transpose.each do |person, name| expect(fmt_msg).to include person_link(person, class: "mention hovercardable", display_name: name) end end - it "should work correct when message is escaped html" do - fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(@test_txt), @people) + it "adds the links to the formatted message and uses the names from the people" do + fmt_msg = Diaspora::Mentionable.format(test_text_without_names, people) - [@people, @names].transpose.each do |person, name| + people.each do |person| + expect(fmt_msg).to include person_link(person, class: "mention hovercardable", display_name: person.name) + end + end + + it "should work correct when message is escaped html" do + fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(test_text_with_names), people) + + [people, names].transpose.each do |person, name| expect(fmt_msg).to include person_link(person, class: "mention hovercardable", display_name: name) end end it "escapes the link title (name)" do name = "" - test_txt = "two @{#{name}; #{@people[0].diaspora_handle}} and finally" + test_txt = "two @{#{name}; #{people[0].diaspora_handle}} and finally" - fmt_msg = Diaspora::Mentionable.format(test_txt, @people) + fmt_msg = Diaspora::Mentionable.format(test_txt, people) expect(fmt_msg).not_to include(name) expect(fmt_msg).to include(">", "<", "'") # ">", "<", "'" @@ -69,9 +77,9 @@ STR context "plain text output" do it "removes mention markup and displays unformatted name" do - fmt_msg = Diaspora::Mentionable.format(@test_txt, @people, plain_text: true) + fmt_msg = Diaspora::Mentionable.format(test_text_with_names, people, plain_text: true) - @names.each do |name| + names.each do |name| expect(fmt_msg).to include CGI.escapeHTML(name) end expect(fmt_msg).not_to include "", "hovercardable" @@ -79,27 +87,46 @@ STR end it "leaves the names of people that cannot be found" do - fmt_msg = Diaspora::Mentionable.format(@test_txt, []) - expect(fmt_msg).to eql @test_txt_plain + test_txt_plain = <<-STR +This post contains a lot of mentions +one Alice A, +two Bob B and finally +three "Eve> E. +STR + + fmt_msg = Diaspora::Mentionable.format(test_text_with_names, []) + expect(fmt_msg).to eql test_txt_plain + end + + it "uses the diaspora ID when the person cannot be found" do + test_txt_plain = <<-STR +This post contains a lot of mentions +one #{people[0].diaspora_handle}, +two #{people[1].diaspora_handle} and finally +three #{people[2].diaspora_handle}. +STR + + fmt_msg = Diaspora::Mentionable.format(test_text_without_names, []) + expect(fmt_msg).to eql test_txt_plain end end describe ".people_from_string" do it "extracts the mentioned people from the text" do - ppl = Diaspora::Mentionable.people_from_string(@test_txt) - expect(ppl).to match_array(@people) + ppl = Diaspora::Mentionable.people_from_string(test_text_with_names) + expect(ppl).to match_array(people) end it "extracts the mentioned people from the text without name" do - text = "test @{#{@people[0].diaspora_handle}} test" + text = "test @{#{people[0].diaspora_handle}} test" ppl = Diaspora::Mentionable.people_from_string(text) - expect(ppl).to match_array([@people[0]]) + expect(ppl).to match_array([people[0]]) end it "extracts the mentioned people from the text mixed mentions (with and without name)" do - text = "@{#{@people[0].diaspora_handle}} and @{#{@names[1]}; #{@people[1].diaspora_handle}}" + text = "@{#{people[0].diaspora_handle}} and @{#{names[1]}; #{people[1].diaspora_handle}}" ppl = Diaspora::Mentionable.people_from_string(text) - expect(ppl).to match_array([@people[0], @people[1]]) + expect(ppl).to match_array([people[0], people[1]]) end describe "returns an empty array if nobody was found" do @@ -135,51 +162,55 @@ STR end describe ".filter_people" do + let(:user_a) { FactoryGirl.create(:user_with_aspect, username: "user_a") } + let(:user_b) { FactoryGirl.create(:user, username: "user_b") } + let(:user_c) { FactoryGirl.create(:user, username: "user_c") } + before do - @user_a = FactoryGirl.create(:user_with_aspect, username: "user_a") - @user_b = FactoryGirl.create(:user, username: "user_b") - @user_c = FactoryGirl.create(:user, username: "user_c") + user_a.aspects.create!(name: "second") - @user_a.aspects.create!(name: "second") - - @mention_b = "@{user B; #{@user_b.diaspora_handle}}" - @mention_c = "@{user C; #{@user_c.diaspora_handle}}" - - @user_a.share_with(@user_b.person, @user_a.aspects.where(name: "generic")) - @user_a.share_with(@user_c.person, @user_a.aspects.where(name: "second")) - - @test_txt_b = "mentioning #{@mention_b}" - @test_txt_c = "mentioning #{@mention_c}" - @test_txt_bc = "mentioning #{@mention_b}} and #{@mention_c}" + user_a.share_with(user_b.person, user_a.aspects.where(name: "generic")) + user_a.share_with(user_c.person, user_a.aspects.where(name: "second")) end it "filters mention, if contact is not in a given aspect" do + mention = "@{user C; #{user_c.diaspora_handle}}" txt = Diaspora::Mentionable.filter_people( - @test_txt_c, - @user_a.aspects.where(name: "generic").first.contacts.map(&:person_id) + "mentioning #{mention}", + user_a.aspects.where(name: "generic").first.contacts.map(&:person_id) ) expect(txt).to include("user C") - expect(txt).to include(local_or_remote_person_path(@user_c.person)) + expect(txt).to include(local_or_remote_person_path(user_c.person)) expect(txt).not_to include("href") - expect(txt).not_to include(@mention_c) + expect(txt).not_to include(mention) end it "leaves mention, if contact is in a given aspect" do + mention = "@{user B; #{user_b.diaspora_handle}}" txt = Diaspora::Mentionable.filter_people( - @test_txt_b, - @user_a.aspects.where(name: "generic").first.contacts.map(&:person_id) + "mentioning #{mention}", + user_a.aspects.where(name: "generic").first.contacts.map(&:person_id) ) expect(txt).to include("user B") - expect(txt).to include(@mention_b) + expect(txt).to include(mention) + end + + it "works if the person cannot be found" do + expect(Person).to receive(:find_or_fetch_by_identifier).with("non_existing_user@example.org").and_return(nil) + + mention = "@{non_existing_user@example.org}" + txt = Diaspora::Mentionable.filter_people("mentioning #{mention}", []) + + expect(txt).to eq "mentioning non_existing_user@example.org" end end describe ".backport_mention_syntax" do it "replaces the new syntax with the old syntax" do - text = "mention @{#{@people[0].diaspora_handle}} text" - expected_text = "mention @{#{@people[0].name}; #{@people[0].diaspora_handle}} text" + text = "mention @{#{people[0].diaspora_handle}} text" + expected_text = "mention @{#{people[0].name}; #{people[0].diaspora_handle}} text" expect(Diaspora::Mentionable.backport_mention_syntax(text)).to eq(expected_text) end @@ -192,7 +223,7 @@ STR end it "does not change the text, when the mention includes a name" do - text = "mention @{#{@names[0]}; #{@people[0].diaspora_handle}} text" + text = "mention @{#{names[0]}; #{people[0].diaspora_handle}} text" expect(Diaspora::Mentionable.backport_mention_syntax(text)).to eq(text) end diff --git a/spec/lib/diaspora/message_renderer_spec.rb b/spec/lib/diaspora/message_renderer_spec.rb index 539edec2b..78611a324 100644 --- a/spec/lib/diaspora/message_renderer_spec.rb +++ b/spec/lib/diaspora/message_renderer_spec.rb @@ -202,6 +202,11 @@ describe Diaspora::MessageRenderer do msg = message("@{Alice; #{alice.diaspora_handle}} is cool", mentioned_people: alice.person) expect(msg.plain_text_without_markdown).to eq "Alice is cool" end + + it "uses the diaspora ID when the person cannot be found" do + msg = message("@{#{alice.diaspora_handle}} is cool", mentioned_people: []) + expect(msg.plain_text_without_markdown).to eq "#{alice.diaspora_handle} is cool" + end end end