search

wp-cli / extension-command

Manages plugins and themes, including installs, activations, and updates.
MIT License
90 stars 79 forks source link

Default to false when the "network" flag is omitted in the activate command #364

Closed raicem closed 1 year ago

raicem commented 1 year ago

Fixes #348

For the activate command, we're defaulting the $network_wide option to NULL if the --network flag is omitted. This causes type errors further down in the code, as shown in issue #348.

I thought about doing the same for the deactivate command as well, but for deactivation, passing null is probably what we should keep since it has its own meaning in the WordPress' deactivate_plugins function.

danielbachhuber commented 1 year ago

@raicem Planning to finish this one up?

raicem commented 1 year ago

Yes @danielbachhuber. I struggled to figure out tests. Can I get another week? Otherwise I'll close the PR.

danielbachhuber commented 1 year ago

@raicem Yep, take your time. Feel free to stop by the #cli channel on WordPress.org Slack if you'd like help further.

raicem commented 1 year ago

@danielbachhuber Thank you for your patience with this :bow: I managed to write a test for this regression now. I hope it makes sense!

While reported in the original issue #348, I can't replicate the type problem with wp plugin deactivate. When deactivating, WP CLI doesn't (and probably can't) cause a type error since the value it passes down to the deactivate_plugins function is not used later in the code.

This is not the case when activating a plugin. The $network_wide variable we pass to activate_plugin is directly passed down to the activate_{$plugin} hook and causes a type error. Therefore I only fixed and added a test for wp plugin activate.

swissspidy commented 1 year ago

While reported in the original issue #348, I can't replicate the type problem with wp plugin deactivate. When deactivating, WP CLI doesn't (and probably can't) cause a type error since the value it passes down to the deactivate_plugins function is not used later in the code.

This is not the case when activating a plugin. The $network_wide variable we pass to activate_plugin is directly passed down to the activate_{$plugin} hook and causes a type error. Therefore I only fixed and added a test for wp plugin activate.

Thanks for digging into this!

IIRC I originally ran into this with the activate command, but included deactivate in the bug report too so that we check whether it's affected as well. So thanks for confirming that the deactivate command is not affected!

raicem commented 1 year ago

Thank you @swissspidy for the explanation!

My tests failed, probably because the example code in my test doesn't play nicely with PHP 5.6. I'll check that and fix it later today.