diff --git a/app/models/notification.rb b/app/models/notification.rb index 11511462c49ccf48f659d0e04d6606f37d797bad..aaa56d53d827d7e281f5526f00bd4ce76027d765 100644 --- a/app/models/notification.rb +++ b/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 diff --git a/app/models/notifications/also_commented.rb b/app/models/notifications/also_commented.rb index c20c88df4fd09fa9dce3f74b1604dbbe7598358a..a345566f3e922596596794299dd90f0487111899 100644 --- a/app/models/notifications/also_commented.rb +++ b/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 diff --git a/app/models/notifications/mentioned.rb b/app/models/notifications/mentioned.rb index 9636d574b93b2b8d8c9ad97692d131405f1f87f5..bede65ee4afca849c1cd405e61c8b25aa90e9072 100644 --- a/app/models/notifications/mentioned.rb +++ b/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 diff --git a/app/models/notifications/reshared.rb b/app/models/notifications/reshared.rb index 51f089e35552f8a6b60721a842d7cd49ad7ebf4c..0b438a483eb9204dc8ad45b6975105df38691714 100644 --- a/app/models/notifications/reshared.rb +++ b/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 diff --git a/app/models/notifications/started_sharing.rb b/app/models/notifications/started_sharing.rb index bae3a3a1ea575fad1df1e119acbf86d58ff6fbcb..dfe6dfd55944a9236968efc535e790e2caca52bc 100644 --- a/app/models/notifications/started_sharing.rb +++ b/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 diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index a2ecbb0767abbe1a9c404f2bdcfae4162ff3e7d8..6008ac5f595b678de1c0eb01e0b44920a61fab47 100644 --- a/spec/models/notification_spec.rb +++ b/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) diff --git a/spec/models/notifications/also_commented_spec.rb b/spec/models/notifications/also_commented_spec.rb index 74b69b6f456f47767fc5ca6efd9e814cea927b9a..72361bf030a0425c47fb8b9901bfa69f28c7f618 100644 --- a/spec/models/notifications/also_commented_spec.rb +++ b/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 diff --git a/spec/models/notifications/mentioned_spec.rb b/spec/models/notifications/mentioned_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..cb63bb9ffb77b36fcb4e8921aab369a29e8097ce --- /dev/null +++ b/spec/models/notifications/mentioned_spec.rb @@ -0,0 +1,72 @@ +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 diff --git a/spec/models/notifications/reshared_spec.rb b/spec/models/notifications/reshared_spec.rb index 5c22ab350f9dde9eb8b69f440e8f98c6f82d8b7f..203ed5230b2ea27f9f884c7ac9ec939f2d9206a9 100644 --- a/spec/models/notifications/reshared_spec.rb +++ b/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 diff --git a/spec/models/notifications/started_sharing_spec.rb b/spec/models/notifications/started_sharing_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..29fb2d4e7cfa8b6f2cefddd7f0948089faa5b36c --- /dev/null +++ b/spec/models/notifications/started_sharing_spec.rb @@ -0,0 +1,33 @@ +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