Skip to content

Commit

Permalink
Fix: view_mail does not duplicate headers
Browse files Browse the repository at this point in the history
With a view mail we have to call `mail` twice, once to get the body of
the email from the view and once again to override the format contents.

We were passing in any custom headers for both calls which was resulting
in duplicate headers in the message.

By calling `mail` without the headers, we can get the body and not
modify the headers, leaving that for the second call.

Interestingly adding custom headers is pointless, as they will never end
up in the email from Notify - we may want to stop messing with the
headers at all - I think it is a hangover from the v1 implementation.

This was kindly identified by @inulty-dfe in #162 who are testing the
header Mail::Fields that are potentially duplicated.
  • Loading branch information
mec committed Oct 18, 2024
1 parent 7a4a091 commit 72254a5
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog]

- Fixed bug in `view_mail` and `template_mail` that meant the options were being
included in the preview email headers for no reason - thanks to @inulty-dfe
- Fixed a bug in `view_mail` that meant custom headers in the preview email were
duplicated - thanks to @inulty-dfe

## [2.0.0] - 2024-04-01

Expand Down
7 changes: 5 additions & 2 deletions lib/mail/notify/mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,12 @@ def view_mail(template_id, options)
subject = options[:subject]
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
# 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
#
# We do not pass the headers as the call to `mail` will keep adding headers resulting in
# duplication when we have to call it again later.
body = mail.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.
Expand Down
28 changes: 28 additions & 0 deletions spec/mail/notify/mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,20 @@
expect(message.header[:reply_to_id]).to be_nil
expect(message.header[:reference]).to be_nil
end

it "sets custom headers only once" do
message_params = {
template_id: "template-id",
to: "[email protected]",
subject: "Test subject",
custom_header: "custom header value"
}

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 header value")
end
end

describe "#template_email" do
Expand Down Expand Up @@ -236,6 +250,20 @@
expect(message.header[:reply_to_id]).to be_nil
expect(message.header[:reference]).to be_nil
end

it "sets custom headers only once" do
message_params = {
template_id: "template-id",
to: "[email protected]",
subject: "Test subject",
custom_header: "custom header value"
}

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 header value")
end
end

describe "#blank_allowed" do
Expand Down

0 comments on commit 72254a5

Please sign in to comment.