AdminMailer broken [v5.5]

[orenyk]

2016-01-15

This has been identified as an issue with the way AdminMailer sets the from address in e-mails on deployments with a relative root. See below.

Original

Reported by BMEC, notes are being entered during checkin but the reservation is not being included in the hourly notes e-mail. Pretty critical!

[orenyk]

This was likely also due to e-mails being disabled this morning (via the DISABLE_EMAILS environment variable). I've created and checked out a dummy reservation on BMEC and hopefully they will see it in an upcoming notes e-mail; if they do we can just close this issue. I'll update tomorrow once I hear back from them.

[orenyk]

Similar to #1428, this isn't turning out to be so straightforward. The notes_unsent flag was set when I checked out a Reservation with notes, and then later unset, implying that the rake task ran. I'm just trying to confirm whether or not the e-mail went out without reservations or if no note e-mail was sent at all. I do know that some e-mails have been going out so the mailer is performing deliveries in general; this is getting pretty frustrating.

[orenyk]

Thanks to @dgoerger we've figured this out - when we updated the no-reply address for AdminMailer e-mails in #1240 we broke things since it's including the relative root (so [email protected]/STUFF) which leads to an invalid from address and the mail server refusing to deliver the e-mail. We'll have to update that code - I'm going to get this done over the weekend, include a CHANGELOG update, and release that as v5.5.2 for immediate deployment so that admin e-mails start sending again. All the other issues will be worked on over the rest of the month for release in v5.5.3.

[orenyk]

The relevant line is here.

[orenyk]

Turns out we were incorrectly incorporating the relative root into the ActionMailer::Base.default_url_options hash in config/environments/production.rb - we should be specifying a :script_name value with the relative root prepended by a forward slash instead. See here for more details.

[orenyk]

Deploying to DEV to test; will ask @dgoerger to enable e-mails for the SDMP instance tomorrow so we can see if it works.

[orenyk]

PR's issued for release-v5.5 and master, waiting for testing on DEV tomorrow before merging.

[dgoerger]

@orenyk I've enabled emails for dev /sdmp and restarted the app. Can you test it and let me know?

[orenyk]

Bingo, worked almost perfectly 😌. The site I found that introduced me to the appropriate setting incorrectly added a leading slash to the relative root so I'll update that later today and then we can merge this in. Thanks!

[orenyk]

Note that I verified the notify_admin_on_create setting, now just making sure that the notes e-mails work as expected.

[dgoerger]

Great! If you need to do more deploys to dev /sdmp, let me know and I'll change the installer script to enable emails at point of install, rather than me subsequently editing the conf and restarting the app? (then disable emails in dev /sdmp again once testing is complete)

[orenyk]

@dgoerger we'll probably want to test again tomorrow today if you want to go ahead and do whatever it is you suggested in your last comment 😛. Thanks!

[dgoerger]

@orenyk ok done! Emails are enabled in /sdmp dev per the installer, so we can test and not have to edit the .env file or restart the app. (Other dev instances remain with emails disabled.)

[orenyk]

Worked perfectly, thanks!