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

Multiple tax rates with tax inclusive pricing returning 1 cent off totals #23917

Closed
3 tasks done
jessepearson opened this issue Jun 12, 2019 · 3 comments · Fixed by #24024
Closed
3 tasks done

Multiple tax rates with tax inclusive pricing returning 1 cent off totals #23917

jessepearson opened this issue Jun 12, 2019 · 3 comments · Fixed by #24024
Assignees
Labels
type: bug The issue is a confirmed bug.

Comments

@jessepearson
Copy link

jessepearson commented Jun 12, 2019

Describe the bug
This is a question, but may turn into a bug.

If two tax rates are used, along with tax inclusive pricing, the total for products will become incorrect in the cart.

Related tickets
2080235-zen

To Reproduce
Steps to reproduce the behavior:

  1. Set two tax rates at 9%, one with priority of 1, the other priority of 2
  2. Create two products, one with a cost of 599.00, and the other 4.50
  3. Add the 599 product to the cart, the total is 599.00
  4. Remove the 599 product and add the 4.50 to the cart, total is 4.49

Screenshots

Image Link: https://cld.wthms.co/4BsntR


Image Link: https://cld.wthms.co/IJyV67


Image Link: https://cld.wthms.co/BDHgKZ

Expected behavior
I would expect that the total would be the same as the product prices for both instances

Isolating the problem (mark completed items with an [x]):

  • I have deactivated other plugins and confirmed this bug occurs when only WooCommerce plugin is active.
  • This bug happens with a default WordPress theme active, or Storefront.
  • I can reproduce this bug consistently using the steps above.

WordPress Environment

``` ` ### WordPress Environment ###

WordPress address (URL): http://clean.test
Site address (URL): http://clean.test
WC Version: 3.6.4
Log Directory Writable: ✔
WP Version: 5.2.1
WP Multisite: –
WP Memory Limit: 2 GB
WP Debug Mode: ✔
WP Cron: ✔
Language: en_US
External object cache: –

Server Environment

Server Info: nginx/1.15.2
PHP Version: 7.2.13
PHP Post Max Size: 128 MB
PHP Time Limit: 300
PHP Max Input Vars: 2500
cURL Version: 7.63.0
OpenSSL/1.0.2q

SUHOSIN Installed: –
MySQL Version: 5.7.21
Max Upload Size: 128 MB
Default Timezone is UTC: ✔
fsockopen/cURL: ✔
SoapClient: ✔
DOMDocument: ✔
GZip: ✔
Multibyte String: ✔
Remote Post: ✔
Remote Get: ✔

Database

WC Database Version: 3.6.4
WC Database Prefix: wp_
MaxMind GeoIP Database: ✔
Total Database Size: 5.38MB
Database Data Size: 3.78MB
Database Index Size: 1.60MB
wp_woocommerce_sessions: Data: 0.03MB + Index: 0.02MB
wp_woocommerce_api_keys: Data: 0.02MB + Index: 0.03MB
wp_woocommerce_attribute_taxonomies: Data: 0.02MB + Index: 0.02MB
wp_woocommerce_downloadable_product_permissions: Data: 0.02MB + Index: 0.05MB
wp_woocommerce_order_items: Data: 0.02MB + Index: 0.02MB
wp_woocommerce_order_itemmeta: Data: 0.02MB + Index: 0.03MB
wp_woocommerce_tax_rates: Data: 0.02MB + Index: 0.06MB
wp_woocommerce_tax_rate_locations: Data: 0.02MB + Index: 0.03MB
wp_woocommerce_shipping_zones: Data: 0.02MB + Index: 0.00MB
wp_woocommerce_shipping_zone_locations: Data: 0.02MB + Index: 0.03MB
wp_woocommerce_shipping_zone_methods: Data: 0.02MB + Index: 0.00MB
wp_woocommerce_payment_tokens: Data: 0.02MB + Index: 0.02MB
wp_woocommerce_payment_tokenmeta: Data: 0.02MB + Index: 0.03MB
wp_woocommerce_log: Data: 0.02MB + Index: 0.02MB
wp_commentmeta: Data: 0.02MB + Index: 0.03MB
wp_comments: Data: 0.06MB + Index: 0.09MB
wp_failed_jobs: Data: 0.02MB + Index: 0.00MB
wp_links: Data: 0.02MB + Index: 0.02MB
wp_mailchimp_carts: Data: 0.02MB + Index: 0.00MB
wp_options: Data: 2.02MB + Index: 0.05MB
wp_postmeta: Data: 0.34MB + Index: 0.36MB
wp_posts: Data: 0.14MB + Index: 0.06MB
wp_queue: Data: 0.02MB + Index: 0.00MB
wp_termmeta: Data: 0.02MB + Index: 0.03MB
wp_terms: Data: 0.02MB + Index: 0.03MB
wp_term_relationships: Data: 0.02MB + Index: 0.02MB
wp_term_taxonomy: Data: 0.02MB + Index: 0.03MB
wp_usermeta: Data: 0.02MB + Index: 0.03MB
wp_users: Data: 0.02MB + Index: 0.05MB
wp_wcpv_commissions: Data: 0.02MB + Index: 0.00MB
wp_wcpv_per_product_shipping_rules: Data: 0.02MB + Index: 0.00MB
wp_wc_admin_notes: Data: 0.02MB + Index: 0.00MB
wp_wc_admin_note_actions: Data: 0.02MB + Index: 0.02MB
wp_wc_bookings_availability: Data: 0.02MB + Index: 0.02MB
wp_wc_bookings_availabilitymeta: Data: 0.02MB + Index: 0.03MB
wp_wc_booking_relationships: Data: 0.02MB + Index: 0.03MB
wp_wc_customer_lookup: Data: 0.02MB + Index: 0.03MB
wp_wc_download_log: Data: 0.02MB + Index: 0.03MB
wp_wc_order_coupon_lookup: Data: 0.02MB + Index: 0.03MB
wp_wc_order_product_lookup: Data: 0.02MB + Index: 0.06MB
wp_wc_order_stats: Data: 0.02MB + Index: 0.05MB
wp_wc_order_tax_lookup: Data: 0.02MB + Index: 0.03MB
wp_wc_product_meta_lookup: Data: 0.02MB + Index: 0.09MB
wp_wc_webhooks: Data: 0.02MB + Index: 0.02MB
wp_wpml_mails: Data: 0.41MB + Index: 0.00MB

Post Type Counts

attachment: 2
bookable_person: 13
bookable_resource: 15
page: 6
post: 2
product: 46
product_variation: 12
revision: 1
scheduled-action: 58
shop_order: 2
user_request: 3
wc_booking: 151

Security

Secure connection (HTTPS): ❌
Your store is not using HTTPS. Learn more about HTTPS and SSL Certificates.
Hide errors from visitors: ✔

Active Plugins (1)

WooCommerce: by Automattic – 3.6.4

Settings

API Enabled: –
Force SSL: –
Currency: USD ($)
Currency Position: left
Thousand Separator: ,
Decimal Separator: .
Number of Decimals: 2
Taxonomies: Product Types: booking (booking)
external (external)
grouped (grouped)
simple (simple)
variable (variable)

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: ✔

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: 2.4.3 – 2.5.0 is available
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: –

Action Scheduler

Complete: 57
Oldest: 2019-06-04 07:55:36 -0400
Newest: 2019-06-12 10:50:00 -0400

Pending: 0
Oldest: –
Newest: –

Canceled: 1
Oldest: 2019-06-12 11:50:00 -0400
Newest: 2019-06-12 11:50:00 -0400

In-progress: 0
Oldest: –
Newest: –

Failed: 0
Oldest: –
Newest: –

`

</details>
@jessepearson jessepearson added type: bug The issue is a confirmed bug. question labels Jun 12, 2019
@jessepearson
Copy link
Author

Forgot to mention things I did:

  • Tried changing Round tax at subtotal level, instead of rounding per line setting, but no change.
  • Tried setting WC_ROUNDING_PRECISION value different, but no change.

@vedanshujain vedanshujain self-assigned this Jun 13, 2019
@vedanshujain
Copy link
Contributor

This is happening because of rounding off errors. There is a workaround by setting Number of decimals to atleast 3 in /wp-admin/admin.php?page=wc-settings&tab=generalt/wp-admin/admin.php?page=wc-settings&tab=general

Another workaround would be to use slightly different price, like 598.

Keep on reading if you'd like to know the root cause.

For items where taxes are included in prices, we calculate tax based on the price and then subtract it from price to get the pre-tax cost. This calculation is needed to add line items in final order screen with separate tax lines.

Unfortunately, since we usually display amounts up to two decimal points, inconsistencies that are added through rounding cannot be prevented in some cases.

If we accurately calculate the cost everywhere and only round when displaying, there will be inconsistencies between the displayed tax lines, and actual tax values used for computation.

For egs, if for a product:

Price including tax: 599.00 and tax are applied in two slabs of

then,

Cost: 507.6271186
Tax 1: 45.68644068
Tax 2: 45.68644068
Total: 598.99999996 ~= 599.00
Error: 0

Displayed Cost: 507.63
Displayed Tax 1: 45.69
Displayed Tax 2: 45.69
Total: 599.01
Error: -0.01

So, in cases like these, we have to pick either using the displayed rounded values to calculate total cost, which will be slightly inaccurate. Or we have to pick un-rounded values for calculation, which will be accurate, but inconsistent from what we will display to users. As you can guess, currently we pick rounded values for calculation which can be slightly inaccurate at times.

If we would have picked the un-rounded values instead, this is how it would have looked (total is correct, but tax amount etc don't add up)
Screenshot 2019-06-17 at 10 13 57 PM

I was planning to add a commit to pick the un-rounded values when rounding at line subtotal setting is enabled, but stopped because I am not yet sure if that is the correct solution.

Checkout this sheet to see what would be the probable errors in calculations when prices are inclusive of taxes
Woo Dummy Tax.xlsx

@vedanshujain
Copy link
Contributor

I thought about this again, and I would think that displaying slightly inaccurate tax break up is better than displaying slightly inaccurate total. So there will be cases, like in screenshot in above comment, where subtotal+taxes with slightly different that displayed total, but displayed total will be accurate. IMO its better UX then displaying weird total .

What do you think? @woocommerce/proton ?

vedanshujain added a commit that referenced this issue Jun 27, 2019
We were earlier rounding different tax rate values while they are merged, even if rounding at subtotal setting is enabled. This increases the rounding error, especially when prices are inclusive of taxes, and thus there is a chance that the total will be slightly different from when add the original values. For egs: #23917 .

This commit changes this behavior to round *after* we have summed all the precise unround values. Similar for items prices, we now round as late as possible, if rounding at subtotal is enabled.
vedanshujain added a commit that referenced this issue Jun 27, 2019
We were earlier rounding different tax rate values while they are merged, even if rounding at subtotal setting is enabled. This increases the rounding error, especially when prices are inclusive of taxes, and thus there is a chance that the total will be slightly different from when add the original values. For egs: #23917 .

This commit changes this behavior to round *after* we have summed all the precise unround values. Similar for items prices, we now round as late as possible, if rounding at subtotal is enabled.
vedanshujain added a commit that referenced this issue Jul 4, 2019
We were earlier rounding different tax rate values while they are merged, even if rounding at subtotal setting is enabled. This increases the rounding error, especially when prices are inclusive of taxes, and thus there is a chance that the total will be slightly different from when add the original values. For egs: #23917 .

This commit changes this behavior to round *after* we have summed all the precise unround values. Similar for items prices, we now round as late as possible, if rounding at subtotal is enabled.
vedanshujain added a commit that referenced this issue Jul 4, 2019
We were earlier rounding different tax rate values while they are merged, even if rounding at subtotal setting is enabled. This increases the rounding error, especially when prices are inclusive of taxes, and thus there is a chance that the total will be slightly different from when add the original values. For egs: #23917 .

This commit changes this behavior to round *after* we have summed all the precise unround values. Similar for items prices, we now round as late as possible, if rounding at subtotal is enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants