replace mentions of out-of-aspect people in limited posts with just a

markdown link to their profile (fixes #2516)

add failing spec for #4160 / #2516

extend the spec a bit more

refactor mention handling in a status message

add method for filtering mentions by aspects

wire mention filtering into the status message model, adapt a few tests to
work properly

cosmetic changes

shorten helper methods

add changelog entry

Changelog.md

@@ -18,6 +18,7 @@
 * Show medium avatar in hovercard [#4203](https://github.com/diaspora/diaspora/pull/4203)
 * Fix posting to Twitter [#2758](https://github.com/diaspora/diaspora/issues/2758)
 * Don't show hovercards for current user in comments [#3999](https://github.com/diaspora/diaspora/issues/3999)
+* Replace mentions of out-of-aspect people with markdown links [#4161](https://github.com/diaspora/diaspora/pull/4161)
 
 ## Features
 
@@ -45,13 +46,13 @@ To update do the following:
 1. Before updating (even before the `git pull`!) stop your application
    server (Unicorn by default, started through Foreman).
 2. In case you did already run `git pull` checkout v0.0.3.4:
-   
+
    ```
    git fetch origin
    git checkout v0.0.3.4
    bundle
    ```
-   
+
 3. Start Resque web (you'll need temporary access to port 5678, check
    your Firewall if needed!):
 
@@ -76,7 +77,7 @@ To update do the following:
 
    Don't forget to close the port on the Firewall again, if you had to open it.
 6. In case you needed to do step 2., run:
-   
+
    ```
    git checkout master
    bundle

app/helpers/markdownify_helper.rb

@@ -35,15 +35,15 @@ def markdownify(target, render_options={})
 
     message = markdown.render(message).html_safe
 
-    if target.respond_to?(:format_mentions)
-      message = target.format_mentions(message)
+    if target.respond_to?(:mentioned_people)
+      message = Diaspora::Mentionable.format(message, target.mentioned_people)
     end
 
     message = Diaspora::Taggable.format_tags(message, :no_escape => true)
 
     return message.html_safe
   end
-  
+
   def strip_markdown(text)
     renderer = Redcarpet::Markdown.new(Redcarpet::Render::StripDown, :autolink => true)
     renderer.render(text).strip

app/models/status_message.rb

@@ -28,6 +28,7 @@ class StatusMessage < Post
   attr_accessible :text, :provider_display_name, :frame_name
   attr_accessor :oembed_url
 
+  before_create :filter_mentions
   after_create :create_mentions
   after_create :queue_gather_oembed_data, :if => :contains_oembed_url_in_text?
 
@@ -75,41 +76,29 @@ def formatted_message(opts={})
     return self.raw_message unless self.raw_message
 
     escaped_message = opts[:plain_text] ? self.raw_message : ERB::Util.h(self.raw_message)
-    mentioned_message = self.format_mentions(escaped_message, opts)
+    mentioned_message = Diaspora::Mentionable.format(escaped_message, self.mentioned_people, opts)
     Diaspora::Taggable.format_tags(mentioned_message, opts.merge(:no_escape => true))
   end
 
-  def format_mentions(text, opts = {})
-    form_message = text.to_str.gsub(Mention::REGEX) do |matched_string|
-      people = self.mentioned_people
-      person = people.detect{ |p|
-        p.diaspora_handle == $~[2] unless p.nil?
-      }
-
-      if opts[:plain_text]
-        person ? ERB::Util.h(person.name) : ERB::Util.h($~[1])
-      else
-        person ? person_link(person, :class => 'mention hovercardable') : ERB::Util.h($~[1])
-      end
-    end
-    form_message
-  end
-
   def mentioned_people
     if self.persisted?
       create_mentions if self.mentions.empty?
       self.mentions.includes(:person => :profile).map{ |mention| mention.person }
     else
-      mentioned_people_from_string
+      Diaspora::Mentionable.people_from_string(self.raw_message)
     end
   end
 
+  ## TODO ----
+  # don't put presentation logic in the model!
   def mentioned_people_names
     self.mentioned_people.map(&:name).join(', ')
   end
+  ## ---- ----
 
   def create_mentions
-    mentioned_people_from_string.each do |person|
+    ppl = Diaspora::Mentionable.people_from_string(self.raw_message)
+    ppl.each do |person|
       self.mentions.find_or_create_by_person_id(person.id)
     end
   end
@@ -122,13 +111,6 @@ def notify_person(person)
     self.mentions.where(:person_id => person.id).first.try(:notify_recipient)
   end
 
-  def mentioned_people_from_string
-    identifiers = self.raw_message.scan(Mention::REGEX).map do |match|
-      match.last
-    end
-    identifiers.empty? ? [] : Person.where(:diaspora_handle => identifiers)
-  end
-
   def after_dispatch(sender)
     self.update_and_dispatch_attached_photos(sender)
   end
@@ -178,6 +160,15 @@ def presence_of_content
     end
   end
 
+  def filter_mentions
+    return if self.public? || self.aspects.empty?
+
+    author_usr = self.author.try(:owner)
+    aspect_ids = self.aspects.map(&:id)
+
+    self.raw_message = Diaspora::Mentionable.filter_for_aspects(self.raw_message, author_usr, *aspect_ids)
+  end
+
   private
   def self.tag_stream(tag_ids)
     joins(:taggings).where('taggings.tag_id IN (?)', tag_ids)

lib/diaspora/mentionable.rb

@@ -0,0 +1,134 @@
+
+module Diaspora::Mentionable
+
+  # regex for finding mention markup in plain text
+  # ex.
+  #   "message @{User Name; user@pod.net} text"
+  #   will yield "User Name" and "user@pod.net"
+  REGEX = /@\{([^;]+); ([^\}]+)\}/
+
+  # class attribute that will be added to all mention html links
+  PERSON_HREF_CLASS = "mention hovercardable"
+
+  # takes a message text and returns the text with mentions in (html escaped)
+  # plain text or formatted with html markup linking to user profiles.
+  # default is html output.
+  #
+  # @param [String] text containing mentions
+  # @param [Array<Person>] list of mentioned people
+  # @param [Hash] formatting options
+  # @return [String] formatted message
+  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
+
+    fmt_msg
+  end
+
+  # takes a message text and returns an array of people constructed from the
+  # contained mentions
+  #
+  # @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
+
+    return [] if identifiers.empty?
+    Person.where(diaspora_handle: identifiers)
+  end
+
+  # takes a message text and converts mentions for people that are not in the
+  # given aspects to simple markdown links, leaving only mentions for people who
+  # will actually be able to receive notifications for being mentioned.
+  #
+  # @param [String] message text
+  # @param [User] aspect owner
+  # @param [Mixed] array containing aspect ids or "all"
+  # @return [String] message text with filtered mentions
+  def self.filter_for_aspects(msg_text, user, *aspects)
+    aspect_ids = MentionsInternal.get_aspect_ids(user, *aspects)
+
+    mentioned_ppl = people_from_string(msg_text)
+    aspects_ppl = AspectMembership.where(aspect_id: aspect_ids)
+                                  .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] }
+
+      mention = match_str
+      mention = MentionsInternal.profile_link(person, m[1]) unless aspects_ppl.include?(person)
+
+      mention
+    end
+
+    filtered_msg
+  end
+
+  private
+
+  # inline module for namespacing
+  module MentionsInternal
+    extend ::PeopleHelper
+
+    # output a formatted mention link as defined by the given options,
+    # use the fallback name if the person is unavailable
+    # @see Diaspora::Mentions#format
+    #
+    # @param [Person] AR Person
+    # @param [String] fallback name
+    # @param [Hash] formatting options
+    def self.mention_link(person, fallback_name, *opts)
+      return fallback_name unless person.present?
+
+      if opts.include?(:plain_text)
+        person.name
+      else
+        person_link(person, class: PERSON_HREF_CLASS)
+      end
+    end
+
+    # output a markdown formatted link to the given person or the given fallback
+    # string, in case the person is not present
+    #
+    # @param [Person] AR Person
+    # @param [String] fallback name
+    # @return [String] markdown person link
+    def self.profile_link(person, fallback_name)
+      return fallback_name unless person.present?
+
+      "[#{person.name}](#{local_or_remote_person_path(person)})"
+    end
+
+    # takes a user and an array of aspect ids or an array containing "all" as
+    # the first element. will do some checking on ids and return them in an array
+    # in case of "all", returns an array with all the users aspect ids
+    #
+    # @param [User] owner of the aspects
+    # @param [Array] aspect ids or "all"
+    # @return [Array] aspect ids
+    def self.get_aspect_ids(user, *aspects)
+      return [] if aspects.empty?
+
+      if (!aspects.first.kind_of?(Integer)) && aspects.first.to_sym == :all
+        return user.aspects.pluck(:id)
+      end
+
+      ids = aspects.select { |id| Integer(id) != nil } # only numeric
+
+      #make sure they really belong to the user
+      user.aspects.where(id: ids).pluck(:id)
+    end
+  end
+
+end

lib/publisher.rb

@@ -28,8 +28,8 @@ def explain?
   private
   def formatted_message
     if self.prefill.present?
-      StatusMessage.new(:text => self.prefill).
-        format_mentions(self.prefill, :plain_text => true)
+      sm = StatusMessage.new(:text => self.prefill)
+      Diaspora::Mentionable.format(sm.raw_message, sm.mentioned_people, plain_text: true)
     end
   end
 end

spec/factories.rb

@@ -99,6 +99,14 @@ def r_str
     end
   end
 
+  factory(:status_message_in_aspect, parent: :status_message) do
+    self.public false
+    after :build do |sm|
+      sm.author = FactoryGirl.create(:user_with_aspect).person
+      sm.aspects << sm.author.owner.aspects.first
+    end
+  end
+
   factory(:photo) do
     sequence(:random_string) {|n| SecureRandom.hex(10) }
     association :author, :factory => :person

spec/integration/mentioning_spec.rb

@@ -0,0 +1,58 @@
+
+require 'spec_helper'
+
+module MentioningSpecHelpers
+  def default_aspect
+    @user1.aspects.where(name: 'generic')
+  end
+
+  def text_mentioning(user)
+    handle = user.diaspora_handle
+    "this is a text mentioning @{Mention User ; #{handle}} ... have fun testing!"
+  end
+
+  def notifications_about_mentioning(user)
+    Notifications::Mentioned.where(recipient_id: user.id)
+  end
+
+  def stream_for(user)
+    stream = Stream::Multi.new(user)
+    stream.posts
+  end
+
+  def users_connected?(user1, user2)
+    user1.contacts.where(person_id: user2.person).count > 0
+  end
+end
+
+
+describe 'mentioning' do
+  include MentioningSpecHelpers
+
+  before do
+    @user1 = FactoryGirl.create :user_with_aspect
+    @user2 = FactoryGirl.create :user
+    @user3 = FactoryGirl.create :user
+
+    @user1.share_with(@user2.person, default_aspect)
+  end
+
+  # see: https://github.com/diaspora/diaspora/issues/4160
+  it 'only mentions people that are in the target aspect' do
+    users_connected?(@user1, @user2).should be_true
+    users_connected?(@user1, @user3).should be_false
+
+    status_msg = nil
+    lambda do
+      status_msg = @user1.post(:status_message, {text: text_mentioning(@user3), to: default_aspect})
+    end.should change(Post, :count).by(1)
+
+    status_msg.should_not be_nil
+    status_msg.public?.should be_false
+    status_msg.text.should include(@user3.name)
+
+    notifications_about_mentioning(@user3).should be_empty
+    stream_for(@user3).map { |item| item.id }.should_not include(status_msg.id)
+  end
+
+end