Skip to content
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

PHP Notice: Undefined property: Automatic_Upgrader_Skin::$plugin_active #589

Closed
raamdev opened this issue Oct 10, 2015 · 7 comments
Closed

PHP Notice: Undefined property: Automatic_Upgrader_Skin::$plugin_active #589

raamdev opened this issue Oct 10, 2015 · 7 comments

Comments

@raamdev
Copy link
Contributor

@raamdev raamdev commented Oct 10, 2015

A user reported (via internal ticket) the following PHP Notice in their error log:

[09-Oct-2015 11:46:33 UTC] PHP Notice: Undefined property: Automatic_Upgrader_Skin::$plugin_active in /home/example/public_html/testing/wp-content/plugins/zencache-pro/src/includes/closures/Plugin/WcpUpdaterUtils.php on line 46

Here's that line in the source code: https://github.com/websharks/zencache-pro/blob/151004/src/includes/closures/Plugin/WcpUpdaterUtils.php#L46

It looks like we need to check if isset($skin->plugin_active) before referencing it on that line. Agreed @jaswsinc?

@raamdev raamdev added this to the Next Release (Pro) milestone Oct 10, 2015
@jaswrks
Copy link

@jaswrks jaswrks commented Oct 11, 2015

It looks like we could also change $skin->plugin_active === true to:

is_plugin_active($data['plugin'])

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Oct 11, 2015

@jaswsinc I agree. That's even better. Thanks!

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Oct 11, 2015

Next Pro Release Changelog:

  • Bug Fix: Fixed an issue where a PHP Notice was generated when an inactive WordPress component was being upgraded. This issue did not have any adverse affect on the site, but this fix resolves the issue so that the notice won't appear in PHP logs. See Issue #589.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Oct 11, 2015

@jaswsinc Can you confirm that wpsharks/comet-cache-pro@542a6f8 is correct before I close this issue? I didn't see any use of $skin in the code once I replaced $skin->plugin_active, but I want to double-check that I'm not missing something.

Also, I assume that we do need to keep the \WP_Upgrader $upgrader_instance function parameter for the upgrader_process_complete hook, correct?

@jaswrks
Copy link

@jaswrks jaswrks commented Oct 12, 2015

Can you confirm that wpsharks/comet-cache-pro@542a6f8 is correct before I close this issue?

Yep, wpsharks/comet-cache-pro@542a6f8 looks like a good choice to me also.

Also, I assume that we do need to keep the \WP_Upgrader $upgrader_instance function parameter for the upgrader_process_complete hook, correct?

It looks like we aren't using it any longer. However, we are using the second parameter $data, so I'd say just leave it. The type hint for \WP_Upgrader $upgrader_instance is a sort of built-in sanity check for us. If that happens to change for some reason it will start throwing an exception and we can get it fixed.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Oct 14, 2015

@jaswsinc Cool. Thanks for confirming! 😃

@raamdev raamdev closed this Oct 14, 2015
@raamdev raamdev self-assigned this Nov 1, 2015
@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Nov 14, 2015

ZenCache v151114 has been released and includes changes from this GitHub Issue. See the v151114 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#589).

@wpsharks wpsharks locked and limited conversation to collaborators Nov 14, 2015
@raamdev raamdev removed their assignment Apr 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants