search

backdrop-contrib / smtp

Backdrop CMS port of the SMTP module for Drupal 7
GNU General Public License v2.0
4 stars 5 forks source link

Upgrading from 1.3.4 to 1.7.0 disables mail delivery #20

Closed bugfolder closed 1 year ago

bugfolder commented 1 year ago

We saw this when upgrading the Backdrop sites to the latest version of SMTP, 1.7.0. The "Turn on delivery of emails" setting (shown here as "On") is set to neither "On" nor "Off", which is then interpreted as "Off".

smtp setting

This setting is config variable 'smpt_deliver', which was introduced in 1.7.0 via this commit:

https://github.com/backdrop-contrib/smtp/commit/ff6debb75ce45f7ebd3e763dfa24d6e329ae0fb6

It was introduced into the smtp.settings config file introduced as TRUE, so new installs receive it as turned on, but there is no update hook to initialize the config setting for existing installations, so it is left uninitialized, and the net effect is that upgraders' outgoing email is silently turned off.

argiepiano commented 1 year ago

The rest of the new configurations introduced there are also not updated/added when you update to version 1.7.0, unless you go to the admin UI and save.

At this point I'm not sure what might be the best way forward. Perhaps another update_n hook that checks if smtp_deliver exists in the config file, and if not, adds this key and the others to the config file. I assume that, in the previous version of the module, there was no way to turn on/off the module, so it's safe to assume it should be on by default if its enabled.

bugfolder commented 1 year ago

Perhaps another update_n hook that checks if smtp_deliver exists in the config file, and if not, adds this key and the others to the config file.

Yeah, that's what I was thinking. The default for new installations is TRUE, so it seems that should be the default for upgrades, too.

bugfolder commented 1 year ago

PR submitted. To test:

The only missing variable that actually affects behavior is 'smtp_deliver', since for the others, the default value is what you get when casting the NULL returned from config_get(), but for completeness, I'm initializing all of the new variables to what they are in the current default config file.