WPprodigy / woocommerce-product-fees

Add additional fees at checkout based on products that are in the cart.
https://wordpress.org/plugins/woocommerce-product-fees/
Other
25 stars 19 forks source link

Glitch with subscriptions #23

Closed stickypages closed 7 years ago

stickypages commented 7 years ago

Thought I would open up a new issue for this.

When I added a subscription for my cart, that does NOT have a "fee". It still seems to add a fee. Small issue from what I can see.

2017-02-24 13_01_47-cart

WPprodigy commented 7 years ago

Hmm, having a subscription product in the cart seems to double other fees. And if the subscription has a fee, it also doubles it's own fee.

Unsure why atm, but when subs is active or a subscription product is in the cart, this loop runs through each product twice: https://github.com/WPprodigy/woocommerce-product-fees/blob/master/classes/class-woocommerce-product-fees.php#L58

Edit: Goes deeper. The plugin actually loads twice w/ the above conditions https://github.com/WPprodigy/woocommerce-product-fees/blob/master/woocommerce-product-fees.php#L26-L32

WPprodigy commented 7 years ago

Dug into subscriptions a little bit.

add_action( 'plugins_loaded', __CLASS__ . '::load_dependant_classes' ); loads class-wc-product-subscription.php. Commenting out either line solves the bug.

This plugin also uses plugins_loaded, so I tried using different priority levels for each but nothing changed. Also tried implementing an $instance check in this plugin so it could only be loaded once, still no luck.

Starting to lean towards this being something weird in Subs. Can reproduce with a snippet like this. It will print each product's ID twice in the error log when refreshing the cart page (when a simple product and subscription product are in the cart):

 add_action( 'woocommerce_cart_calculate_fees', 'wcpf_testing_fees', 15 );
 function wcpf_testing_fees() {
     WC()->cart->add_fee( 'Custom Fee', '5' );

     foreach( WC()->cart->get_cart() as $cart_item => $values ) {
         error_log('looping through for product ID #' . $values['product_id']);
     }
 }

I can think of a few workarounds, but pretty curious why a subscription product in the cart causes a double loop.

thenbrent commented 7 years ago

but pretty curious why a subscription product in the cart causes a double loop.

@WPprodigy depending on what is actually causing your code to run twice, it might not be just a double loop, it might be a 1...n loop, where n is the number of different billing schedule combinations of subscription products in the cart.

It appears like a double here, but I suspect that may be because you have just one unique schedule for one subscription product in the cart. Add a few more products with different billing schedules, and you may see that loop increase.

Subscriptions needs to run cart calculations for each set of recurring totals to make sure shipping/taxes/fees/discounts etc. are all correct. To do this, Subscriptions clones the WooCommerce cart to make "recurring carts" for each unique set of recurring totals. You can see this code here: https://github.com/Prospress/woocommerce-subscriptions/blob/master/includes/class-wc-subscriptions-cart.php#L245:L322

I think that is what is causing the issue here, because your code assumes everytime the 'woocommerce_cart_calculate_fees' hook is called, it's being called by the global WC()->cart, but it can be called by different instances of the cart.

A solution is probably to use the cart instance passed to your code via the 'woocommerce_cart_calculate_fees' hook, instead of the global WC()->cart->get_cart() instance.

For example, the snippet you provided above to reproduce would it be modified to do it this like:

 add_action( 'woocommerce_cart_calculate_fees', 'wcpf_testing_fees', 15, 1 );
 function wcpf_testing_fees( $cart ) {
     $cart->add_fee( 'Custom Fee', '5' );

     foreach( $cart->get_cart() as $cart_item => $values ) {
         error_log('looping through for product ID #' . $values['product_id']);
     }
 }

A diff of the changes is available here: https://www.diffchecker.com/oFogIDxd

That will still add the log entry twice, but it shouldn't add 2x the fees to the global cart. It should only add 1x the fees to each instance of the $cart passed to your callback.

If you're curious, there is some in-depth background on this grouping and calculation code in these docs:

WPprodigy commented 7 years ago

Thanks for the help @thenbrent :)