From 69eaefc45930bc1247596dd458472c0641fceb10 Mon Sep 17 00:00:00 2001
From: Laurent Destailleur <eldy@destailleur.fr>
Date: Tue, 13 May 2014 10:12:45 +0200
Subject: [PATCH] Fix: The way we tested if a ref/id already exists was wrong.
Introduce a common static method for this. Removed verifNumRef method. Fix:
Add missing logs
---
htdocs/comm/propal/class/propal.class.php | 18 +++--
htdocs/commande/class/commande.class.php | 26 +++++--
htdocs/compta/facture/class/facture.class.php | 18 ++++-
htdocs/core/class/commonobject.class.php | 68 +++++++++++--------
htdocs/core/lib/geturl.lib.php | 2 +-
htdocs/fichinter/class/fichinter.class.php | 34 ++++------
htdocs/product/class/product.class.php | 46 +++----------
htdocs/product/liste.php | 1 -
htdocs/product/reassort.php | 1 -
test/phpunit/CommandeFournisseurTest.php | 23 +------
test/phpunit/CommandeTest.php | 21 ------
test/phpunit/CommonObjectTest.php | 22 ------
test/phpunit/ContratTest.php | 23 -------
test/phpunit/FactureFournisseurTest.php | 19 ------
test/phpunit/ProjectTest.php | 24 +------
test/phpunit/PropalTest.php | 22 ------
16 files changed, 114 insertions(+), 254 deletions(-)
diff --git a/htdocs/comm/propal/class/propal.class.php b/htdocs/comm/propal/class/propal.class.php
index 8a4ff80f713..2744c3eec4d 100644
--- a/htdocs/comm/propal/class/propal.class.php
+++ b/htdocs/comm/propal/class/propal.class.php
@@ -673,16 +673,26 @@ class Propal extends CommonObject
dol_syslog(get_class($this)."::create ".$this->error, LOG_ERR);
return -3;
}
+
+ // Check parameters
+ if (! empty($this->ref)) // We check that ref is not already used
+ {
+ $result=self::isExistingObject($this->element, 0, $this->ref); // Check ref is not yet used
+ if ($result > 0)
+ {
+ $this->error='ErrorRefAlreadyExists';
+ dol_syslog(get_class($this)."::create ".$this->error,LOG_WARNING);
+ $this->db->rollback();
+ return -1;
+ }
+ }
+
if (empty($this->date))
{
$this->error="Date of proposal is required";
dol_syslog(get_class($this)."::create ".$this->error, LOG_ERR);
return -4;
}
- if (! empty($this->ref))
- {
- $result=$this->verifyNumRef(); // Check ref is not yet used
- }
$this->db->begin();
diff --git a/htdocs/commande/class/commande.class.php b/htdocs/commande/class/commande.class.php
index 5c4d1d0e131..dd5bb59c678 100644
--- a/htdocs/commande/class/commande.class.php
+++ b/htdocs/commande/class/commande.class.php
@@ -612,6 +612,18 @@ class Commande extends CommonOrder
dol_syslog(get_class($this)."::create user=".$user->id);
// Check parameters
+ if (! empty($this->ref)) // We check that ref is not already used
+ {
+ $result=self::isExistingObject($this->element, 0, $this->ref); // Check ref is not yet used
+ if ($result > 0)
+ {
+ $this->error='ErrorRefAlreadyExists';
+ dol_syslog(get_class($this)."::create ".$this->error,LOG_WARNING);
+ $this->db->rollback();
+ return -1;
+ }
+ }
+
$soc = new Societe($this->db);
$result=$soc->fetch($this->socid);
if ($result < 0)
@@ -1039,7 +1051,7 @@ class Commande extends CommonOrder
function addline($desc, $pu_ht, $qty, $txtva, $txlocaltax1=0, $txlocaltax2=0, $fk_product=0, $remise_percent=0, $info_bits=0, $fk_remise_except=0, $price_base_type='HT', $pu_ttc=0, $date_start='', $date_end='', $type=0, $rang=-1, $special_code=0, $fk_parent_line=0, $fk_fournprice=null, $pa_ht=0, $label='',$array_option=0)
{
global $mysoc, $conf, $langs;
-
+
$commandeid=$this->id;
dol_syslog(get_class($this)."::addline commandeid=$commandeid, desc=$desc, pu_ht=$pu_ht, qty=$qty, txtva=$txtva, fk_product=$fk_product, remise_percent=$remise_percent, info_bits=$info_bits, fk_remise_except=$fk_remise_except, price_base_type=$price_base_type, pu_ttc=$pu_ttc, date_start=$date_start, date_end=$date_end, type=$type", LOG_DEBUG);
@@ -1086,9 +1098,9 @@ class Commande extends CommonOrder
// qty, pu, remise_percent et txtva
// TRES IMPORTANT: C'est au moment de l'insertion ligne qu'on doit stocker
// la part ht, tva et ttc, et ce au niveau de la ligne qui a son propre taux tva.
-
+
$localtaxes_type=getLocalTaxesFromRate($txtva,0,$mysoc);
-
+
$tabprice = calcul_price_total($qty, $pu, $remise_percent, $txtva, $txlocaltax1, $txlocaltax2, 0, $price_base_type, $info_bits, $type,'', $localtaxes_type);
$total_ht = $tabprice[0];
$total_tva = $tabprice[1];
@@ -1103,14 +1115,14 @@ class Commande extends CommonOrder
$rangmax = $this->line_max($fk_parent_line);
$rangtouse = $rangmax + 1;
}
-
+
$product_type=$type;
if (!empty($fk_product))
{
$product=new Product($this->db);
$result=$product->fetch($fk_product);
$product_type=$product->type;
-
+
if($conf->global->STOCK_MUST_BE_ENOUGH_FOR_ORDER && $product_type == 0 && $product->stock_reel < $qty) {
$this->error=$langs->trans('ErrorStockIsNotEnough');
$this->db->rollback();
@@ -2303,9 +2315,9 @@ class Commande extends CommonOrder
// qty, pu, remise_percent et txtva
// TRES IMPORTANT: C'est au moment de l'insertion ligne qu'on doit stocker
// la part ht, tva et ttc, et ce au niveau de la ligne qui a son propre taux tva.
-
+
$localtaxes_type=getLocalTaxesFromRate($txtva,0,$mysoc);
-
+
$tabprice=calcul_price_total($qty, $pu, $remise_percent, $txtva, $txlocaltax1, $txlocaltax2, 0, $price_base_type, $info_bits, $type, '', $localtaxes_type);
$total_ht = $tabprice[0];
$total_tva = $tabprice[1];
diff --git a/htdocs/compta/facture/class/facture.class.php b/htdocs/compta/facture/class/facture.class.php
index 8a2907c3522..65736e06097 100644
--- a/htdocs/compta/facture/class/facture.class.php
+++ b/htdocs/compta/facture/class/facture.class.php
@@ -3493,7 +3493,7 @@ class FactureLigne extends CommonInvoiceLine
}
/**
- * Insert line in database
+ * Insert line into database
*
* @param int $notrigger 1 no triggers
* @return int <0 if KO, >0 if OK
@@ -3531,7 +3531,21 @@ class FactureLigne extends CommonInvoiceLine
}
// Check parameters
- if ($this->product_type < 0) return -1;
+ if ($this->product_type < 0)
+ {
+ $this->error='ErrorProductTypeMustBe0orMore';
+ return -1;
+ }
+ if (! empty($this->fk_product))
+ {
+ // Check product exists
+ $result=Product::isExistingObject('product', $this->fk_product);
+ if ($result <= 0)
+ {
+ $this->error='ErrorProductIdDoesNotExists';
+ return -1;
+ }
+ }
$this->db->begin();
diff --git a/htdocs/core/class/commonobject.class.php b/htdocs/core/class/commonobject.class.php
index f6e5e2e815e..ec8a9be586b 100644
--- a/htdocs/core/class/commonobject.class.php
+++ b/htdocs/core/class/commonobject.class.php
@@ -53,6 +53,42 @@ abstract class CommonObject
// No constructor as it is an abstract class
+ /**
+ * Check an object id/ref exists
+ * If you don't need/want to instantiate object and just need to know if object exists, use this method instead of fetch
+ *
+ * @param string $element String of element ('product', 'facture', ...)
+ * @param int $id Id of object
+ * @param string $ref Ref of object to check
+ * @param string $ref_ext Ref ext of object to check
+ * @return int <0 if KO, 0 if OK but not found, >0 if OK and exists
+ */
+ static function isExistingObject($element, $id, $ref='', $ref_ext='')
+ {
+ global $db;
+
+ $sql = "SELECT rowid, ref, ref_ext";
+ $sql.= " FROM ".MAIN_DB_PREFIX.$element;
+ if ($id > 0) $sql.= " WHERE rowid = ".$db->escape($id);
+ else if ($ref) $sql.= " WHERE ref = '".$db->escape($ref)."'";
+ else if ($ref_ext) $sql.= " WHERE ref_ext = '".$db->escape($ref_ext)."'";
+ else {
+ $error='ErrorWrongParameters';
+ dol_print_error(get_class()."::isExistingObject ".$error, LOG_ERR);
+ return -1;
+ }
+
+ dol_syslog(get_class()."::isExistingObject sql=".$sql);
+ $resql = $db->query($sql);
+ if ($resql)
+ {
+ $num=$db->num_rows($resql);
+ if ($num > 0) return 1;
+ else return 0;
+ }
+ return -1;
+ }
+
/**
* Method to output saved errors
*
@@ -114,33 +150,6 @@ abstract class CommonObject
return dol_format_address($this, $withcountry, $sep);
}
- /**
- * Check if ref is used.
- *
- * @return int <0 if KO, 0 if not found, >0 if found
- */
- function verifyNumRef()
- {
- global $conf;
-
- $sql = "SELECT rowid";
- $sql.= " FROM ".MAIN_DB_PREFIX.$this->table_element;
- $sql.= " WHERE ref = '".$this->ref."'";
- $sql.= " AND entity = ".$conf->entity;
- dol_syslog(get_class($this)."::verifyNumRef sql=".$sql, LOG_DEBUG);
- $resql = $this->db->query($sql);
- if ($resql)
- {
- $num = $this->db->num_rows($resql);
- return $num;
- }
- else
- {
- $this->error=$this->db->lasterror();
- dol_syslog(get_class($this)."::verifyNumRef ".$this->error, LOG_ERR);
- return -1;
- }
- }
/**
* Add a link between element $this->element and a contact
@@ -2052,7 +2061,7 @@ abstract class CommonObject
/**
- * Get special code of line
+ * Get special code of a line
*
* @param int $lineid Id of line
* @return int Special code
@@ -3088,7 +3097,8 @@ abstract class CommonObject
* @param string $force_price True of not
* @return mixed Array with info
*/
- function getMarginInfos($force_price=false) {
+ function getMarginInfos($force_price=false)
+ {
global $conf;
require_once DOL_DOCUMENT_ROOT.'/fourn/class/fournisseur.product.class.php';
diff --git a/htdocs/core/lib/geturl.lib.php b/htdocs/core/lib/geturl.lib.php
index 15604c0ce5d..8a9ac1385be 100644
--- a/htdocs/core/lib/geturl.lib.php
+++ b/htdocs/core/lib/geturl.lib.php
@@ -26,7 +26,7 @@
*
* @param string $url URL to call.
* @param string $postorget 'POST', 'GET', 'HEAD'
- * @param string $param Paraemeters of URL (x=value1&y=value2)
+ * @param string $param Parameters of URL (x=value1&y=value2)
* @param string $followlocation 1=Follow location, 0=Do not follow
* @param array $addheaders Array of string to add into header. Example: ('Accept: application/xrds+xml', ....)
* @return array Returns an associative array containing the response from the server array('content'=>response,'curl_error_no'=>errno,'curl_error_msg'=>errmsg...)
diff --git a/htdocs/fichinter/class/fichinter.class.php b/htdocs/fichinter/class/fichinter.class.php
index bae211525ea..d7bb50682fe 100644
--- a/htdocs/fichinter/class/fichinter.class.php
+++ b/htdocs/fichinter/class/fichinter.class.php
@@ -95,36 +95,28 @@ class Fichinter extends CommonObject
dol_syslog(get_class($this)."::create ref=".$this->ref);
// Check parameters
- if (! is_numeric($this->duree)) {
- $this->duree = 0;
+ if (! empty($this->ref)) // We check that ref is not already used
+ {
+ $result=self::isExistingObject($this->element, 0, $this->ref); // Check ref is not yet used
+ if ($result > 0)
+ {
+ $this->error='ErrorRefAlreadyExists';
+ dol_syslog(get_class($this)."::create ".$this->error,LOG_WARNING);
+ $this->db->rollback();
+ return -1;
+ }
}
+ if (! is_numeric($this->duree)) $this->duree = 0;
+
if ($this->socid <= 0)
{
$this->error='ErrorBadParameterForFunc';
dol_syslog(get_class($this)."::create ".$this->error,LOG_ERR);
return -1;
}
- // on verifie si la ref n'est pas utilisee
+
$soc = new Societe($this->db);
$result=$soc->fetch($this->socid);
- if (! empty($this->ref))
- {
- $result=$this->verifyNumRef(); // Check ref is not yet used
- if ($result > 0)
- {
- $this->error='ErrorRefAlreadyExists';
- dol_syslog(get_class($this)."::create ".$this->error,LOG_WARNING);
- $this->db->rollback();
- return -3;
- }
- else if ($result < 0)
- {
- $this->error=$this->db->error();
- dol_syslog(get_class($this)."::create ".$this->error,LOG_ERR);
- $this->db->rollback();
- return -2;
- }
- }
$now=dol_now();
diff --git a/htdocs/product/class/product.class.php b/htdocs/product/class/product.class.php
index 50ca2d1b6c4..ae1c0f2f43c 100644
--- a/htdocs/product/class/product.class.php
+++ b/htdocs/product/class/product.class.php
@@ -2276,6 +2276,7 @@ class Product extends CommonObject
$sql.= " FROM ".MAIN_DB_PREFIX."product_price ";
$sql.= " WHERE fk_product = ". $fromId;
+ dol_syslog(get_class($this).'::clone_price sql='.$sql);
if (! $this->db->query($sql))
{
$this->db->rollback();
@@ -2285,6 +2286,13 @@ class Product extends CommonObject
return 1;
}
+ /**
+ * Clone links between products
+ *
+ * @param int $fromId Product id
+ * @param int $toId Product id
+ * @return number
+ */
function clone_associations($fromId, $toId)
{
$this->db->begin();
@@ -2293,6 +2301,7 @@ class Product extends CommonObject
$sql.= " SELECT null, $toId, fk_product_fils, qty FROM ".MAIN_DB_PREFIX."product_association";
$sql.= " WHERE fk_product_pere = '".$fromId."'";
+ dol_syslog(get_class($this).'::clone_association sql='.$sql);
if (! $this->db->query($sql))
{
$this->db->rollback();
@@ -2336,6 +2345,7 @@ class Product extends CommonObject
$sql.= " FROM ".MAIN_DB_PREFIX."product_fournisseur_price";
$sql.= " WHERE fk_product = ".$fromId;
+ dol_syslog(get_class($this).'::clone_fournisseurs sql='.$sql);
$resql=$this->db->query($sql);
if (! $resql)
{
@@ -2894,42 +2904,6 @@ class Product extends CommonObject
}
}
- /**
- * Deplace fichier recupere sur internet (utilise pour interface avec OSC)
- *
- * @param string $sdir Repertoire destination finale
- * @param string $file url de l'image
- * @return void
- */
- function add_photo_web($sdir, $file)
- {
- $dir = $sdir .'/'. get_exdir($this->id,2) . $this->id ."/";
- $dir .= "photos/";
-
- $dir_osencoded=dol_osencode($dir);
- if (! file_exists($dir_osencoded))
- {
- dol_syslog("Product Create ".$dir);
- dol_mkdir($dir);
- }
-
- if (file_exists($dir_osencoded))
- {
- // Cree fichier en taille vignette
- // TODO A faire
-
- // Cree fichier en taille origine
- $content = @file_get_contents($file);
- if( $content)
- {
- $nom = basename($file);
- $im = fopen(dol_osencode($dir.$nom),'wb');
- fwrite($im, $content);
- fclose($im);
- }
- }
- }
-
/**
* Affiche la premiere photo du produit
*
diff --git a/htdocs/product/liste.php b/htdocs/product/liste.php
index 36904eb0378..821686b026c 100644
--- a/htdocs/product/liste.php
+++ b/htdocs/product/liste.php
@@ -68,7 +68,6 @@ $limit = $conf->liste_limit;
// Get object canvas (By default, this is not defined, so standard usage of dolibarr)
-//$object->getCanvas($id);
$canvas=GETPOST("canvas");
$objcanvas='';
if (! empty($canvas))
diff --git a/htdocs/product/reassort.php b/htdocs/product/reassort.php
index c185eeaf389..4522304b887 100644
--- a/htdocs/product/reassort.php
+++ b/htdocs/product/reassort.php
@@ -62,7 +62,6 @@ $search_sale = GETPOST("search_sale");
$search_categ = GETPOST("search_categ");
// Get object canvas (By default, this is not defined, so standard usage of dolibarr)
-//$object->getCanvas($id);
$canvas=GETPOST("canvas");
$objcanvas='';
if (! empty($canvas))
diff --git a/test/phpunit/CommandeFournisseurTest.php b/test/phpunit/CommandeFournisseurTest.php
index 74f048dece8..2c83b4fdfc3 100644
--- a/test/phpunit/CommandeFournisseurTest.php
+++ b/test/phpunit/CommandeFournisseurTest.php
@@ -136,7 +136,7 @@ class CommandeFournisseurTest extends PHPUnit_Framework_TestCase
$product=new ProductFournisseur($db);
$product->fetch(0,'PIDRESS');
if ($product->id <= 0) { print "\n".__METHOD__." A product with ref PIDRESS must exists into database"; die(); }
-
+
$quantity=10;
$ref_fourn='SUPPLIER_REF_PHPUNIT';
$tva_tx=19.6;
@@ -337,26 +337,5 @@ class CommandeFournisseurTest extends PHPUnit_Framework_TestCase
return $result;
}
- /**
- * testVerifyNumRef
- *
- * @return void
- */
- public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new CommandeFournisseur($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }
}
?>
\ No newline at end of file
diff --git a/test/phpunit/CommandeTest.php b/test/phpunit/CommandeTest.php
index c77dd39d0c2..451e886a761 100644
--- a/test/phpunit/CommandeTest.php
+++ b/test/phpunit/CommandeTest.php
@@ -264,26 +264,5 @@ class CommandeTest extends PHPUnit_Framework_TestCase
return $result;
}
- /**
- * testVerifyNumRef
- *
- * @return void
- */
- public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new Commande($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }
}
?>
\ No newline at end of file
diff --git a/test/phpunit/CommonObjectTest.php b/test/phpunit/CommonObjectTest.php
index a01982508e3..cbdde31e14d 100644
--- a/test/phpunit/CommonObjectTest.php
+++ b/test/phpunit/CommonObjectTest.php
@@ -115,28 +115,6 @@ class CommonObjectTest extends PHPUnit_Framework_TestCase
}
- /**
- * testVerifyNumRef
- *
- * @return void
- */
- public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new Commande($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }
-
/**
* testFetchUser
*
diff --git a/test/phpunit/ContratTest.php b/test/phpunit/ContratTest.php
index 2c9eaf6dc9d..c42d62cd3b1 100644
--- a/test/phpunit/ContratTest.php
+++ b/test/phpunit/ContratTest.php
@@ -241,27 +241,4 @@ class ContratTest extends PHPUnit_Framework_TestCase
return $result;
}
-
- /**
- * testVerifyNumRef
- *
- * @return int
- */
- public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new Contrat($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }
-
}
diff --git a/test/phpunit/FactureFournisseurTest.php b/test/phpunit/FactureFournisseurTest.php
index 7fe3dbd3214..2cacb073b60 100644
--- a/test/phpunit/FactureFournisseurTest.php
+++ b/test/phpunit/FactureFournisseurTest.php
@@ -264,24 +264,5 @@ class FactureFournisseurTest extends PHPUnit_Framework_TestCase
return $result;
}
- /**
- *
- */
- /*public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new Facture($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }*/
}
?>
\ No newline at end of file
diff --git a/test/phpunit/ProjectTest.php b/test/phpunit/ProjectTest.php
index d9a5d8d09b4..9ed50eb2475 100644
--- a/test/phpunit/ProjectTest.php
+++ b/test/phpunit/ProjectTest.php
@@ -185,7 +185,7 @@ class ProjectTest extends PHPUnit_Framework_TestCase
$this->assertLessThan($result, 0);
return $localobject;
}
-
+
/**
* testProjectOther
*
@@ -232,27 +232,5 @@ class ProjectTest extends PHPUnit_Framework_TestCase
return $result;
}
- /**
- * testVerifyNumRef
- *
- * @return void
- */
- public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new Project($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }
-
}
?>
\ No newline at end of file
diff --git a/test/phpunit/PropalTest.php b/test/phpunit/PropalTest.php
index 772a792e367..805ee7f908a 100644
--- a/test/phpunit/PropalTest.php
+++ b/test/phpunit/PropalTest.php
@@ -264,27 +264,5 @@ class PropalTest extends PHPUnit_Framework_TestCase
return $result;
}
- /**
- * testVerifyNumRef
- *
- * @return void
- */
- public function testVerifyNumRef()
- {
- global $conf,$user,$langs,$db;
- $conf=$this->savconf;
- $user=$this->savuser;
- $langs=$this->savlangs;
- $db=$this->savdb;
-
- $localobject=new Propal($this->savdb);
- $result=$localobject->ref='refthatdoesnotexists';
- $result=$localobject->VerifyNumRef();
-
- print __METHOD__." result=".$result."\n";
- $this->assertEquals($result, 0);
- return $result;
- }
-
}
?>
\ No newline at end of file
--
GitLab