Multiple calls to ActionMailer::Base#mail produces multiple Mail::Fields in the headers

.rubocop.yml

@@ -11,4 +11,12 @@ Style/Documentation:
 
 Metrics/BlockLength:
   Exclude:
-    - 'spec/**/*'
+    - 'spec/**/*'
+
+Style/StringLiterals:
+  Enabled: true
+  EnforcedStyle: double_quotes
+
+Layout/SpaceInsideHashLiteralBraces:
+  Enabled: true
+  EnforcedStyle: no_space

lib/mail/notify/mailer.rb

@@ -3,13 +3,14 @@
 module Mail
   module Notify
     ##
-    # The Mail Notify base Mailer class, overridden in Rails applications to provide the additional
-    # Notify behaviour along with the application behaviour.
+    # The Mail Notify base Mailer class, overridden in Rails applications to
+    # provide the additional Notify behaviour along with the application
+    # behaviour.
 
     class Mailer < ActionMailer::Base
       ##
-      # Set a default from address, will only be used in previews if a from address is not supplied
-      # by subclasses
+      # Set a default from address, will only be used in previews if a from
+      # address is not supplied by subclasses
 
       default from: "[email protected]"
 
@@ -25,26 +26,31 @@ class Mailer < ActionMailer::Base
       #
       # Add any additional headers in the options hash.
       #
-      # A default subject is supplied as ActionMailer requires one, however it will never be used as
-      # the subject is assumed to be managed in the Notify template.
+      # A default subject is supplied as ActionMailer requires one, however it
+      # will never be used as the subject is assumed to be managed in the
+      # Notify template.
 
       def template_mail(template_id, options)
         raise ArgumentError, "You must specify a Notify template ID" if template_id.blank?
-        raise ArgumentError, "You must specify a to address" if options[:to].nil? || options[:to].blank?
+
+        if options[:to].nil? || options[:to].blank?
+          raise ArgumentError,
+                "You must specify a to address"
+        end
 
         message.template_id = template_id
         message.reply_to_id = options[:reply_to_id]
         message.reference = options[:reference]
 
         message.personalisation = options[:personalisation] || {}
 
-        headers = options.except([:personalisation, :reply_to_id, :reference])
+        headers = options.except(:personalisation, :reply_to_id, :reference)
 
         headers[:subject] = "Subject managed in Notify" unless options[:subject]
 
-        # We have to set the html and the plain text content to nil to prevent Rails from looking
-        # for the content in the views. We replace nil with the content returned from Notify before
-        # sending or previewing
+        # We have to set the html and the plain text content to nil to prevent
+        # Rails from looking for the content in the views. We replace nil with
+        # the content returned from Notify before sending or previewing
         mail(headers) do |format|
           format.text { nil }
           format.html { nil }
@@ -74,14 +80,35 @@ def view_mail(template_id, options)
         message.reference = options[:reference]
 
         subject = options[:subject]
-        headers = options.except([:personalisation, :reply_to_id, :reference])
+        headers = options.except(:personalisation, :reply_to_id, :reference)
+
+        # we have to render the view for the message and grab the raw source,
+        # then we set that as the body in the personalisation for sending to
+        # the Notify API. 
+        #
+        # Calling the #mail method is not idempotent. It
+        # modifies state by setting instance variables on the message.
+        # Specifically, it sets @_message. mail generates message headers for
+        # the options passed in. Each time it is called with the same headers
+        # it adds another header field. This results in something like this
+        #
+        # mail({custom_header => 123})
+        # message.header['custom_header']
+        # #=> Mail::Field
+        #
+        # mail({custom_header => 123})
+        # message.header['custom_header']
+        # #=> [Mail::Field..., Mail::Field...]
+        #
+        original_message = message.dup
 
-        # we have to render the view for the message and grab the raw source, then we set that as the
-        # body in the personalisation for sending to the Notify API.
         body = mail(headers).body.raw_source
 
-        # The 'view mail' works by sending a subject and body as personalisation options, these are
-        # then used in the Notify template to provide content.
+        @_message = original_message
+
+        # The 'view mail' works by sending a subject and body as
+        # personalisation options, these are then used in the Notify template
+        # to provide content.
         message.personalisation = {subject: subject, body: body}
 
         mail(headers) do |format|

spec/mail/notify/mailer_spec.rb

@@ -6,15 +6,54 @@
 RSpec.describe Mail::Notify::Mailer do
   describe "#view_mail" do
     it "sets the message template id" do
-      message_params = {template_id: "template-id", to: "[email protected]", subject: "Test subject"}
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject"}
 
       message = TestMailer.with(message_params).test_view_mail
 
       expect(message.template_id).to eql("template-id")
     end
 
+    it "sets a custom value as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", custom_header: "custom"}
+
+      message = TestMailer.with(message_params).test_view_mail
+
+      expect(message.header[:custom_header]).to be_a(Mail::Field)
+      expect(message.header[:custom_header].value).to eq('custom')
+    end
+
+    it "does not set reply_to_id as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", reply_to_id: "123"}
+
+      message = TestMailer.with(message_params).test_view_mail
+
+      expect(message.header[:reply_to_id]).to be_nil
+    end
+
+    it "does not set reference as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", reference: "ref-123"}
+
+      message = TestMailer.with(message_params).test_view_mail
+
+      expect(message.header[:reference]).to be_nil
+    end
+
+    it "does not set personalisation as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", personalisation: "Dear sir"}
+
+      message = TestMailer.with(message_params).test_view_mail
+
+      expect(message.header[:personalisation]).to be_nil
+    end
+
     it "sets the message subject" do
-      message_params = {template_id: "template-id", to: "[email protected]", subject: "Test subject"}
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject"}
 
       message = TestMailer.with(message_params).test_view_mail
 
@@ -23,7 +62,8 @@
     end
 
     it "sets the message to address" do
-      message_params = {template_id: "template-id", to: "[email protected]", subject: "Test subject"}
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject"}
 
       message = TestMailer.with(message_params).test_view_mail
 
@@ -32,15 +72,17 @@
     end
 
     it "sets the subject on personalisation" do
-      message_params = {template_id: "template-id", to: "[email protected]", subject: "Test subject"}
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject"}
 
       message = TestMailer.with(message_params).test_view_mail
 
       expect(message.personalisation[:subject]).to eql("Test subject")
     end
 
     it "sets the body on personalisation" do
-      message_params = {template_id: "template-id", to: "[email protected]", subject: "Test subject"}
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject"}
 
       message = TestMailer.with(message_params).test_view_mail
 
@@ -126,6 +168,33 @@
       expect(message.template_id).to eql("template-id")
     end
 
+    it "does not set reply_to_id as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", reply_to_id: "123"}
+
+      message = TestMailer.with(message_params).test_template_mail
+
+      expect(message.header[:reply_to_id]).to be_nil
+    end
+
+    it "does not set reference as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", reference: "ref-123"}
+
+      message = TestMailer.with(message_params).test_template_mail
+
+      expect(message.header[:reference]).to be_nil
+    end
+
+    it "does not set personalisation as a header" do
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "Test subject", personalisation: "Dear sir"}
+
+      message = TestMailer.with(message_params).test_template_mail
+
+      expect(message.header[:personalisation]).to be_nil
+    end
+
     it "sets the message to address" do
       message_params = {template_id: "template-id", to: "[email protected]"}
 
@@ -145,7 +214,8 @@
     end
 
     it "sets the subject if one is passed, even though it will not be used" do
-      message_params = {template_id: "template-id", to: "[email protected]", subject: "My subject"}
+      message_params = {template_id: "template-id", to: "[email protected]",
+                        subject: "My subject"}
 
       message = TestMailer.with(message_params).test_template_mail