-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display warning in plugin list info if plugin version is higher than expected #157
Display warning in plugin list info if plugin version is higher than expected #157
Conversation
PR is in draft as I couldn't find an any data of current theme version like plugin. For plugin that data is available in response's |
@thrijith The only thing I can think of right now is to go through the themes that state they don't provide an update and do an extra query to retrieve the latest version of these themes through the |
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.
I flagged some mostly minor changes in here. The code in general looks good, but we need those tests to make sure it actually what we expect.
For testing, I think you're best bet is to assemble and load a small custom plugin snippet in Behat that uses the site_transient_update_plugins
hook to modify the data that was retrieved from the API before running the command.
@schlessera I have added a test for verifying the change. Please check and let me know if something needs change. |
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.
A small reliability improvement, and then we're good to merge.
features/plugin.feature
Outdated
*/ | ||
|
||
add_filter( 'site_transient_update_plugins', function( $value ) { | ||
if ( isset( $value->no_update['hello-dolly/hello.php'] ) ) { |
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.
I think this could potentially cause sporadic test failures where an update will be noticed after the installation step in line 555.
I think it would be better to do the following instead, without a prior check:
unset ( $value->response['hello-dolly/hello.php'] );
(in case an actual update was found)$value->no_update['hello-dolly/hello.php']->new_version = '1.5';
to fake the versions as you already did.
This should always reliable trigger the error condition, if I've read the code correctly.
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.
Sorry for the back and forth, but I think there's still a logical flaw in there.
Alright, let's merge this version now. We'll probably soon find out if we forgot something else now...;) Thanks for the PR! |
Fixes #142
Theme List