let mention-regex only match usable strings

closes #6658

Changelog.md

@@ -2,6 +2,7 @@
 
 ## Refactor
 * Internationalize controller rescue\_from text [#6554](https://github.com/diaspora/diaspora/pull/6554)
+* Make mention parsing a bit more robust [#6658](https://github.com/diaspora/diaspora/pull/6658)
 
 ## Bug fixes
 * Fix plural rules handling more than wanted as "one" [#6630](https://github.com/diaspora/diaspora/pull/6630)

app/controllers/status_messages_controller.rb

@@ -65,6 +65,7 @@ class StatusMessagesController < ApplicationController
   end
 
   def handle_create_error(error)
+    logger.debug error
     respond_to do |format|
       format.html { redirect_to :back }
       format.mobile { redirect_to stream_path }

lib/diaspora/mentionable.rb

@@ -5,7 +5,7 @@ module Diaspora::Mentionable
   # ex.
   #   "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"

spec/lib/diaspora/mentionable_spec.rb

@@ -1,5 +1,5 @@
 
-require 'spec_helper'
+require "spec_helper"
 
 describe Diaspora::Mentionable do
   include PeopleHelper
@@ -21,26 +21,26 @@ STR
     @status_msg = FactoryGirl.build(:status_message, text: @test_txt)
   end
 
-  describe '#format' do
+  describe "#format" do
-    context 'html output' do
+    context "html output" do
-      it 'adds the links to the formatted message' do
+      it "adds the links to the formatted message" do
         fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people)
 
         @people.each do |person|
-          expect(fmt_msg).to include person_link(person, class: 'mention hovercardable')
+          expect(fmt_msg).to include person_link(person, class: "mention hovercardable")
         end
       end
 
-      it 'should work correct when message is escaped html' do
+      it "should work correct when message is escaped html" do
         raw_msg = @status_msg.raw_message
-        fmt_msg = Diaspora::Mentionable.format(CGI::escapeHTML(raw_msg), @people)
+        fmt_msg = Diaspora::Mentionable.format(CGI.escapeHTML(raw_msg), @people)
 
         @people.each do |person|
-          expect(fmt_msg).to include person_link(person, class: 'mention hovercardable')
+          expect(fmt_msg).to include person_link(person, class: "mention hovercardable")
         end
       end
 
-      it 'escapes the link title (name)' do
+      it "escapes the link title (name)" do
         p = @people[0].profile
         p.first_name = "</a><script>alert('h')</script>"
         p.save!
@@ -52,8 +52,8 @@ STR
       end
     end
 
-    context 'plain text output' do
+    context "plain text output" do
-      it 'removes mention markup and displays unformatted name' do
+      it "removes mention markup and displays unformatted name" do
         fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, @people, plain_text: true)
 
         @people.each do |person|
@@ -63,73 +63,73 @@ STR
       end
     end
 
-    it 'leaves the name of people that cannot be found' do
+    it "leaves the name of people that cannot be found" do
       fmt_msg = Diaspora::Mentionable.format(@status_msg.raw_message, [])
       expect(fmt_msg).to eql @test_txt_plain
     end
   end
 
-  describe '#people_from_string' do
+  describe "#people_from_string" do
-    it 'extracts the mentioned people from the text' do
+    it "extracts the mentioned people from the text" do
       ppl = Diaspora::Mentionable.people_from_string(@test_txt)
       expect(ppl).to include(*@people)
     end
 
-    describe 'returns an empty array if nobody was found' do
+    describe "returns an empty array if nobody was found" do
-      it 'gets a post without mentions' do
+      it "gets a post without mentions" do
         ppl = Diaspora::Mentionable.people_from_string("post w/o mentions")
         expect(ppl).to be_empty
       end
 
-      it 'gets a post with invalid handles' do
+      it "gets a post with invalid handles" do
-        ppl = Diaspora::Mentionable.people_from_string("@{a; xxx@xxx.xx} @{b; yyy@yyyy.yyy}")
+        ppl = Diaspora::Mentionable.people_from_string("@{a; xxx@xxx.xx} @{b; yyy@yyyy.yyy} @{...} @{bla; blubb}")
         expect(ppl).to be_empty
       end
     end
   end
 
-  describe '#filter_for_aspects' do
+  describe "#filter_for_aspects" do
     before do
-      @user_A = FactoryGirl.create(:user_with_aspect, :username => "user_a")
+      @user_a = FactoryGirl.create(:user_with_aspect, username: "user_a")
-      @user_B = FactoryGirl.create(:user, :username => "user_b")
+      @user_b = FactoryGirl.create(:user, username: "user_b")
-      @user_C = FactoryGirl.create(:user, :username => "user_c")
+      @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_b = "@{user B; #{@user_b.diaspora_handle}}"
-      @mention_C = "@{user C; #{@user_C.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_b.person, @user_a.aspects.where(name: "generic"))
-      @user_A.share_with(@user_C.person, @user_A.aspects.where(name: 'second'))
+      @user_a.share_with(@user_c.person, @user_a.aspects.where(name: "second"))
 
-      @test_txt_B = "mentioning #{@mention_B}"
+      @test_txt_b = "mentioning #{@mention_b}"
-      @test_txt_C = "mentioning #{@mention_C}"
+      @test_txt_c = "mentioning #{@mention_c}"
-      @test_txt_BC = "mentioning #{@mention_B}} and #{@mention_C}"
+      @test_txt_bc = "mentioning #{@mention_b}} and #{@mention_c}"
     end
 
-    it 'filters mention, if contact is not in a given aspect' do
+    it "filters mention, if contact is not in a given aspect" do
-      aspect_id = @user_A.aspects.where(name: 'generic').first.id
+      aspect_id = @user_a.aspects.where(name: "generic").first.id
-      txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_C, @user_A, aspect_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.person.name)
-      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_c)
     end
 
-    it 'leaves mention, if contact is in a given aspect' do
+    it "leaves mention, if contact is in a given aspect" do
-      aspect_id = @user_A.aspects.where(name: 'generic').first.id
+      aspect_id = @user_a.aspects.where(name: "generic").first.id
-      txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_B, @user_A, aspect_id)
+      txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_b, @user_a, aspect_id)
 
       expect(txt).to include("user B")
-      expect(txt).to include(@mention_B)
+      expect(txt).to include(@mention_b)
     end
 
-    it 'recognizes "all" as keyword for aspects' do
+    it "recognizes 'all' as keyword for aspects" do
-      txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_BC, @user_A, "all")
+      txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_bc, @user_a, "all")
 
-      expect(txt).to include(@mention_B)
+      expect(txt).to include(@mention_b)
-      expect(txt).to include(@mention_C)
+      expect(txt).to include(@mention_c)
     end
   end
 end

spec/models/mention_spec.rb

@@ -9,7 +9,6 @@ describe Mention, :type => :model do
     before do
       @user = alice
       @aspect1 = @user.aspects.create(:name => 'second_aspect')
-
     end
 
     it 'notifies the person being mentioned' do