Fix plaintext mode for mentionable

Also slight refactors to it.

Changelog.md

@@ -18,6 +18,7 @@
 * Do not add a space after adding a mention [#4767](https://github.com/diaspora/diaspora/issues/4767)
 * Fix active user statistics by saving a last seen timestamp for users [#4734](https://github.com/diaspora/diaspora/issues/4734)
 * Render HTML in atom user feed [#4835](https://github.com/diaspora/diaspora/pull/4835)
+* Fix plaintext mode of Mentionable [#4294](https://github.com/diaspora/diaspora/issues/4294)
 
 ## Features
 * You can report a single post by clicking the correct icon in the controler section [#4517](https://github.com/diaspora/diaspora/pull/4517)

lib/diaspora/mentionable.rb

@@ -18,17 +18,15 @@ module Diaspora::Mentionable
   # @param [Array<Person>] list of mentioned people
   # @param [Hash] formatting options
   # @return [String] formatted message
-  def self.format(msg_text, people, *opts)
+  def self.format(msg_text, people, opts={})
     people = [*people]
-    fmt_msg = msg_text.to_s.gsub(REGEX) do |match_str|
-      # for some reason gsub doesn't always produce MatchData...
-      m = REGEX.match(match_str)
-      person = people.detect{ |p| p.diaspora_handle == m[2] }
 
-      ERB::Util.h(MentionsInternal.mention_link(person, m[1], *opts))
-    end
+    msg_text.to_s.gsub(REGEX) {|match_str|
+      name, handle = match_str.match(REGEX).captures
+      person = people.find {|p| p.diaspora_handle == handle }
 
-    fmt_msg
+      ERB::Util.h(MentionsInternal.mention_link(person, name, opts))
+    }
   end
 
   # takes a message text and returns an array of people constructed from the
@@ -37,9 +35,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|
-      match.last
-    end
+    identifiers = msg_text.to_s.scan(REGEX).map(&:last)
 
     return [] if identifiers.empty?
     Person.where(diaspora_handle: identifiers)
@@ -61,18 +57,14 @@ module Diaspora::Mentionable
                                   .includes(:contact => :person)
                                   .map(&:person)
 
-    filtered_msg = msg_text.to_s.gsub(REGEX) do |match_str|
-      # for some reason gsub doesn't always produce MatchData...
-      m = REGEX.match(match_str)
-      person = mentioned_ppl.detect{ |p| p.diaspora_handle == m[2] }
+    msg_text.to_s.gsub(REGEX) {|match_str|
+      name, handle = match_str.match(REGEX).captures
+      person = mentioned_ppl.find {|p| p.diaspora_handle == handle }
 
-      mention = match_str
-      mention = MentionsInternal.profile_link(person, m[1]) unless aspects_ppl.include?(person)
+      mention = MentionsInternal.profile_link(person, name) unless aspects_ppl.include?(person)
 
-      mention
-    end
-
-    filtered_msg
+      mention || match_str
+    }
   end
 
   private
@@ -88,10 +80,10 @@ module Diaspora::Mentionable
     # @param [Person] AR Person
     # @param [String] fallback name
     # @param [Hash] formatting options
-    def self.mention_link(person, fallback_name, *opts)
+    def self.mention_link(person, fallback_name, opts)
       return fallback_name unless person.present?
 
-      if opts.include?(:plain_text)
+      if opts[:plain_text]
         person.name
       else
         person_link(person, class: PERSON_HREF_CLASS)
@@ -120,11 +112,11 @@ module Diaspora::Mentionable
     def self.get_aspect_ids(user, *aspects)
       return [] if aspects.empty?
 
-      if (!aspects.first.kind_of?(Integer)) && aspects.first.to_sym == :all
+      if (!aspects.first.is_a?(Integer)) && aspects.first.to_s == 'all'
         return user.aspects.pluck(:id)
       end
 
-      ids = aspects.select { |id| Integer(id) != nil } # only numeric
+      ids = aspects.reject {|id| Integer(id) == nil } # only numeric
 
       #make sure they really belong to the user
       user.aspects.where(id: ids).pluck(:id)

spec/lib/diaspora/mentionable_spec.rb

@@ -18,7 +18,6 @@ one Alice A,
 two Bob B and finally
 three Eve E.
 STR
-    @short_txt = "@{M1; m1@a.at} text @{M2 ; m2@b.be}text @{M3; m3@c.ca}"
     @status_msg = FactoryGirl.build(:status_message, text: @test_txt)
   end
 
@@ -27,9 +26,9 @@ STR
       it 'adds the links to the formatted message' do
         fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people)
 
-        fmt_msg.should include(person_link(@people[0], class: 'mention hovercardable'))
-        fmt_msg.should include(person_link(@people[1], class: 'mention hovercardable'))
-        fmt_msg.should include(person_link(@people[2], class: 'mention hovercardable'))
+        @people.each do |person|
+          fmt_msg.should include person_link(person, class: 'mention hovercardable')
+        end
       end
 
       it 'escapes the link title (name)' do
@@ -46,10 +45,12 @@ STR
 
     context 'plain text output' do
       it 'removes mention markup and displays unformatted name' do
-        s_msg = FactoryGirl.build(:status_message, text: @short_txt)
-        fmt_msg = Diaspora::Mentionable.format(s_msg.raw_message, @people, plain_text: true)
+        fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people, plain_text: true)
 
-        fmt_msg.should eql "M1 text M2 text M3"
+        @people.each do |person|
+          fmt_msg.should include person.first_name
+        end
+        fmt_msg.should_not include "<a", "</a>", "hovercardable"
       end
     end
 
@@ -95,8 +96,6 @@ STR
       @test_txt_B = "mentioning #{@mention_B}"
       @test_txt_C = "mentioning #{@mention_C}"
       @test_txt_BC = "mentioning #{@mention_B}} and #{@mention_C}"
-
-      Diaspora::Mentionable.stub(:current_user).and_return(@user_A)
     end
 
     it 'filters mention, if contact is not in a given aspect' do