From 8e3816e64e1ee91568978b4b5569a2a8e1db9c9e Mon Sep 17 00:00:00 2001 From: Faldrian Date: Wed, 27 Jan 2016 21:21:50 +0100 Subject: [PATCH] let mention-regex only match usable strings closes #6658 --- Changelog.md | 1 + app/controllers/status_messages_controller.rb | 1 + lib/diaspora/mentionable.rb | 2 +- spec/lib/diaspora/mentionable_spec.rb | 88 +++++++++---------- spec/models/mention_spec.rb | 1 - 5 files changed, 47 insertions(+), 46 deletions(-) diff --git a/Changelog.md b/Changelog.md index 8a313b0e4..1950e38aa 100644 --- a/Changelog.md +++ b/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) diff --git a/app/controllers/status_messages_controller.rb b/app/controllers/status_messages_controller.rb index 2109d3b71..a7a4ebea0 100644 --- a/app/controllers/status_messages_controller.rb +++ b/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 } diff --git a/lib/diaspora/mentionable.rb b/lib/diaspora/mentionable.rb index 425b5b379..f64857f77 100644 --- a/lib/diaspora/mentionable.rb +++ b/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" diff --git a/spec/lib/diaspora/mentionable_spec.rb b/spec/lib/diaspora/mentionable_spec.rb index d8f5235de..a328fe7cc 100644 --- a/spec/lib/diaspora/mentionable_spec.rb +++ b/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 - context 'html output' do - it 'adds the links to the formatted message' do + describe "#format" do + context "html output" 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 = "" p.save! @@ -52,8 +52,8 @@ STR end end - context 'plain text output' do - it 'removes mention markup and displays unformatted name' do + context "plain text output" 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 - it 'extracts the mentioned people from the text' 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 include(*@people) end - describe 'returns an empty array if nobody was found' do - it 'gets a post without mentions' do + 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") expect(ppl).to be_empty end - it 'gets a post with invalid handles' do - ppl = Diaspora::Mentionable.people_from_string("@{a; xxx@xxx.xx} @{b; yyy@yyyy.yyy}") + it "gets a post with invalid handles" do + 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_B = FactoryGirl.create(:user, :username => "user_b") - @user_C = FactoryGirl.create(:user, :username => "user_c") + @user_a = FactoryGirl.create(:user_with_aspect, username: "user_a") + @user_b = FactoryGirl.create(:user, username: "user_b") + @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_C = "@{user C; #{@user_C.diaspora_handle}}" + @mention_b = "@{user B; #{@user_b.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_C.person, @user_A.aspects.where(name: 'second')) + @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")) - @test_txt_B = "mentioning #{@mention_B}" - @test_txt_C = "mentioning #{@mention_C}" - @test_txt_BC = "mentioning #{@mention_B}} and #{@mention_C}" + @test_txt_b = "mentioning #{@mention_b}" + @test_txt_c = "mentioning #{@mention_c}" + @test_txt_bc = "mentioning #{@mention_b}} and #{@mention_c}" end - it 'filters mention, if contact is not in a given aspect' do - aspect_id = @user_A.aspects.where(name: 'generic').first.id - txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_C, @user_A, aspect_id) + it "filters mention, if contact is not in a given aspect" do + 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(local_or_remote_person_path(@user_C.person)) + expect(txt).to include(@user_c.person.name) + 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 - aspect_id = @user_A.aspects.where(name: 'generic').first.id - txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_B, @user_A, aspect_id) + it "leaves mention, if contact is in a given aspect" do + aspect_id = @user_a.aspects.where(name: "generic").first.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 - txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_BC, @user_A, "all") + it "recognizes 'all' as keyword for aspects" do + txt = Diaspora::Mentionable.filter_for_aspects(@test_txt_bc, @user_a, "all") - expect(txt).to include(@mention_B) - expect(txt).to include(@mention_C) + expect(txt).to include(@mention_b) + expect(txt).to include(@mention_c) end end end diff --git a/spec/models/mention_spec.rb b/spec/models/mention_spec.rb index 27444cbc5..a3eaa103b 100644 --- a/spec/models/mention_spec.rb +++ b/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