Fix calculation of PMP of product.

Also add phpunit test to avoid this in future.

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;
 	}

htdocs/product/stock/class/mouvementstock.class.php

@@ -1,6 +1,6 @@
 <?php
 /* Copyright (C) 2003-2006 Rodolphe Quiedeville <rodolphe@quiedeville.org>
- * Copyright (C) 2005-2013 Laurent Destailleur  <eldy@users.sourceforge.net>
+ * Copyright (C) 2005-2015 Laurent Destailleur  <eldy@users.sourceforge.net>
  * Copyright (C) 2011      Jean Heimburger      <jean@tiaris.info>
  * Copyright (C) 2014	   Cedric GROSS	        <c.gross@kreiz-it.fr>
  *
@@ -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.= " (pmp, reel, fk_entrepot, fk_product) VALUES ";
-					$sql.= " (".$newpmpwarehouse.", ".$qty.", ".$entrepot_id.", ".$fk_product.")";
+					//$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.
+    }
 }

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');

test/phpunit/MouvementStockTest.php

@@ -0,0 +1,255 @@
+<?php
+/* Copyright (C) 2010 Laurent Destailleur  <eldy@users.sourceforge.net>
+ *
+ * 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 <http://www.gnu.org/licenses/>.
+ * 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;
+    }
+
+}