From 9fe9eee18a55f09a32b7edca7e0a642bea553ca7 Mon Sep 17 00:00:00 2001 From: Alexandre Janniaux Date: Sat, 21 Oct 2023 16:28:30 +0200 Subject: [PATCH] FIX|Fix some minor issues on Reception and add a new test for it (#26310) * reception: reOpen: mirror $this->statut in $this->status $this->statut is the deprecated variable, $this->status should also get the correct value. * reception: setDraft: mirror $this->statut in $this->status $this->statut is the deprecated variable, $this->status should also get the correct value. * reception: add missing $weight field Fix warning: Undefined property: Reception::$weight * societe: add country_id field * reception: use getDolGlobalInt when suitable Using empty() implies that the value actually exists on the stdClass at $conf->global, but it's not always the case. getDolGlobalInt will handle this smoothly by checking first, which solves warnings like those: Undefined property: stdClass::$STOCK_CALCULATE_ON_RECEPTION * ReceptionTest: add new test The test checks the usual workflow of the Reception class, with, on the one hand, the common CRUD operations: - create - fetch - update - delete And on the other hand, the status handling for Reception: - valid: STATUS_DRAFT -> STATUS_VALID - setClosed: STATUS_VALID -> STATUS_CLOSED - reOpen: STATUS_CLOSED -> STATUS_VALID - setDraft: STATUS_VALID -> STATUS_DRAFT The stocks lines are not tested yet, and the error cases, like any other transition not described above, are not tested either. The permissions for some of the operations are hardcoded for the test and there is no failure check when the permission is not set yet. --- htdocs/reception/class/reception.class.php | 37 +- htdocs/societe/class/societe.class.php | 5 + test/phpunit/AllTests.php | 2 + test/phpunit/ReceptionTest.php | 383 +++++++++++++++++++++ 4 files changed, 410 insertions(+), 17 deletions(-) create mode 100644 test/phpunit/ReceptionTest.php diff --git a/htdocs/reception/class/reception.class.php b/htdocs/reception/class/reception.class.php index 39a4353a3c4..4e399444f89 100644 --- a/htdocs/reception/class/reception.class.php +++ b/htdocs/reception/class/reception.class.php @@ -83,6 +83,7 @@ class Reception extends CommonObject public $billed; public $model_pdf; + public $weight; public $trueWeight; public $weight_units; public $trueWidth; @@ -503,8 +504,8 @@ class Reception extends CommonObject return 0; } - if (!((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->reception->creer)) - || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->reception->reception_advance->validate)))) { + if (!((!getDolGlobalInt('MAIN_USE_ADVANCED_PERMS') && !empty($user->rights->reception->creer)) + || (getDolGlobalInt('MAIN_USE_ADVANCED_PERMS') && !empty($user->rights->reception->reception_advance->validate)))) { $this->error = 'Permission denied'; dol_syslog(get_class($this)."::valid ".$this->error, LOG_ERR); return -1; @@ -545,7 +546,7 @@ class Reception extends CommonObject } // If stock increment is done on reception (recommanded choice) - if (!$error && isModEnabled('stock') && !empty($conf->global->STOCK_CALCULATE_ON_RECEPTION)) { + if (!$error && isModEnabled('stock') && getDolGlobalInt('STOCK_CALCULATE_ON_RECEPTION')) { require_once DOL_DOCUMENT_ROOT.'/product/stock/class/mouvementstock.class.php'; $langs->load("agenda"); @@ -730,7 +731,7 @@ class Reception extends CommonObject $supplierorderdispatch = new CommandeFournisseurDispatch($this->db); $filter = array('t.fk_commande'=>$this->origin_id); - if (!empty($conf->global->SUPPLIER_ORDER_USE_DISPATCH_STATUS)) { + if (getDolGlobalInt('SUPPLIER_ORDER_USE_DISPATCH_STATUS')) { $filter['t.status'] = 1; // Restrict to lines with status validated } @@ -750,7 +751,7 @@ class Reception extends CommonObject // qty wished in order supplier (origin) foreach ($this->commandeFournisseur->lines as $origin_line) { // exclude lines not qualified for reception - if (empty($conf->global->STOCK_SUPPORTS_SERVICES) && $origin_line->product_type > 0) { + if (!getDolGlobalInt('STOCK_SUPPORTS_SERVICES') && $origin_line->product_type > 0) { continue; } @@ -764,7 +765,7 @@ class Reception extends CommonObject if (count($diff_array) == 0 && count($keys_in_wished_not_in_received) == 0 && count($keys_in_received_not_in_wished) == 0) { // no diff => mean everything is received $status = CommandeFournisseur::STATUS_RECEIVED_COMPLETELY; - } elseif (!empty($conf->global->SUPPLIER_ORDER_MORE_THAN_WISHED)) { + } elseif (getDolGlobalInt('SUPPLIER_ORDER_MORE_THAN_WISHED')) { // set totally received if more products received than ordered $close = 0; @@ -829,7 +830,7 @@ class Reception extends CommonObject if (isModEnabled('stock') && !empty($supplierorderline->fk_product)) { $fk_product = $supplierorderline->fk_product; - if (!($entrepot_id > 0) && empty($conf->global->STOCK_WAREHOUSE_NOT_REQUIRED_FOR_RECEPTIONS)) { + if (!($entrepot_id > 0) && !getDolGlobalInt('STOCK_WAREHOUSE_NOT_REQUIRED_FOR_RECEPTIONS')) { $langs->load("errors"); $this->error = $langs->trans("ErrorWarehouseRequiredIntoReceptionLine"); return -1; @@ -853,7 +854,7 @@ class Reception extends CommonObject // extrafields $line->array_options = $supplierorderline->array_options; - if (empty($conf->global->MAIN_EXTRAFIELDS_DISABLED) && is_array($array_options) && count($array_options) > 0) { + if (!getDolGlobalInt('MAIN_EXTRAFIELDS_DISABLED') && is_array($array_options) && count($array_options) > 0) { foreach ($array_options as $key => $value) { $line->array_options[$key] = $value; } @@ -1027,7 +1028,7 @@ class Reception extends CommonObject $this->db->begin(); // Stock control - if (isModEnabled('stock') && $conf->global->STOCK_CALCULATE_ON_RECEPTION && $this->statut > 0) { + if (isModEnabled('stock') && !getDolGlobalInt('STOCK_CALCULATE_ON_RECEPTION') && $this->statut > 0) { require_once DOL_DOCUMENT_ROOT."/product/stock/class/mouvementstock.class.php"; $langs->load("agenda"); @@ -1250,7 +1251,7 @@ class Reception extends CommonObject $linkclose = ''; if (empty($notooltip)) { - if (!empty($conf->global->MAIN_OPTIMIZEFORTEXTBROWSER)) { + if (getDolGlobalInt('MAIN_OPTIMIZEFORTEXTBROWSER')) { $label = $langs->trans("Reception"); $linkclose .= ' alt="'.dol_escape_htmltag($label, 1).'"'; } @@ -1556,7 +1557,7 @@ class Reception extends CommonObject foreach ($order->lines as $line) { $lineid = $line->id; $qty = $line->qty; - if (($line->product_type == 0 || !empty($conf->global->STOCK_SUPPORTS_SERVICES)) && $order->receptions[$lineid] < $qty) { + if (($line->product_type == 0 || getDolGlobalInt('STOCK_SUPPORTS_SERVICES')) && $order->receptions[$lineid] < $qty) { $receptions_match_order = 0; $text = 'Qty for order line id '.$lineid.' is '.$qty.'. However in the receptions with status Reception::STATUS_CLOSED='.self::STATUS_CLOSED.' we have qty = '.$order->receptions[$lineid].', so we can t close order'; dol_syslog($text); @@ -1573,7 +1574,7 @@ class Reception extends CommonObject $this->status = self::STATUS_CLOSED; // If stock increment is done on closing - if (!$error && isModEnabled('stock') && !empty($conf->global->STOCK_CALCULATE_ON_RECEPTION_CLOSE)) { + if (!$error && isModEnabled('stock') && getDolGlobalInt('STOCK_CALCULATE_ON_RECEPTION_CLOSE')) { require_once DOL_DOCUMENT_ROOT.'/product/stock/class/mouvementstock.class.php'; $langs->load("agenda"); @@ -1722,11 +1723,12 @@ class Reception extends CommonObject $resql = $this->db->query($sql); if ($resql) { - $this->statut = 1; + $this->statut = self::STATUS_VALIDATED; + $this->status = self::STATUS_VALIDATED; $this->billed = 0; // If stock increment is done on closing - if (!$error && isModEnabled('stock') && !empty($conf->global->STOCK_CALCULATE_ON_RECEPTION_CLOSE)) { + if (!$error && isModEnabled('stock') && getDolGlobalInt('STOCK_CALCULATE_ON_RECEPTION_CLOSE')) { require_once DOL_DOCUMENT_ROOT.'/product/stock/class/mouvementstock.class.php'; $numref = $this->ref; $langs->load("agenda"); @@ -1843,8 +1845,8 @@ class Reception extends CommonObject return 0; } - if (!((empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->reception->creer)) - || (!empty($conf->global->MAIN_USE_ADVANCED_PERMS) && !empty($user->rights->reception->reception_advance->validate)))) { + if (!((!getDolGlobalInt('MAIN_USE_ADVANCED_PERMS') && !empty($user->rights->reception->creer)) + || (getDolGlobalInt('MAIN_USE_ADVANCED_PERMS') && !empty($user->rights->reception->reception_advance->validate)))) { $this->error = 'Permission denied'; return -1; } @@ -1858,7 +1860,7 @@ class Reception extends CommonObject dol_syslog(__METHOD__, LOG_DEBUG); if ($this->db->query($sql)) { // If stock increment is done on closing - if (!$error && isModEnabled('stock') && !empty($conf->global->STOCK_CALCULATE_ON_RECEPTION)) { + if (!$error && isModEnabled('stock') && getDolGlobalInt('STOCK_CALCULATE_ON_RECEPTION')) { require_once DOL_DOCUMENT_ROOT.'/product/stock/class/mouvementstock.class.php'; $langs->load("agenda"); @@ -1958,6 +1960,7 @@ class Reception extends CommonObject if (!$error) { $this->statut = self::STATUS_DRAFT; + $this->status = self::STATUS_DRAFT; $this->db->commit(); return 1; } else { diff --git a/htdocs/societe/class/societe.class.php b/htdocs/societe/class/societe.class.php index cca0c5abdb5..0a415146b21 100644 --- a/htdocs/societe/class/societe.class.php +++ b/htdocs/societe/class/societe.class.php @@ -320,6 +320,11 @@ class Societe extends CommonObject */ public $region; + /** + * @var int country_id + */ + public $country_id; + /** * @var string State code * @deprecated Use state_code instead diff --git a/test/phpunit/AllTests.php b/test/phpunit/AllTests.php index 4d1ae67052c..6deaafd7707 100644 --- a/test/phpunit/AllTests.php +++ b/test/phpunit/AllTests.php @@ -140,6 +140,8 @@ class AllTests $suite->addTestSuite('ActionCommTest'); require_once dirname(__FILE__).'/SocieteTest.php'; $suite->addTestSuite('SocieteTest'); + require_once dirname(__FILE__).'/ReceptionTest.php'; + $suite->addTestSuite('ReceptionTest'); require_once dirname(__FILE__).'/ContactTest.php'; $suite->addTestSuite('ContactTest'); require_once dirname(__FILE__).'/AdherentTest.php'; diff --git a/test/phpunit/ReceptionTest.php b/test/phpunit/ReceptionTest.php new file mode 100644 index 00000000000..01034f397ce --- /dev/null +++ b/test/phpunit/ReceptionTest.php @@ -0,0 +1,383 @@ + + * + * 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 https://www.gnu.org/ + */ + +/** + * \file test/phpunit/ReceptionTest.php + * \ingroup test + * \brief PHPUnit test for the Reception code + * \remarks To run this script as CLI: phpunit filename.php + */ + +global $conf,$user,$langs,$db; + +require_once dirname(__FILE__).'/../../htdocs/master.inc.php'; +require_once dirname(__FILE__).'/../../htdocs/societe/class/societe.class.php'; +require_once dirname(__FILE__).'/../../htdocs/reception/class/reception.class.php'; +$langs->load("dict"); + +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 ReceptionTest extends PHPUnit\Framework\TestCase +{ + protected $savconf; + protected $savuser; + protected $savlangs; + protected $savdb; + + /** + * Constructor + * We save global variables into local variables + * + * @param string $name Name + * @return SocieteTest + */ + public function __construct($name = '') + { + parent::__construct($name); + + //$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"; + } + + /** + * setUpBeforeClass + * + * @return void + */ + public static function setUpBeforeClass(): void + { + 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"; + } + + /** + * tearDownAfterClass + * + * @return void + */ + public static function tearDownAfterClass(): void + { + global $conf,$user,$langs,$db; + $db->rollback(); + + print __METHOD__."\n"; + } + + /** + * Init phpunit tests + * + * @return void + */ + protected function setUp(): void + { + 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(): void + { + print __METHOD__."\n"; + } + + /** + * testSocieteCreate + * + * @return int + */ + public function testReceptionCreate() + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + $soc = new Societe($db); + $soc->name = "ReceptionTest Unittest"; + $soc_id = $soc->create($user); + $this->assertLessThanOrEqual($soc_id, 0, + "Cannot create Societe object: ". + $soc->errorsToString() + ); + + $localobject = new Reception($db); + $localobject->socid = $soc_id; + $result = $localobject->create($user); + $this->assertLessThanOrEqual($result, 0, "Cannot create Reception object:\n". + $localobject->errorsToString()); + return $result; + } + + /** + * testReceptionFetch + * + * Check that a Reception object can be fetched from database. + * + * @param $id The id of an existing Reception object to fetch. + * + * @depends testReceptionCreate + * @return Reception $localobject + */ + public function testReceptionFetch($id) + { + global $db; + + $localobject = new Reception($db); + $result = $localobject->fetch($id); + print __METHOD__." id=".$id." result=".$result."\n"; + $this->assertLessThan($result, 0); + + return $localobject; + } + + /** + * testReceptionUpdate + * + * Check that a Reception object can be updated. + * + * @param $localobject An existing Reception object to update. + * + * @depends testReceptionFetch + * @return Reception a Reception object with data fetched and name changed + */ + public function testReceptionUpdate($localobject) + { + global $user; + + $localobject->name = "foobar"; + + $result = $localobject->update($localobject->id, $user); + print __METHOD__." id=".$localobject->id." result=".$result."\n"; + $this->assertLessThan($result, 0, $localobject->errorsToString()); + + return $localobject; + } + + /** + * testReceptionValid + * + * Check that a Reception with status == Reception::STATUS_DRAFT can be + * re-opened with the Reception::reOpen() function. + * + * @param $localobject An existing Reception object to validate. + * + * @depends testReceptionUpdate + * @return Reception a Reception object with data fetched and STATUS_VALIDATED + */ + public function testReceptionValid($localobject) + { + global $db, $user, $conf; + + $conf->global->MAIN_USE_ADVANCED_PERMS = ''; + $user->rights->reception = new stdClass; + $user->rights->reception->creer = 1; + + $result = $user->fetch($user->id); + $this->assertLessThan($result, 0, $user->errorsToString()); + + $result = $localobject->fetch($localobject->id); + $this->assertLessThan($result, 0, $localobject->errorsToString()); + $this->assertEquals(Reception::STATUS_DRAFT, $localobject->statut); + $this->assertEquals(Reception::STATUS_DRAFT, $localobject->status); + + $result = $localobject->valid($user); + print __METHOD__." id=".$localobject->id." result=".$result."\n"; + $this->assertLessThan($result, 0, $localobject->errorsToString()); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->statut); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->status); + + $obj = new Reception($db); + $obj->fetch($localobject->id); + $this->assertEquals(Reception::STATUS_VALIDATED, $obj->statut); + $this->assertEquals(Reception::STATUS_VALIDATED, $obj->status); + return $obj; + } + + /** + * testReceptionSetClosed + * + * Check that a Reception can be closed with the Reception::setClosed() + * function, after it has been validated. + * + * @param $localobject An existing validated Reception object to close. + * + * @depends testReceptionValid + * @return Reception a Reception object with data fetched and STATUS_CLOSED + */ + public function testReceptionSetClosed($localobject) + { + global $db, $user; + + $result = $localobject->fetch($localobject->id); + $this->assertLessThanOrEqual($result, 0, "Cannot fetch Reception object:\n". + $localobject->errorsToString()); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->statut); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->status); + + $result = $localobject->setClosed($user); + $this->assertLessThanOrEqual($result, 0, "Cannot close Reception object:\n". + $localobject->errorsToString()); + $this->assertEquals(Reception::STATUS_CLOSED, $localobject->status, + "Checking that \$localobject->status is STATUS_CLOSED"); + $this->assertEquals(Reception::STATUS_CLOSED, $localobject->statut, + "Checking that \$localobject->statut is STATUS_CLOSED"); + + $obj = new Reception($db); + $result = $obj->fetch($localobject->id); + $this->assertLessThanOrEqual($result, 0, "Cannot fetch Reception object:\n". + $obj->errorsToString()); + $this->assertEquals(Reception::STATUS_CLOSED, $obj->status, + "Checking that \$obj->status is STATUS_CLOSED"); + $this->assertEquals(Reception::STATUS_CLOSED, $obj->statut, + "Checking that \$obj->statut is STATUS_CLOSED"); + + return $obj; + } + + /** + * testReceptionReOpen + * + * Check that a Reception with status == Reception::STATUS_CLOSED can be + * re-opened with the Reception::reOpen() function. + * + * @param $localobject An existing closed Reception object to re-open. + * + * @depends testReceptionSetClosed + * @return Reception a Reception object with data fetched and STATUS_VALIDATED + */ + public function testReceptionReOpen($localobject) + { + global $db; + + $result = $localobject->fetch($localobject->id); + $this->assertLessThanOrEqual($result, 0, "Cannot fetch Reception object:\n". + $localobject->errorsToString()); + + $this->assertEquals(Reception::STATUS_CLOSED, $localobject->status); + $this->assertEquals(Reception::STATUS_CLOSED, $localobject->statut); + + $result = $localobject->reOpen(); + $this->assertLessThanOrEqual($result, 0, "Cannot reOpen Reception object:\n". + $localobject->errorsToString()); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->statut); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->status); + + $obj = new Reception($db); + $obj->fetch($localobject->id); + $this->assertEquals(Reception::STATUS_VALIDATED, $obj->statut); + $this->assertEquals(Reception::STATUS_VALIDATED, $obj->status); + + return $obj; + } + + /** + * testReceptionSetDraft + * + * Check that a Reception with status == Reception::STATUS_CLOSED can be + * re-opened with the Reception::reOpen() function. + * + * @param $localobject An existing validated Reception object to mark as Draft. + * + * @depends testReceptionReOpen + * @return Reception a Reception object with data fetched and STATUS_DRAFT + */ + public function testReceptionSetDraft($localobject) + { + global $db, $user, $conf; + + //$conf->global->MAIN_USE_ADVANCED_PERMS = 1; + //$user->rights->reception->creer = 1; + //$user->rights->reception_advance->validate = 1; + + $result = $localobject->fetch($localobject->id); + $this->assertLessThan($result, 0); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->statut); + $this->assertEquals(Reception::STATUS_VALIDATED, $localobject->status); + + $result = $localobject->setDraft($user); + $this->assertLessThanOrEqual($result, 0, "Cannot setDraft on Reception object:\n". + $localobject->errorsToString()); + $this->assertEquals(Reception::STATUS_DRAFT, $localobject->statut); + $this->assertEquals(Reception::STATUS_DRAFT, $localobject->status); + + $obj = new Reception($db); + $obj->fetch($localobject->id); + $this->assertEquals(Reception::STATUS_DRAFT, $obj->statut); + $this->assertEquals(Reception::STATUS_DRAFT, $obj->status); + + return $obj; + } + + /** + * testReceptionDelete + * + * Check that a Reception object can be deleted. + * + * @param $localobject An existing Reception object to delete. + * + * @depends testReceptionReOpen + * @return int the result of the delete operation + */ + public function testReceptionDelete($localobject) + { + global $db, $user; + + $result = $localobject->delete($user); + print __METHOD__." id=".$localobject->id." result=".$result."\n"; + $this->assertLessThanOrEqual($result, 0); + + $soc = new Societe($db); + $result = $soc->delete($localobject->socid); + $this->assertLessThanOrEqual($result, 0); + + return $result; + } +}