N°1150 AtributeCustomFields : FromJSONToValue is now delegated to the handler

Method was always returning null before

core/attributedef.class.inc.php

@@ -801,8 +801,8 @@ abstract class AttributeDefinition
 	/**
 	 * force an allowed value (type conversion and possibly forces a value as mySQL would do upon writing!
 	 *
-	 * @param $proposedValue
-	 * @param $oHostObj
+	 * @param mixed $proposedValue
+	 * @param \DBObject $oHostObj
 	 *
 	 * @return mixed
 	 */
@@ -992,17 +992,21 @@ abstract class AttributeDefinition
 	}
 
 	/**
-	 * Helper to form a value, given JSON decoded data
+	 * Helper to form a value, given JSON decoded data. This way the attribute itself handles the transformation from the JSON structure to the expected data (the one that
+	 * needs to be used in the {@see \DBObject::Set()} method).
+	 *
+	 * Note that for CSV and XML this isn't done yet (no delegation to the attribute but switch/case inside controllers) :/
+	 *
+	 * @see GetForJSON for the reverse operation
 	 *
 	 * @param string $json JSON encoded value
 	 *
-	 * @return mixed JSON decoded data
+	 * @return mixed JSON decoded data, depending on the attribute type
 	 *
-	 * @see GetForJSON for the reverse operation
 	 */
 	public function FromJSONToValue($json)
 	{
-		// Passthrough in most of the cases
+		// Pass-through in most of the cases
 		return $json;
 	}
 
@@ -13048,6 +13052,7 @@ class AttributeCustomFields extends AttributeDefinition
 	public function GetHandler($aValues = null)
 	{
 		$sHandlerClass = $this->Get('handler_class');
+		/** @var \TemplateFieldsHandler $oHandler */
 		$oHandler = new $sHandlerClass($this->GetCode());
 		if (!is_null($aValues))
 		{
@@ -13072,36 +13077,50 @@ class AttributeCustomFields extends AttributeDefinition
 	/**
 	 * Makes the string representation out of the values given by the form defined in GetDisplayForm
 	 */
-	public function ReadValueFromPostedForm($oHostObject, $sFormPrefix)
-	{
-		$aRawData = json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$this->GetCode()}", '{}', 'raw_data'),	true);
+	public function ReadValueFromPostedForm($oHostObject, $sFormPrefix) {
+		$aRawData = json_decode(utils::ReadPostedParam("attr_{$sFormPrefix}{$this->GetCode()}", '{}', 'raw_data'), true);
 		if ($aRawData != null) {
 			return new ormCustomFieldsValue($oHostObject, $this->GetCode(), $aRawData);
-		} else{
+		}
+		else {
 			return null;
 		}
 	}
 
-	public function MakeRealValue($proposedValue, $oHostObject)
-	{
-		if (is_object($proposedValue) && ($proposedValue instanceof ormCustomFieldsValue))
-		{
+	public function MakeRealValue($proposedValue, $oHostObject) {
+		if (is_object($proposedValue) && ($proposedValue instanceof ormCustomFieldsValue)) {
+			if (false === $oHostObject->IsNew()) {
+				// In that case we need additional keys : see \TemplateFieldsHandler::DoBuildForm
+				$aRequestTemplateValues = $proposedValue->GetValues();
+				if (false === array_key_exists('current_template_id', $aRequestTemplateValues)) {
+					$aRequestTemplateValues['current_template_id'] = $aRequestTemplateValues['template_id'];
+					$aRequestTemplateValues['current_template_data'] = $aRequestTemplateValues['template_data'];
+					$proposedValue = new ormCustomFieldsValue($oHostObject, $this->GetCode(), $aRequestTemplateValues);
+				}
+			}
+
+			if (is_null($proposedValue->GetHostObject())) {
+				// the object might not be set : for example in \AttributeCustomFields::FromJSONToValue we don't have the object available :(
+				$proposedValue->SetHostObject($oHostObject);
+			}
+
 			return $proposedValue;
 		}
-		elseif (is_string($proposedValue))
-		{
+
+		if (is_string($proposedValue)) {
 			$aValues = json_decode($proposedValue, true);
 
 			return new ormCustomFieldsValue($oHostObject, $this->GetCode(), $aValues);
 		}
-		elseif (is_array($proposedValue))
-		{
+
+		if (is_array($proposedValue)) {
 			return new ormCustomFieldsValue($oHostObject, $this->GetCode(), $proposedValue);
 		}
-		elseif (is_null($proposedValue))
-		{
+
+		if (is_null($proposedValue)) {
 			return new ormCustomFieldsValue($oHostObject, $this->GetCode());
 		}
+
 		throw new Exception('Unexpected type for the value of a custom fields attribute: '.gettype($proposedValue));
 	}
 
@@ -13388,11 +13407,13 @@ class AttributeCustomFields extends AttributeDefinition
 	/**
 	 * @inheritDoc
 	 *
-	 * @return ?\ormCustomFieldsValue
+	 * @return ?\ormCustomFieldsValue with empty host object as we don't have it here (most consumers don't have an object in their context, for example in \RestUtils::GetObjectSetFromKey)
+	 *                  The host object will be set in {@see MakeRealValue}
+	 *                  All the necessary checks will be done in {@see CheckValue}
 	 */
 	public function FromJSONToValue($json)
 	{
-		return null;
+		return ormCustomFieldsValue::FromJSONToValue($json, $this);
 	}
 
 	public function Equals($val1, $val2)

core/customfieldshandler.class.inc.php

@@ -23,10 +23,22 @@
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
-abstract class CustomFieldsHandler
-{
+abstract class CustomFieldsHandler {
+	/** @var string $sAttCode */
 	protected $sAttCode;
+	/** @var array{
+	 *          legacy: int,
+	 *          extradata_id: string,
+	 *          _template_name: string,
+	 *          template_id: string,
+	 *          template_data: string,
+	 *          user_data: array<string, mixed>,
+	 *          current_template_id: string,
+	 *          current_template_data: string,
+	 *     } $aValues same as {@see \ormCustomFieldsValue::$aCurrentValues}
+	 */
 	protected $aValues;
+	/** @var \Combodo\iTop\Form\Form $oForm */
 	protected $oForm;
 
 	/**
@@ -118,6 +130,21 @@ abstract class CustomFieldsHandler
 		return null;
 	}
 
+	/**
+	 * @param \stdClass|null $json
+	 * @param string $sAttCode
+	 *
+	 * @return \ormCustomFieldsValue|null
+	 *
+	 * @since 3.1.0 N°1150 Method creation
+	 */
+	public function FromJSONToValue(?stdClass $json, string $sAttCode): ?ormCustomFieldsValue
+	{
+		// Default impl doing nothing, to avoid errors on children not having this method
+		return null;
+	}
+
+
 	/**
 	 * @param DBObject $oHostObject
 	 *

core/dbobjectsearch.class.php

@@ -400,7 +400,7 @@ class DBObjectSearch extends DBSearch
 	}
 
 	/**
-	 * Important: If you need to add a condition on the same $sFilterCode several times with different $value values; do not use this method as the previous $value occurences will be replaced by the last. Instead use:
+	 * Important: If you need to add a condition on the same $sFilterCode several times with different $value values; do not use this method as the previous $value occurrences will be replaced by the last. Instead use:
 	 *  * {@see \DBObjectSearch::AddConditionExpression()} in loops to add conditions one by one
 	 *  * {@see \DBObjectSearch::AddConditionForInOperatorUsingParam()} for IN/NOT IN queries with lots of params at once
 	 *

core/ormcustomfieldsvalue.class.inc.php

@@ -35,10 +35,11 @@ class ormCustomFieldsValue
 	 *          _template_name: string,
 	 *          template_id: string,
 	 *          template_data: string,
-	 *          user_data: string,
+	 *          user_data: array<string, mixed>,
 	 *          current_template_id: string,
 	 *          current_template_data: string,
-	 *     } $aCurrentValues Containing JSON encoded strings in template_data/current_template_data, user_data.
+	 *     } $aCurrentValues Containing JSON encoded strings in template_data/current_template_data.
+	 *          The user_data key contains an array with field code as key and field value as value
 	 *          Warning, current_* are mandatory for data to be saved in a DBUpdate() call !
 	 */
 	protected $aCurrentValues;
@@ -48,13 +49,31 @@ class ormCustomFieldsValue
 	 * @param string $sAttCode
 	 * @param array $aCurrentValues
 	 */
-	public function __construct(DBObject $oHostObject, $sAttCode, $aCurrentValues = null)
+	public function __construct(?DBObject $oHostObject, $sAttCode, $aCurrentValues = null)
 	{
 		$this->oHostObject = $oHostObject;
 		$this->sAttCode = $sAttCode;
 		$this->aCurrentValues = $aCurrentValues;
 	}
 
+	/**
+	 * @return \DBObject|null
+	 */
+	public function GetHostObject(): ?DBObject
+	{
+		return $this->oHostObject;
+	}
+
+	/**
+	 * @param \DBObject|null $oHostObject
+	 *
+	 * @return void
+	 */
+	public function SetHostObject(?DBObject $oHostObject): void
+	{
+		$this->oHostObject = $oHostObject;
+	}
+
 	public function GetValues()
 	{
 		return $this->aCurrentValues;
@@ -62,6 +81,7 @@ class ormCustomFieldsValue
 
 	/**
 	 * Wrapper used when the only thing you have is the value...
+	 *
 	 * @return \Combodo\iTop\Form\Form
 	 */
 	public function GetForm($sFormPrefix = null)
@@ -96,6 +116,19 @@ class ormCustomFieldsValue
 		return $this->GetHandler()->GetAsJSON($this->aCurrentValues);
 	}
 
+	/**
+	 * @param string|null $json
+	 * @param \AttributeDefinition $oAttDef
+	 *
+	 * @return \ormCustomFieldsValue
+	 *
+	 * @since 3.1.0 N°1150 Method creation
+	 */
+	public static function FromJSONToValue(?stdClass $json, AttributeCustomFields $oAttDef)
+	{
+		return $oAttDef->GetHandler()->FromJSONToValue($json, $oAttDef->GetCode());
+	}
+
 	/**
 	 * @return \CustomFieldsHandler
 	 * @throws \Exception
@@ -103,6 +136,7 @@ class ormCustomFieldsValue
 	 */
 	final protected function GetHandler()
 	{
+		/** @var \AttributeCustomFields $oAttDef */
 		$oAttDef = MetaModel::GetAttributeDef(get_class($this->oHostObject), $this->sAttCode);
 
 		return $oAttDef->GetHandler($this->GetValues());

tests/php-unit-tests/ItopDataTestCase.php

@@ -48,6 +48,7 @@ use Server;
 use TagSetFieldData;
 use Ticket;
 use URP_UserProfile;
+use UserRequest;
 use VirtualHost;
 use VirtualMachine;
 use XMLDataLoader;
@@ -328,7 +329,7 @@ class ItopDataTestCase extends ItopTestCase
 		$aUserRequestParams = array_merge($aUserRequestDefaultParams, $aUserRequestCustomParams);
 
 		/** @var \UserRequest $oTicket */
-		$oTicket = $this->createObject('UserRequest', $aUserRequestParams);
+		$oTicket = $this->createObject(UserRequest::class, $aUserRequestParams);
 		$this->debug("Created {$oTicket->Get('title')} ({$oTicket->Get('ref')})");
 
 		return $oTicket;