johnbillion/user-switching

PHP notice when user-switching is used together with WooCommerce and MailPoet

Closed this issue · 2 comments

rodrigoprimo commented

Over at MailPoet, we got a report from one of our users that they get the following PHP notice when using user-switching to switch to a different user:

PHP Notice: get_cart was called <strong>incorrectly</strong>. Get cart should not be called before the wp_loaded action.

Backtrace:

require('wp-load.php'), 
require_once('wp-config.php'),
require_once('wp-settings.php'), 
do_action('init'), 
WP_Hook->do_action, 
WP_Hook->apply_filters, 
user_switching->action_init, 
switch_to_user, 
do_action('switch_to_user'), 
WP_Hook->do_action, 
WP_Hook->apply_filters, 
user_switching->forget_woocommerce_session, 
WC_Session_Handler->forget_session, 
wc_empty_cart, 
WC_Cart->empty_cart, 
do_action('woocommerce_cart_emptied'), 
WP_Hook->do_action, 
WP_Hook->apply_filters, 
MailPoet\AutomaticEmails\WooCommerce\Events\AbandonedCart->handleCartChange, 
WC_Cart->is_empty, 
WC_Cart->get_cart, 
wc_doing_it_wrong 

Please see <a href="https://wordpress.org/support/article/debugging-in-wordpress/">Debugging in WordPress</a> for more information. (This message was added in version 2.3.) in /var/www/html/wp-includes/functions.php:5311

user-switching calls WC_Session_Handler::forget_session() before the wp_loaded action which per se is not a problem (https://github.com/johnbillion/user-switching/blob/develop/user-switching.php#L852). But when this method is called, WooCommerce calls WC_Cart::empty_cart() and MailPoet hooks into an action that is executed inside this method to call WC_Cart::get_cart(). That is why the user is seeing the notice.

I don't have a strong opinion on whether this should be fixed in WooCommerce, MailPoet, or User Switching. If we fix this in MallPoet, there is a chance that other users will see the same notice when WooCommerce and User Switching are used in combination with another plugin. I found another user reporting the same, but they didn't provide enough details for us to tell if they were using MailPoet or something else: https://wordpress.org/support/topic/get-cart-should-not-be-called-before-the-wp_loaded-action/

I'm opening this issue to start a conversation and to ask if, in your opinion, it would make sense to change User Switching to execute user_switching::forget_woocommerce_session() after wp_loaded?

johnbillion commented

Thanks for the report. I was actually the one who introduced the forget_session() method into WooCommerce specifically so User Switching could instruct it to "forget" the session when switching users (woocommerce/woocommerce#21991).

I'm not completely sure but it seems like this should be fixed in the MailPoet plugin. The root problem is that MailPoet is calling WC_Cart::get_cart() without checking whether wp_loaded has fired. A plugin other than User Switching could call WC_Session_Handler::forget_session() before wp_loaded and MailPoet would see the same problem.

Thoughts?

rodrigoprimo commented

Thanks for the quick response. I'm not completely sure a well but I tend to agree with you. I will proceed with your suggestion of changing MailPoet to check if wp_loaded has fired before calling WC_Cart::get_cart().

  • 👍1