Skip to content
This repository was archived by the owner on Oct 4, 2019. It is now read-only.

Fix plugin auto update logic #37

Merged
merged 3 commits into from
Aug 2, 2017
Merged

Fix plugin auto update logic #37

merged 3 commits into from
Aug 2, 2017

Conversation

emdashcodes
Copy link

@emdashcodes emdashcodes commented Aug 1, 2017

Auto updates were previously added in #16.

There was a problem with the auto-update logic. It worked with the testing instructions I provided (i.e. manually running wp_maybe_auto_update()), but doesn't seem to work out in the wild :(.

I've spent quite a bit of time trying to track it down and I think I've discovered why.

The relevant part seems to be where we bail early for on $transient->checked around line 60. This doesn't seem to always be set - specifically in the case of DOING_CRON.

I also looked at what our very own Woo Helper is doing, as well as a couple other GH update plugins and they don't bail like this. They check against the internal version of the plugin and add to the list of plugins to be updated.

The other addition here I made is storing the raw GitHub response (containing the latest release) in a transient. It's a short lived transient, but I wanted to avoid making multiple calls to the endpoint in rapid succession.

To Test (preferably on your AT site):

  • Download, install, and activate this helper plugin:
    reschedule-version-check.zip. It allows you to easily schedule the next version check to make testing easier, and will send a WordPress debug email with auto-update results.
  • Download
    https://cloudup.com/c5PYieevCBA
    which contains a zipped copy of this branch. There is one different between this zip and the PR here. I have set the version to 0.7.9, so that when you are testing, the version will be seen as out of date.
  • Upload this plugin over your existing wc-api-dev plugin and activate.
  • Go to https://:site?version-check=reschedule which should schedule a version check/update for 2 minutes from now.
  • Wait for two minutes.
  • Visit any page on your site, like the front page. WP's cron system will kick in.

You should receive an email like this:

notice

@ryelle
Copy link
Contributor

ryelle commented Aug 1, 2017

This is tricky to test. The linked wc-api-dev plugin doesn't have the version set back, it's set to 0.8.1, and I originally changed the header values not the constant. Anyway, I finally got the update test to work as you described, and got the email. While I was reloading wp-admin, I noticed that after the update check, the update indicator in the admin bar went from "1 update" to "2 updates" after a refresh. I then saw that I got the autoupdate email, and when I went to the updates page there were no updates pending, and wc-api-dev is correctly 0.8.0.

I'm out of time today to test further, but I can track down the "2 updates" bug tomorrow morning if you don't have any immediate ideas. (It could also be that I reloaded while it was autoupdating, but I don't think that should give me "2 updates").

@emdashcodes
Copy link
Author

The linked wc-api-dev plugin doesn't have the version set back

Ahh, of course I linked the wrong one. It's been a long day of digging in WP core code. I've updated my testing directions with the correct copy.

While I was reloading wp-admin, I noticed that after the update check, the update indicator in the admin bar went from "1 update" to "2 updates" after a refresh. I then saw that I got the autoupdate email, and when I went to the updates page there were no updates pending, and wc-api-dev is correctly 0.8.0.

I just tried testing this again myself and I only had a "1" appear in the wp-admin plugins menu.

That sequence of actions makes me think that one of those loads is what finally kicked off the autoupdate (which is why you then got the email). The way WP CRON works is by setting a timestamp for next run, and then, on the next page load after that, the queued action is ran.

WordPress plugin's system will still list the plugin as being available for update, giving users a chance to update since the auto-update code normally only runs twice a day -- so it's likely they would see a notice at some point.

However I don't have any immediate ideas about why you would see 2 instead of 1 if nothing else updated (since that indicator can also show themes and core updates). Any help tracking it down is appreciated.

@ryelle
Copy link
Contributor

ryelle commented Aug 2, 2017

It looks like the second update is Jetpack, which I think is coming from the beta testing plugin, so it's probably not an issue. After removing the beta-tester I'm not seeing the 2 updates anymore.

I do still see the update indicator even after the update succeeds, and it sticks around until I go to the updates page (which I assume clears the transient and forces a check again).

Some debugging on this leads me to the step right after updating, where the transient is cleared and rebuilt. auto-update.php still sees the old WC_API_Dev::CURRENT_VERSION when doing the version compare — likely because the page hasn't refreshed yet, so it's still running the "old" plugin code. I'm not sure how we could force this kind of check in the modify_transient function, or how other updater plugins handle this. Would this go away if the plugin is updated to code with the new checking functionality (rather than "downgrading" to 0.8.0)?

Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

Looks good - correctly updated the plugin and removed the update indicator.

@emdashcodes emdashcodes merged commit 1b5d6f3 into master Aug 2, 2017
@emdashcodes emdashcodes deleted the fix/auto-update branch August 2, 2017 19:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants