From 4ad5660615a4026b332a137a3977cd4a0d6e3b63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 15 Feb 2016 17:53:07 +0100
Subject: [PATCH] add slightly more logs when skipping email notifications

---
 app/jobs/regular/user_email.rb | 40 +++++++++++++++++-----------------
 lib/email/sender.rb            |  6 ++---
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb
index 14f34b42c2c..404dd07f8e5 100644
--- a/app/jobs/regular/user_email.rb
+++ b/app/jobs/regular/user_email.rb
@@ -6,19 +6,20 @@ module Jobs
   class UserEmail < Jobs::Base
 
     def execute(args)
-      notification, post = nil
-
       raise Discourse::InvalidParameters.new(:user_id) unless args[:user_id].present?
       raise Discourse::InvalidParameters.new(:type)    unless args[:type].present?
 
+      post = nil
+      notification = nil
       type = args[:type]
-
       user = User.find_by(id: args[:user_id])
+      to_address = args[:to_address].presence || user.try(:email).presence || "no_email_found"
+
+      set_skip_context(type, args[:user_id], to_address)
 
-      set_skip_context(type, args[:user_id], args[:to_address].presence || user.try(:email).presence || "no_email_found")
       return skip(I18n.t("email_log.no_user", user_id: args[:user_id])) unless user
 
-      if args[:post_id]
+      if args[:post_id].present?
         post = Post.find_by(id: args[:post_id])
         return skip(I18n.t('email_log.post_not_found', post_id: args[:post_id])) unless post.present?
       end
@@ -27,18 +28,17 @@ module Jobs
         notification = Notification.find_by(id: args[:notification_id])
       end
 
-      message, skip_reason = message_for_email( user,
-                                   post,
-                                   type,
-                                   notification,
-                                   args[:notification_type],
-                                   args[:notification_data_hash],
-                                   args[:email_token],
-                                   args[:to_address] )
-
+      message, skip_reason = message_for_email(user,
+                                               post,
+                                               type,
+                                               notification,
+                                               args[:notification_type],
+                                               args[:notification_data_hash],
+                                               args[:email_token],
+                                               args[:to_address])
 
       if message
-        Email::Sender.new(message, args[:type], user).send
+        Email::Sender.new(message, type, user).send
       else
         skip_reason
       end
@@ -51,8 +51,8 @@ module Jobs
     NOTIFICATIONS_SENT_BY_MAILING_LIST ||= Set.new %w{posted replied mentioned group_mentioned quoted}
 
    def message_for_email(user, post, type, notification,
-                          notification_type=nil, notification_data_hash=nil,
-                          email_token=nil, to_address=nil)
+                         notification_type=nil, notification_data_hash=nil,
+                         email_token=nil, to_address=nil)
 
       set_skip_context(type, user.id, to_address || user.email)
 
@@ -75,7 +75,7 @@ module Jobs
       end
 
       if notification || notification_type
-        email_args[:notification_type] ||= notification_type || notification.try(:notification_type)
+        email_args[:notification_type]      ||= notification_type      || notification.try(:notification_type)
         email_args[:notification_data_hash] ||= notification_data_hash || notification.try(:data_hash)
 
         unless String === email_args[:notification_type]
@@ -113,7 +113,7 @@ module Jobs
 
       # Update the to address if we have a custom one
       if message && to_address.present?
-        message.to = [to_address]
+        message.to = to_address
       end
 
       [message, nil]
@@ -143,7 +143,7 @@ module Jobs
         to_address: @skip_context[:to_address],
         user_id: @skip_context[:user_id],
         skipped: true,
-        skipped_reason: reason,
+        skipped_reason: "[UserEmail] #{reason}",
       )
     end
 
diff --git a/lib/email/sender.rb b/lib/email/sender.rb
index e1f81c84fb1..f0cef9ff227 100644
--- a/lib/email/sender.rb
+++ b/lib/email/sender.rb
@@ -23,7 +23,7 @@ module Email
 
     def send
       return if SiteSetting.disable_emails
-      return skip(I18n.t('email_log.message_blank')) if @message.blank?
+      return skip(I18n.t('email_log.message_blank'))    if @message.blank?
       return skip(I18n.t('email_log.message_to_blank')) if @message.to.blank?
 
       if @message.text_part
@@ -135,7 +135,7 @@ module Email
 
     def to_address
       @to_address ||= begin
-        to = @message ? @message.to : nil
+        to = @message.try(:to)
         to = to.first if Array === to
         to.presence || "no_email_found"
       end
@@ -167,7 +167,7 @@ module Email
         to_address: to_address,
         user_id: @user.try(:id),
         skipped: true,
-        skipped_reason: reason
+        skipped_reason: "[Sender] #{reason}"
       )
     end