Problem with the price2num() function

[catrielr]

Bug

Problem with the price2num() function

Lang configuration (ES_ar)
SeparatorDecimal=,
SeparatorThousand=.

Scale barcode:
2002100002753

Takepos barcode ruler
other:2+ref:4+qu:3+qd:3+other:1

The problem is here:
in the price2mun() function:

			if ($option == 2 && $thousand == '.' && preg_match('/\.(\d\d\d)$/', (string) $amount)) {	// It means the . is used as a thousand separator and string come from input data, so 1.123 is 1123
			$amount = str_replace($thousand, '', $amount);
		}

$amount = 0.275 (this comes from takepos ajax)
$options = 2
$thousand = "."

When it goes through here, it converts $amount to "0275" and then ends up being an integer of 275

Dolibarr Version

No response

Environment PHP

No response

Environment Database

No response

Steps to reproduce the behavior and expected behavior

No response

Attached files

No response

[mdeweerd]

Apparently you already had a look at the code.
I would think that the problem is with the value provided to price2num to be converted using the option set to 2.

I assume that only the _formated price values should be converted using localisation and in that case, this should be the amount used when a localised value is needed (to later use in price2num(..,..,2).

dolibarr/htdocs/takepos/ajax/ajax.php

Lines 375 to 389 in bdd8dc0

	$row = array(
	'rowid' => $obj->rowid,
	'ref' => $obj->ref,
	'label' => $obj->label,
	'tosell' => $obj->tosell,
	'tobuy' => $obj->tobuy,
	'barcode' => $obj->barcode,
	'price' => empty($objProd->multiprices[$pricelevel]) ? $obj->price : $objProd->multiprices[$pricelevel],
	'price_ttc' => empty($objProd->multiprices_ttc[$pricelevel]) ? $obj->price_ttc : $objProd->multiprices_ttc[$pricelevel],
	'object' => 'product',
	'img' => $ig,
	'qty' => 1,
	'price_formated' => price(price2num(empty($objProd->multiprices[$pricelevel]) ? $obj->price : $objProd->multiprices[$pricelevel], 'MT'), 1, $langs, 1, -1, -1, $conf->currency),
	'price_ttc_formated' => price(price2num(empty($objProd->multiprices_ttc[$pricelevel]) ? $obj->price_ttc : $objProd->multiprices_ttc[$pricelevel], 'MT'), 1, $langs, 1, -1, -1, $conf->currency)
	);

[catrielr]

@mdeweerd
The problem isn't with the price, but with the quantity.
The barcode contains the REF and the QTY of the product (it's a weighable product). In this case, the quantity is 0.275

In ajax.php, the barcode is decomposed according to the barcode rules already established in the configuration.

						$qty = 1;
						if (isset($barcode_value_list['qu'])) {
							$qty_str = $barcode_value_list['qu'];
							if (isset($barcode_value_list['qd'])) {
								$qty_str .= '.' . $barcode_value_list['qd'];
							}
							$qty = (float) $qty_str;
						}

The ajax request returns $row[0]["qty"]= 0.275, so far so good, then in invoice.php:

$qty = GETPOSTISSET('qty') ? GETPOSTFLOAT('qty') : 1;

(line 714 dolibarr 22.0.2)

And this is where the problem with the regional settings I described earlier begins:

function GETPOSTFLOAT($paramname, $rounding = '')
{
	// price2num() is used to sanitize any valid user input (such as "1 234.5", "1 234,5", "1'234,5", "1·234,5", "1,234.5", etc.)
	return (float) price2num(GETPOST($paramname), $rounding, 2);
}

[mdeweerd]

Ok, so this is an editable field which means it can be user input or it can be machine input in this specific case.
Both are interpreted as user input (when submitted to invoice.php) and user input is supposed to be in the locale.
So I believe that one approach is to add a "qty_formatted" to the fields in ajax.php and then use this localized field here:

dolibarr/htdocs/takepos/index.php

Line 838 in bdd8dc0

ClickProduct(0, data[0]['qty']);

Relevant code where qty is "used":

dolibarr/htdocs/takepos/index.php

Line 560 in bdd8dc0

$("#poslines").load("invoice.php?action=addline&token=<?php echo newToken(); ?>&place="+place+"&idproduct="+idproduct+"&qty="+qty+"&invoiceid="+invoiceid, function() {

dolibarr/htdocs/takepos/index.php

Line 1318 in bdd8dc0

<button type="button" id="qty" class="calcbutton2" onclick="Edit('qty')"><?php echo $langs->trans("Qty"); ?></button>

[catrielr]

For me, the solution is here:

replace in price2num function:

from:

		if ($option == 2 && $thousand == '.' && preg_match('/\.(\d\d\d)$/', (string) $amount)) {	// It means the . is used as a thousand separator and string come from input data, so 1.123 is 1123
			$amount = str_replace($thousand, '', $amount);
		}

To:

// Avoid interpreting 0.xxx as thousands (e.g., 0.275 should be decimal).
if (
    $option == 2 &&
    $thousand == '.' &&
    preg_match('/\.(\d\d\d)$/', (string) $amount) &&
    !preg_match('/^0\.\d+$/', (string) $amount)   // <-- added exception
) {
    $amount = str_replace($thousand, '', $amount);
}

Not only for takepos but for any value that needs to be formatted as price2num

[mdeweerd]

The '$option' does not need to be '2', so when it is '2', this means that the input is localised.
The issue would still appear when the floating number is 10.230 for instance because it would become 10230 which is still wrong.

As invoice.php is expecting a localized number in qty because of GETPOSTFLOAT(), which is valid here because it is a user field.
So qty should be localized. By getting a localized value from ajax.php the value is available to use in the line mentioned earlier.

I could not see an automation test case for this, so while I could update the code, I am not in a position to test it.

I would just add 'qty_formatted'=>price(price2num($qty, 'MS')), in the $row assignment near line 267 of ajax.php and change ClickProduct(0, data[0]['qty']); into ClickProduct(0, data[0]['qty_formatted']);

[eldy]

For me, the solution is here:

replace in price2num function:

from:

		if ($option == 2 && $thousand == '.' && preg_match('/\.(\d\d\d)$/', (string) $amount)) {	// It means the . is used as a thousand separator and string come from input data, so 1.123 is 1123
			$amount = str_replace($thousand, '', $amount);
		}

To:

// Avoid interpreting 0.xxx as thousands (e.g., 0.275 should be decimal).
if (
    $option == 2 &&
    $thousand == '.' &&
    preg_match('/\.(\d\d\d)$/', (string) $amount) &&
    !preg_match('/^0\.\d+$/', (string) $amount)   // <-- added exception
) {
    $amount = str_replace($thousand, '', $amount);
}

Not only for takepos but for any value that needs to be formatted as price2num

I confirm the analysis of mdeweerd.
The pb is not the price2num.
But the code use GETPOSTFLOAT that use price2num with param 2 on qty. Param 2 means "the value is in local end user format, thanks to convert it into international format". But here value is received so already into international format. So convertion is done when it should not (so done twice)
And solution too.
When using the screen, the qty is in user local format.
When using the barcode, the qty is in international format.
And the code that is using the data expect the data to be in user local format.
So solution can be to provide the 2 values in 2 different parameters (one in local format, the other in int format) like suggested by @mdeweerd but this means we will use qty_formated and qty and there is a risk to mix them when all the code suppose that value from form and retreived with GETPOST are user data even if name is just qty.
So I recommend instead another solution: adding a parameter when data is coming from barcode reader to say "hey, qty is already in international format, don't use GETPOSTFLOAT with param option user input=2 but use it with param option 1 for no user input".
i have modified the GETPOSTFLOAT to be able to manage this param in the 2 ways (See 54f767f )

[catrielr]

I know this isn't a serious problem, but every time I use "," as a decimal separator, I have a problem with qty, which interprets it as a whole number.

[mdeweerd]

@catrielr Please provide the use case, dolibarr version and locale (country/language) in which you observe this. The above discussion was mainly related to a qty embedded in the barcode.