From 9cc79de31b12a5ded581c7a3d2fc4f9453879ea7 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Tue, 1 Mar 2022 00:01:13 +0100 Subject: [PATCH] Clean code --- htdocs/compta/facture/class/facture.class.php | 3 +- htdocs/hrm/class/evaluation.class.php | 75 ------------------- htdocs/product/class/productbatch.class.php | 39 +++++----- test/phpunit/CodingPhpTest.php | 8 +- 4 files changed, 25 insertions(+), 100 deletions(-) diff --git a/htdocs/compta/facture/class/facture.class.php b/htdocs/compta/facture/class/facture.class.php index 33720b8345e..70aebe6932c 100644 --- a/htdocs/compta/facture/class/facture.class.php +++ b/htdocs/compta/facture/class/facture.class.php @@ -2643,6 +2643,7 @@ class Facture extends CommonInvoice require_once DOL_DOCUMENT_ROOT.'/product/stock/class/entrepot.class.php'; $productStatic = new Product($this->db); $warehouseStatic = new Entrepot($this->db); + $productbatch = new ProductBatch($this->db); } $now = dol_now(); @@ -2785,7 +2786,7 @@ class Facture extends CommonInvoice $sortorder = 'ASC,ASC,ASC,ASC'; } - $resBatchList = Productbatch::findAllForProduct($this->db, $productStatic->id, $idwarehouse, (!empty($conf->global->STOCK_ALLOW_NEGATIVE_TRANSFER) ? null : 0), $sortfield, $sortorder); + $resBatchList = $productbatch->findAllForProduct($productStatic->id, $idwarehouse, (!empty($conf->global->STOCK_ALLOW_NEGATIVE_TRANSFER) ? null : 0), $sortfield, $sortorder); if (!is_array($resBatchList)) { $error++; $this->error = $this->db->lasterror(); diff --git a/htdocs/hrm/class/evaluation.class.php b/htdocs/hrm/class/evaluation.class.php index 5e8124fe09a..8050eb08ec9 100644 --- a/htdocs/hrm/class/evaluation.class.php +++ b/htdocs/hrm/class/evaluation.class.php @@ -1067,79 +1067,4 @@ class Evaluation extends CommonObject return $error; } - - /** - * @param string $action - * @param int $selected - */ - // public function printObjectLines($action, $selected = 0) - // { - // global $conf, $hookmanager, $langs, $user, $extrafields, $object; - // // TODO We should not use global var for this - // global $inputalsopricewithtax, $usemargins, $disableedit, $disablemove, $disableremove, $outputalsopricetotalwithtax; - // - // // Define usemargins - //// $usemargins = 0; - //// if (!empty($conf->margin->enabled) && !empty($this->element) && in_array($this->element, array('facture', 'facturerec', 'propal', 'commande'))) { - //// $usemargins = 1; - //// } - // - // $num = count($this->lines); - // - // // Line extrafield - // if (!is_object($extrafields)) { - // require_once DOL_DOCUMENT_ROOT.'/core/class/extrafields.class.php'; - // $extrafields = new ExtraFields($this->db); - // } - // $extrafields->fetch_name_optionals_label($this->table_element_line); - // - // $parameters = array('num'=>$num, 'selected'=>$selected, 'table_element_line'=>$this->table_element_line); - // $reshook = $hookmanager->executeHooks('printObjectLineTitle', $parameters, $this, $action); // Note that $action and $object may have been modified by some hooks - // if (empty($reshook)) { - // // Output template part (modules that overwrite templates must declare this into descriptor) - // // Note: This is deprecated. If you need to overwrite the tpl file, use instead the hook. - // include dol_buildpath('hrm/core/tpl/objectline_title.tpl.php'); - // } - // - // $i = 0; - // - // print "\n"; - // foreach ($this->lines as $line) { - // //Line extrafield - // $line->fetch_optionals(); - // - // //if (is_object($hookmanager) && (($line->product_type == 9 && ! empty($line->special_code)) || ! empty($line->fk_parent_line))) - // if (is_object($hookmanager)) { // Old code is commented on preceding line. - // if (empty($line->fk_parent_line)) { - // $parameters = array('line'=>$line, 'num'=>$num, 'i'=>$i, 'selected'=>$selected, 'table_element_line'=>$line->table_element); - // $reshook = $hookmanager->executeHooks('printObjectLine', $parameters, $this, $action); // Note that $action and $object may have been modified by some hooks - // } else { - // $parameters = array('line'=>$line, 'num'=>$num, 'i'=>$i, 'selected'=>$selected, 'table_element_line'=>$line->table_element, 'fk_parent_line'=>$line->fk_parent_line); - // $reshook = $hookmanager->executeHooks('printObjectSubLine', $parameters, $this, $action); // Note that $action and $object may have been modified by some hooks - // } - // } - // if (empty($reshook)) { - // $this->printObjectLine($action, $line); - // } - // - // $i++; - // } - // print "\n"; - // } - - // public function printObjectLine($action, $line) - // { - // global $conf, $langs, $user, $object, $hookmanager; - // global $form; - // global $object_rights, $disableedit, $disablemove, $disableremove; // TODO We should not use global var for this ! - // - // $object_rights = $this->getRights(); - // - // $element = $this->element; - // - // $text = ''; - // $description = ''; - // - // include dol_buildpath('hrm/tpl/objectline_view.tpl.php'); - // } } diff --git a/htdocs/product/class/productbatch.class.php b/htdocs/product/class/productbatch.class.php index 5149dd2c708..e1ac2ea1eed 100644 --- a/htdocs/product/class/productbatch.class.php +++ b/htdocs/product/class/productbatch.class.php @@ -428,13 +428,13 @@ class Productbatch extends CommonObject /** * Return all batch detail records for a given product and warehouse * - * @param DoliDB $db database object + * @param DoliDB $dbs database object * @param int $fk_product_stock id product_stock for objet * @param int $with_qty 1 = doesn't return line with 0 quantity * @param int $fk_product If set to a product id, get eatby and sellby from table llx_product_lot * @return array <0 if KO, array of batch */ - public static function findAll($db, $fk_product_stock, $with_qty = 0, $fk_product = 0) + public static function findAll($dbs, $fk_product_stock, $with_qty = 0, $fk_product = 0) { global $conf; @@ -453,9 +453,9 @@ class Productbatch extends CommonObject $sql .= ", pl.rowid as lotid, pl.eatby as eatby, pl.sellby as sellby"; // TODO May add extrafields to ? } - $sql .= " FROM ".$db->prefix()."product_batch as t"; + $sql .= " FROM ".$dbs->prefix()."product_batch as t"; if ($fk_product > 0) { - $sql .= " LEFT JOIN ".$db->prefix()."product_lot as pl ON pl.fk_product = ".((int) $fk_product)." AND pl.batch = t.batch"; + $sql .= " LEFT JOIN ".$dbs->prefix()."product_lot as pl ON pl.fk_product = ".((int) $fk_product)." AND pl.batch = t.batch"; // TODO May add extrafields to ? } $sql .= " WHERE fk_product_stock=".((int) $fk_product_stock); @@ -470,9 +470,9 @@ class Productbatch extends CommonObject $sql .= ", t.qty ".(!empty($conf->global->DO_NOT_TRY_TO_DEFRAGMENT_STOCKS_WAREHOUSE)?'DESC':'ASC'); // Note : qty ASC is important for expedition card, to avoid stock fragmentation dol_syslog("productbatch::findAll", LOG_DEBUG); - $resql = $db->query($sql); + $resql = $dbs->query($sql); if ($resql) { - $num = $db->num_rows($resql); + $num = $dbs->num_rows($resql); $i = 0; while ($i < $num) { $obj = $db->fetch_object($resql); @@ -503,7 +503,6 @@ class Productbatch extends CommonObject /** * Return all batch for a product and a warehouse * - * @param DoliDB $db Database object * @param int $fk_product Id of product * @param int $fk_warehouse Id of warehouse * @param int $qty_min [=NULL] Minimum quantity @@ -513,7 +512,7 @@ class Productbatch extends CommonObject * * @throws Exception */ - public static function findAllForProduct($db, $fk_product, $fk_warehouse = 0, $qty_min = null, $sortfield = null, $sortorder = null) + public function findAllForProduct($fk_product, $fk_warehouse = 0, $qty_min = null, $sortfield = null, $sortorder = null) { $productBatchList = array(); @@ -526,10 +525,10 @@ class Productbatch extends CommonObject $sql .= ", pl.sellby"; $sql .= ", pl.eatby"; $sql .= ", pb.qty"; - $sql .= " FROM ".$db->prefix()."product_lot as pl"; - $sql .= " LEFT JOIN ".$db->prefix()."product as p ON p.rowid = pl.fk_product"; - $sql .= " LEFT JOIN ".$db->prefix()."product_batch AS pb ON pl.batch = pb.batch"; - $sql .= " LEFT JOIN ".$db->prefix()."product_stock AS ps ON ps.rowid = pb.fk_product_stock"; + $sql .= " FROM ".$this->db->prefix()."product_lot as pl"; + $sql .= " LEFT JOIN ".$this->db->prefix()."product as p ON p.rowid = pl.fk_product"; + $sql .= " LEFT JOIN ".$this->db->prefix()."product_batch AS pb ON pl.batch = pb.batch"; + $sql .= " LEFT JOIN ".$this->db->prefix()."product_stock AS ps ON ps.rowid = pb.fk_product_stock"; $sql .= " WHERE p.entity IN (".getEntity('product').")"; $sql .= " AND pl.fk_product = ".((int) $fk_product); if ($fk_warehouse > 0) { @@ -538,25 +537,25 @@ class Productbatch extends CommonObject if ($qty_min !== null) { $sql .= " AND pb.qty > ".((float) price2num($qty_min, 'MS')); } - $sql .= $db->order($sortfield, $sortorder); + $sql .= $this->db->order($sortfield, $sortorder); - $resql = $db->query($sql); + $resql = $this->db->query($sql); if ($resql) { - while ($obj = $db->fetch_object($resql)) { - $productBatch = new self($db); + while ($obj = $this->db->fetch_object($resql)) { + $productBatch = new self($this->db); $productBatch->id = $obj->rowid; $productBatch->fk_product = $obj->fk_product; $productBatch->batch = $obj->batch; - $productBatch->eatby = $db->jdate($obj->eatby); - $productBatch->sellby = $db->jdate($obj->sellby); + $productBatch->eatby = $this->db->jdate($obj->eatby); + $productBatch->sellby = $this->db->jdate($obj->sellby); $productBatch->qty = $obj->qty; $productBatchList[] = $productBatch; } - $db->free($resql); + $this->db->free($resql); return $productBatchList; } else { - dol_syslog(__METHOD__.' Error: '.$db->lasterror(), LOG_ERR); + dol_syslog(__METHOD__.' Error: '.$this->db->lasterror(), LOG_ERR); return -1; } } diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 9b0ef597a6f..47ac31d0547 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -213,8 +213,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase 'multicurrency.class.php', 'productbatch.class.php', 'reception.class.php', - 'infobox.class.php', - 'link.class.php', + 'infobox.class.php' ))) { // Must not find $db-> $ok=true; @@ -231,10 +230,11 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase } } else { if (! in_array($file['name'], array( + 'objectline_view.tpl.php', 'extrafieldsinexport.inc.php', 'DolQueryCollector.php' ))) { - // Must must not found $this->db-> + // Must not found $this->db-> $ok=true; $matches=array(); // Check string $this->db-> into a non class.php file (it shoud be $db-> into such classes) @@ -244,7 +244,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase break; } //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found string $this->db-> in '.$file['relativename']); + $this->assertTrue($ok, 'Found string "$this->db->" in '.$file['relativename']); //exit; } }