Proper subscription notifictations (WP Cron and other enhancements)

[mechter]

So I had my issues with the current BCC implementation of subscription notification emails, as had others (see e.g. #2162).

I didn't want to mess with core, so I created my own code to work around the problems and put it in a plugin. I've published this plugin here: ​https://wordpress.org/plugins/asyncronous-bbpress-subscriptions/. @Robkk suggested I add this to core, so I've incorporated the plugin into BBPress and it's now a solid notification system in place of the old one. Here are the changes in short:

• Uses WP Cron to send subscription notifications instead of BCC lists
• Revised subject and message texts
• Makes one DB query for all recipients instead of one get_userdata() call per recipient
• Uses admin address as From address, so your users can actually reach you when something is wrong with these notifications
• Can set proper bounce address (Return-Path) via filter
• Recipients emails are of format "Display Name <email@…>", i.e. they now contain the user's name
• Messages contain exactly 42 dashes in a row, in memory of Douglas Adams

filters added:
'bbp_subscription_email_from'
'bbp_bounce_address' ( sets PHPMailer::$Sender )
'bbp_subscription_email_recipients'
'bbp_forum_subscription_email_subject'
'bbp_forum_subscription_email_message'
'bbp_topic_subscription_email_subject'
'bbp_topic_subscription_email_message'

filters removed:
'bbp_get_do_not_reply_address'
'bbp_subscription_mail_message'
'bbp_subscription_mail_title'
'bbp_subscription_from_email'
'bbp_subscription_mail_headers'
'bbp_subscription_to_email'
'bbp_forum_subscription_mail_message'
'bbp_forum_subscription_mail_title'
'bbp_subscription_from_email'

changes signature of actions (now without parameters):
'bbp_pre_notify_subscribers'
'bbp_post_notify_subscribers'

I've tested the new subscription system in trunk and it works for me.

[mechter]

A first patch, tested and works for me

[netweb]

@mechter Awesome, thanks for this, will take a look soon :)

[mechter]

Improved patch

[mechter]

Improved the patch some more, the functionality is now completely encapsulated and all that's left for the old notification functions is to invoke a factory call (actually the factory is encapsulated, too, only success/failure is returned to the calling function). Enjoy.

Time to get back to my real project (that uses bbPress). :)

[mechter]

Fixed return values

[mechter]

re-added check for empty user id array before performing query

[mechter]

filter 'bbp_subscription_email_recipients' is now applied even when no recipients exists so that plugin authors can manually add recipients. I'm done patching for now ;)

[netweb]

@r-a-y Based on your feedback in #2162 any chance you could take a look at this?

(Might also be beneficial to the current emails discussions currently occurring for BuddyPress)

[tharsheblows]

I have a couple of concerns:

• this breaks backwards compatibility if anyone is using those filters or their own thing for subscription emails on the bbp_new_reply / bbp_new_topic actions.
• on sites which aren't very active, it could take a while for cron to run.

I really like this though, I use cron to send mine too and agree it can work quite well. One thing I do is add a one-click unsubscribe link to the emails which works whether the user is logged in or not. This not only helps keep down the number of people hitting spam on the emails but also the amount of email I get asking for help in unsubscribing.

[mechter]

It's been a while, good to see the patch has not been forgotten about.

Regarding your concerns, @tharsheblows:

I streamlined the filter/action API for clarity and consistency. If we are to retain backwards compatibility, we could simply add the old filters back in and mark them obsolete.

Actually, WPCron should run instantly. When a message is posted, the job is scheduled for immediate execution. Then the user is redirected to view their new message and that GET call should trigger WPCron.

I really like your one-click unsubsribe, let's add that instead of the unsubscribe instructions.

[tharsheblows]

Ha ha ha, I forgot about the page reload! It's been one of those summers. Yes, adding the old filters back in would be great and using the old functions to trigger the cron job so users don't get two subscription emails after updating bbPress.

Here are the conditions for my unsubscribe code. Please let me know if you see any issues with them! This works for my site but might not be right for everyone:

• works if logged in as person trying to unsubscribe
• works if not logged in
• does not work if logged in as someone else but gives non-personalised instructions to unsubscribe
• there's no time limit on the links, they will work years from now unless you delete the plugin

The unsubscribe code works like this (I am doing this from a quick look at mine, I might be missing something...):

• on plugin init: add salt to wp_options (this never ever changes whereas WP salts might)
• make a hash value with user id, topic id and the salt (not terribly a secure one but it doesn't matter)
• unsubscribe link is the link to the topic with query args: user id, topic id and hash value
• add action to eg bbp_template_before_single_topic to check for query args and unsubscribe if everything checks out (compare topic id to current topic, user id to current user and check hash value to make sure user id and topic id haven't been altered)
• adds a template notice at the top to let you know the results of the unsubscribe attempt

I would rather someone be accidentally unsubscribed than not be able to do it. My spam rates have gone down a great deal since using this and I can't remember the last time someone emailed me annoyed / furious they couldn't unsubscribe. :)

[mechter]

Okay guys, what is taking so long? @netweb you wrote you'd take a look "soon", that was 3 months ago. Here is a more backwards compatible patch to address @tharsheblows' concerns. Before I even consider adding more cool features, like an unsubscribe link, I'd like to see this patch making its way into trunk.

filters reintroduced for backwards compatibility:

bbp_subscription_mail_message
bbp_subscription_mail_title
bbp_forum_subscription_mail_message
bbp_forum_subscription_mail_title
bbp_subscription_from_email
bbp_get_do_not_reply_address
bbp_subscription_mail_headers

filters that kept their original functionality:

bbp_topic_subscription_user_ids
bbp_forum_subscription_user_ids

newly added filters for functionality and subscription API streamlining:

bbp_subscription_email_from
bbp_bounce_address
bbp_subscription_email_recipients
bbp_forum_subscription_email_subject
bbp_forum_subscription_email_message
bbp_topic_subscription_email_subject
bbp_topic_subscription_email_message

removed filter:

bbp_subscription_to_email

(this filter is useless now and likely to do more harm than anything else, for valid use cases, use bbp_subscription_email_recipients)

removed function:

bbp_get_do_not_reply_address

(I would feel bad if I kept that, BBP doesn't need a "do not reply" address anymore)

[mechter]

Backwards compatible filters

[mechter]

small bugfix for backwards compatible filters

[mechter]

missed one

[mechter]

Added filters to selectively disable the async part. Reflects recent update to AsynCRONous bbPress Subscriptions (requested feature). Patched against current trunk.

[johnjamesjacoby]

Thanks a bunch for continuing to work on this.

In the interests of getting 2.6 out, I'm marking this for 2.7 early so we can review this and give it proper attention.

This all looks really great.