From 17b6d3a2fe011c19b7283559d9d9444d519f5c88 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 24 Nov 2014 11:40:21 -0500 Subject: [PATCH] FIX: Mailing list mode was not checking for user deleted posts --- .../notify_mailing_list_subscribers.rb | 7 +-- .../notify_mailing_list_subscribers_spec.rb | 60 +++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 spec/jobs/notify_mailing_list_subscribers_spec.rb diff --git a/app/jobs/regular/notify_mailing_list_subscribers.rb b/app/jobs/regular/notify_mailing_list_subscribers.rb index c8db65406..5ee789797 100644 --- a/app/jobs/regular/notify_mailing_list_subscribers.rb +++ b/app/jobs/regular/notify_mailing_list_subscribers.rb @@ -4,13 +4,10 @@ module Jobs def execute(args) post_id = args[:post_id] - if post_id - post = Post.with_deleted.find_by(id: post_id) - # our topic can be deleted as well - return if (post && post.trashed?) || !post.topic - end + post = post_id ? Post.with_deleted.find_by(id: post_id) : nil raise Discourse::InvalidParameters.new(:post_id) unless post + return if post.trashed? || post.user_deleted? || (!post.topic) users = User.activated.not_blocked.not_suspended.real diff --git a/spec/jobs/notify_mailing_list_subscribers_spec.rb b/spec/jobs/notify_mailing_list_subscribers_spec.rb new file mode 100644 index 000000000..c7b9923b3 --- /dev/null +++ b/spec/jobs/notify_mailing_list_subscribers_spec.rb @@ -0,0 +1,60 @@ +require "spec_helper" + +describe Jobs::NotifyMailingListSubscribers do + + context "with mailing list on" do + let(:user) { Fabricate(:user, mailing_list_mode: true) } + + context "with a valid post" do + let!(:post) { Fabricate(:post, user: user) } + + it "sends the email to the user" do + UserNotifications.expects(:mailing_list_notify).with(user, post).once + Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id) + end + end + + context "with a deleted post" do + let!(:post) { Fabricate(:post, user: user, deleted_at: Time.now) } + + it "doesn't send the email to the user" do + UserNotifications.expects(:mailing_list_notify).with(user, post).never + Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id) + end + end + + context "with a user_deleted post" do + let!(:post) { Fabricate(:post, user: user, user_deleted: true) } + + it "doesn't send the email to the user" do + UserNotifications.expects(:mailing_list_notify).with(user, post).never + Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id) + end + end + + context "with a deleted topic" do + let!(:post) { Fabricate(:post, user: user) } + + before do + post.topic.update_column(:deleted_at, Time.now) + end + + it "doesn't send the email to the user" do + UserNotifications.expects(:mailing_list_notify).with(user, post).never + Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id) + end + end + + end + + context "with mailing list off" do + let(:user) { Fabricate(:user, mailing_list_mode: false) } + let!(:post) { Fabricate(:post, user: user) } + + it "doesn't send the email to the user" do + UserNotifications.expects(:mailing_list_notify).never + Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id) + end + end + +end