Merge pull request #7300 from SuperTux88/new-mention-syntax

New mention syntax - backend changes

Changelog.md

@@ -6,6 +6,7 @@
 
 ## Features
 * Add support for mentions in comments to the backend [#6818](https://github.com/diaspora/diaspora/pull/6818)
+* Add support for new mention syntax [#7300](https://github.com/diaspora/diaspora/pull/7300)
 
 # 0.6.4.0
 

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)
@@ -32,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))
     }
@@ -45,10 +42,7 @@ module Diaspora::Mentionable
   # @param [String] text containing mentions
   # @return [Array<Person>] 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 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
@@ -64,18 +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
 
-  private
+  # 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
@@ -113,5 +119,4 @@ module Diaspora::Mentionable
       "[#{display_name.presence || person.name}](#{local_or_remote_person_path(person)})"
     end
   end
-
 end

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

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

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")
@@ -143,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

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

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