From 98789f28bbf5000dc00b507861aa8b22db56743a Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Tue, 22 Sep 2020 12:07:05 +0200 Subject: [PATCH] =?UTF-8?q?N=C2=B03198=20Relations=20Table=20Mode=20:=20co?= =?UTF-8?q?ntrol=20duplicates=20server=20side?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/attributedef.class.inc.php | 17 ++--- core/dbobject.class.php | 86 +++++++++++++++++++----- dictionaries/en.dictionary.itop.core.php | 2 + dictionaries/fr.dictionary.itop.core.php | 2 + 4 files changed, 79 insertions(+), 28 deletions(-) diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index 731ac9688..9bacfd819 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -295,7 +295,7 @@ abstract class AttributeDefinition * @param \DBObject $oHostObject * @param $value Object error if any, null otherwise * - * @return bool + * @return bool|string true for no errors, false or error message otherwise */ public function CheckValue(DBObject $oHostObject, $value) { @@ -2273,22 +2273,17 @@ class AttributeLinkedSetIndirect extends AttributeLinkedSet /** @var \AttributeExternalKey $oExtKeyToRemote */ $oExtKeyToRemote = MetaModel::GetAttributeDef($this->GetLinkedClass(), $this->GetExtKeyToRemote()); $sRemoteClass = $oExtKeyToRemote->GetTargetClass(); - foreach(MetaModel::ListAttributeDefs($sRemoteClass) as $sRemoteAttCode => $oRemoteAttDef) - { - if (!$oRemoteAttDef instanceof AttributeLinkedSetIndirect) - { + foreach(MetaModel::ListAttributeDefs($sRemoteClass) as $sRemoteAttCode => $oRemoteAttDef) { + if (!$oRemoteAttDef instanceof AttributeLinkedSetIndirect) { continue; } - if ($oRemoteAttDef->GetLinkedClass() != $this->GetLinkedClass()) - { + if ($oRemoteAttDef->GetLinkedClass() != $this->GetLinkedClass()) { continue; } - if ($oRemoteAttDef->GetExtKeyToMe() != $this->GetExtKeyToRemote()) - { + if ($oRemoteAttDef->GetExtKeyToMe() != $this->GetExtKeyToRemote()) { continue; } - if ($oRemoteAttDef->GetExtKeyToRemote() != $this->GetExtKeyToMe()) - { + if ($oRemoteAttDef->GetExtKeyToRemote() != $this->GetExtKeyToMe()) { continue; } $oRet = $oRemoteAttDef; diff --git a/core/dbobject.class.php b/core/dbobject.class.php index 1f0c0877f..a12a0882e 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -1822,13 +1822,13 @@ abstract class DBObject implements iDisplay /** * Check if the given (or current) value is suitable for the attribute * - * @api - * @api-advanced - * - * @param string $sAttCode - * @param boolean|string $value true if successful, the error description otherwise + * @api + * @api-advanced * - * @return bool|string + * @param string $sAttCode + * @param mixed $value if not passed, then will get value using Get method + * + * @return bool|string true if successful, the error description otherwise * @throws \ArchivedObjectException * @throws \CoreException * @throws \OQLException @@ -2108,23 +2108,75 @@ abstract class DBObject implements iDisplay } $aChildClassesWithRuleDisabled = MetaModel::GetChildClassesWithDisabledUniquenessRule($sRuleRootClass, $sUniquenessRuleId); - if (!empty($aChildClassesWithRuleDisabled)) - { + if (!empty($aChildClassesWithRuleDisabled)) { $oUniquenessQuery->AddConditionForInOperatorUsingParam('finalclass', $aChildClassesWithRuleDisabled, false); } return $oUniquenessQuery; } + /** + * @param string $sAttCode + * @param mixed $value + * + * @uses m_aCheckWarnings to log to user duplicates found + * + * @since 2.8.0 N°3198 check duplicates if necessary :
+ * Before we could only add or remove lnk using the uilinks widget. This widget has a filter based on existing values, and so + * forbids to add duplicates.
+ * Now we can modify existing entries using the extkey widget, and the widget doesn't have (yet !) such + * a filter. The widget will be updated later on, but as a first step we're checking for duplicates server side. This is a non + * blocking warning, as it was already possible to add duplicates by other means. + */ + protected function DoCheckLinkedSetDuplicates($sAttCode, $value) + { + $oAttDef = MetaModel::GetAttributeDef(get_class($this), $sAttCode); + + if (!($oAttDef instanceof AttributeLinkedSetIndirect)) { + return; + } + + if ($oAttDef->DuplicatesAllowed()) { + return true; + } + + /** @var \ormLinkSet $value */ + $aModifiedLnk = $value->ListModifiedLinks(); + $sExtKeyToRemote = $oAttDef->GetExtKeyToRemote(); + $aExistingRemotesId = $value->GetColumnAsArray($sExtKeyToRemote, true); + $aExistingRemotesFriendlyName = $value->GetColumnAsArray($sExtKeyToRemote.'_friendlyname', true); + $aDuplicatesFields = []; + foreach ($aModifiedLnk as $oModifiedLnk) { + $iModifiedLnkId = $oModifiedLnk->GetKey(); + $iModifiedLnkRemoteId = $oModifiedLnk->Get($sExtKeyToRemote); + $aExistingRemotesIdToCheck = array_filter($aExistingRemotesId, function ($iLnkKey) use ($iModifiedLnkId) { + return ($iLnkKey != $iModifiedLnkId); + }, ARRAY_FILTER_USE_KEY); + + if (!in_array($iModifiedLnkRemoteId, $aExistingRemotesIdToCheck, true)) { + continue; + } + + $iLnkId = $oModifiedLnk->GetKey(); + $aDuplicatesFields[] = $aExistingRemotesFriendlyName[$iLnkId]; + } + + if (!empty($aDuplicatesFields)) { + $this->m_aCheckWarnings[] = Dict::Format('Core:AttributeLinkedSetDuplicatesFound', + $oAttDef->GetLabel(), + implode(', ', $aDuplicatesFields)); + } + } + /** * Check integrity rules (before inserting or updating the object) * - * **This method is not meant to be called directly, use DBObject::CheckToWrite()!** + * **This method is not meant to be called directly, use DBObject::CheckToWrite()!** * Errors should be inserted in $m_aCheckIssues and $m_aCheckWarnings arrays - * - * @overwritable-hook You can extend this method in order to provide your own logic. - * @see CheckToWrite() - * @see $m_aCheckIssues + * + * @overwritable-hook You can extend this method in order to provide your own logic. + * @see CheckToWrite() + * @see $m_aCheckIssues * @see $m_aCheckWarnings * * @throws \ArchivedObjectException @@ -2140,14 +2192,14 @@ abstract class DBObject implements iDisplay $aChanges = $this->ListChanges(); - foreach($aChanges as $sAttCode => $value) - { + foreach($aChanges as $sAttCode => $value) { $res = $this->CheckValue($sAttCode); - if ($res !== true) - { + if ($res !== true) { // $res contains the error description $this->m_aCheckIssues[] = "Unexpected value for attribute '$sAttCode': $res"; } + + $this->DoCheckLinkedSetDuplicates($sAttCode, $value); } if (count($this->m_aCheckIssues) > 0) { diff --git a/dictionaries/en.dictionary.itop.core.php b/dictionaries/en.dictionary.itop.core.php index 7c7ef2973..c320f1a4f 100644 --- a/dictionaries/en.dictionary.itop.core.php +++ b/dictionaries/en.dictionary.itop.core.php @@ -33,6 +33,8 @@ Dict::Add('EN US', 'English', 'English', array( 'Core:AttributeLinkedSet' => 'Array of objects', 'Core:AttributeLinkedSet+' => 'Any kind of objects of the same class or subclass', + 'Core:AttributeLinkedSetDuplicatesFound' => 'Duplicates in the \'%1$s\' field : %2$s', + 'Core:AttributeDashboard' => 'Dashboard', 'Core:AttributeDashboard+' => '', diff --git a/dictionaries/fr.dictionary.itop.core.php b/dictionaries/fr.dictionary.itop.core.php index 0bea1b975..54146e648 100644 --- a/dictionaries/fr.dictionary.itop.core.php +++ b/dictionaries/fr.dictionary.itop.core.php @@ -31,6 +31,8 @@ Dict::Add('FR FR', 'French', 'Français', array( 'Core:AttributeLinkedSet' => 'Objets liés (1-n)', 'Core:AttributeLinkedSet+' => 'Liste d\'objets d\'une classe donnée et pointant sur l\'objet courant', + 'Core:AttributeLinkedSetDuplicatesFound' => 'Des doublons sont présents dans le champ \'%1$s\' : %2$s', + 'Core:AttributeDashboard' => 'Tableau de bord', 'Core:AttributeDashboard+' => '',