First of all this module is really great, and very extensive! congratulations!

We use this module in our typical dev/staging/production environments, and often copy down the database from production to staging and development servers for testing. Sometimes this requires cron to be run, but I couldnt find a really clean way to turn off subscription notification emails only (while leaving other parts of the site that require email enabled).

It would be awesome if there was a global variable (eg in the variables table) that allowed us to turn on/off subscription notification emails depending on the server, using settings files $conf variable overrides.

Patch to follow in the first comment once this issue's node id is created ;)

Comments

jwilson3’s picture

Heres a patch that wraps the call to drupal_mail() in an if statement that checks for the presence of the 'subscriptions_mail_send' variable.

On our production site we force this setting in code:
sites/example.com/settings.php

// Subscriptions:  force sending of emails on production server.
$conf['subscriptions_mail_send'] = 1;

On our staging and development sites, we block this setting in code:
sites/staging.example.com/settings.php
sites/example.local/settings.php

// Subscriptions:  block sending of emails on staging server.
$conf['subscriptions_mail_send'] = 0;
jwilson3’s picture

Status: Active » Needs review

Forgot to update status.

salvis’s picture

Status: Needs review » Needs work

I already have enough people who come here claiming that "Subscriptions does not send mail" — I'm not so keen to build in a mechanism for people to shoot themselves in the foot, especially one that is completely hidden.

Yet I see your need and I'm willing to accept a patch with the following characteristics:
— call the variable 'subscriptions_mail_trash_silently' and reverse the logic; default to FALSE
— remove the variable in hook_uninstall()
— document the usage and purpose near the bottom of the README.txt file
— adjust the watchdog item and summary messages stating something like "notification has been discarded due to the subscriptions_mail_trash_silently variable being set"
— put an error message on the settings page if the variable is !empty(), similar to what you get when you set cron percentage to 0:
'cron discards all notifications because you've set the subscriptions_mail_trash_silently variable to TRUE — see the README.txt file.'

jwilson3’s picture

> I already have enough people who come here claiming that "Subscriptions does not send mail" —

I can sympathize with your concern about people having issues with email not working, and I commend you for trying to deal with them, even though those issues often have nothing to do with your module.

> call the variable 'subscriptions_mail_trash_silently' and reverse the logic; default to FALSE

The patch and idea was originally based on functionality in the MailLog module who's email sending paradigm involves a maillog_send variable, which defaults to TRUE, and if you want/need to, you can disable it in settings.php or from the admin settings page. Only difference here is that you can't accidentally modify this in the admin screen and leave it in an "unhealthy" state, without knowing exactly what you are doing; it HAS to be done from code, specifically so that novices dont shoot themselves in the foot ;) Anyway, in the end, making it a killswitch that defaults to OFF instead of a passthrough that defaults to ON, it's 6 and one half dozen.

Will work on a patch this weekend that adjusts to your requests in #3.

damienmckenna’s picture

Why not just use Maillog?

jwilson3’s picture

>Why not just use Maillog?

This is to prevent notification emails from ever being sent from staging and development servers. If maillog is accidentally left enabled on staging while the client is testing other email functionality, then these subscription notification emails will get sent on every cron run. I want to strongarm specifically this module to prevent it from sending out any updates to real end-users that may be subscribed to specific content types, while the team or client is testing the site.

jwilson3’s picture

Status: Needs work » Needs review
StatusFileSize
new2.93 KB
jwilson3’s picture

Fixed wordwrap.

salvis’s picture

Status: Needs review » Fixed

Requiring an innocently named variable to be present with a certain value is harder to get right than requiring a variable with a nasty name to be absent. We want people to gravitate towards normal operation.

Committed with some modifications, thanks.

jwilson3’s picture

Thanks salvis. cheers.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.