Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Header handling #165

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Fix: Header handling #165

wants to merge 3 commits into from

Commits on Oct 18, 2024

  1. Fix: options included in the headers 

    The call to `except` here should not be an array as there is no array of
these symbol to exclude from the hash.

This issue was kindly identified by @inulty-dfe in #162.
    @mec
    mec committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    4c3c409 View commit details
    Browse the repository at this point in the history

  2. Fix: view_mail does not duplicate headers 

    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.
    @mec
    mec committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    7199a3d View commit details
    Browse the repository at this point in the history

  3. Refactor mailer methods 

    We only keep a Mail::Message for use in previews, it is never actually
sent, email sending is handled by Notify, not the host application.

The Notify API does not accept additional custom headers, so passing
custom headers to the message via the call to `mail` serves no purpose
as those headers will never be used (other than in the previews, where
they get shown).

This work refactors the two mailer methods so that only the `to` and
`subject` headers are set on the Mail::Message returned.
    @mec
    mec committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    c628284 View commit details
    Browse the repository at this point in the history