From 1f1a2b660f916a7b70c4e08a9d14696049563c9f Mon Sep 17 00:00:00 2001 From: Timothee Date: Fri, 21 Jun 2024 12:36:35 +0200 Subject: [PATCH 1/3] =?UTF-8?q?N=C2=B07581=20Improve=20error=20message=20r?= =?UTF-8?q?eadability=20during=20object=20creation/modification=20in=20the?= =?UTF-8?q?=20portal=20(regression=20introduced=20with=20N=C2=B07545)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../CoreCannotSaveObjectException.php | 18 ++++++++++++++++++ .../portal/src/Form/ObjectFormManager.php | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/application/exceptions/CoreCannotSaveObjectException.php b/application/exceptions/CoreCannotSaveObjectException.php index dcbe20ff5..82f8b1945 100644 --- a/application/exceptions/CoreCannotSaveObjectException.php +++ b/application/exceptions/CoreCannotSaveObjectException.php @@ -60,6 +60,24 @@ class CoreCannotSaveObjectException extends CoreException return $sContent; } + public function getTextMessage() + { + $sTitle = Dict::S('UI:Error:SaveFailed'); + $sContent = utils::HtmlEntities($sTitle); + + if (count($this->aIssues) == 1) { + $sIssue = reset($this->aIssues); + $sContent .= utils::HtmlEntities($sIssue); + } else { + foreach ($this->aIssues as $sError) { + $sContent .= " ".utils::HtmlEntities($sError).", "; + } + } + + return $sContent; + } + + public function getIssues() { return $this->aIssues; diff --git a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php index 3ffb2933e..915e229e3 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php @@ -1167,7 +1167,7 @@ class ObjectFormManager extends FormManager { $this->oObject->DBWrite(); } catch (CoreCannotSaveObjectException $e) { - throw new Exception($e->getHtmlMessage()); + throw new Exception($e->getTextMessage()); } catch (InvalidExternalKeyValueException $e) { ExceptionLog::LogException($e, $e->getContextData()); $bExceptionLogged = true; From 0904a21e3fdc2eca5d48a97a66072ae7a4108294 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 24 Jun 2024 11:50:20 +0200 Subject: [PATCH 2/3] :white_check_mark: Cleanup ItopTestCase --- .../src/BaseTestCase/ItopTestCase.php | 45 ++----------------- 1 file changed, 4 insertions(+), 41 deletions(-) diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 90df080d0..89e3aac85 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -85,23 +85,6 @@ abstract class ItopTestCase extends TestCase require_once APPROOT . $sFileRelPath; } - /** - * Helper to load a module file. The caller test must be in that module ! - * Will browse dir up to find a module.*.php - * - * @param string $sFileRelPath for example 'portal/src/Helper/ApplicationHelper.php' - * @since 2.7.10 3.1.1 3.2.0 N°6709 method creation - */ - protected function RequireOnceCurrentModuleFile(string $sFileRelPath): void - { - $aStack = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 1); - $sCallerFileFullPath = $aStack[0]['file']; - $sCallerDir = dirname($sCallerFileFullPath); - - $sModuleRootPath = static::GetFirstDirUpContainingFile($sCallerDir, 'module.*.php'); - require_once $sModuleRootPath . $sFileRelPath; - } - /** * Require once a unit test file (eg. a mock class) from its relative path from the *current* dir. * This ensure that required files don't crash when unit tests dir is moved in the iTop structure (see N°5608) @@ -119,26 +102,6 @@ abstract class ItopTestCase extends TestCase require_once $sCallerDirAbsPath . DIRECTORY_SEPARATOR . $sFileRelPath; } - private static function GetFirstDirUpContainingFile(string $sSearchPath, string $sFileToFindGlobPattern): ?string - { - for ($iDepth = 0; $iDepth < 8; $iDepth++) { - $aGlobFiles = glob($sSearchPath . '/' . $sFileToFindGlobPattern); - if (is_array($aGlobFiles) && (count($aGlobFiles) > 0)) { - return $sSearchPath . '/'; - } - $iOffsetSep = strrpos($sSearchPath, '/'); - if ($iOffsetSep === false) { - $iOffsetSep = strrpos($sSearchPath, '\\'); - if ($iOffsetSep === false) { - // Do not throw an exception here as PHPUnit will not show it clearly when determing the list of test to perform - return 'Could not find the approot file in ' . $sSearchPath; - } - } - $sSearchPath = substr($sSearchPath, 0, $iOffsetSep); - } - return null; - } - protected function debug($sMsg) { if (DEBUG_UNIT_TEST) { @@ -153,7 +116,7 @@ abstract class ItopTestCase extends TestCase public function GetMicroTime() { - list($uSec, $sec) = explode(" ", microtime()); + [$uSec, $sec] = explode(" ", microtime()); return ((float)$uSec + (float)$sec); } @@ -195,7 +158,7 @@ abstract class ItopTestCase extends TestCase /** * @param string $sObjectClass for example DBObject::class * @param string $sMethodName - * @param object $oObject + * @param object|null $oObject * @param array $aArgs * * @return mixed method result @@ -238,7 +201,7 @@ abstract class ItopTestCase extends TestCase * @throws \ReflectionException * @since 2.7.8 3.0.3 3.1.0 */ - public function GetNonPublicProperty(object $oObject, string $sProperty) + public function GetNonPublicProperty($oObject, string $sProperty) { $oProperty = $this->GetProperty(get_class($oObject), $sProperty); @@ -307,7 +270,7 @@ abstract class ItopTestCase extends TestCase * @throws \ReflectionException * @since 2.7.8 3.0.3 3.1.0 */ - public function SetNonPublicProperty(object $oObject, string $sProperty, $value) + public function SetNonPublicProperty($oObject, string $sProperty, $value) { $oProperty = $this->GetProperty(get_class($oObject), $sProperty); $oProperty->setValue($oObject, $value); From cceb6809e743f032a991ef9487a194af0a81f173 Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Mon, 24 Jun 2024 14:20:33 +0200 Subject: [PATCH 3/3] :white_check_mark: Fix CI --- .../src/BaseTestCase/ItopTestCase.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 7b7d40725..8573a81d9 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -169,6 +169,27 @@ abstract class ItopTestCase extends TestCase return $sAppRootPath . '/'; } + private static function GetFirstDirUpContainingFile(string $sSearchPath, string $sFileToFindGlobPattern): ?string + { + for ($iDepth = 0; $iDepth < 8; $iDepth++) { + $aGlobFiles = glob($sSearchPath . '/' . $sFileToFindGlobPattern); + if (is_array($aGlobFiles) && (count($aGlobFiles) > 0)) { + return $sSearchPath . '/'; + } + $iOffsetSep = strrpos($sSearchPath, '/'); + if ($iOffsetSep === false) { + $iOffsetSep = strrpos($sSearchPath, '\\'); + if ($iOffsetSep === false) { + // Do not throw an exception here as PHPUnit will not show it clearly when determing the list of test to perform + return 'Could not find the approot file in ' . $sSearchPath; + } + } + $sSearchPath = substr($sSearchPath, 0, $iOffsetSep); + } + return null; + } + + /** * Overload this method to require necessary files through {@see \Combodo\iTop\Test\UnitTest\ItopTestCase::RequireOnceItopFile()} *