create NotificationService: send notifications after receive

app/models/notification.rb

@@ -3,15 +3,15 @@
 #   the COPYRIGHT file.
 #
 class Notification < ActiveRecord::Base
-  belongs_to :recipient, :class_name => 'User'
-  has_many :notification_actors, :dependent => :destroy
-  has_many :actors, :class_name => 'Person', :through => :notification_actors, :source => :person
-  belongs_to :target, :polymorphic => true
+  belongs_to :recipient, class_name: "User"
+  has_many :notification_actors, dependent: :destroy
+  has_many :actors, class_name: "Person", through: :notification_actors, source: :person
+  belongs_to :target, polymorphic: true
 
   attr_accessor :note_html
 
   def self.for(recipient, opts={})
-    self.where(opts.merge!(:recipient_id => recipient.id)).order('updated_at desc')
+    where(opts.merge!(recipient_id: recipient.id)).order("updated_at DESC")
   end
 
   def self.notify(recipient, target, actor)
@@ -33,11 +33,11 @@ class Notification < ActiveRecord::Base
   end
 
   def as_json(opts={})
-    super(opts.merge(:methods => :note_html))
+    super(opts.merge(methods: :note_html))
   end
 
   def email_the_user(target, actor)
-    self.recipient.mail(self.mail_job, self.recipient_id, actor.id, target.id)
+    recipient.mail(mail_job, recipient_id, actor.id, target.id)
   end
 
   def set_read_state( read_state )
@@ -45,14 +45,13 @@ class Notification < ActiveRecord::Base
   end
 
   def mail_job
-    raise NotImplementedError.new('Subclass this.')
+    raise NotImplementedError.new("Subclass this.")
   end
 
-  def effective_target
-    self.popup_translation_key == "notifications.mentioned" ? self.target.post : self.target
+  def linked_object
+    target
   end
 
-private
   def self.concatenate_or_create(recipient, target, actor, notification_type)
     return nil if suppress_notification?(recipient, target)
 
@@ -76,7 +75,6 @@ private
     end
   end
 
-
   def self.make_notification(recipient, target, actor, notification_type)
     return nil if suppress_notification?(recipient, target)
     n = notification_type.new(:target => target,
@@ -87,9 +85,28 @@ private
     n
   end
 
+  def self.concatenate_or_create(recipient, target, actor)
+    return nil if suppress_notification?(recipient, target)
+
+    find_or_initialize_by(recipient: recipient, target: target, unread: true).tap do |notification|
+      notification.actors |= [actor]
+      # Explicitly touch the notification to update updated_at whenever new actor is inserted in notification.
+      if notification.new_record? || notification.changed?
+        notification.save!
+      else
+        notification.touch
+      end
+    end
+  end
+
+  def self.create_notification(recipient_id, target, actor)
+    create(recipient_id: recipient_id, target: target, actors: [actor])
+  end
+
   def self.suppress_notification?(recipient, post)
     post.is_a?(Post) && recipient.is_shareable_hidden?(post)
   end
+  private_class_method :suppress_notification?
 
   def self.types
     {

app/models/notifications/also_commented.rb

@@ -1,17 +1,26 @@
-class Notifications::AlsoCommented < Notification
-  def mail_job
-    Workers::Mail::AlsoCommented
-  end
+module Notifications
+  class AlsoCommented < Notification
+    def mail_job
+      Workers::Mail::AlsoCommented
+    end
 
-  def popup_translation_key
-    'notifications.also_commented'
-  end
+    def popup_translation_key
+      "notifications.also_commented"
+    end
 
-  def deleted_translation_key
-    'notifications.also_commented_deleted'
-  end
+    def deleted_translation_key
+      "notifications.also_commented_deleted"
+    end
 
-  def linked_object
-    Post.where(:id => self.target_id).first
+    def self.notify(comment, _recipient_user_ids)
+      actor = comment.author
+      commentable = comment.commentable
+      recipient_ids = commentable.participants.local.where.not(id: [commentable.author_id, actor.id]).pluck(:owner_id)
+
+      User.where(id: recipient_ids).find_each do |recipient|
+        concatenate_or_create(recipient, commentable, actor)
+          .try {|notification| notification.email_the_user(comment, actor) }
+      end
+    end
   end
 end

app/models/notifications/comment_on_post.rb

@@ -1,17 +1,24 @@
-class Notifications::CommentOnPost < Notification
-  def mail_job
-    Workers::Mail::CommentOnPost
-  end
+module Notifications
+  class CommentOnPost < Notification
+    def mail_job
+      Workers::Mail::CommentOnPost
+    end
 
-  def popup_translation_key
-    'notifications.comment_on_post'
-  end
+    def popup_translation_key
+      "notifications.comment_on_post"
+    end
 
-  def deleted_translation_key
-    'notifications.also_commented_deleted'
-  end
+    def deleted_translation_key
+      "notifications.also_commented_deleted"
+    end
 
-  def linked_object
-    Post.where(:id => self.target_id).first
+    def self.notify(comment, _recipient_user_ids)
+      actor = comment.author
+      commentable_author = comment.commentable.author
+
+      return unless commentable_author.local? && actor != commentable_author
+
+      concatenate_or_create(commentable_author.owner, comment.commentable, actor).email_the_user(comment, actor)
+    end
   end
 end

app/models/notifications/liked.rb

@@ -1,19 +1,24 @@
-class Notifications::Liked < Notification
-  def mail_job
-    Workers::Mail::Liked
-  end
+module Notifications
+  class Liked < Notification
+    def mail_job
+      Workers::Mail::Liked
+    end
 
-  def popup_translation_key
-    'notifications.liked'
-  end
+    def popup_translation_key
+      "notifications.liked"
+    end
 
-  def deleted_translation_key
-    'notifications.liked_post_deleted'
-  end
+    def deleted_translation_key
+      "notifications.liked_post_deleted"
+    end
 
-  def linked_object
-    post = self.target
-    post = post.target if post.is_a? Like
-    post
+    def self.notify(like, _recipient_user_ids)
+      actor = like.author
+      target_author = like.target.author
+
+      return unless like.target_type == "Post" && target_author.local? && actor != target_author
+
+      concatenate_or_create(target_author.owner, like.target, actor).email_the_user(like, actor)
+    end
   end
 end

app/models/notifications/mentioned.rb

@@ -1,17 +1,31 @@
-class Notifications::Mentioned < Notification
-  def mail_job
-    Workers::Mail::Mentioned
-  end
+module Notifications
+  class Mentioned < Notification
+    def mail_job
+      Workers::Mail::Mentioned
+    end
 
-  def popup_translation_key
-    'notifications.mentioned'
-  end
+    def popup_translation_key
+      "notifications.mentioned"
+    end
 
-  def deleted_translation_key
-    'notifications.mentioned_deleted'
-  end
+    def deleted_translation_key
+      "notifications.mentioned_deleted"
+    end
 
-  def linked_object
-    Mention.find(self.target_id).post
+    def linked_object
+      target.post
+    end
+
+    def self.notify(mentionable, recipient_user_ids)
+      actor = mentionable.author
+
+      mentionable.mentions.select {|mention| mention.person.local? }.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
   end
 end

app/models/notifications/private_message.rb

@@ -1,15 +1,30 @@
-class Notifications::PrivateMessage < Notification
-  def mail_job
-    Workers::Mail::PrivateMessage
-  end
-  def popup_translation_key
-    'notifications.private_message'
-  end
-  def self.make_notification(recipient, target, actor, notification_type)
-    n = notification_type.new(:target => target,
-                               :recipient_id => recipient.id)
-    target.increase_unread(recipient)
-    n.actors << actor
-    n
+module Notifications
+  class PrivateMessage < Notification
+    def mail_job
+      Workers::Mail::PrivateMessage
+    end
+
+    def popup_translation_key
+      "notifications.private_message"
+    end
+
+    def self.notify(object, recipient_user_ids)
+      case object
+      when Conversation
+        object.messages.each do |message|
+          recipient_ids = recipient_user_ids - [message.author.owner_id]
+          User.where(id: recipient_ids).find_each {|recipient| notify_message(message, recipient) }
+        end
+      when Message
+        recipients = object.conversation.participants.select(&:local?) - [object.author]
+        recipients.each {|recipient| notify_message(object, recipient.owner) }
+      end
+    end
+
+    def self.notify_message(message, recipient)
+      message.increase_unread(recipient)
+      new(recipient: recipient).email_the_user(message, message.author)
+    end
+    private_class_method :notify_message
   end
 end

app/models/notifications/request_accepted.rb

@@ -1,8 +0,0 @@
-class Notifications::RequestAccepted < Notification
-  def mail_job
-    Workers::Mail::RequestAcceptance
-  end
-  def popup_translation_key
-    'notifications.request_accepted'
-  end
-end

app/models/notifications/reshared.rb

@@ -1,17 +1,22 @@
-class Notifications::Reshared < Notification
-  def mail_job
-    Workers::Mail::Reshared
-  end
+module Notifications
+  class Reshared < Notification
+    def mail_job
+      Workers::Mail::Reshared
+    end
 
-  def popup_translation_key
-    'notifications.reshared'
-  end
+    def popup_translation_key
+      "notifications.reshared"
+    end
 
-  def deleted_translation_key
-    'notifications.reshared_post_deleted'
-  end
+    def deleted_translation_key
+      "notifications.reshared_post_deleted"
+    end
 
-  def linked_object
-    self.target
+    def self.notify(reshare, _recipient_user_ids)
+      return unless reshare.root.present? && reshare.root.author.local?
+
+      actor = reshare.author
+      concatenate_or_create(reshare.root.author.owner, reshare.root, actor).email_the_user(reshare, actor)
+    end
   end
 end

app/models/notifications/started_sharing.rb

@@ -1,20 +1,16 @@
-class Notifications::StartedSharing < Notification
-  def mail_job
-    Workers::Mail::StartedSharing
+module Notifications
+  class StartedSharing < Notification
+    def mail_job
+      Workers::Mail::StartedSharing
+    end
+
+    def popup_translation_key
+      "notifications.started_sharing"
+    end
+
+    def self.notify(contact, _recipient_user_ids)
+      sender = contact.person
+      create_notification(contact.user_id, sender, sender).email_the_user(sender, sender)
+    end
   end
-
-  def popup_translation_key
-    'notifications.started_sharing'
-  end
-
-  def email_the_user(target, actor)
-    super(target.sender, actor)
-  end
-
-  private
-
-  def self.make_notification(recipient, target, actor, notification_type)
-    super(recipient, target.sender, actor, notification_type)
-  end
-
 end

app/services/notification_service.rb

@@ -0,0 +1,21 @@
+class NotificationService
+  NOTIFICATION_TYPES = {
+    Comment       => [Notifications::CommentOnPost, Notifications::AlsoCommented],
+    Like          => [Notifications::Liked],
+    StatusMessage => [Notifications::Mentioned],
+    Conversation  => [Notifications::PrivateMessage],
+    Message       => [Notifications::PrivateMessage],
+    Reshare       => [Notifications::Reshared],
+    Contact       => [Notifications::StartedSharing]
+  }.freeze
+
+  def notify(object, recipient_user_ids)
+    notification_types(object).each {|type| type.notify(object, recipient_user_ids) }
+  end
+
+  private
+
+  def notification_types(object)
+    NOTIFICATION_TYPES.fetch(object.class, [])
+  end
+end

app/views/notifications/_notification.haml

@@ -1,9 +1,9 @@
 .media.stream_element{:data=>{:guid => note.id, :type => (Notification.types.key(note.type) || '') }, :class => (note.unread ? 'unread' : 'read')}
   .unread-toggle.pull-right
     %i.entypo-eye{title: (note.unread ? t("notifications.index.mark_read") : t("notifications.index.mark_unread"))}
-  - if note.type == "Notifications::StartedSharing" && contact = current_user.contact_for(note.effective_target)
+  - if note.type == "Notifications::StartedSharing" && contact = current_user.contact_for(note.target)
     .pull-right
-      = aspect_membership_dropdown(contact, note.effective_target, "right")
+      = aspect_membership_dropdown(contact, note.target, "right")
 
   .media-object.pull-left
     = person_image_link note.actors.first, :size => :thumb_small, :class => 'hovercardable'

app/workers/receive_local.rb

@@ -5,7 +5,7 @@ module Workers
     def perform(object_class_string, object_id, recipient_user_ids)
       object = object_class_string.constantize.find(object_id)
       # TODO: create visibilities
-      # TODO: send notifications
+      NotificationService.new.notify(object, recipient_user_ids)
     rescue ActiveRecord::RecordNotFound # Already deleted before the job could run
     end
   end

spec/integration/federation/receive_federation_messages_spec.rb

@@ -92,14 +92,13 @@ describe "Receive federation messages feature" do
       expect(new_contact).not_to be_nil
       expect(new_contact.sharing).to eq(true)
 
-      # TODO: handle notifications
-      # expect(
-      #   Notifications::StartedSharing.exists?(
-      #     recipient_id: alice.id,
-      #     target_type:  "Person",
-      #     target_id:    sender.person.id
-      #   )
-      # ).to be_truthy
+      expect(
+        Notifications::StartedSharing.exists?(
+          recipient_id: alice.id,
+          target_type:  "Person",
+          target_id:    sender.person.id
+        )
+      ).to be_truthy
     end
 
     context "with sharing" do

spec/integration/federation/shared_receive_stream_items.rb

@@ -26,8 +26,6 @@ shared_examples_for "messages which are indifferent about sharing fact" do
 
     describe "notifications are sent where required" do
       it "for comment on local post" do
-        skip("TODO: handle notifications") # TODO
-
         entity = create_relayable_entity(:comment_entity, local_parent, remote_user_on_pod_b.diaspora_handle)
         post_message(generate_xml(entity, sender, recipient), recipient)
 
@@ -41,8 +39,6 @@ shared_examples_for "messages which are indifferent about sharing fact" do
       end
 
       it "for like on local post" do
-        skip("TODO: handle notifications") # TODO
-
         entity = create_relayable_entity(:like_entity, local_parent, remote_user_on_pod_b.diaspora_handle)
         post_message(generate_xml(entity, sender, recipient), recipient)
 
@@ -134,8 +130,6 @@ shared_examples_for "messages which can't be send without sharing" do
     # this one shouldn't depend on the sharing fact. this must be fixed
     describe "notifications are sent where required" do
       it "for comment on remote post where we participate" do
-        skip("TODO: handle notifications") # TODO
-
         alice.participate!(remote_parent)
         author_id = remote_user_on_pod_c.diaspora_handle
         entity = create_relayable_entity(:comment_entity, remote_parent, author_id)

spec/models/notifications/also_commented_spec.rb

@@ -0,0 +1,47 @@
+#   Copyright (c) 2010-2011, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+require "spec_helper"
+
+describe Notifications::AlsoCommented, type: :model do
+  let(:sm) { FactoryGirl.build(:status_message, author: alice.person, public: true) }
+  let(:comment) { FactoryGirl.create(:comment, commentable: sm) }
+  let(:notification) { Notifications::AlsoCommented.new(recipient: bob) }
+
+  describe ".notify" do
+    it "does not notify the commentable author" do
+      expect(Notifications::AlsoCommented).not_to receive(:concatenate_or_create)
+
+      Notifications::AlsoCommented.notify(comment, [])
+    end
+
+    it "notifies a local participant" do
+      bob.participate!(sm)
+
+      expect(Notifications::AlsoCommented).to receive(:concatenate_or_create).with(
+        bob, sm, comment.author
+      ).and_return(notification)
+      expect(bob).to receive(:mail).with(Workers::Mail::AlsoCommented, bob.id, comment.author.id, comment.id)
+
+      Notifications::AlsoCommented.notify(comment, [])
+    end
+
+    it "does not notify the a remote participant" do
+      FactoryGirl.create(:participation, target: sm)
+
+      expect(Notifications::AlsoCommented).not_to receive(:concatenate_or_create)
+
+      Notifications::AlsoCommented.notify(comment, [])
+    end
+
+    it "does not notify the author of the comment" do
+      bob.participate!(sm)
+      comment = FactoryGirl.create(:comment, commentable: sm, author: bob.person)
+
+      expect(Notifications::AlsoCommented).not_to receive(:concatenate_or_create)
+
+      Notifications::AlsoCommented.notify(comment, [])
+    end
+  end
+end