don't create notifications if the notification-actor is ignored

Also move "shareable hidden"-logic to AlsoCommented, because it is the
only one that needs it. And write some specs for mentioned and started
sharing notifications.

Fixes #6294

app/models/notification.rb

@@ -29,7 +29,7 @@ class Notification < ActiveRecord::Base
   end
 
   def self.concatenate_or_create(recipient, target, actor)
-    return nil if suppress_notification?(recipient, target)
+    return nil if suppress_notification?(recipient, actor)
 
     find_or_initialize_by(recipient: recipient, target: target, unread: true).tap do |notification|
       notification.actors |= [actor]
@@ -42,12 +42,14 @@ class Notification < ActiveRecord::Base
     end
   end
 
-  def self.create_notification(recipient_id, target, actor)
-    create(recipient_id: recipient_id, target: target, actors: [actor])
+  def self.create_notification(recipient, target, actor)
+    return nil if suppress_notification?(recipient, actor)
+
+    create(recipient: recipient, target: target, actors: [actor])
   end
 
-  private_class_method def self.suppress_notification?(recipient, post)
-    post.is_a?(Post) && recipient.is_shareable_hidden?(post)
+  private_class_method def self.suppress_notification?(recipient, actor)
+    recipient.blocks.where(person: actor).exists?
   end
 
   def self.types

app/models/notifications/also_commented.rb

@@ -18,8 +18,9 @@ module Notifications
       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) }
+        next if recipient.is_shareable_hidden?(commentable)
+
+        concatenate_or_create(recipient, commentable, actor).try(:email_the_user, comment, actor)
       end
     end
   end

app/models/notifications/mentioned.rb

@@ -24,7 +24,7 @@ module Notifications
 
         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)
+        create_notification(recipient.owner, mention, actor).try(:email_the_user, mention, actor)
       end
     end
   end

app/models/notifications/reshared.rb

@@ -16,7 +16,7 @@ module Notifications
       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)
+      concatenate_or_create(reshare.root.author.owner, reshare.root, actor).try(:email_the_user, reshare, actor)
     end
   end
 end

app/models/notifications/started_sharing.rb

@@ -10,7 +10,7 @@ module Notifications
 
     def self.notify(contact, _recipient_user_ids)
       sender = contact.person
-      create_notification(contact.user_id, sender, sender).email_the_user(sender, sender)
+      create_notification(contact.user, sender, sender).try(:email_the_user, sender, sender)
     end
 
     def contact

spec/models/notification_spec.rb

@@ -70,14 +70,14 @@ describe Notification, :type => :model do
     end
 
     it "appends the actors to the already existing notification" do
-      notification = Notification.create_notification(alice.id, @sm, @person)
+      notification = Notification.create_notification(alice, @sm, @person)
       expect {
         Notification.concatenate_or_create(alice, @sm, eve.person)
       }.to change(notification.actors, :count).by(1)
     end
 
     it "doesn't append the actor to an existing notification if it is already there" do
-      notification = Notification.create_notification(alice.id, @sm, @person)
+      notification = Notification.create_notification(alice, @sm, @person)
       expect {
         Notification.concatenate_or_create(alice, @sm, @person)
       }.not_to change(notification.actors, :count)

spec/models/notifications/also_commented_spec.rb

@@ -43,5 +43,25 @@ describe Notifications::AlsoCommented, type: :model do
 
       Notifications::AlsoCommented.notify(comment, [])
     end
+
+    it "does not notify if the commentable is hidden" do
+      bob.participate!(sm)
+      bob.add_hidden_shareable(sm.class.base_class.to_s, sm.id.to_s)
+
+      expect(Notifications::AlsoCommented).not_to receive(:concatenate_or_create)
+
+      Notifications::AlsoCommented.notify(comment, [])
+    end
+
+    it "does not notify if the author of the comment is ignored" do
+      bob.participate!(sm)
+      bob.blocks.create(person: comment.author)
+
+      expect_any_instance_of(Notifications::AlsoCommented).not_to receive(:email_the_user)
+
+      Notifications::AlsoCommented.notify(comment, [])
+
+      expect(Notifications::AlsoCommented.where(target: sm)).not_to exist
+    end
   end
 end

spec/models/notifications/mentioned_spec.rb

+require "spec_helper"
+
+describe Notifications::Mentioned, type: :model do
+  let(:sm) {
+    FactoryGirl.create(:status_message, author: alice.person, text: "hi @{bob; #{bob.diaspora_handle}}", public: true)
+  }
+  let(:mentioned_notification) { Notifications::Mentioned.new(recipient: bob) }
+
+  describe ".notify" do
+    it "calls create_notification with mention" do
+      expect(Notifications::Mentioned).to receive(:create_notification).with(
+        bob, sm.mentions.first, sm.author
+      ).and_return(mentioned_notification)
+
+      Notifications::Mentioned.notify(sm, [])
+    end
+
+    it "sends an email to the mentioned person" do
+      allow(Notifications::Mentioned).to receive(:create_notification).and_return(mentioned_notification)
+      expect(bob).to receive(:mail).with(Workers::Mail::Mentioned, bob.id, sm.author.id, sm.mentions.first.id)
+
+      Notifications::Mentioned.notify(sm, [])
+    end
+
+    it "does nothing if the mentioned person is not local" do
+      sm = FactoryGirl.create(
+        :status_message,
+        author: alice.person,
+        text:   "hi @{raphael; #{remote_raphael.diaspora_handle}}",
+        public: true
+      )
+      expect(Notifications::Mentioned).not_to receive(:create_notification)
+
+      Notifications::Mentioned.notify(sm, [])
+    end
+
+    it "does not notify if the author of the post is ignored" do
+      bob.blocks.create(person: sm.author)
+
+      expect_any_instance_of(Notifications::Mentioned).not_to receive(:email_the_user)
+
+      Notifications::Mentioned.notify(sm, [])
+
+      expect(Notifications::Mentioned.where(target: sm.mentions.first)).not_to exist
+    end
+
+    context "with private post" do
+      let(:private_sm) {
+        FactoryGirl.create(
+          :status_message,
+          author: remote_raphael,
+          text:   "hi @{bob; #{bob.diaspora_handle}}",
+          public: false
+        )
+      }
+
+      it "calls create_notification if the mentioned person is a recipient of the post" do
+        expect(Notifications::Mentioned).to receive(:create_notification).with(
+          bob, private_sm.mentions.first, private_sm.author
+        ).and_return(mentioned_notification)
+
+        Notifications::Mentioned.notify(private_sm, [bob.id])
+      end
+
+      it "does not call create_notification if the mentioned person is not a recipient of the post" do
+        expect(Notifications::Mentioned).not_to receive(:create_notification)
+
+        Notifications::Mentioned.notify(private_sm, [alice.id])
+      end
+    end
+  end
+end

spec/models/notifications/reshared_spec.rb

@@ -38,5 +38,15 @@ describe Notifications::Reshared, type: :model do
 
       Notifications::Reshared.notify(reshare, [])
     end
+
+    it "does not notify if the author of the reshare is ignored" do
+      alice.blocks.create(person: reshare.author)
+
+      expect_any_instance_of(Notifications::Reshared).not_to receive(:email_the_user)
+
+      Notifications::Reshared.notify(reshare, [])
+
+      expect(Notifications::Reshared.where(target: sm)).not_to exist
+    end
   end
 end

spec/models/notifications/started_sharing_spec.rb

+require "spec_helper"
+
+describe Notifications::StartedSharing, type: :model do
+  let(:contact) { alice.contact_for(bob.person) }
+  let(:started_sharing_notification) { Notifications::StartedSharing.new(recipient: alice) }
+
+  describe ".notify" do
+    it "calls create_notification with sender" do
+      expect(Notifications::StartedSharing).to receive(:create_notification).with(
+        alice, bob.person, bob.person
+      ).and_return(started_sharing_notification)
+
+      Notifications::StartedSharing.notify(contact, [])
+    end
+
+    it "sends an email to the contacted user" do
+      allow(Notifications::StartedSharing).to receive(:create_notification).and_return(started_sharing_notification)
+      expect(alice).to receive(:mail).with(Workers::Mail::StartedSharing, alice.id, bob.person.id, bob.person.id)
+
+      Notifications::StartedSharing.notify(contact, [])
+    end
+
+    it "does not notify if the sender of the contact is ignored" do
+      alice.blocks.create(person: contact.person)
+
+      expect_any_instance_of(Notifications::StartedSharing).not_to receive(:email_the_user)
+
+      Notifications::StartedSharing.notify(contact, [])
+
+      expect(Notifications::StartedSharing.where(target: bob.person)).not_to exist
+    end
+  end
+end