Don't send comment emails to disabled users

[shawnhooper]

This plugin addresses issue #5.

In the process, I also created a generic is_user_disabled() function that checks the usermeta value. This was being done in two places already, and this PR added a third.

(Same PR as submitted yesterday, I just moved this to a branch in my fork so I could work on some other PRs)

[jaredatch]

I have an idea for a different approach for this @shawnhooper and you can tell me what you think.

I think a better solution would be to apply this globally.

By this, I mean if a user is disabled, they should never receive emails, regardless of the source. The comments email (addressed in the PR) is the obvious use case, but I think given other core emails along with plugins that may be activated on the site, a better approach would be to do this on a site-wide level.

What I envision this entailing:

• Filtering wp_mail() $to arguments via the wp_mail filter

• Create a new method, get_disabled_users that does a user meta query and fetches all disabled users. These users are stored in an array inside option that will be created (if it doesn't exist) ja_disabled_users. Probably could make the user ID they key with the email as the value. This way fetching disabled users in the future won't trigger user queries again.

• Enabling/disabling user methods would update the option

• Deleting users would need remove them from the option if they were previously disabled

• Filter wp_mail - explode $to into an array if it's not already. Fetch disabled users via get_disabled_users method. Then loop through the email addresses provided and if any of them are in contained in the array returned from get_disabled_users unset them from the $to before returning the final arguments.

This method would, at least in theory, prevent the disabled user(s) from ever getting an email regardless of the source. Definitely open to feedback.

[shawnhooper]

@jaredatch In theory, I agree with your idea, filtering wp_mail() would be really easy to do, and would stop all email being sent out as long as the plugins were coded to use wp_mail().

My only hesitation would be a case, where, for example, a left a company's web content team, and therefore had their account disabled. But they still work there, and should be receiving the results from a feedback form on the website. This approach would block that email.

The solution would be of course just to lower that user's account to Subscriber role or something like that.

Thoughts?

[jaredatch]

I think that scenario, while possible, is more edge case than the chance of disabled users getting unwanted emails that are sent from non-comment sources. So IMO it's worth proceeding with the risk of that possibly happening.

I also agree with that you said - at that point changing the user to subscriber would definitely make more sense.

I'm not sure how useful it would be (because I can't really think of a good use case at this moment) but we could always wrap the accounts returned from get_disabled_users in a filter. This way if, for whatever reason, a disabled user still needed to be able to get emails, they could use the filter to allow this to happen.

[shawnhooper]

Ooh, I love that combined wp_mail & disabled users filter idea. It would solve both of our concerns.

[shawnhooper]

@jaredatch I've implemented the changes we discussed. Rather get create a get_disabled_users function, I went with a get_disabled_users_email_addresses function. Since that's what we're dealing with when it comes to wp_mail it was streamlined and easy to do an array_diff on rather than having an array of WP_User objects.

[jaredatch]

@shawnhooper thanks!

The only thing missing I see which I would like, is the disabled users should be cached in an option.

Then when a user is disabled, enabled, or deleted, the option should be updated.

The logic behind this is for site performance. On large sites the frequency of wp_mail can be very high. Right now that's every email runs a query through all the users which also includes a join. So that's something that should be avoided. For small sites there would be no difference. Larger sites or sites with 10k users would definitely suffer a performance impact that could be measured.

Running the query one time, storing the results in an option, then simply maintaining that option after that will allow us to avoid possible performance issues. The options are already autoloaded anyways, so anytime (after the initial build) that a mail fires off and we reference the disabled users, it should be pretty much instant.

You've already contributed a ton (thanks!) so I have no problem at all making this adjustment after merging the PR, I just wanted to let you know what my intentions are :)

[shawnhooper]

@jaredatch Good call, as this data won't change frequently.

If you accept the PR for the adding hooks to the plugin, I've already modified the code that saves the Disabled state so that it only updates when a change has been made. This would be a good spot to put the code that resets the option value. You'd also need to trigger it if their email address changes, since the option would contain their email addresses.

Cheers.