From 68d849419b4281451171322f2f5537f707fa4411 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Mon, 10 Jun 2013 12:02:04 -0400
Subject: [PATCH] FIX: Don't email '(user deleted)' posts. Seriously.

---
 lib/jobs/user_email.rb                  | 29 +++++++++++--------------
 spec/components/jobs/user_email_spec.rb | 11 ++++++++--
 spec/components/topic_query_spec.rb     | 17 ---------------
 spec/models/post_alert_observer_spec.rb |  1 -
 4 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/lib/jobs/user_email.rb b/lib/jobs/user_email.rb
index e59f68babd7..432c5a295c2 100644
--- a/lib/jobs/user_email.rb
+++ b/lib/jobs/user_email.rb
@@ -27,41 +27,38 @@ module Jobs
         post = Post.where(id: args[:post_id]).first
         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
       end
 
       email_args[:email_token] = args[:email_token] if args[:email_token].present?
 
-
       notification = nil
       notification = Notification.where(id: args[:notification_id]).first if args[:notification_id].present?
-
       if notification.present?
 
         # Don't email a user about a post when we've seen them recently.
         return if seen_recently
 
         # 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
-        end
+        email_args[:post] ||= notification.post
         email_args[:notification] = notification
 
         # Don't send email if the notification this email is about has already been read
         return if notification.read?
       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
       raise Discourse::InvalidParameters.new(:type) unless UserNotifications.respond_to?(args[:type])
 
diff --git a/spec/components/jobs/user_email_spec.rb b/spec/components/jobs/user_email_spec.rb
index 785e9f51f98..79f1179994f 100644
--- a/spec/components/jobs/user_email_spec.rb
+++ b/spec/components/jobs/user_email_spec.rb
@@ -80,11 +80,12 @@ describe Jobs::UserEmail 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
         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)
       end
 
@@ -94,6 +95,12 @@ describe Jobs::UserEmail do
         Jobs::UserEmail.new.execute(type: :user_mentioned, user_id: user.id, notification_id: notification.id)
       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
diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb
index c09548322eb..f190ca105ee 100644
--- a/spec/components/topic_query_spec.rb
+++ b/spec/components/topic_query_spec.rb
@@ -96,30 +96,13 @@ describe TopicQuery do
     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 'with no data' do
-
       it "has no unread topics" do
         topic_query.list_unread.topics.should be_blank
         topic_query.unread_count.should == 0
       end
-
     end
 
     context 'with read data' do
diff --git a/spec/models/post_alert_observer_spec.rb b/spec/models/post_alert_observer_spec.rb
index 50f7beed53f..e0d8fa92316 100644
--- a/spec/models/post_alert_observer_spec.rb
+++ b/spec/models/post_alert_observer_spec.rb
@@ -105,7 +105,6 @@ describe PostAlertObserver do
         mention_post
       }.should_not change(evil_trout.notifications, :count)
     end
-
   end
 
   context 'moderator action post' do