Allow From, Reply-To addresses to be overridden in settings table

[Holmes98]

I've changed the website to send emails using train@nzoi.org.nz, but this currently causes emails to be sent with incorrect headers:

X-Google-Original-From: nztrain@gmail.com
Reply-To: nztrain@gmail.com

This would also allow setting the sender's display name, e.g. "NZOI Training" <train@nzoi.org.nz>; currently Gmail just shows the sender as "train" or "nztrain".

[coveralls]

coverage: 37.529% (+0.006%) from 37.523%
when pulling a3d1f09 on change-mailer-address
into 51571c5 on master.

[tom93]

Hi @Holmes98! I'm curious -- why do these need to be set separately? I would expect to have a single email address for both SMTP auth and "From". Otherwise it's effectively spoofing the sender, which I think would be better to avoid if possible.

[Holmes98]

I don't know why, but maybe there are cases where the SMTP username isn't the same as the email address.

I guess we could still set the fields from system/mailer/username to avoid repetition, though then you wouldn't be able to set the display name and I didn't want to overcomplicate things.

[Holmes98]

Actually, it's probably just because Action Mailer has other delivery methods than SMTP (e.g. sendmail).

[tom93]

Actually, how did you change the website to send emails using train@nzoi.org.nz? I assumed it was by changing system/mailer/username (and generating a password etc.), but then I don't see how there would still be references to nztrain@gmail.com.

[Holmes98]

Yeah, I just changed the system/mailer settings, but those are only used for SMTP authentication.

For emails from Devise, the from/reply_to parameters seem to be set from the mailer_sender setting (but only if you don't set the default from/reply_to parameters; I still left it in though as it seems to break the specs if commented out):

nztrain/config/initializers/devise.rb

Line 13 in 8e102ad

config.mailer_sender = "nztrain@gmail.com"

Emails from the admin panel currently use this (which I removed in this PR):

nztrain/app/mailers/admin_mailer.rb

Line 2 in 8e102ad

default :from => "nztrain@gmail.com"

[tom93]

Right, thanks for digging that up. I consider that a bug -- we shouldn't hard-code that email address. How about I write a separate PR to change it to use system/mailer/username, and then this PR would only be necessary for setting a custom display name?

[Holmes98]

Sure.

[tom93]

I'm mostly done (branch "pr/remove-hard-coded-nztrain-email" of my fork, commits).

One question is how the settings should work. I would strongly prefer that in the simplest case (where the admin doesn't want to set a pretty display name), the email would only have to be configured in one setting. Here are the options I like:

A. "system/mailer/email" for auth and default "From:", setting "system/mailer/from" to override "From:" (may contain a display name).

B. "system/mailer/email" for auth and "From:", "system/mailer/display_name" (optional) added to "From:".

(For both options, a migration could rename "system/mailer/username" to "system/mailer/email".)

Also, is Reply-To useful, or was it just a workaround?

[Holmes98]

That looks reasonable. I'm fine with either of those options, or even just hardcoding the display name to "NZOI Training", since there are already hardcoded references to that elsewhere on the site.

Note that config.action_mailer.default() is broken[1], so we must use default_options=.

Nice find, I guess that's why the specs were breaking for me earlier.

Reply-To was mostly a workaround. I could see it maybe being useful for redirecting responses to an mailbox that's actually monitored, but it's probably not important.

[tom93]

I implemented the hard-coded display name and opened #273.
Are you okay with using that and dropping this PR?