From a0d200d209ab8e94d55d847d1e196c9519edbf94 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 27 Jan 2017 23:44:35 +0100 Subject: [PATCH 1/3] Update regex for new mention syntax See #7276 --- lib/diaspora/mentionable.rb | 20 +++++--------- spec/lib/diaspora/mentionable_spec.rb | 38 ++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/lib/diaspora/mentionable.rb b/lib/diaspora/mentionable.rb index c922096af..4029054d3 100644 --- a/lib/diaspora/mentionable.rb +++ b/lib/diaspora/mentionable.rb @@ -1,23 +1,20 @@ module Diaspora::Mentionable - # regex for finding mention markup in plain text - # ex. + # regex for finding mention markup in plain text: + # "message @{user@pod.net} text" + # it can also contain a name, which gets used as the link text: # "message @{User Name; user@pod.net} text" # will yield "User Name" and "user@pod.net" - REGEX = /(@\{(.+?; [^\}]+)\})/ + REGEX = /@\{(?:([^\}]+?); )?([^\} ]+)\}/ # class attribute that will be added to all mention html links PERSON_HREF_CLASS = "mention hovercardable" def self.mention_attrs(mention_str) - mention = mention_str.match(REGEX)[2] - del_pos = mention.rindex(/;/) + name, diaspora_id = mention_str.match(REGEX).captures - name = mention[0..(del_pos - 1)].strip - handle = mention[(del_pos + 1)..-1].strip - - [name, handle] + [name.try(:strip), diaspora_id.strip] end # takes a message text and returns the text with mentions in (html escaped) @@ -46,7 +43,7 @@ module Diaspora::Mentionable # @return [Array] array of people def self.people_from_string(msg_text) identifiers = msg_text.to_s.scan(REGEX).map do |match_str| - _, identifier = mention_attrs(match_str.first) + identifier = match_str.second.strip identifier if Validation::Rule::DiasporaId.new.valid_value?(identifier) end @@ -72,8 +69,6 @@ module Diaspora::Mentionable } end - private - private_class_method def self.find_or_fetch_person_by_identifier(identifier) Person.find_or_fetch_by_identifier(identifier) rescue DiasporaFederation::Discovery::DiscoveryError @@ -113,5 +108,4 @@ module Diaspora::Mentionable "[#{display_name.presence || 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 de8e2fcac..6a6274ad6 100644 --- a/spec/lib/diaspora/mentionable_spec.rb +++ b/spec/lib/diaspora/mentionable_spec.rb @@ -18,7 +18,27 @@ three "Eve> E. STR end - describe "#format" do + 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) + 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) + 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) + 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) @@ -64,12 +84,24 @@ STR end end - describe "#people_from_string" do + 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) end + it "extracts the mentioned people from the text without name" do + text = "test @{#{@people[0].diaspora_handle}} test" + ppl = Diaspora::Mentionable.people_from_string(text) + 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}}" + ppl = Diaspora::Mentionable.people_from_string(text) + expect(ppl).to match_array([@people[0], @people[1]]) + end + describe "returns an empty array if nobody was found" do it "gets a post without mentions" do ppl = Diaspora::Mentionable.people_from_string("post w/o mentions") @@ -102,7 +134,7 @@ STR end end - describe "#filter_people" do + describe ".filter_people" do before do @user_a = FactoryGirl.create(:user_with_aspect, username: "user_a") @user_b = FactoryGirl.create(:user, username: "user_b") From 322f92110a44f9c0cd77592b71f53b4f41b78174 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 28 Jan 2017 23:54:06 +0100 Subject: [PATCH 2/3] Backport new mention syntax to old syntax for backward compatibility See #7276 --- lib/diaspora/mentionable.rb | 29 ++++++++++++++------- lib/diaspora/mentions_container.rb | 5 ++++ spec/lib/diaspora/mentionable_spec.rb | 25 ++++++++++++++++++ spec/shared_behaviors/mentions_container.rb | 10 +++++++ 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/lib/diaspora/mentionable.rb b/lib/diaspora/mentionable.rb index 4029054d3..f1b8cc918 100644 --- a/lib/diaspora/mentionable.rb +++ b/lib/diaspora/mentionable.rb @@ -29,8 +29,8 @@ module Diaspora::Mentionable people = [*people] msg_text.to_s.gsub(REGEX) {|match_str| - name, handle = mention_attrs(match_str) - person = people.find {|p| p.diaspora_handle == handle } + 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)) } @@ -42,10 +42,7 @@ module Diaspora::Mentionable # @param [String] text containing mentions # @return [Array] array of people def self.people_from_string(msg_text) - identifiers = msg_text.to_s.scan(REGEX).map do |match_str| - identifier = match_str.second.strip - identifier if Validation::Rule::DiasporaId.new.valid_value?(identifier) - end + identifiers = msg_text.to_s.scan(REGEX).map {|match_str| match_str.second.strip } identifiers.compact.uniq.map {|identifier| find_or_fetch_person_by_identifier(identifier) }.compact end @@ -61,16 +58,30 @@ module Diaspora::Mentionable mentioned_ppl = people_from_string(msg_text) msg_text.to_s.gsub(REGEX) {|match_str| - name, handle = mention_attrs(match_str) - person = mentioned_ppl.find {|p| p.diaspora_handle == handle } + 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 } end + # Regex to find mentions with new syntax, only used for backporting to old syntax + NEW_SYNTAX_REGEX = /@\{[^ ]+\}/ + + # replaces new syntax with old syntax, to be compatible with old pods + # @deprecated remove when most of the posts can handle the new syntax + def self.backport_mention_syntax(text) + text.to_s.gsub(NEW_SYNTAX_REGEX) do |match_str| + _, diaspora_id = mention_attrs(match_str) + person = find_or_fetch_person_by_identifier(diaspora_id) + old_syntax = "@{#{person.name}; #{diaspora_id}}" if person + old_syntax || match_str + end + end + private_class_method def self.find_or_fetch_person_by_identifier(identifier) - Person.find_or_fetch_by_identifier(identifier) + Person.find_or_fetch_by_identifier(identifier) if Validation::Rule::DiasporaId.new.valid_value?(identifier) rescue DiasporaFederation::Discovery::DiscoveryError nil end diff --git a/lib/diaspora/mentions_container.rb b/lib/diaspora/mentions_container.rb index 5364aa5ae..6aea4acf5 100644 --- a/lib/diaspora/mentions_container.rb +++ b/lib/diaspora/mentions_container.rb @@ -3,6 +3,11 @@ module Diaspora extend ActiveSupport::Concern included do + before_create do + # TODO: remove when most of the posts can handle the new syntax + self.text = Diaspora::Mentionable.backport_mention_syntax(text) if text + end + after_create :create_mentions has_many :mentions, as: :mentions_container, dependent: :destroy end diff --git a/spec/lib/diaspora/mentionable_spec.rb b/spec/lib/diaspora/mentionable_spec.rb index 6a6274ad6..b01161a82 100644 --- a/spec/lib/diaspora/mentionable_spec.rb +++ b/spec/lib/diaspora/mentionable_spec.rb @@ -175,4 +175,29 @@ STR expect(txt).to include(@mention_b) 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" + expect(Diaspora::Mentionable.backport_mention_syntax(text)).to eq(expected_text) + end + + it "does not change the text, when the mention includes a name" do + text = "mention @{#{@names[0]}; #{@people[0].diaspora_handle}} text" + expect(Diaspora::Mentionable.backport_mention_syntax(text)).to eq(text) + end + + it "does not change the text, when the person is not found" do + text = "mention @{non_existing_user@example.org} text" + expect(Person).to receive(:find_or_fetch_by_identifier).with("non_existing_user@example.org").and_return(nil) + expect(Diaspora::Mentionable.backport_mention_syntax(text)).to eq(text) + end + + it "does not change the text, when the diaspora ID is invalid" do + text = "mention @{invalid_diaspora_id} text" + expect(Person).not_to receive(:find_or_fetch_by_identifier) + expect(Diaspora::Mentionable.backport_mention_syntax(text)).to eq(text) + end + end end diff --git a/spec/shared_behaviors/mentions_container.rb b/spec/shared_behaviors/mentions_container.rb index 87dedfef5..42633e6e9 100644 --- a/spec/shared_behaviors/mentions_container.rb +++ b/spec/shared_behaviors/mentions_container.rb @@ -10,6 +10,16 @@ shared_examples_for "it is mentions container" do target } + describe ".before_create" do + it "backports mention syntax to old syntax" do + text = "mention @{#{people[0].diaspora_handle}} text" + expected_text = "mention @{#{people[0].name}; #{people[0].diaspora_handle}} text" + obj = FactoryGirl.build(described_class.to_s.underscore.to_sym, text: text, author: alice.person) + obj.save + expect(obj.text).to eq(expected_text) + end + end + describe ".after_create" do it "calls create_mentions" do expect(target).to receive(:create_mentions).and_call_original From 0e89d601178a339851c81c4a1b2d53ce347aab62 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sun, 29 Jan 2017 01:45:32 +0100 Subject: [PATCH 3/3] Use name of mentioned person in plain text --- lib/diaspora/message_renderer.rb | 2 +- spec/lib/diaspora/message_renderer_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/diaspora/message_renderer.rb b/lib/diaspora/message_renderer.rb index de616f3d9..9540db631 100644 --- a/lib/diaspora/message_renderer.rb +++ b/lib/diaspora/message_renderer.rb @@ -79,7 +79,7 @@ module Diaspora end def make_mentions_plain_text - @message = Diaspora::Mentionable.format message, [], plain_text: true + @message = Diaspora::Mentionable.format message, options[:mentioned_people], plain_text: true end def render_tags diff --git a/spec/lib/diaspora/message_renderer_spec.rb b/spec/lib/diaspora/message_renderer_spec.rb index fa0aa3b3b..539edec2b 100644 --- a/spec/lib/diaspora/message_renderer_spec.rb +++ b/spec/lib/diaspora/message_renderer_spec.rb @@ -191,6 +191,18 @@ describe Diaspora::MessageRenderer do text = "#hashtag message" expect(message(text).plain_text_without_markdown).to eq text end + + context "with mention" do + it "contains the name of the mentioned person" do + msg = message("@{#{alice.diaspora_handle}} is cool", mentioned_people: alice.person) + expect(msg.plain_text_without_markdown).to eq "#{alice.name} is cool" + end + + it "uses the name from mention when the mention contains a name" 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 + end end describe "#urls" do