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

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

Closed
haszari opened this issue Dec 14, 2022 · 15 comments · Fixed by #5489
Closed

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

haszari opened this issue Dec 14, 2022 · 15 comments · Fixed by #5489
Assignees
Labels
category: core WC Payments core related issues, where it’s obvious. component: customer multi-currency Issues related to customer multi-currency project component: wc subscriptions integration Issues affecting subscriptions with WC Subscriptions plugin active. component: wcpay subscriptions Issues related to Stripe Billing Subscriptions priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue is a confirmed bug.

Comments

@haszari
Copy link
Contributor

haszari commented Dec 14, 2022

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
@haszari haszari added type: bug The issue is a confirmed bug. component: customer multi-currency Issues related to customer multi-currency project component: wcpay subscriptions Issues related to Stripe Billing Subscriptions component: wc subscriptions integration Issues affecting subscriptions with WC Subscriptions plugin active. priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. labels Dec 14, 2022
@jessy-p jessy-p added the category: core WC Payments core related issues, where it’s obvious. label Dec 29, 2022
@RadoslavGeorgiev
Copy link
Contributor

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

@tpaksu
Copy link
Contributor

tpaksu commented Jan 24, 2023

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

@dwainm
Copy link
Contributor

dwainm commented Jan 26, 2023

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

@tpaksu
Copy link
Contributor

tpaksu commented Jan 26, 2023

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

@jessepearson jessepearson self-assigned this Jan 30, 2023
@jessepearson
Copy link
Contributor

This is happening due to the filter priority change in this PR:
#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
Copy link
Contributor

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
Copy link
Contributor Author

haszari commented Jan 30, 2023

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

csmcneill commented Apr 30, 2023

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
Copy link
Member

maxlaf commented May 1, 2023

6240945-zen

@jessepearson
Copy link
Contributor

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: core WC Payments core related issues, where it’s obvious. component: customer multi-currency Issues related to customer multi-currency project component: wc subscriptions integration Issues affecting subscriptions with WC Subscriptions plugin active. component: wcpay subscriptions Issues related to Stripe Billing Subscriptions priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue is a confirmed bug.
Projects
None yet
10 participants