diff --git a/app/helpers/people_helper.rb b/app/helpers/people_helper.rb index 8d5e14c11..18461351e 100644 --- a/app/helpers/people_helper.rb +++ b/app/helpers/people_helper.rb @@ -28,7 +28,9 @@ module PeopleHelper opts[:class] << " self" if defined?(user_signed_in?) && user_signed_in? && current_user.person == person opts[:class] << " hovercardable" if defined?(user_signed_in?) && user_signed_in? && current_user.person != person remote_or_hovercard_link = Rails.application.routes.url_helpers.person_path(person).html_safe - ""\ + "#{html_escape_once(opts[:display_name] || person.name)}"\ + .html_safe end def person_image_tag(person, size = :thumb_small) @@ -58,7 +60,7 @@ module PeopleHelper absolute = opts.delete(:absolute) if person.local? - username = person.diaspora_handle.split('@')[0] + username = person.username unless username.include?('.') opts.merge!(:username => username) if absolute diff --git a/lib/diaspora/mentionable.rb b/lib/diaspora/mentionable.rb index f64857f77..147364cb3 100644 --- a/lib/diaspora/mentionable.rb +++ b/lib/diaspora/mentionable.rb @@ -14,8 +14,8 @@ module Diaspora::Mentionable mention = mention_str.match(REGEX)[2] del_pos = mention.rindex(/;/) - name = mention[0..(del_pos-1)].strip - handle = mention[(del_pos+1)..-1].strip + name = mention[0..(del_pos - 1)].strip + handle = mention[(del_pos + 1)..-1].strip [name, handle] end @@ -85,33 +85,33 @@ module Diaspora::Mentionable module MentionsInternal extend ::PeopleHelper - # output a formatted mention link as defined by the given options, - # use the fallback name if the person is unavailable + # output a formatted mention link as defined by the given arguments. + # if the display name is blank, falls back to the person's name. # @see Diaspora::Mentions#format # # @param [Person] AR Person - # @param [String] fallback name + # @param [String] display name # @param [Hash] formatting options - def self.mention_link(person, fallback_name, opts) - return fallback_name unless person.present? + def self.mention_link(person, display_name, opts) + return display_name unless person.present? if opts[:plain_text] - person.name + display_name.presence || person.name else - person_link(person, class: PERSON_HREF_CLASS) + person_link(person, class: PERSON_HREF_CLASS, display_name: display_name) end end - # output a markdown formatted link to the given person or the given fallback - # string, in case the person is not present + # output a markdown formatted link to the given person with the display name as the link text. + # if the display name is blank, falls back to the person's name. # # @param [Person] AR Person - # @param [String] fallback name + # @param [String] display name # @return [String] markdown person link - def self.profile_link(person, fallback_name) - return fallback_name unless person.present? + def self.profile_link(person, display_name) + return display_name unless person.present? - "[#{person.name}](#{local_or_remote_person_path(person)})" + "[#{display_name.presence || person.name}](#{local_or_remote_person_path(person)})" end # takes a user and an array of aspect ids or an array containing "all" as diff --git a/spec/helpers/people_helper_spec.rb b/spec/helpers/people_helper_spec.rb index 24f0bf445..58b46b73a 100644 --- a/spec/helpers/people_helper_spec.rb +++ b/spec/helpers/people_helper_spec.rb @@ -77,6 +77,11 @@ describe PeopleHelper, :type => :helper do it 'links by id for a local user' do expect(person_link(@user.person)).to include "href='#{person_path(@user.person)}'" end + + it "recognizes the 'display_name' option" do + display_name = "string used as a name" + expect(person_link(@person, display_name: display_name)).to match(%r{]+>#{display_name}}) + end end describe '#local_or_remote_person_path' do diff --git a/spec/integration/mentioning_spec.rb b/spec/integration/mentioning_spec.rb index 8d93ebd81..360c7465f 100644 --- a/spec/integration/mentioning_spec.rb +++ b/spec/integration/mentioning_spec.rb @@ -52,6 +52,7 @@ describe "mentioning", type: :request do expect(status_msg.public?).to be false expect(status_msg.text).to include(@user3.name) expect(status_msg.text).not_to include(@user3.diaspora_handle) + expect(status_msg.text).to include(user_profile_path(username: @user3.username)) expect(stream_for(@user3).map(&:id)).not_to include(status_msg.id) expect(mention_stream_for(@user3).map(&:id)).not_to include(status_msg.id) diff --git a/spec/lib/diaspora/mentionable_spec.rb b/spec/lib/diaspora/mentionable_spec.rb index efc0860b0..280fcbac1 100644 --- a/spec/lib/diaspora/mentionable_spec.rb +++ b/spec/lib/diaspora/mentionable_spec.rb @@ -6,11 +6,12 @@ describe Diaspora::Mentionable do before do @people = [alice, bob, eve].map(&:person) + @names = %w(Alice\ A Bob\ B "Eve>\ E) @test_txt = <<-STR This post contains a lot of mentions -one @{Alice A; #{@people[0].diaspora_handle}}, -two @{Bob B; #{@people[1].diaspora_handle}} and finally -three @{"Eve> E; #{@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 This post contains a lot of mentions @@ -18,53 +19,50 @@ one Alice A, two Bob B and finally three "Eve> E. STR - @status_msg = FactoryGirl.build(:status_message, text: @test_txt) end describe "#format" do context "html output" do it "adds the links to the formatted message" do - fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people) + fmt_msg = Diaspora::Mentionable.format(@test_txt, @people) - @people.each do |person| - expect(fmt_msg).to include person_link(person, class: "mention hovercardable") + [@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 - raw_msg = @status_msg.text - fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(raw_msg), @people) + fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(@test_txt), @people) - @people.each do |person| - expect(fmt_msg).to include person_link(person, class: "mention hovercardable") + [@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 - p = @people[0].profile - p.first_name = "" - p.save! + name = "" + test_txt = "two @{#{name}; #{@people[0].diaspora_handle}} and finally" - fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people) + fmt_msg = Diaspora::Mentionable.format(test_txt, @people) - expect(fmt_msg).not_to include(p.first_name) + expect(fmt_msg).not_to include(name) expect(fmt_msg).to include(">", "<", "'") # ">", "<", "'" end end context "plain text output" do it "removes mention markup and displays unformatted name" do - fmt_msg = Diaspora::Mentionable.format(@status_msg.text, @people, plain_text: true) + fmt_msg = Diaspora::Mentionable.format(@test_txt, @people, plain_text: true) - @people.each do |person| - expect(fmt_msg).to include person.first_name + @names.each do |name| + expect(fmt_msg).to include CGI.escapeHTML(name) end expect(fmt_msg).not_to include "", "hovercardable" end end - it "leaves the name of people that cannot be found" do - fmt_msg = Diaspora::Mentionable.format(@status_msg.text, []) + 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 end end @@ -111,7 +109,7 @@ STR aspect_id = @user_a.aspects.where(name: "generic").first.id txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_c, @user_a, aspect_id) - expect(txt).to include(@user_c.person.name) + expect(txt).to include("user C") 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)