diff --git a/.rubocop.yml b/.rubocop.yml index 763fa0c..1ec0288 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -11,4 +11,12 @@ Style/Documentation: Metrics/BlockLength: Exclude: - - 'spec/**/*' \ No newline at end of file + - 'spec/**/*' + +Style/StringLiterals: + Enabled: true + EnforcedStyle: double_quotes + +Layout/SpaceInsideHashLiteralBraces: + Enabled: true + EnforcedStyle: no_space diff --git a/lib/mail/notify/mailer.rb b/lib/mail/notify/mailer.rb index cd871fd..86dc433 100644 --- a/lib/mail/notify/mailer.rb +++ b/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: "preview@notifications.service.gov.uk" @@ -25,12 +26,17 @@ 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] @@ -38,13 +44,13 @@ def template_mail(template_id, options) 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| diff --git a/spec/mail/notify/mailer_spec.rb b/spec/mail/notify/mailer_spec.rb index 00d852b..791ed8d 100644 --- a/spec/mail/notify/mailer_spec.rb +++ b/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: "test.name@email.co.uk", subject: "Test subject"} + message_params = {template_id: "template-id", to: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk", subject: "Test subject"} + message_params = {template_id: "template-id", to: "test.name@email.co.uk", + 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: "test.name@email.co.uk", subject: "Test subject"} + message_params = {template_id: "template-id", to: "test.name@email.co.uk", + subject: "Test subject"} message = TestMailer.with(message_params).test_view_mail @@ -32,7 +72,8 @@ end it "sets the subject on personalisation" do - message_params = {template_id: "template-id", to: "test.name@email.co.uk", subject: "Test subject"} + message_params = {template_id: "template-id", to: "test.name@email.co.uk", + subject: "Test subject"} message = TestMailer.with(message_params).test_view_mail @@ -40,7 +81,8 @@ end it "sets the body on personalisation" do - message_params = {template_id: "template-id", to: "test.name@email.co.uk", subject: "Test subject"} + message_params = {template_id: "template-id", to: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk", + 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: "test.name@email.co.uk"} @@ -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: "test.name@email.co.uk", subject: "My subject"} + message_params = {template_id: "template-id", to: "test.name@email.co.uk", + subject: "My subject"} message = TestMailer.with(message_params).test_template_mail