Automattic / woocommerce-services

WooCommerce Services is a feature plugin that integrates hosted services into WooCommerce (3.0+), and currently includes automated tax rates and the ability to purchase and print USPS shipping labels.
GNU General Public License v2.0
107 stars 20 forks source link

Subscription Synchronizer adding free trial with automated taxes enabled #2478

Closed bborman22 closed 2 years ago

bborman22 commented 3 years ago

We had a user report an issue where the items in their cart were getting "With Array free trial" appended to their products names in the cart when using a combination of: WCS&T with automated taxes, Subscriptions with Synchronization, and All Products for WooCommerce Subscriptions.

There was some additional research on this issue and the apparent culprit is in this line: https://github.com/Automattic/woocommerce-services/blob/82b23b61496ca9aa8f57b1a7001e531e090cc1ce/classes/class-wc-connect-taxjar-integration.php#L639

And one proposed solution would be to unset the free trial once the $unit_price has been captured:

if ( class_exists( 'WC_Subscriptions_Synchroniser' ) ) {
    WC_Subscriptions_Synchroniser::maybe_unset_free_trial();
}

Markup 2021-09-28 at 10 56 37

Markup 2021-09-28 at 10 59 34

p1632852336307200-slack-C07414X4N

csmcneill commented 3 years ago

Issue reported in 4330767-zen.

manospsyx commented 3 years ago

Some additional context/analysis in this thread: https://a8c.slack.com/archives/C07414X4N/p1632852336307200

Summary:

The issue is not limited to Product Bundles or the All Products add-on, and seems to also affect vanilla subscription products with a synchronized renewal schedule. For example, the second line item in this screen grab is a Simple subscription product that is synchronized to renew every Sunday:

Image 2020-08-13 at 11 09 47 a m

Here's what happens when I enable Automated Taxes:

Image 2020-08-13 at 11 11 28 a m

(note the "1 day free trial" on the Simple Subscription product)

You can find some debug output in that Slack thread which indicates that the mocked free trial applied at https://github.com/Automattic/woocommerce-services/blob/82b23b61496ca9aa8f57b1a7001e531e090cc1ce/classes/class-wc-connect-taxjar-integration.php#L639 is not unset correctly.

dustinparker commented 2 years ago

Hey everyone,

Array free trial issue I haven't been able to replicate the "Array free trial" part of the issue yet. Would someone mind posting some screenshots of their WC Subscriptions settings and both product settings so I can try to replicate an exact setup? Thanks!

Mini cart total issue The mini cart price appears to show the price that will be charged on the current day, so shows $0.00 if the cart only has non-prorated subscriptions in the cart which are synchronized to a different day. For example, if my subscriptions are set to renew every Sunday but today is Friday, the mini cart total is $0.00. However, if today is Sunday, then my cart shows the total of those subscriptions since they will be charged today.

I didn't find any documentation about what the mini cart should show for non-prorated, synchronized subscription products, but it does this even with WC S&T deactivated so appears it would have to be fixed in WC Subscriptions extension if that is indeed a bug.

manospsyx commented 2 years ago

I haven't been able to replicate the "Array free trial" part of the issue yet.

Hey @dustinparker 👋

The issue manifests itself differently when adding:

This screenshot outlines the difference (note the "1 day free trial" on the Simple Subscription product).

Would someone mind posting some screenshots of their WC Subscriptions settings and both product settings so I can try to replicate an exact setup? Thanks!

From https://a8c.slack.com/archives/C07414X4N/p1632856126313700?thread_ts=1632852336.307200&cid=C07414X4N --

Synchronization settings under WooCommerce > Settings > Subscriptions look like this: https://share.getcloudapp.com/Wnu0gLQ6

...and here's the Simple Subscription product configuration: https://share.getcloudapp.com/kpunj9PD

There's also a backtrace there that should help you understand why maybe_set_free_trial needs to be followed by maybe_unset_free_trial. Feel free to ping me in Slack if any questions come up! ☺️

dustinparker commented 2 years ago

Thanks @manospsyx ! I'm able to recreate the issue now and am looking into it.