Merge dd54baa into c5849b4

Gemfile

@@ -257,6 +257,8 @@ group :development do
   # Debugging
   gem "pry"
   gem "pry-byebug"
+  gem "pry-rescue"
+  gem "pry-stack_explorer"
 
   # test coverage
   gem "simplecov", "0.11.2", require: false

Gemfile.lock

@@ -65,6 +65,8 @@ GEM
       railties
     bcrypt (3.1.11)
     bindata (2.3.0)
+    binding_of_caller (0.7.2)
+      debug_inspector (>= 0.0.1)
     bootstrap-sass (3.3.6)
       autoprefixer-rails (>= 5.2.1)
       sass (>= 3.3.4)
@@ -165,6 +167,7 @@ GEM
       railties (>= 3, < 5)
     cucumber-wire (0.0.1)
     database_cleaner (1.5.3)
+    debug_inspector (0.0.2)
     devise (3.5.6)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
@@ -444,6 +447,7 @@ GEM
       i18n-inflector (~> 2.6)
       railties (>= 3.0.0)
     inflecto (0.0.2)
+    interception (0.5)
     ipaddress (0.8.3)
     jasmine (2.4.0)
       jasmine-core (~> 2.4)
@@ -607,6 +611,12 @@ GEM
     pry-byebug (3.4.0)
       byebug (~> 9.0)
       pry (~> 0.10)
+    pry-rescue (1.4.4)
+      interception (>= 0.5)
+      pry
+    pry-stack_explorer (0.4.9.2)
+      binding_of_caller (>= 0.7)
+      pry (>= 0.9.11)
     quiet_assets (1.1.0)
       railties (>= 3.1, < 5.0)
     rack (1.6.4)
@@ -986,6 +996,8 @@ DEPENDENCIES
   pronto-scss (= 0.6.0)
   pry
   pry-byebug
+  pry-rescue
+  pry-stack_explorer
   quiet_assets (= 1.1.0)
   rack-cors (= 0.4.0)
   rack-google-analytics (= 1.2.0)

app/controllers/status_messages_controller.rb

@@ -80,7 +80,10 @@ def handle_create_error(error)
 
   def handle_mention_feedback(status_message)
     return unless comes_from_others_profile_page?
-    flash[:notice] = t("status_messages.create.success", names: status_message.mentioned_people_names)
+    flash[:notice] = t(
+      "status_messages.create.success",
+      names: PersonPresenter.people_names(status_message.mentioned_people)
+    )
   end
 
   def comes_from_others_profile_page?

app/helpers/notifications_helper.rb

@@ -4,40 +4,46 @@ module NotificationsHelper
 
   def object_link(note, actors)
     target_type = note.popup_translation_key
-    actors_count = note.actors.size
+    opts = {actors: actors, count: note.actors.size}
 
-    if note.instance_of?(Notifications::Mentioned)
-      if post = note.linked_object
-        translation(target_type,
-                    actors:    actors,
-                    count:     actors_count,
-                    post_link: link_to(post_page_title(post), post_path(post)).html_safe)
-      else
-        t(note.deleted_translation_key, :actors => actors, :count => actors_count).html_safe
+    if note.respond_to?(:linked_object)
+      if note.linked_object.nil? && note.respond_to?(:deleted_translation_key)
+        target_type = note.deleted_translation_key
+      elsif note.is_a?(Notifications::Mentioned)
+        opts.merge!(opts_for_mentioned(note))
+      elsif %w(Notifications::CommentOnPost Notifications::AlsoCommented Notifications::Reshared Notifications::Liked)
+            .include?(note.type)
+        opts.merge!(opts_for_post(note.linked_object))
       end
-    elsif note.instance_of?(Notifications::CommentOnPost) || note.instance_of?(Notifications::AlsoCommented) || note.instance_of?(Notifications::Reshared) || note.instance_of?(Notifications::Liked)
-      if post = note.linked_object
-        translation(target_type,
-                    actors:      actors,
-                    count:       actors_count,
-                    post_author: h(post.author_name),
-                    post_link:   link_to(post_page_title(post),
-                                         post_path(post),
-                                         data:  {ref: post.id},
-                                         class: "hard_object_link").html_safe)
-      else
-        t(note.deleted_translation_key, :actors => actors, :count => actors_count).html_safe
-      end
-    else #Notifications:StartedSharing, etc.
-      translation(target_type, :actors => actors, :count => actors_count)
     end
+    translation(target_type, opts)
   end
 
   def translation(target_type, opts = {})
-    {:post_author => nil}.merge!(opts)
     t("#{target_type}", opts).html_safe
   end
 
+  def opts_for_post(post)
+    {
+      post_author: h(post.author_name),
+      post_link:   link_to(post_page_title(post),
+                           post_path(post),
+                           data:  {ref: post.id},
+                           class: "hard_object_link").html_safe
+    }
+  end
+
+  def opts_for_mentioned(note)
+    post = note.linked_object.instance_of?(Comment) ? note.linked_object.parent : note.linked_object
+    {
+      post_link: link_to(post_page_title(post), post_path(post)).html_safe
+    }.tap {|opts|
+      if note.linked_object.instance_of?(Comment)
+        opts.merge!(comment_path: post_path(post, anchor: note.linked_object.guid).html_safe)
+      end
+    }
+  end
+
   def notification_people_link(note, people=nil)
     actors =people || note.actors
     number_of_actors = actors.size

app/mailers/notification_mailers/mentioned.rb

@@ -4,7 +4,7 @@ class Mentioned < NotificationMailers::Base
     delegate :author_name, to: :post, prefix: true
 
     def set_headers(target_id)
-      @post = Mention.find_by_id(target_id).post
+      @post = Mention.find_by_id(target_id).mentions_container
 
       @headers[:subject] = I18n.t('notifier.mentioned.subject', :name => @sender.name)
     end

app/models/comment.rb

@@ -11,6 +11,7 @@ class Comment < ActiveRecord::Base
 
   include Diaspora::Taggable
   include Diaspora::Likeable
+  include Diaspora::MentionsContainer
 
   acts_as_taggable_on :tags
   extract_tags_from :text
@@ -23,6 +24,7 @@ class Comment < ActiveRecord::Base
   delegate :name, to: :author, prefix: true
   delegate :comment_email_subject, to: :parent
   delegate :author_name, to: :parent, prefix: true
+  delegate :public, to: :parent
 
   validates :text, :presence => true, :length => {:maximum => 65535}
   validates :parent, :presence => true #should be in relayable (pending on fixing Message)
@@ -32,6 +34,8 @@ class Comment < ActiveRecord::Base
   scope :including_author, -> { includes(:author => :profile) }
   scope :for_a_stream,  -> { including_author.merge(order('created_at ASC')) }
 
+  after_create :create_mentions
+
   before_save do
     self.text.strip! unless self.text.nil?
   end
@@ -46,14 +50,23 @@ class Comment < ActiveRecord::Base
     participation.unparticipate! if participation.present?
   end
 
-  def message
-    @message ||= Diaspora::MessageRenderer.new text
-  end
-
   def text= text
      self[:text] = text.to_s.strip #to_s if for nil, for whatever reason
   end
 
+  def people_allowed_to_be_mentioned
+    return @people_allowed_to_be_mentioned unless @people_allowed_to_be_mentioned.nil?
+    contacts = author.owner.try(:contact_people) if parent.public?
+
+    @people_allowed_to_be_mentioned = [*parent.participants, *contacts, parent.author].uniq
+  end
+
+  def subscribers
+    super.tap {|subscribers|
+      subscribers.concat(mentioned_people) if parent.author.local?
+    }
+  end
+
   class Generator < Diaspora::Federated::Generator
     def self.federated_class
       Comment
@@ -64,6 +77,12 @@ def initialize(person, target, text)
       super(person, target)
     end
 
+    def build(*args)
+      super.tap do |comment|
+        comment.filter_mentions(comment.people_allowed_to_be_mentioned)
+      end
+    end
+
     def relayable_options
       {:post => @target, :text => @text}
     end

app/models/mention.rb

@@ -3,14 +3,28 @@
 #   the COPYRIGHT file.
 
 class Mention < ActiveRecord::Base
-  belongs_to :post
+  belongs_to :mentions_container, polymorphic: :true
+  belongs_to :post, foreign_key: :mentions_container_id, foreign_type: "Post"
   belongs_to :person
-  validates :post, presence: true
+  validates :mentions_container, presence: true
   validates :person, presence: true
+  validate :person_is_allowed_to_be_mentioned
+
+  scope :local, -> {
+    joins(:person).where.not(people: {owner_id: nil})
+  }
 
   after_destroy :delete_notification
 
   def delete_notification
     Notification.where(target_type: self.class.name, target_id: id).destroy_all
   end
+
+  private
+
+  def person_is_allowed_to_be_mentioned
+    ppl = mentions_container.people_allowed_to_be_mentioned
+    return if ppl == "all" || ppl.include?(person)
+    errors.add(:base, "Person can't be mentioned in the #{mentions_container}")
+  end
 end

app/models/notifications/mentioned.rb

@@ -13,19 +13,26 @@ def deleted_translation_key
     end
 
     def linked_object
-      target.post
+      target.mentions_container
     end
 
     def self.notify(mentionable, recipient_user_ids)
       actor = mentionable.author
+      relevant_mentions = filter_mentions(
+        mentionable.find_or_create_mentions.local.where.not(person: mentionable.author),
+        mentionable,
+        recipient_user_ids
+      )
 
-      mentionable.mentions.select {|mention| mention.person.local? }.each do |mention|
+      relevant_mentions.each do |mention|
         recipient = mention.person
-
-        next if recipient == actor || !(mentionable.public || recipient_user_ids.include?(recipient.owner_id))
-
         create_notification(recipient.owner_id, mention, actor).email_the_user(mention, actor)
       end
     end
+
+    def self.filter_mentions(mentions, mentionable, recipient_user_ids)
+      return mentions if mentionable.public
+      mentions.where(person: Person.where(owner_id: recipient_user_ids).ids)
+    end
   end
 end

app/models/notifications/mentioned_in_comment.rb

@@ -0,0 +1,15 @@
+module Notifications
+  class MentionedInComment < Mentioned
+    def popup_translation_key
+      "notifications.mentioned_in_comment"
+    end
+
+    def deleted_translation_key
+      "notifications.mentioned_in_comment_deleted"
+    end
+
+    def self.filter_mentions(mentions, _mentionable, _recipient_user_ids)
+      mentions
+    end
+  end
+end

app/models/post.rb

@@ -12,16 +12,15 @@ class Post < ActiveRecord::Base
   include Diaspora::Likeable
   include Diaspora::Commentable
   include Diaspora::Shareable
+  include Diaspora::MentionsContainer
 
   has_many :participations, dependent: :delete_all, as: :target, inverse_of: :target
-  has_many :participants, class_name: "Person", through: :participations, source: :author
+  has_many :participants, through: :participations, source: :author
 
   attr_accessor :user_like
 
   has_many :reports, as: :item
 
-  has_many :mentions, dependent: :destroy
-
   has_many :reshares, class_name: "Reshare", foreign_key: :root_guid, primary_key: :guid
   has_many :resharers, class_name: "Person", through: :reshares, source: :author
 
@@ -59,7 +58,6 @@ def post_type
   end
 
   def root; end
-  def mentioned_people; []; end
   def photos; []; end
 
   #prevents error when trying to access @post.address in a post different than Reshare and StatusMessage types;

app/models/status_message.rb

@@ -23,7 +23,6 @@ class StatusMessage < Post
   attr_accessor :oembed_url
   attr_accessor :open_graph_url
 
-  after_create :create_mentions
   after_commit :queue_gather_oembed_data, :on => :create, :if => :contains_oembed_url_in_text?
   after_commit :queue_gather_open_graph_data, :on => :create, :if => :contains_open_graph_url_in_text?
 
@@ -52,37 +51,6 @@ def nsfw
     text.try(:match, /#nsfw/i) || super
   end
 
-  def message
-    @message ||= Diaspora::MessageRenderer.new(text, mentioned_people: mentioned_people)
-  end
-
-  def mentioned_people
-    if self.persisted?
-      create_mentions if self.mentions.empty?
-      self.mentions.includes(:person => :profile).map{ |mention| mention.person }
-    else
-      Diaspora::Mentionable.people_from_string(text)
-    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
-    ppl = Diaspora::Mentionable.people_from_string(text)
-    ppl.each do |person|
-      self.mentions.find_or_create_by(person_id: person.id)
-    end
-  end
-
-  def mentions?(person)
-    mentioned_people.include? person
-  end
-
   def comment_email_subject
     message.title
   end
@@ -121,6 +89,22 @@ def post_location
     }
   end
 
+  def self.people_allowed_to_be_mentioned(public, aspects)
+    public ? "all" : AspectMembership.where(aspect_id: aspects).map(&:person)
+  end
+
+  def people_allowed_to_be_mentioned
+    @aspects_ppl ||= StatusMessage.people_allowed_to_be_mentioned(public?, aspects).tap {|ppl|
+      ppl.concat(share_visibilities.map {|sv| sv.user.person }) unless public?
+    }
+  end
+
+  def subscribers
+    super.tap {|subscribers|
+      subscribers.concat(mentioned_people)
+    }
+  end
+
   private
 
   def presence_of_content