FIX: Don't email '(user deleted)' posts. Seriously.

This commit is contained in:
Robin Ward 2013-06-10 12:02:04 -04:00
parent 3b7d3aa487
commit 68d849419b
4 changed files with 22 additions and 36 deletions

View File

@ -27,41 +27,38 @@ module Jobs
post = Post.where(id: args[:post_id]).first post = Post.where(id: args[:post_id]).first
return unless post.present? return unless post.present?
# Topic may be deleted
return unless post.topic
# Don't email posts that were deleted
return if post.user_deleted?
# Don't send the email if the user has read the post
return if PostTiming.where(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id).present?
email_args[:post] = post email_args[:post] = post
end end
email_args[:email_token] = args[:email_token] if args[:email_token].present? email_args[:email_token] = args[:email_token] if args[:email_token].present?
notification = nil notification = nil
notification = Notification.where(id: args[:notification_id]).first if args[:notification_id].present? notification = Notification.where(id: args[:notification_id]).first if args[:notification_id].present?
if notification.present? if notification.present?
# Don't email a user about a post when we've seen them recently. # Don't email a user about a post when we've seen them recently.
return if seen_recently return if seen_recently
# Load the post if present # Load the post if present
if notification.post.present?
# Don't email a user if the topic has been deleted
return unless notification.post.topic.present?
email_args[:post] ||= notification.post email_args[:post] ||= notification.post
end
email_args[:notification] = notification email_args[:notification] = notification
# Don't send email if the notification this email is about has already been read # Don't send email if the notification this email is about has already been read
return if notification.read? return if notification.read?
end end
# Check that the post has not been deleted or read
if email_args[:post]
post = email_args[:post]
# Don't email about deleted topics or user deleted posts
return if post.topic.blank? || post.user_deleted?
# Don't send the email if the user has read the post
return if PostTiming.where(topic_id: post.topic_id, post_number: post.post_number, user_id: user.id).present?
end
# Make sure that mailer exists # Make sure that mailer exists
raise Discourse::InvalidParameters.new(:type) unless UserNotifications.respond_to?(args[:type]) raise Discourse::InvalidParameters.new(:type) unless UserNotifications.respond_to?(args[:type])

View File

@ -80,11 +80,12 @@ describe Jobs::UserEmail do
context 'notification' do context 'notification' do
let!(:notification) { Fabricate(:notification, user: user)} let(:post) { Fabricate(:post, user: user) }
let!(:notification) { Fabricate(:notification, user: user, topic: post.topic, post_number: post.post_number)}
it 'passes a notification as an argument when a notification_id is present' do it 'passes a notification as an argument when a notification_id is present' do
EmailSender.any_instance.expects(:send) EmailSender.any_instance.expects(:send)
UserNotifications.expects(:user_mentioned).with(user, notification: notification).returns(mailer) UserNotifications.expects(:user_mentioned).with(user, notification: notification, post: post).returns(mailer)
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
end end
@ -94,6 +95,12 @@ describe Jobs::UserEmail do
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id) Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
end end
it "doesn't send the email if the post has been user deleted" do
EmailSender.any_instance.expects(:send).never
post.update_column(:user_deleted, true)
Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
end
end end
end end

View File

@ -96,30 +96,13 @@ describe TopicQuery do
end end
end end
pending 'hot' do
let(:cold_category) { Fabricate(:category, name: 'brrrrrr', hotness: 5) }
let(:hot_category) { Fabricate(:category, name: 'yeeouch', hotness: 10) }
let!(:t1) { Fabricate(:topic, category: cold_category)}
let!(:t2) { Fabricate(:topic, category: hot_category)}
let!(:t3) { Fabricate(:topic, category: hot_category)}
let!(:t4) { Fabricate(:topic, category: cold_category)}
it "returns the hot categories first" do
topic_query.list_hot.topics.should == [t3, t2, t4, t1]
end
end
context 'unread / read topics' do context 'unread / read topics' do
context 'with no data' do context 'with no data' do
it "has no unread topics" do it "has no unread topics" do
topic_query.list_unread.topics.should be_blank topic_query.list_unread.topics.should be_blank
topic_query.unread_count.should == 0 topic_query.unread_count.should == 0
end end
end end
context 'with read data' do context 'with read data' do

View File

@ -105,7 +105,6 @@ describe PostAlertObserver do
mention_post mention_post
}.should_not change(evil_trout.notifications, :count) }.should_not change(evil_trout.notifications, :count)
end end
end end
context 'moderator action post' do context 'moderator action post' do