zencart / zencart

Zen Cart® is a full-function e-commerce application for your website.
https://github.com/zencart/zencart/releases
Other
375 stars 233 forks source link

Currency conversions result in incorrect values #6077

Open lat9 opened 10 months ago

lat9 commented 10 months ago

Using master branch as of 2023-12-09 7:56PM Eastern Time, with the demo products, running PHP 8.2.

No logs are generated, just that incorrect

lat9 commented 10 months ago

Update: It's not just the quantity of four. Using the same settings for the original post and dropping the product quantity to 1, with a US$5.00 flat-rate shipping (£4.18, using the same conversion ratio), the order's total calculation is off by a penny as displayed on the checkout_payment page: image

brittainmark commented 10 months ago

Add 4 of these to the cart; the cart total displays as £133.58 instead of £133.60.

This looks like the conversion is done on the total USD cost 39.99 4 0.83509400 = 133.58 same thing for the total. £37.5708

So technically the figures are correct, even though they do not add up.

Rather than straight rounding, you could use PHP_ROUND_UP when you round to always round to the next penny. This would work for the total, but could cause a penny plus on the total. Additionally, you could display the unit price to more decimal places so that the item price is correct. Finally, you could add a note to explain that currency conversion may cause rounding errors.

OR

Have to change how the calculation is done. i.e. convert unit price and round, then apply multiples. For the total, each figure would have to be rounded before they are summed and not use the total from orders_totaltable

OR

Display all the items in their default currency and only round the total with PHP_ROUND_UP set so you don't lose a cent.

I expect you have already worked all this out. Just wanted to clarify.

lat9 commented 10 months ago

Yep, I saw that processing. What I'm grappling with is the possibility that there are stores that have products that are unit-priced at less than 0.01; for example, a hardware store that sells nails at 0.0625 per nail with a minimum of 100. For that pricing, you'd want qty * price to be 'currency-converted' and then tax applied.

P.S. Since we're dealing with store's profit and loss, the calculations are not technically correct since they neither make sense nor do they add up!

brittainmark commented 10 months ago

That been the case. Should you stick with the store base currency and just display the total converted to the guest currency?

lat9 commented 10 months ago

That been the case. Should you stick with the store base currency and just display the total converted to the guest currency?

Could you elaborate on your comment? I don't understand the suggestion.

brittainmark commented 10 months ago

Display all the figures in the base currency USD. an just display an extra line of field displaying the final total in GBP. image

This way the accounts are in the local currency and the only change is for the currency conversion on the total. Don't know if that is a good idea or not.

lat9 commented 8 months ago

TL;DR

This issue has its basis in:

  1. Various pricing elements for a site, whether products or orders, are stored in the database in the site's default currency.
  2. The shopping_cart class rounds prices stored in its totals based on the currently-selected currency.

I was able to coerce the shopping_cart page's products' table to reflect the correct total by changing this section of the page's header_php.php

https://github.com/zencart/zencart/blob/832a82036cb0a0b66bf6d59ae1687f93c199ed93/includes/modules/pages/shopping_cart/header_php.php#L116-L123

to


    $tax_rate = zen_get_tax_rate($products[$i]['tax_class_id']);

    // $ppe is product price each, before one-time charges added
    $ppe = $products[$i]['final_price'];
    $ppe = zen_round(zen_add_tax($ppe, $tax_rate), $currencies->get_decimal_places($_SESSION['currency']));

    // -----
    // If pricing is currently being displayed in a currency *other than* the site's
    // default, convert the per-item pricing now so that any quantity > 1 will be
    // multiplying based on that currency-converted value.
    //
    if ($_SESSION['currency'] !== DEFAULT_CURRENCY) {
        $ppe = $currencies->value($ppe);
    }

    // $ppt is product price total, before one-time charges added
    $ppt = $ppe * $products[$i]['quantity'];

    // -----
    // NOTE: Telling the currencies' formatter not to convert to the current currency
    // in use, since any conversion was performed when determining the product's
    // per-item price.
    //
    $productsPriceEach = $currencies->format($ppe, false) . ($products[$i]['onetime_charges'] != 0 ? '<br>' . $currencies->display_price($products[$i]['onetime_charges'], $tax_rate, 1) : '');
    $productsPriceTotal = $currencies->format($ppt, false) . ($products[$i]['onetime_charges'] != 0 ? '<br>' . $currencies->display_price($products[$i]['onetime_charges'], $tax_rate, 1) : '');

That's only a small part of the equation, though, as the shopping-cart class' total calculation is still off.

The products' total summation (i.e. an order's subtotal) are performed using the site's default currency and once a product's default-currency price is multiplied by the in-cart quantity, the rounding that occurs in that multiplication leads to the errant value for the cart's selected currency.

So far, I've found the following modules to require change for these calculations:

  1. /includes/classes/order.php
  2. /includes/classes/shopping_cart.php
  3. /includes/modules/pages/shopping_cart/header_php.php
  4. /includes/templates/*/templates/tpl_account_history_info_default.php (since, for whatever reason, those calculations are left to the templating rather than being performed in the page's header_php.php)
  5. /includes/template/*/templates/tpl_checkout_confirmation_default.php (ditto)
  6. /admin/orders.php

That said, the correction for this issue will be extremely complicated since it requires changes for the templating as well.