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

getTotalWeightVolume uses wrong scale #11861

Closed
simicar29 opened this issue Sep 11, 2019 · 5 comments
Closed

getTotalWeightVolume uses wrong scale #11861

simicar29 opened this issue Sep 11, 2019 · 5 comments
Labels
Bug This is a bug (something does not work as expected)

Comments

@simicar29
Copy link
Contributor

Bug

Function getTotalWeightVolume from core/class/commonobject.class.php uses unit index instead of dictionary 'scale' to convert values

Environment

  • Version: 10.01
  • OS: Centos 7.6
  • Web server: nging 1.17.3
  • PHP: 7.3.9
  • Database: maraiadb 5.5.64
  • URL(s): comm/propal/card.php

Expected and actual behavior

Total weight and volume should be computed and converted from defined to reference unit.
Aberrant values are returned by the implemented function

Steps to reproduce the behavior

Add product with weight/volume and units defined to proposals => displayed total is aberrant.

Proposed resolution : modify lines in function

@@ -3684,24 +3691,24 @@ abstract class CommonObject

                    $weightUnit=0;
                    $volumeUnit=0;
  •                   if (! empty($weight_units)) $weightUnit = $weight_units;
    
  •                   if (! empty($volume_units)) $volumeUnit = $volume_units;
    
  •                   if (! empty($weight_units)) $weightUnit = getDictvalue(MAIN_DB_PREFIX.'c_units', 'scale', $weight_units);
    
  •                   if (! empty($volume_units)) $volumeUnit = getDictvalue(MAIN_DB_PREFIX.'c_units', 'scale', $volume_units);
    
                      if (empty($totalWeight)) $totalWeight=0;  // Avoid warning because $totalWeight is ''
                      if (empty($totalVolume)) $totalVolume=0;  // Avoid warning because $totalVolume is ''
    
                      //var_dump($line->volume_units);
    
  •                   if ($weight_units < 50)   // >50 means a standard unit (power of 10 of official unit), > 50 means an exotic unit (like 
    
  •                   if ($weightUnit < 50)   // >50 means a standard unit (power of 10 of official unit), > 50 means an exotic unit (like in
                      {
                              $trueWeightUnit=pow(10, $weightUnit);
                              $totalWeight += $weight * $qty * $trueWeightUnit;
                      }
                      else {
    
  •           if ($weight_units == 99) {
    
  •           if ($weightUnit == 99) {
                      // conversion 1 Pound = 0.45359237 KG
                      $trueWeightUnit = 0.45359237;
                      $totalWeight += $weight * $qty * $trueWeightUnit;
    
  •           } elseif ($weight_units == 98) {
    
  •           } elseif ($weightUnit == 98) {
                      // conversion 1 Ounce = 0.0283495 KG
                      $trueWeightUnit = 0.0283495;
                      $totalWeight += $weight * $qty * $trueWeightUnit;
    

@@ -3709,7 +3716,7 @@ abstract class CommonObject
else
$totalWeight += $weight * $qty; // This may be wrong if we mix different units
}

  •                   if ($volume_units < 50)   // >50 means a standard unit (power of 10 of official unit), > 50 means an exotic unit (like 
    
  •                   if ($volumeUnit < 50)   // >50 means a standard unit (power of 10 of official unit), > 50 means an exotic unit (like in
                      {
                              //print $line->volume."x".$line->volume_units."x".($line->volume_units < 50)."x".$volumeUnit;
                              $trueVolumeUnit=pow(10, $volumeUnit);
    
@simicar29 simicar29 added the Bug This is a bug (something does not work as expected) label Sep 11, 2019
@tiaris
Copy link
Contributor

tiaris commented Sep 24, 2019

By introducing the c_units table and class, we create big issues in weight_calculation
product.weigh_unit (and all *.weight_unit db fields) will now have the rowid of unit in c_unit, instead of the scale value.
But for conversion we must use the c_units.scale value.
Actually, all weight calculations and conversions are false because based on rowids instead of scale.

Fetch methods of class core/class/cunits.class.php never fetch the scale field, but conversion functions still need this info
in commonobject
line 3690 if ($weight_units < 50 ... weight_unit should be the scale value but you wil have the rowid
same in function getTotalWeightVolume()

Also there can be big migration problems because until dolibarr 9 you will have on a product weight = 100 weight_unit = -3 for a product of 100g How do you convert this in the new system ?

@simicar29
Copy link
Contributor Author

On the proposal card, you have a computed volume based on product line volumes.

ComputedVolume

This is the role of the getTotalWeightVolume function. I am sorry the copy paste of my git diff resulted in a crappy report... In short, this function will convert values with differing units (say liters of milliliters) to a reference unit (cubic meters). The problem is that it is not using the right parameter for the scaling (it uses the unit index, which may have previously been chosen to reflect this scale, but this is presently not the case and the dictionary defines a scale value which should be used).
I observed the issue on volumes, but it is probably true for weights as well.

This has been reported in the forum as well : https://www.dolibarr.org/forum/5-bugs-on-cvs-or-demo-version/29558-severe-problem-with-calculated-weight-and-volume

@eldy
Copy link
Member

eldy commented Sep 27, 2019

OK. The field is visible only when there is a weight > 0

@simicar29
Copy link
Contributor Author

Thank you, that was impressively fast !

@dolibarr95
Copy link
Contributor

Hi !
I've applied the fix in my Dolibarr 10.0.1 but still have the bug :

  • Product "Belt" weight:71 grams
  • Order 4 Belts Calculated weight:2 840 kilogram

Tks for your help

French forum :
https://www.dolibarr.fr/forum/527-bugs-sur-la-version-stable-courante/65206-dolibarr-v-10-0-1-poids-calcule-des-commandes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug (something does not work as expected)
Projects
None yet
Development

No branches or pull requests

4 participants