Skip to content
This repository has been archived by the owner on May 29, 2020. It is now read-only.

Add functions to handle plugins updates notification emails #54

Merged
merged 3 commits into from
Mar 13, 2020

Conversation

audrasjb
Copy link
Owner

This pull request adds the following functions:

- wp_autoupdates_automatic_updates_complete_notification()
This function is hooked on automatic_updates_complete core hook which triggers right after the updates attempts. The function checks the updates results and triggers wp_autoupdates_send_email_notification() function according to the type of results (success, failed, or mixed).

- wp_autoupdates_send_email_notification()
This function builds and sends the email notification, depending of the type of results (success, failed, or mixed).
wp_autoupdates_notifications_email filters can be used to customize email notifications.

@audrasjb audrasjb added this to the 0.3.0 milestone Mar 12, 2020
@audrasjb
Copy link
Owner Author

Screenshot: example of Email notification on Gmail, in case of successful update:

Capture d’écran 2020-03-12 à 13 59 43

@pbiron
Copy link
Collaborator

pbiron commented Mar 12, 2020

Initial test of successfully auto-updating a single plugin: email message successfully sent/received!

Now I just gotta figure out how to force an auto-update failure to make sure the email message reflects that :-)

@bookdude13
Copy link
Contributor

I wonder if you could use some sort of proxy like Burp Suite and corrupt the response?

@pbiron
Copy link
Collaborator

pbiron commented Mar 12, 2020

I wonder if you could use some sort of proxy like Burp Suite and corrupt the response?

I've figured out that hooking into core's upgrader_pre_install and returning a WP_Error is a pretty effective way of simulating a failure.

…ceeded when determining whether to send email notifications.
Copy link
Collaborator

@pbiron pbiron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this to what is in the trac patch.

Successful updates always set result = true. Failed updates set it to either null or a WP_Error. In the case of WP_Error, the intval() call was incorrectly treating those as successes. Also, calling intval() on an object raises a PHP notice. The PHP Manual says:

intval() should not be used on objects, as doing so will emit an E_NOTICE level error and return 1.

If there was a specific reason you used inval() for the comparison, then it should be changed to:

if ( ! is_object( $update_result->result ) && 1 === inval( $update_result->result ) ) {
...
}

@pbiron
Copy link
Collaborator

pbiron commented Mar 12, 2020

After making the one change mentioned above, I think this works as expected:

  1. successful updates generate an email noting the success
  2. failed updates generate an email noting the failure
  3. mixed updates generate an email noting some successes and some failures

We might want to iterate on the text of the actual messages later, but the basic mechanism seems to work.

@pbiron
Copy link
Collaborator

pbiron commented Mar 12, 2020

The other thing I discovered is that on multisite, when an auto-update fails for a plugin that is active, the site is left in maintenance mode.

As far as I can tell, that is unrelated to this plugin, although we should investigate further before the core merge...because with more plugins being auto-updated the risk of ending up stuck in maintenance mode will go up. See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-plugin-upgrader.php#L251 for where the site gets placed into maintenance mode.

@audrasjb
Copy link
Owner Author

audrasjb commented Mar 13, 2020

Ok great, I tested your changes and it works on my side, I think we can merge this pull request.

I think it's worthwhile to release it as soon as possible so the content of the email could be iterated. Does it sound good to you?

The other thing I discovered is that on multisite, when an auto-update fails for a plugin that is active, the site is left in maintenance mode.
As far as I can tell, that is unrelated to this plugin, although we should investigate further before the core merge...because with more plugins being auto-updated the risk of ending up stuck in maintenance mode will go up. See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-plugin-upgrader.php#L251 for where the site gets placed into maintenance mode.

This is a very important information. Do we already have a ticket on Trac for this issue?

@pbiron pbiron merged commit 382ded2 into master Mar 13, 2020
@pbiron pbiron deleted the add/plugin-updates-email-notifications branch March 13, 2020 13:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants