Navigation Menu

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

Fix WCA fatal with bail conditions for WCA execution paths #32814

Merged
merged 6 commits into from Apr 29, 2022

Conversation

ilyasfoo
Copy link
Contributor

@ilyasfoo ilyasfoo commented Apr 28, 2022

Changes proposed in this Pull Request:

Closes #32797. This PR:

  1. Attempts to resolve fatal errors when WCAdmin plugin is activated alongside
  2. Deactivate WCAdmin plugin when it's detected

How to test the changes in this Pull Request:

Build WooCommerce zip

  1. In your local, run git cherry-pick 1bbf2f50d390acad83b023d1ebe61224944fd102 to get proper WC version in autoloader (Original PR). Otherwise WCAdmin plugin may take precedence
  2. Run pnpm nx build:zip woocommerce to build the zip

Testing on existing activated plugin

  1. In a JN, install the latest stable WooCommerce
  2. Install and activate WooCommerce Admin
  3. Build the plugin in your local and upload it to the JN instance
  4. Observe no fatal error

Testing new activation

  1. Install and activate WooCommerce Admin
  2. Observe no fatal error and a notice appears after the plugin is deactivated

image

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@ilyasfoo ilyasfoo requested a review from a team April 28, 2022 06:36
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Apr 28, 2022
if ( ! defined( 'WC_ABSPATH' ) ) {
return;
}
return WCAdminAssets::should_use_minified_js_file( $script_debug );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No deprecation notice via wc_deprecated_function since it also causes output during activation, which would break plugin activation.

@@ -84,30 +84,29 @@ public function __construct() {
*/
remove_action( 'admin_print_scripts', 'print_emoji_detection_script' );

add_action( 'admin_init', array( __CLASS__, 'is_using_installed_wc_admin_plugin' ) );
add_action( 'admin_init', array( __CLASS__, 'deactivate_wc_admin_plugin' ) );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing the now obsolete version check with deactivation check

@ilyasfoo ilyasfoo added this to the 6.5.0 milestone Apr 29, 2022
@ilyasfoo ilyasfoo added the release: cherry-pick Commits from this PR also needs to be added to current release branch. label Apr 29, 2022
Copy link
Contributor

@adrianduffell adrianduffell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tested well and LGTM! Thanks for wrangling this @ilyasfoo 👍 🚀

@ilyasfoo ilyasfoo merged commit 959ba86 into trunk Apr 29, 2022
@ilyasfoo ilyasfoo deleted the fix/32797-wca-fatal branch April 29, 2022 02:50
@github-actions
Copy link
Contributor

Hi @ilyasfoo, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[WC 6.5 RC1] Fatal error with WooCommerce Admin 3.3.2 activated
3 participants