Filters for Plugin/Theme Update Email Notifications

[ronalfy]

I've had several requests to enable core update emails, but disable emails for plugins/themes.

If this idea is approved, I would love to be the one to tackle this as I have yet to contribute to core.

Goal is to keep send_core_update_notification_email intact, but allow filters for both plugin/theme background update notifications.

Apparent use-case is a user wants core emails, but not plugin/theme emails. One filter rules them all currently from what I can see in the code-base.

Example use-case in the wild: ​https://wordpress.org/support/topic/for-on-the-wishlist?replies=8

[ronalfy]

Patch to add theme, plugin, translation files.

[ronalfy]

Simplified filter.

[ronalfy]

Patch 2 simplifies the filter calls and moves everything to one filter since we're already passing the type.

[ronalfy]

Bumping for patch. Sorry.

[ronalfy]

One Filter to Rule Them All

[ronalfy]

Patch 3 solves the issue since we're already passing type for context. No reason to have 3 conditionals.

[johnbillion]

Thanks for the patch, Ronald!

This filter makes sense. Here's some feedback on your patch:

• If the filter returns false for one or more of the update types, it prevents the email from being sent for all subsequent update types too. The return statement after the filter should probably be a continue. Then below the foreach loop you could return if the $body variable is empty.
• The diff should be generated from the root of your repo so other people can apply the patch without having to specify which file is to be patched.
• There's a typo in the docblock: translators instead of translations.
• Passing $this as the third parameter to the filter would be a good idea so the filter receives as much context as possible.

[ronalfy]

Thank you! I'll revisit this tomorow and revise my patch. Thank you SO much for the feedback.

Kinda curious why 4.3 was moved back to 3.7. Please pardon my noob-ness.

[ronalfy]

Continue instead of return. Empty body check.

[ronalfy]

@johnbillion,

Added the requested changes and worked with my internal testing.

Anything I missed? Ping me on slack (ronaldhuereca). May need some mentoring here if patch is submitted incorrectly.

[johnbillion]

In 34543:

Introduce a send_update_notification_email which controls whether an update notification email is sent for background updates. This filter allows control over each of the update types (plugin, theme, translation) and compliments the automatic_updates_send_debug_email and send_core_update_notification_email filters.

Fixes #33932
Props ronalfy

[ronalfy]

Just an FYI. I was instructed to create a new ticket.

There is a bug for the committed patch when I tested against WordPress 4.4 Beta 1. Here is the new ticket.

#34434

[ocean90]

[34543] is incomplete because there is a second loop for the "UPDATE LOG" part. This and the bug mentioned in #34434 sounds like we should revert [34543] for 4.4 and give it another try in 4.5.

[ronalfy]

Replying to ocean90:

[34543] is incomplete because there is a second loop for the "UPDATE LOG" part. This and the bug mentioned in #34434 sounds like we should revert [34543] for 4.4 and give it another try in 4.5.

Agreed on the revert unless patch for #34434 can be committed. Will post a new patch when 4.5 is in the pipeline.

[ocean90]

In 35619:

Upgrade: Revert [34543] because of incomplete and incorrect functionality.

See #33932.
Fixes #34434.

[johnbillion]

@ronalfy Do you want to tackle this again?

[ronalfy]

Replying to johnbillion:

@ronalfy Do you want to tackle this again?

Sure! I can do that. I'll try again this weekend.

[ronalfy]

Filter to disable e-mails for plugins, themes, and translations

[ronalfy]

@johnbillion Added Patch 5. I've tested this with background updates for themes, plugins, core, and various combinations of each to ensure the patch is correct.

Can you please look over and offer any suggestions?

Thanks!

PS: I may have to redo this since there seems to be a bug (see #34878) with automatic updates at the moment.

[jorbin]

There hasn't been any recent activity on this ticket and the beta 1 deadline for 4.5 is tomorrow, so punting.

[ronalfy]

Filter to disable e-mails for plugins, themes, and translations

[kidsguide]

I have tested patch.6.diff and all works great. Each filter did what it was suppose to do.

[ronalfy]

@ocean90 @adamsilverstein would you be willing to consider this patch for WordPress 4.6?

A common use case is if a user has automatic updates for plugins/themes/translations. We have a fairly popular plugin (60k) installs where users are constantly complaining of the massive influx of notification e-mails with the only option to turn them off using a all-is-on, all-is-off filter.

I've thoroughly tested this with plugin updates, translation updates, theme updates, and core minor/update updates.

[ronalfy]

Filter to disable e-mails for plugins, themes, and translations

[ronalfy]

Added patch based on recs from @adamsilverstein to modify class var directly instead of creating new var.

Tested the same as above.

[welcher]

Adding unit tests

[welcher]

I added a rather large patch that contains unit test. The reason for the size is that there seemed to be no tests related to the send_debug_email method or the upgrader component ( at least that I could find ) so I needed to add them.

The tests added here have a baseline test for the method and some tests for the filter. There is also a fix in place here that addresses #36609 which was discovered while writing the tests for this patch.

Any feedback or direction is welcome.

[ronalfy]

@welcher found a bug in the patch. I think we need a similar path to where I essentially clone the class var instead of modifying it directly. https://core.trac.wordpress.org/attachment/ticket/33932/patch.6.diff

Modifying it directly can cause the action automatic_updates_complete to not pass the proper data.

[ronalfy]

Add filter and unit tests for automatic update emails

[ronalfy]

Added patch to fix bug in @welcher's patch when patch7 was added.

We need to basically clone the class var because the action automatic_updates_complete needs the full dataset.

Modifying the class variable will make this hook unreliable, so I chose to create a method variable instead to eliminate this conflict.

[ocean90]

Some feedback about 33932.patch​:

• The doc block for send_update_notification_email needs to be aligned with a whitespace before the *.
• The documentation should start with "Filter whether to".
• It seems like the current documentation was copied from the send_core_update_notification_email filter. The new filter should have its own documentation because the current one is wrong.
• It's also worth to mention, that the suggested filter lives in send_debug_email, naming it send_update_notification_email feels wrong.
• Changes to tests/phpunit/tests/mail.php should be part of a separate ticket. It seems like [37307] fixed parts of this already.
• In test_send_update_notification_email_filter_no_emails() the comment for send_update_notification_email should be just // Disable all notifications.
• In test_send_update_notification_email_filter_remove_single_type() the comment for send_update_notification_email is wrong (copy/paste error).
• In test_send_update_notification_email_filter_remove_single_type() the variable variable $$type looks odd. The alignment should be improved too.

[ronalfy]

Filter to disable e-mails for plugins, themes, and translations

[ronalfy]

Thanks for the feedback @ocean90. I changed the filter name to have better context. It's now send_debug_notification_email.

Based on the feedback on the unit tests, I removed them and just included the filter in this patch.

@welcher would you be up to opening a separate ticket and applying your unit tests based on the above feedback?

THANKS!

[ronalfy]

Filter to disable e-mails for plugins, themes, and translations

[ronalfy]

@ocean90 applied new patch. If committed, recommend @kidsguide for props as he helped with testing.