From c9b569ad1a1eaf52bc3eefb85a82b34309c9beb2 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sat, 21 Mar 2015 15:31:11 +0100 Subject: [PATCH] Fix calculation of PMP of product. Also add phpunit test to avoid this in future. --- htdocs/product/class/product.class.php | 33 ++- .../stock/class/mouvementstock.class.php | 65 +++-- test/phpunit/AllTests.php | 11 +- test/phpunit/MouvementStockTest.php | 255 ++++++++++++++++++ 4 files changed, 332 insertions(+), 32 deletions(-) create mode 100644 test/phpunit/MouvementStockTest.php diff --git a/htdocs/product/class/product.class.php b/htdocs/product/class/product.class.php index 3f66a5f51fd..31f652a18ab 100755 --- a/htdocs/product/class/product.class.php +++ b/htdocs/product/class/product.class.php @@ -673,7 +673,7 @@ class Product extends CommonObject $sql.= ", desiredstock = " . ((isset($this->desiredstock) && $this->desiredstock != '') ? $this->desiredstock : "null"); $sql.= " WHERE rowid = " . $id; - dol_syslog(get_class($this)."update", LOG_DEBUG); + dol_syslog(get_class($this)."::update", LOG_DEBUG); $resql=$this->db->query($sql); if ($resql) @@ -916,16 +916,16 @@ class Product extends CommonObject foreach ($langs_available as $key => $value) { - $sql = "SELECT rowid"; - $sql.= " FROM ".MAIN_DB_PREFIX."product_lang"; - $sql.= " WHERE fk_product=".$this->id; - $sql.= " AND lang='".$key."'"; - - $result = $this->db->query($sql); - if ($key == $current_lang) { - if ($this->db->num_rows($result)) // si aucune ligne dans la base + $sql = "SELECT rowid"; + $sql.= " FROM ".MAIN_DB_PREFIX."product_lang"; + $sql.= " WHERE fk_product=".$this->id; + $sql.= " AND lang='".$key."'"; + + $result = $this->db->query($sql); + + if ($this->db->num_rows($result)) // if there is already a description line for this language { $sql2 = "UPDATE ".MAIN_DB_PREFIX."product_lang"; $sql2.= " SET label='".$this->db->escape($this->libelle)."',"; @@ -947,9 +947,16 @@ class Product extends CommonObject return -1; } } - else if (isset($this->multilangs["$key"])) + else if (isset($this->multilangs[$key])) { - if ($this->db->num_rows($result)) // si aucune ligne dans la base + $sql = "SELECT rowid"; + $sql.= " FROM ".MAIN_DB_PREFIX."product_lang"; + $sql.= " WHERE fk_product=".$this->id; + $sql.= " AND lang='".$key."'"; + + $result = $this->db->query($sql); + + if ($this->db->num_rows($result)) // if there is already a description line for this language { $sql2 = "UPDATE ".MAIN_DB_PREFIX."product_lang"; $sql2.= " SET label='".$this->db->escape($this->multilangs["$key"]["label"])."',"; @@ -974,6 +981,10 @@ class Product extends CommonObject return -1; } } + else + { + // language is not current language and we didn't provide a multilang description for this language + } } return 1; } diff --git a/htdocs/product/stock/class/mouvementstock.class.php b/htdocs/product/stock/class/mouvementstock.class.php index 39dd2ff5f67..b0f3c227e9d 100644 --- a/htdocs/product/stock/class/mouvementstock.class.php +++ b/htdocs/product/stock/class/mouvementstock.class.php @@ -1,6 +1,6 @@ - * Copyright (C) 2005-2013 Laurent Destailleur + * Copyright (C) 2005-2015 Laurent Destailleur * Copyright (C) 2011 Jean Heimburger * Copyright (C) 2014 Cedric GROSS * @@ -21,7 +21,7 @@ /** * \file htdocs/product/stock/class/mouvementstock.class.php * \ingroup stock - * \brief Fichier de la classe de gestion des mouvements de stocks + * \brief File of class to manage stock movement (input or output) */ @@ -33,6 +33,12 @@ class MouvementStock extends CommonObject var $error; var $db; + var $product_id; + var $entrepot_id; + var $qty; + var $type; + + /** * Constructor * @@ -49,10 +55,11 @@ class MouvementStock extends CommonObject * @param User $user User object * @param int $fk_product Id of product * @param int $entrepot_id Id of warehouse - * @param int $qty Qty of movement (can be <0 or >0) + * @param int $qty Qty of movement (can be <0 or >0 depending on parameter type) * @param int $type Direction of movement: * 0=input (stock increase after stock transfert), 1=output (stock decrease after stock transfer), * 2=output (stock decrease), 3=input (stock increase) + * Note that qty should be > 0 with 0 or 3, < 0 with 1 or 2. * @param int $price Unit price HT of product, used to calculate average weighted price (PMP in french). If 0, average weighted price is not changed. * @param string $label Label of stock movement * @param string $inventorycode Inventory code @@ -156,9 +163,9 @@ class MouvementStock extends CommonObject // Define current values for qty and pmp $oldqty=$product->stock_reel; - $oldqtywarehouse=0; $oldpmp=$product->pmp; - $oldpmpwarehouse=0; + $oldqtywarehouse=0; + //$oldpmpwarehouse=0; // Test if there is already a record for couple (warehouse / product) $num = 0; @@ -176,7 +183,7 @@ class MouvementStock extends CommonObject { $num = 1; $oldqtywarehouse = $obj->reel; - $oldpmpwarehouse = $obj->pmp; + //$oldpmpwarehouse = $obj->pmp; $fk_product_stock = $obj->rowid; } $this->db->free($resql); @@ -190,8 +197,7 @@ class MouvementStock extends CommonObject // Calculate new PMP. $newpmp=0; - $newpmpwarehouse=0; - /* + //$newpmpwarehouse=0; if (! $error) { // Note: PMP is calculated on stock input only (type of movement = 0 or 3). If type == 0 or 3, qty should be > 0. @@ -200,7 +206,7 @@ class MouvementStock extends CommonObject { // If we will change PMP for the warehouse we edit and the product, we must first check/clean that PMP is defined // on every stock entry with old value (so global updated value will match recalculated value from product_stock) - $sql = "UPDATE ".MAIN_DB_PREFIX."product_stock SET pmp = ".($oldpmp?$oldpmp:'0'); + /* $sql = "UPDATE ".MAIN_DB_PREFIX."product_stock SET pmp = ".($oldpmp?$oldpmp:'0'); $sql.= " WHERE pmp = 0 AND fk_product = ".$fk_product; dol_syslog(get_class($this)."::_create", LOG_DEBUG); $resql=$this->db->query($sql); @@ -209,18 +215,19 @@ class MouvementStock extends CommonObject $this->error=$this->db->lasterror(); $error = -4; } - + */ $oldqtytouse=($oldqty >= 0?$oldqty:0); // We make a test on oldpmp>0 to avoid to use normal rule on old data with no pmp field defined if ($oldpmp > 0) $newpmp=price2num((($oldqtytouse * $oldpmp) + ($qty * $price)) / ($oldqtytouse + $qty), 'MU'); else { - $newpmp=$price; // For this product, PMP was not yet set. We will set it later. + $newpmp=$price; // For this product, PMP was not yet set. We set it to input price. } + /* $oldqtywarehousetouse=$oldqtywarehouse; if ($oldpmpwarehouse > 0) $newpmpwarehouse=price2num((($oldqtywarehousetouse * $oldpmpwarehouse) + ($qty * $price)) / ($oldqtywarehousetouse + $qty), 'MU'); else $newpmpwarehouse=$price; - + */ //print "oldqtytouse=".$oldqtytouse." oldpmp=".$oldpmp." oldqtywarehousetouse=".$oldqtywarehousetouse." oldpmpwarehouse=".$oldpmpwarehouse." "; //print "qty=".$qty." newpmp=".$newpmp." newpmpwarehouse=".$newpmpwarehouse; //exit; @@ -228,28 +235,31 @@ class MouvementStock extends CommonObject else if ($type == 1 || $type == 2) { // After a stock decrease, we don't change value of PMP for product. + $newpmp = $oldpmp; } else { $newpmp = $oldpmp; - $newpmpwarehouse = $oldpmpwarehouse; + //$newpmpwarehouse = $oldpmpwarehouse; } } - */ - // Update denormalized value of stock in product_stock and product + // Update stock quantity if (! $error) { if ($num > 0) { - $sql = "UPDATE ".MAIN_DB_PREFIX."product_stock SET pmp = ".$newpmpwarehouse.", reel = reel + ".$qty; + //$sql = "UPDATE ".MAIN_DB_PREFIX."product_stock SET pmp = ".$newpmpwarehouse.", reel = reel + ".$qty; + $sql = "UPDATE ".MAIN_DB_PREFIX."product_stock SET reel = reel + ".$qty; $sql.= " WHERE fk_entrepot = ".$entrepot_id." AND fk_product = ".$fk_product; } else { $sql = "INSERT INTO ".MAIN_DB_PREFIX."product_stock"; - $sql.= " (pmp, reel, fk_entrepot, fk_product) VALUES "; - $sql.= " (".$newpmpwarehouse.", ".$qty.", ".$entrepot_id.", ".$fk_product.")"; + //$sql.= " (pmp, reel, fk_entrepot, fk_product) VALUES "; + //$sql.= " (".$newpmpwarehouse.", ".$qty.", ".$entrepot_id.", ".$fk_product.")"; + $sql.= " (reel, fk_entrepot, fk_product) VALUES "; + $sql.= " (".$qty.", ".$entrepot_id.", ".$fk_product.")"; } dol_syslog(get_class($this)."::_create", LOG_DEBUG); @@ -274,6 +284,7 @@ class MouvementStock extends CommonObject if ($result<0) $error++; } + // Update PMP and denormalized value of stock qty at product level if (! $error) { $sql = "UPDATE ".MAIN_DB_PREFIX."product SET pmp = ".$newpmp.", stock = ".$this->db->ifsql("stock IS NULL", 0, "stock") . " + ".$qty; @@ -576,4 +587,22 @@ class MouvementStock extends CommonObject $origin->fetch($fk_origin); return $origin->getNomUrl(1); } + + + /** + * Initialise an instance with random values. + * Used to build previews or test instances. + * id must be 0 if object instance is a specimen. + * + * @return void + */ + function initAsSpecimen() + { + global $user,$langs,$conf,$mysoc; + + // Initialize parameters + $this->id=0; + + // There is no specific properties. All data into insert are provided as method parameter. + } } diff --git a/test/phpunit/AllTests.php b/test/phpunit/AllTests.php index 03d092e976d..b4c7497cb79 100644 --- a/test/phpunit/AllTests.php +++ b/test/phpunit/AllTests.php @@ -31,14 +31,17 @@ global $conf,$user,$langs,$db; //require_once 'PHPUnit/Autoload.php'; require_once dirname(__FILE__).'/../../htdocs/master.inc.php'; -if ($langs->defaultlang != 'en_US') { +if ($langs->defaultlang != 'en_US') +{ print "Error: Default language for company to run tests must be set to en_US or auto. Current is ".$langs->defaultlang."\n"; exit; } -if (! empty($conf->google->enabled)) { +if (! empty($conf->google->enabled)) +{ print "Warning: Google module should not be enabled.\n"; } -if (empty($user->id)) { +if (empty($user->id)) +{ print "Load permissions for admin user nb 1\n"; $user->fetch(1); $user->getrights(); @@ -161,6 +164,8 @@ class AllTests $suite->addTestSuite('HolidayTest'); require_once dirname(__FILE__).'/EntrepotTest.php'; $suite->addTestSuite('EntrepotTest'); + require_once dirname(__FILE__).'/MouvementStockTest.php'; + $suite->addTestSuite('MouvementStockTest'); require_once dirname(__FILE__).'/CategorieTest.php'; $suite->addTestSuite('CategorieTest'); diff --git a/test/phpunit/MouvementStockTest.php b/test/phpunit/MouvementStockTest.php new file mode 100644 index 00000000000..7a6d4348a41 --- /dev/null +++ b/test/phpunit/MouvementStockTest.php @@ -0,0 +1,255 @@ + + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * or see http://www.gnu.org/ + */ + +/** + * \file test/phpunit/MouvementStockTest.php + * \ingroup test + * \brief PHPUnit test + * \remarks To run this script as CLI: phpunit filename.php + */ + +global $conf,$user,$langs,$db; +//define('TEST_DB_FORCE_TYPE','mysql'); // This is to force using mysql driver +//require_once 'PHPUnit/Autoload.php'; +require_once dirname(__FILE__).'/../../htdocs/master.inc.php'; +require_once dirname(__FILE__).'/../../htdocs/product/stock/class/mouvementstock.class.php'; +require_once dirname(__FILE__).'/../../htdocs/product/stock/class/entrepot.class.php'; +require_once dirname(__FILE__).'/../../htdocs/product/class/product.class.php'; + +if (empty($user->id)) +{ + print "Load permissions for admin user nb 1\n"; + $user->fetch(1); + $user->getrights(); +} +$conf->global->MAIN_DISABLE_ALL_MAILS=1; + + +/** + * Class for PHPUnit tests + * + * @backupGlobals disabled + * @backupStaticAttributes enabled + * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. + */ +class MouvementStockTest extends PHPUnit_Framework_TestCase +{ + protected $savconf; + protected $savuser; + protected $savlangs; + protected $savdb; + + /** + * Constructor + * We save global variables into local variables + * + * @return ContratTest + */ + function __construct() + { + //$this->sharedFixture + global $conf,$user,$langs,$db; + $this->savconf=$conf; + $this->savuser=$user; + $this->savlangs=$langs; + $this->savdb=$db; + + print __METHOD__." db->type=".$db->type." user->id=".$user->id; + //print " - db ".$db->db; + print "\n"; + } + + // Static methods + public static function setUpBeforeClass() + { + global $conf,$user,$langs,$db; + $db->begin(); // This is to have all actions inside a transaction even if test launched without suite. + + print __METHOD__."\n"; + } + + // tear down after class + public static function tearDownAfterClass() + { + global $conf,$user,$langs,$db; + $db->rollback(); + + print __METHOD__."\n"; + } + + /** + * Init phpunit tests + * + * @return void + */ + protected function setUp() + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + print __METHOD__."\n"; + } + /** + * End phpunit tests + * + * @return void + */ + protected function tearDown() + { + print __METHOD__."\n"; + } + + /** + * testMouvementCreate + * + * @return int + */ + public function testMouvementCreate() + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + // We create a product for tests + $product1=new Product($db); + $product1->initAsSpecimen(); + $product1->ref.=' 1'; + $product1->label.=' 1'; + $product1id=$product1->create($user); + + $product2=new Product($db); + $product2->initAsSpecimen(); + $product2->ref.=' 2'; + $product2->label.=' 2'; + $product2id=$product2->create($user); + + // We create a product for tests + $warehouse1=new Entrepot($db); + $warehouse1->initAsSpecimen(); + $warehouse1->libelle.=' 1'; + $warehouse1->description.=' 1'; + $warehouse1id=$warehouse1->create($user); + + $warehouse2=new Entrepot($db); + $warehouse2->initAsSpecimen(); + $warehouse2->libelle.=' 2'; + $warehouse2->description.=' 2'; + $warehouse2id=$warehouse2->create($user); + + $localobject=new MouvementStock($this->savdb); + $localobject->initAsSpecimen(); + + // Do a list of movement into warehouse 1 + + // Create an input movement (type = 3) of price 9.9 -> shoul dupdate PMP to 9.9 + $result=$localobject->_create($user, $product1id, $warehouse1id, 10, 3, 9.9, 'Movement for unit test 1', 'Inventory Code Test'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an input movement (type = 3) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse1id, 10, 3, 9.7, 'Movement for unit test 2', 'Inventory Code Test'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an output movement (type = 2) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse1id, -5, 2, 999, 'Movement for unit test 3', 'Inventory Code Test'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an output movement (type = 1) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse1id, 1, 0, 0, 'Input from transfer', 'Transfert X'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an output movement (type = 1) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse1id, -2, 1, 0, 'Output from transfer', 'Transfert Y'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + + // Do same but into warehouse 2 + + // Create an input movement (type = 3) of price 9.9 -> shoul dupdate PMP to 9.9 + $result=$localobject->_create($user, $product1id, $warehouse2id, 10, 3, 9.9, 'Movement for unit test 1 wh 2', 'Inventory Code Test 2'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an input movement (type = 3) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse2id, 10, 3, 9.7, 'Movement for unit test 2 wh 2', 'Inventory Code Test 2'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an output movement (type = 2) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse2id, -5, 2, 999, 'Movement for unit test 3 wh 2', 'Inventory Code Test 2'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an output movement (type = 1) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse2id, 1, 0, 0, 'Input from transfer wh 2', 'Transfert X 2'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + // Create an output movement (type = 1) of price 9.7 -> shoul dupdate PMP to 9.9/9.7 = 9.8 + $result=$localobject->_create($user, $product1id, $warehouse2id, -2, 1, 0, 'Output from transfer wh 2', 'Transfert Y 2'); + print __METHOD__." result=".$result."\n"; + $this->assertLessThan($result, 0); + + + return $localobject; + } + + /** + * testMouvementCheck + * + * @param MouvementStock $localobject Movement object we created + * @return int + * + * @depends testMouvementCreate + * The depends says test is run only if previous is ok + */ + public function testMouvementCheck($localobject) + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + $productid = $localobject->product_id; + $warehouseid = $localobject->entrepot_id; + + print __METHOD__." productid=".$productid."\n"; + + $producttotest = new Product($db); + $producttotest->fetch($productid); + + print __METHOD__." producttotest->stock_reel=".$producttotest->stock_reel."\n"; + $this->assertEquals($producttotest->stock_reel, 28); // 28 is result of stock movement defined into testMouvementCreate + + print __METHOD__." producttotest->pmp=".$producttotest->pmp."\n"; + $this->assertEquals($producttotest->pmp, 9.8); + + return $localobject; + } + +}