Merge pull request #2478 from riking/patch-email

Fix email code & tests

app/jobs/scheduled/poll_mailbox.rb

@@ -44,7 +44,7 @@ module Jobs
         if message_template
           # inform the user about the rejection
           message = Mail::Message.new(mail_string)
-          client_message = RejectionMailer.send_rejection(message.from, message.body, message_template.to_s, "#{e.message}\n\n#{e.backtrace.join("\n")}")
+          client_message = RejectionMailer.send_rejection(message.from, message.body, message.to, message_template)
           Email::Sender.new(client_message, message_template).send
         else
           Discourse.handle_exception(e, { context: "incoming email", mail: mail_string })

app/mailers/rejection_mailer.rb

@@ -3,8 +3,8 @@ require_dependency 'email/message_builder'
 class RejectionMailer < ActionMailer::Base
   include Email::BuildEmailHelper
 
-  def send_rejection(from, body, template, error)
+  def send_rejection(from, body, to_address, template)
-    build_email(from, from: from, template: template, error: error, source: body)
+    build_email(from, template: "system_messages.#{template}", source: body, destination: to_address)
   end
 
   def send_trust_level(from, template)

config/locales/server.en.yml

@@ -1299,66 +1299,45 @@ en:
         %{logs}
         ```
 
-    email_error_notification:
-      subject_template: "Error parsing email"
-      text_body_template: |
-        This is an automated message.
-
-        Parsing an incoming email from `%{from}` failed. Please review the following Error:
-
-        ---
-
-        %{error}
-
-        ---
-
-        The original message follows.
-
-        ---
-
-        %{source}
-
     email_reject_trust_level:
       subject_template: "Email issue -- Insufficient Trust Level"
       text_body_template: |
-        We're sorry, but your email message to %{to} didn't work.
+        We're sorry, but your email message to %{destination} didn't work.
 
         Your account does not have the required trust level to post new topics to this email address. If you believe this is in error, contact a staff member.
 
     email_reject_no_account:
       subject_template: "Email issue -- No Account"
       text_body_template: |
-        We're sorry, but your email message to %{to} didn't work.
+        We're sorry, but your email message to %{destination} didn't work.
 
         There is no known account with this email address. Try sending from a different email address, or contact a staff member.
 
     email_reject_empty:
       subject_template: "Email issue -- No Content"
       text_body_template: |
-        We're sorry, but your email message to %{to} didn't work.
+        We're sorry, but your email message to %{destination} didn't work.
 
         We couldn't find any content in the email.
 
     email_reject_parsing:
       subject_template: "Email issue -- Content unrecognized"
       text_body_template: |
-        We're sorry, but your email message to %{to} didn't work.
+        We're sorry, but your email message to %{destination} didn't work.
 
         The email encoding was unknown or not supported. Try again with UTF-8 plain text.
 
     email_reject_post_error:
       subject_template: "Email issue -- posting error"
       text_body_template: |
-        We're sorry, but your email message to %{to} didn't work.
+        We're sorry, but your email message to %{destination} didn't work.
 
         There was a problem with the formatting. Try again with less formatting, or no formatting.
 
-        %{error}
-
     email_reject_reply_key:
       subject_template: "Email issue -- Bad Reply Key"
       text_body_template: |
-        We're sorry, but your email message to %{to} didn't work.
+        We're sorry, but your email message to %{destination} didn't work.
 
         The provided reply key is invalid or unknown, so we don't know what this email is in reply to. Contact a staff member.
 

lib/email/message_builder.rb

@@ -147,7 +147,6 @@ module Email
       return @from_value if @from_value
       @from_value = @opts[:from] || SiteSetting.notification_email
       @from_value = alias_email(@from_value)
-      @from_value
     end
 
     def reply_by_email_address
@@ -161,8 +160,6 @@ module Email
                                 else
                                   site_alias_email(@reply_by_email_address)
                                 end
-
-      @reply_by_email_address
     end
 
     def alias_email(source)
@@ -171,7 +168,7 @@ module Email
     end
 
     def site_alias_email(source)
-      return "#{SiteSetting.title} <#{source}>"
+      "#{SiteSetting.title} <#{source}>"
     end
 
   end

spec/jobs/poll_mailbox_spec.rb

@@ -43,7 +43,14 @@ describe Jobs::PollMailbox do
   describe "processing email" do
 
     let!(:receiver) { mock }
-    let!(:email_string) { "EMAIL AS A STRING" }
+    let!(:email_string) { <<MAIL
+From: user@example.com
+To: reply+32@discourse.example.net
+Subject: Hi
+
+Email As a String
+MAIL
+    }
     let!(:email) { mock }
 
     before do
@@ -67,17 +74,17 @@ describe Jobs::PollMailbox do
         receiver.expects(:process).raises(Email::Receiver::UserNotSufficientTrustLevelError)
         email.expects(:delete)
 
-        Mail::Message.expects(:new).with(email_string).returns(email)
+        message = Mail::Message.new(email_string)
-
+        Mail::Message.expects(:new).with(email_string).returns(message)
-        email.expects(:from)
-        email.expects(:body)
 
         client_message = mock
-        sender = mock
+        sender_object = mock
 
-        RejectionMailer.expects(:send_rejection).returns(client_message)
+        RejectionMailer.expects(:send_rejection).with(
-        Email::Sender.expects(:new).with(client_message, :email_reject_trust_level).returns(sender)
+            message.from, message.body, message.to, :email_reject_trust_level
-        sender.expects(:send)
+        ).returns(client_message)
+        Email::Sender.expects(:new).with(client_message, :email_reject_trust_level).returns(sender_object)
+        sender_object.expects(:send)
 
         poller.handle_mail(email)
       end
@@ -98,6 +105,8 @@ describe Jobs::PollMailbox do
           receiver.expects(:process).raises(exception)
           email.expects(:delete)
 
+          Discourse.stubs(:handle_exception)
+
           poller.handle_mail(email)
         end