Openfoodnetwork: Email sending fails after changing mail method

[mkllnk]

Scenario: An OFN instance is configured to use a normal email account to send emails. The password of that email account is changed. The OFN admin has to update the password within OFN as well.

Problem: After this change, notification emails fail.

Background:
Emails are sent using a mail method configured via Spree (Admin, Configuration, Mail Methods). An admin can change the mail method. While the Rails application knows about this change, the independent Delayed::Job processes don't notice. They continue to use the old mail method which may not be working any more.

Solution:
The Delayed::Job processes should reload the Spree configuration. I'm not sure what the trigger should be though. Some possibilities:

• Always reload the configuration before using it. This is the safest method, but it could also be slow.
• Reload once hour. This could be another scheduled Delayed::Job. You still have to wait in hour to see the effect of a change.
• Reload on failure. This would be good for recovering in case Spree configuration doesn't work. But it won't update if the old configuration doesn't cause an error. For example, if you want to use a different email address, but the old one is still valid.

Other ideas:

• We could add a "Restart delayed job" button to the admin interface.
• We could add a refresh-config job after changing the config.

[sigmundpetersen]

Another report creating annoyance -> s4

[luisramos0]

spree v2 changed the way the mail method config is done.
we could retest this and see if it is still a problem in v2...

[sauloperez]

I assume this will still fail because it is related to the way Delayed Job processes work. IMO the easiest and more maintainable solution is to add sidenote that this change requires a Delayed Job restart. All those proposed solutions seem a bit overengineering to me. After all, an instance doesn't change the email every day.

[lin-d-hop]

Agree with Pau on this one. Has anyone really been annoyed by this? We've got bigger fish to fry,. #gitcull

[luisramos0]

this just happened again in CA, this is still a problem.
I think it deserves to remain open as an s4, ok?

[Matt-Yorkley]

Yes, this needs to be addressed at some point.

[luisramos0]

In spree_preferences table I find 2 entries with the password: smtp_password in app_config and in mail_method... I dont think the mail_methods one is used, probably left overs from ofn/spree v1.

[luisramos0]

We have seen this problem again today in Canada.
The release was 24 hours ago, the emails started failing 12 hours ago. I restarted unicorn and delayedjob and that did NOT solve the problem.
Then I updated the password in ofn-install and provisioned/deployed CA with that. This has fixed the problem!?

I don't understand how it did solve the problem, because I now realize ofn-install environment variables with SMTP user and SMTP password are only used for seeding: when you are installing a new server from scratch...
This implies that if we remove the SMTP config fields from the mail settings admin page, there will be no way to update the value unless we update the database table directly spree_preferences :-(

So, if we remove the fields from the admin section we need to make ofn-install re-write the values in the secrets file on every provisioning or deploy (basically using method set_mail_configuration in seeds.rb). Does that sound like an acceptable solution?
I have prepared the PR that removes the fields from the UI #5235

[sauloperez]

So, if we remove the fields from the admin section we need to make ofn-install re-write the values in the secrets file on every provisioning or deploy (basically using method set_mail_configuration in seeds.rb). Does that sound like an acceptable solution?

I'm afraid not :upside_down_face:

IMO we should bring in all Spree related to this (if we haven't already) and undo the mixture of different approaches we have to have only the one that suits our needs. IMO this is through ofn-install since changes require a dev to restart servers anyway. Beware, I haven't checked how difficult that is.

Perhaps updating the spree_preferences Db table from ofn-install is a reasonable shortcut? It sounds pretty horrible just to read it out loud...

[luisramos0]

You are saying the suggestion I propose is not acceptable but I think your suggestion is very similar to mine. I think we are agreeing...

I am suggesting that ofn-install does the job, and I suggested running the code that is in set_mail_configuration, which is very similar to what you suggested: updating the DB directly.

Instead of doing it in every provision(deploy, maybe we can have a specific ofn-install job for it?

[mkllnk]

@luisramos0

In spree_preferences table I find 2 entries with the password

I think this changed with the Spree upgrade. The old way was to have a MailMethod. You could have a method for staging and one for production and then "share" the database. Spree 2 got rid of that and has one configuration for sending emails.

we need to make ofn-install re-write the values in the secrets file on every provisioning or deploy

I think that's happening already and that's why you fixed it when updating ofn-secrets.

https://github.com/openfoodfoundation/ofn-install/blob/4c3538ce649802d227434662b0bb4decc88593d5/roles/deploy/tasks/deploy.yml#L103-L106

https://github.com/openfoodfoundation/openfoodnetwork/blob/8532fa16cda0e7dc90e4a930bef95b336e022f08/db/seeds.rb#L5-L21

https://github.com/openfoodfoundation/openfoodnetwork/blob/8532fa16cda0e7dc90e4a930bef95b336e022f08/app/services/mail_configuration.rb#L6-L11

[sauloperez]

Sorry for the confusion @luisramos0 . Now I see I didn't understand what you really meant by

to make ofn-install re-write the values in the secrets file on every provisioning or deploy

[luisramos0]

@sauloperez no problem!

@mkllnk thanks for the help!!!
I didnt see seeds were re-runnable and executed on every deploy. That explains what happened.
It also means we can remove the smtp settings from the admin page and keep just ofn-install settings that will be set on every deploy. We can go ahead with #5235 :tada:

One thing I can add in #5235 is to remove mail methods from spree_preference in a migration.

[romale]

Scenario: An OFN instance is configured to use a normal email account to send emails. The password of that email account is changed. The OFN admin has to update the password within OFN as well.

Problem: After this change, notification emails fail.

strange but i don't have this problem. I changed the smtp password to the user and if I immediately try send test mail, I get an error, but after about 1 minute everything is ok, the test email is sent

[luisramos0]

yes, it's because the test email is not sent through a job. If you place an order, the confirmation email will fail.

[romale]

Ok, i will try to place an order

[romale]

Ok, i will try to place an order

yes, doesn't work