-
Notifications
You must be signed in to change notification settings - Fork 19
Add Auto Update #16
Add Auto Update #16
Conversation
a90f90d to
da1e635
Compare
auto-update.php
Outdated
| * Adds the wc-api-dev plugin to the list of allowed plugins for auto update. | ||
| */ | ||
| function auto_update( $update, $item ) { | ||
| if ( in_array( $item->slug, array( 'wc-api-dev' ) ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use a simple string comparison here?
if ( 'wc-api-dev' === $item->slug ) {
auto-update.php
Outdated
| $request_uri = 'https://api.github.com/repos/woocommerce/wc-api-dev/releases'; | ||
| $response = json_decode( wp_remote_retrieve_body( wp_remote_get( $request_uri ) ), true ); | ||
| if ( is_array( $response ) ) { | ||
| foreach( $response as $entry ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after foreach.
auto-update.php
Outdated
| * Add our plugin to the list of plugins to update, if we find the version is out of date. | ||
| */ | ||
| public function modify_transient( $transient ) { | ||
| if( property_exists( $transient, 'checked' ) && $transient->checked ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after if.
auto-update.php
Outdated
| if( property_exists( $transient, 'checked' ) && $transient->checked ) { | ||
| $checked = $transient->checked; | ||
| $this->maybe_fetch_github_response(); | ||
| $out_of_date = version_compare( $this->github_response['tag_name'], $checked[ $this->file ], 'gt' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check github_response for 'tag_name' and $checked for $this->file to avoid warnings.
Nitpick: WP core uses '>' for version_compare() calls.
auto-update.php
Outdated
| $checked = $transient->checked; | ||
| $this->maybe_fetch_github_response(); | ||
| $out_of_date = version_compare( $this->github_response['tag_name'], $checked[ $this->file ], 'gt' ); | ||
| if( $out_of_date ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after if.
| 'plugin' => $this->file, | ||
| 'slug' => 'wc-api-dev', | ||
| 'package' => $this->github_response['zipball_url'], | ||
| 'new_version' => $this->github_response['tag_name'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid PHP warnings, we should check github_response for these array keys before accessing them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed. For this one, I am now bailing early and not modifying the transient, if we don't have the correct GitHub response.
auto-update.php
Outdated
| */ | ||
| public function plugin_popup( $result, $action, $args ) { | ||
| if ( ! empty( $args->slug ) ) { | ||
| if ( $args->slug === current( explode( '/' , $this->file ) ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not hardcode a check for 'wc-api-dev' here?
auto-update.php
Outdated
| $plugin = array( | ||
| 'name' => $plugin['Name'], | ||
| 'slug' => $this->file, | ||
| 'version' => $this->github_response['tag_name'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about checking github_response before accessing.
auto-update.php
Outdated
| } | ||
|
|
||
| /** | ||
| * Move the updated plugin to the correct directory, and reactivate the plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include a comment as to why this is necessary. I assume because the Github zip has a short hash in the filename?
| /** | ||
| * Move the updated plugin to the correct directory, and reactivate the plugin. | ||
| */ | ||
| public function after_install( $response, $hook_extra, $result ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this need to be restricted to only the wc-api-dev plugin?
jeffstieler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small nitpick, otherwise the changes look good.
| $tag_name = ! empty( $this->github_response['tag_name'] ) ? $this->github_response['tag_name'] : ''; | ||
| $published_at = ! empty( $this->github_response['published_at'] ) ? $this->github_response['published_at'] : ''; | ||
| $body = ! empty( $this->github_response['body'] ) ? $this->github_response['body'] : ''; | ||
| $zipball_url = ! empty( $this->github_response['zipball_url'] ) ? $this->github_response['zipball_url'] : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick - WP coding standard is to test the positive first. See: https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#ternary-operator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeffstieler "An exception would be using ! empty(), as testing for false here is generally more intuitive.".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For shame. That'll learn me to read.
ryelle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, & test worked for me 👍
This PR makes it so that the
wc-api-devplugin can auto update itself as new (non prerelease) releases come out on GitHub.This can be disabled by setting the constant
WC_API_DEV_AUTO_UPDATEto false.To Test:
wc-api-dev. Download and install from zip.auto-update.phpin your root WordPress folder. This is so you can force an auto update, and ignore the lock mechanism... otherwise you have to wait for transients to expire.wc-api-dev.phpandreadme.txtand just set the version to something lower, like 0.4.0. This is to trick the system into thinking the plugin is out of date.php auto-update.phpfrom the command line in your root WordPress folder.wc-api-dev.phpandreadme.txthave the latest version tag, and the auto upgrade code will be gone too, since the current release doesn't have it.