Module no longer working after update "Turn on delivery of emails" disabled?

[jenlampton]

On the admin form, it appears a new option has been added "Turn on delivery of emails". However, there was no update hook to enable this for existing sites, so by updating the module we are essentially breaking email delivery.

[bugfolder]

It looks like smtp_update_1304()overrides some of the behavior of smtp_update_1303(), which (among other things) sets the smtp_deliver config setting to TRUE if it wasn't already set by the user and leaves it alone if the user had already set it (see https://github.com/backdrop-contrib/smtp/issues/20).

Now it appears that smtp_update_1304() sets smtp_deliver to TRUE even if the site owner had disabled delivery by setting it to "Off." Why are we overriding the site owner's setting? There are legitimate reasons for a site owner to turn off SMTP (usually temporarily, but also for local dev sites that you don't want sending out emails).

Could it be that it was overlooked that smtp_deliver was already set by smtp_update_1303()?

[jenlampton]

There are legitimate reasons for a site owner to turn off SMTP (usually temporarily, but also for local dev sites that you don't want sending out emails).

@bugfolder I'm pretty sure ~everyone~ most people on version 1.7.0 got caught by the last update, and don't know email sending is disabled on their sites. This breaks contact forms unrecoverably (since those messages are not saved anywhere) and is a much bigger problem than the few sites that are in development and start sending emails.

Why are we overriding the site owner's setting?

If the default for new installs is TRUE, we should set it that way for people getting this setting for the first time (and allow people to opt-out if they like).

All of my sites using SMTP right now look like this:

Theres no value for that config setting yet, and also no updates that need to be run on this site. If smtp_update_1303() intended to set that value it seems to have failed. That means that anyone (?) who did the last update is currently stuck in not-sending-email mode.

[jenlampton]

Hm, it looks like 1.7.0 has been out for over a year. So I guess we weren't really in any hurry to fix this problem. Most people who ran into this have probably already set a value by now (unless they updated recently). I'd be open to trying again to preserve a FALSE setting. Giving it a go...

[bugfolder]

Presumably your sites that look like the above are running SMTP 1.17.0, right? Update 1303 is only introduced in 1.17.1 (the new release). That is, the new release contains both update 1303 and 1304. So any site running 1.17.0 didn't have update 1303 run yet. I believe 1303 was sufficient to deal with the uninitialized smtp_deliver variable.

[jenlampton]

Update 1303 is only introduced in 1.17.1 (the new release).

Ah - so this issue is essentially a duplicate of another? ... that I missed? 🤦 but people are still out there running 1.7.0 with broken sites. So still not great :(

I had assumed the update went in with the new config options, so if the UI was there, the update had already been run. Never assume, Jen.

[jenlampton]

Ah - so this issue is essentially a duplicate of another? ... that I missed? 🤦

Yes, one you'd already fixed. https://github.com/backdrop-contrib/smtp/issues/20. Why are brains so bad at remembering things. I'm going to close this as dupe and add a commit.

[bugfolder]

Yes. And then maybe do another bugfix release so people don't install 1.7.1 and wipe out their previous FALSE setting (if they haven't yet).

[jenlampton]

@bugfolder I checked the usage stats and there was only one site running 1.17.1 (mine, probably) so I deleted the tag, and made a new release under the same number. I did, however, leave the update hook in place incase anyone has updated and run the update (so they won't miss the next update).

TY for catching this!

[bugfolder]

Sounds good!