Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
173 stars 69 forks source link

Subscriptions sign-up fees are incorrectly converted when using WCPay multi-currency #5292

Closed haszari closed 1 year ago

haszari commented 1 year ago

Describe the bug

Sign-up fees are incorrectly converted with WCPay's multi-currency feature when customers select a currency that differs from the store's base currency.

To reproduce

  1. Set store base currency to USD.
  2. Enable WCPay Subscriptions.
  3. Enable multi-currency and a few different currencies with WCPay. I used CAD and AUD.
  4. Create a simple subscription with a sign-up fee. I used a $100 sign-up fee and $5/month billing interval.
  5. Add the sub to your cart. See the currency in USD on the cart page.
  6. Change it to CAD.
  7. See the correct sign-up fee on the line item price.
  8. See incorrect sign-up fee on the line item subtotal and all subsequent totals.

Screenshots

USD: OVgbKz.png

CAD: SRVV68.png

Expected behavior

I expected the sign-up fee to be properly converted.

Additional details

Identified in 5788077-zen.

Originally reported by @maxlaf in WC Subscriptions repo: 4441-gh-woocommerce/woocommerce-subscriptions

Coupons are also affected by this as they seem to apply to the price, not the subtotal. A 100% sign-up fee discount will only discount the converted price, not the incorrectly converted subtotal.

jZJRqR.png

System status ``` ### WordPress Environment ### WordPress address (URL): https://breakable-hawk.jurassic.ninja Site address (URL): https://breakable-hawk.jurassic.ninja WC Version: 7.1.1 REST API Version: ✔ 7.1.1 WC Blocks Version: ✔ 8.7.6 Action Scheduler Version: ✔ 3.4.0 Log Directory Writable: ✔ WP Version: 6.1.1 WP Multisite: – WP Memory Limit: 256 MB WP Debug Mode: ✔ WP Cron: ✔ Language: en_US External object cache: – ### Server Environment ### Server Info: Apache/2.4.54 (Unix) OpenSSL/1.0.2g PHP Version: 7.4.33 PHP Post Max Size: 1 GB PHP Time Limit: 30 PHP Max Input Vars: 5000 cURL Version: 7.47.0 OpenSSL/1.0.2g SUHOSIN Installed: – MySQL Version: 5.7.33-0ubuntu0.16.04.1-log Max Upload Size: 512 MB Default Timezone is UTC: ✔ fsockopen/cURL: ✔ SoapClient: ✔ DOMDocument: ✔ GZip: ✔ Multibyte String: ✔ Remote Post: ✔ Remote Get: ✔ ### Database ### WC Database Version: 7.1.1 WC Database Prefix: wp_ Total Database Size: 5.90MB Database Data Size: 4.41MB Database Index Size: 1.49MB wp_woocommerce_sessions: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_woocommerce_api_keys: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_woocommerce_attribute_taxonomies: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_woocommerce_downloadable_product_permissions: Data: 0.02MB + Index: 0.06MB + Engine InnoDB wp_woocommerce_order_items: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_woocommerce_order_itemmeta: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_woocommerce_tax_rates: Data: 0.02MB + Index: 0.06MB + Engine InnoDB wp_woocommerce_tax_rate_locations: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_woocommerce_shipping_zones: Data: 0.02MB + Index: 0.00MB + Engine InnoDB wp_woocommerce_shipping_zone_locations: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_woocommerce_shipping_zone_methods: Data: 0.02MB + Index: 0.00MB + Engine InnoDB wp_woocommerce_payment_tokens: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_woocommerce_payment_tokenmeta: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_woocommerce_log: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_actionscheduler_actions: Data: 0.02MB + Index: 0.11MB + Engine InnoDB wp_actionscheduler_claims: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_actionscheduler_groups: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_actionscheduler_logs: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_commentmeta: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_comments: Data: 0.02MB + Index: 0.09MB + Engine InnoDB wp_links: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_options: Data: 3.48MB + Index: 0.08MB + Engine InnoDB wp_postmeta: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_posts: Data: 0.02MB + Index: 0.06MB + Engine InnoDB wp_termmeta: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_terms: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_term_relationships: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_term_taxonomy: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_usermeta: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_users: Data: 0.02MB + Index: 0.05MB + Engine InnoDB wp_wc_admin_notes: Data: 0.05MB + Index: 0.00MB + Engine InnoDB wp_wc_admin_note_actions: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_wc_category_lookup: Data: 0.02MB + Index: 0.00MB + Engine InnoDB wp_wc_customer_lookup: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_wc_download_log: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_wc_order_coupon_lookup: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_wc_order_product_lookup: Data: 0.02MB + Index: 0.06MB + Engine InnoDB wp_wc_order_stats: Data: 0.02MB + Index: 0.05MB + Engine InnoDB wp_wc_order_tax_lookup: Data: 0.02MB + Index: 0.03MB + Engine InnoDB wp_wc_product_attributes_lookup: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_wc_product_download_directories: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_wc_product_meta_lookup: Data: 0.02MB + Index: 0.09MB + Engine InnoDB wp_wc_rate_limits: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_wc_reserved_stock: Data: 0.02MB + Index: 0.00MB + Engine InnoDB wp_wc_tax_rate_classes: Data: 0.02MB + Index: 0.02MB + Engine InnoDB wp_wc_webhooks: Data: 0.02MB + Index: 0.02MB + Engine InnoDB ### Post Type Counts ### attachment: 1 page: 7 post: 2 product: 1 shop_coupon: 1 ### Security ### Secure connection (HTTPS): ✔ Hide errors from visitors: ❌Error messages should not be shown to visitors. ### Active Plugins (6) ### Companion Plugin: by Osk – 1.28 Jetpack: by Automattic – 11.6 WooCommerce Payments Dev Tools: by Automattic – WooCommerce Payments: by Automattic – 5.1.2 WooCommerce Subscriptions: by WooCommerce – 4.7.0 WooCommerce: by Automattic – 7.1.1 ### Inactive Plugins (2) ### Akismet Anti-Spam: by Automattic – 5.0.1 Hello Dolly: by Matt Mullenweg – 1.7.2 ### Settings ### API Enabled: – Force SSL: – Currency: USD ($) Currency Position: left Thousand Separator: , Decimal Separator: . Number of Decimals: 2 Taxonomies: Product Types: external (external) grouped (grouped) simple (simple) subscription (subscription) variable (variable) variable subscription (variable-subscription) Taxonomies: Product Visibility: exclude-from-catalog (exclude-from-catalog) exclude-from-search (exclude-from-search) featured (featured) outofstock (outofstock) rated-1 (rated-1) rated-2 (rated-2) rated-3 (rated-3) rated-4 (rated-4) rated-5 (rated-5) Connected to WooCommerce.com: – Enforce Approved Product Download Directories: ✔ ### WC Pages ### Shop base: #5 - /shop/ Cart: #6 - /cart/ Checkout: #7 - /checkout/ My account: #8 - /my-account/ Terms and conditions: ❌ Page not set ### Theme ### Name: Storefront Version: 4.2.0 Author URL: https://woocommerce.com/ Child Theme: ❌ – If you are modifying WooCommerce on a parent theme that you did not build personally we recommend using a child theme. See: How to create a child theme WooCommerce Support: ✔ ### Templates ### Overrides: – ### Subscriptions ### WCS_DEBUG: ✔ No Subscriptions Mode: ✔ Live Subscriptions Live URL: https://breakable-hawk.jurassic.ninja Subscription Statuses: – WooCommerce Account Connected: ❌ No Report Cache Enabled: ✔ Yes Cache Update Failures: ✔ 0 failure ### Store Setup ### Country / State: United States (US) — California ### Payment Gateway Support ### WooCommerce Payments: products refunds multiple_subscriptions subscription_cancellation subscription_payment_method_change_admin subscription_payment_method_change_customer subscription_payment_method_change subscription_reactivation subscription_suspension subscriptions subscription_amount_changes subscription_date_changes tokenization add_payment_method ### Admin ### Enabled Features: activity-panels analytics coupons customer-effort-score-tracks experimental-products-task experimental-import-products-task experimental-fashion-sample-products shipping-smart-defaults shipping-setting-tour homescreen marketing multichannel-marketing mobile-app-banner navigation onboarding onboarding-tasks remote-inbox-notifications remote-free-extensions payment-gateway-suggestions shipping-label-banner subscriptions store-alerts transient-notices woo-mobile-welcome wc-pay-promotion wc-pay-welcome-page Disabled Features: minified-JavaScript new-product-management-experience settings Daily Cron: ✔ Next scheduled: 2022-12-14 23:16:12 +00:00 Options: ✔ Notes: 43 Onboarding: skipped ### WooCommerce Payments ### Version: 5.1.2 Connected to WPCOM: Yes Blog ID: 213355688 Account ID: acct_1MEhlxFyCZsaX6s4 ### Action Scheduler ### Complete: 7 Oldest: 2022-12-13 23:26:33 +0000 Newest: 2022-12-13 23:26:33 +0000 Pending: 1 Oldest: 2022-12-14 23:18:55 +0000 Newest: 2022-12-14 23:18:55 +0000 ### Status report information ### Generated at: 2022-12-14 00:01:18 +00:00 ```
RadoslavGeorgiev commented 1 year ago

@Automattic/sigma would you be able to handle this issue?

tpaksu commented 1 year ago

Thanks @haszari and @RadoslavGeorgiev for the ping, added to our backlog.

dwainm commented 1 year ago

Thanks @tpaksu , could you also add a rough estimate?

tpaksu commented 1 year ago

@jessepearson has solved something like this recently, I estimated it as 2, but it may be a 1 to him.

jessepearson commented 1 year ago

This is happening due to the filter priority change in this PR: https://github.com/Automattic/woocommerce-payments/pull/4939

Reverting the filter priority to something below 100 fixes the issue, but will likely cause issues that #4939 fixed. The higher priority causes a double conversion to happen during the process here: https://github.com/Automattic/woocommerce-subscriptions-core/blob/trunk/includes/class-wc-subscriptions-cart.php#L708-L713

jessepearson commented 1 year ago

It looks like Product Bundles uses a filter priority of 98: https://github.com/woocommerce/woocommerce-product-bundles/blob/master/includes/class-wc-pb-product-prices.php#L58-L64

And Subscriptions core uses a priority of 100: https://github.com/Automattic/woocommerce-subscriptions-core/blob/trunk/includes/class-wc-subscriptions-cart.php#L708-L713

Setting our priority to 99 appears to fix compatibility with both extensions, however, leaves us very little wiggle room if we need to adjust again.

haszari commented 1 year ago

Thanks @jessepearson – good analysis. Is there anything you can think of to make this more robust? Maybe architects team have some ideas – fyi @jrodger

jessepearson commented 1 year ago

The main issue with trying to convert prices is that we rely on filters run when $product->get_price() (or something similar) is called. That method is called probably close to 15 times if there's one subscription product with a signup fee in the cart, let alone if other products are involved. We have to try to figure out which calls should be filtered and which should be ignored. Changing the filter priority is the easiest, and probably most solid fix at this point.

dan-roboticarm commented 1 year ago

Hi, just wondering if this was still being worked on. Seems there was a potential fix some time ago. Looking forward to it. Thanks.

jessepearson commented 1 year ago

@dan-roboticarm Yes, it's still being worked on. There was a potential fix, however, other issues arose and need to be fixed, as well.

nicdwilson commented 1 year ago

6059878-zen. Shop base currency is USD. Signup fee sets correctly for currencies such as EUR, CAD, and NZD where multiples are low, but for IRP or JPY the value varies dramatically.

b7CLMK.png

W2Zz8L.png

jKfkEV.png

csmcneill commented 1 year ago

6070592-zen

With my findings, a product with a $16 AUD sign up fee is initially converted to $8 USD when it should be $12 USD. Modifying the cart to force a refresh changes the price to $16 USD even though it should still be $12 USD.

maxlaf commented 1 year ago

6240945-zen

jessepearson commented 1 year ago

Currently looking into this.

Since the PR has been sitting for so long, I need to go through each of the testing steps to confirm everything is testing as it should be. I am presently partially through the list, and have also added additional testing to include grouped subscription switching, as mentioned in the PR. I will fix issues as I come across them and set the PR to be reviewed again.

I am hoping to get through everything by the end of tomorrow, but that depends on what comes up during the day.

jessepearson commented 1 year ago

Noting that we were informed to do nothing but tickets to assist support: p1683186380128409-slack-C01B8KNUYSW My team is also on meetup next week, and I have a couple of days of AFK after that. I may not be able to return to working on this until after that AFK on May 17.