wp-cli / extension-command

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

Incorrect message when there is error in plugin activation #396

Closed ernilambar closed 9 months ago

ernilambar commented 9 months ago

Bug Report

Describe the current, buggy behavior Lets say plugin hello.php as header Requires PHP: 9.0

When we run wp plugin activate hello.php then following result appears:

Warning: Failed to activate plugin. Current PHP version (8.1.16) does not meet minimum requirements for Hello Dolly. The plugin requires PHP 9.0..
Success: Plugin already activated.

Message Success: Plugin already activated. here is not correct.

Describe what you would expect as the correct outcome

Message should be updated to reflect actual situation.

Let us know what environment you are running this on

OS:     Darwin 22.6.0 Darwin Kernel Version 22.6.0: Tue Nov  7 21:48:06 PST 2023; root:xnu-8796.141.3.702.9~2/RELEASE_X86_64 x86_64
Shell:  /bin/zsh
PHP binary:     /usr/local/Cellar/php@8.1/8.1.27/bin/php
PHP version:    8.1.27
php.ini used:   /usr/local/etc/php/8.1/php.ini
MySQL binary:
MySQL version:
SQL modes:
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /Users/nilambarsharma/Sites/staging/app/public/wp-content/plugins
WP-CLI packages dir:    /Users/nilambarsharma/.wp-cli/packages/
WP-CLI cache dir:       /Users/nilambarsharma/.wp-cli/cache
WP-CLI global config:   /Users/nilambarsharma/.wp-cli/config.yml
WP-CLI project config:  /Users/nilambarsharma/Sites/staging/wp-cli.local.yml
WP-CLI version: 2.9.0
swissspidy commented 9 months ago

Re-sharing from Slack:

I can confirm this bug.

The reason for this is that \Plugin_Command::activate() calls report_batch_operation_results() here with zero successes and errors:

https://github.com/wp-cli/extension-command/blob/955bd947d95ef91da501179c7e561d229ab3cc19/src/Plugin_Command.php#L377-L380

We could either change the condition to if ( ! $this->chained_command && ( $successes || $errors ) ) { or leverage the $skips parameter of report_batch_operation_results() to indicate the number of skipped plugins because of such a warning.

schlessera commented 9 months ago

@swissspidy I think it would be better to actually increment $errors instead for the cases where we detect an activation failure. This will then make the batch processing just work.