API: Form#loadDataFrom 2nd arg now sets how existing field data is merged with new data

forms/Form.php

@@ -1080,6 +1080,10 @@ class Form extends RequestHandler {
 		return true;
 	}
 
+	const MERGE_DEFAULT = 0;
+	const MERGE_CLEAR_MISSING = 1;
+	const MERGE_IGNORE_FALSEISH = 2;
+
 	/**
 	 * Load data from the given DataObject or array.
 	 * It will call $object->MyField to get the value of MyField.
@@ -1098,20 +1102,43 @@ class Form extends RequestHandler {
 	 * @uses FormField->setValue()
 	 * 
 	 * @param array|DataObject $data
-	 * @param boolean $clearMissingFields By default, fields which don't match
+	 * @param int $mergeStrategy
-	 *  a property or array-key of the passed {@link $data} argument are "left alone",
+	 *  For every field, {@link $data} is interogated whether it contains a relevant property/key, and
-	 *  meaning they retain any previous values (if present). If this flag is set to true,
+	 *  what that property/key's value is.
-	 *  those fields are overwritten with null regardless if they have a match in {@link $data}.
+	 *
-	 * @param $fieldList An optional list of fields to process.  This can be useful when you have a 
+	 *  By default, if {@link $data} does contain a property/key, the fields value is always replaced by {@link $data}'s
+	 *  value, even if that value is null/false/etc. Fields which don't match any property/key in {@link $data} are
+	 *  "left alone", meaning they retain any previous value.
+	 *
+	 *  You can pass a bitmask here to change this behaviour.
+	 *
+	 *  Passing CLEAR_MISSING means that any fields that don't match any property/key in
+	 *  {@link $data} are cleared.
+	 *
+	 *  Passing IGNORE_FALSEISH means that any false-ish value in {@link $data} won't replace
+	 *  a field's value.
+	 *
+	 *  For backwards compatibility reasons, this parameter can also be set to === true, which is the same as passing
+	 *  CLEAR_MISSING
+	 *
+	 * @param $fieldList An optional list of fields to process.  This can be useful when you have a
 	 * form that has some fields that save to one object, and some that save to another.
 	 * @return Form
 	 */
-	public function loadDataFrom($data, $clearMissingFields = false, $fieldList = null) {
+	public function loadDataFrom($data, $mergeStrategy = 0, $fieldList = null) {
 		if(!is_object($data) && !is_array($data)) {
 			user_error("Form::loadDataFrom() not passed an array or an object", E_USER_WARNING);
 			return $this;
 		}
 
+		// Handle the backwards compatible case of passing "true" as the second argument
+		if ($mergeStrategy === true) {
+			$mergeStrategy = self::MERGE_CLEAR_MISSING;
+		}
+		else if ($mergeStrategy === false) {
+			$mergeStrategy = 0;
+		}
+
 		// if an object is passed, save it for historical reference through {@link getRecord()}
 		if(is_object($data)) $this->record = $data;
 
@@ -1125,37 +1152,50 @@ class Form extends RequestHandler {
 			
 			// First check looks for (fieldname)_unchanged, an indicator that we shouldn't overwrite the field value
 			if(is_array($data) && isset($data[$name . '_unchanged'])) continue;
-			
+
-			// get value in different formats
+			// Does this property exist on $data?
-			$hasObjectValue = false;
+			$exists = false;
-			if(
+			// The value from $data for this field
-				is_object($data) 
+			$val = null;
-				&& (
+
-					isset($data->$name)
+			if(is_object($data)) {
-					|| $data->hasMethod($name)
+				$exists = (
-					|| ($data->hasMethod('hasField') && $data->hasField($name))
+					isset($data->$name) ||
-				)
+					$data->hasMethod($name) ||
-			) {
+					($data->hasMethod('hasField') && $data->hasField($name))
-				// We don't actually call the method because it might be slow.  
+				);
-				// In a later release, relation methods will just return references to the query that should be
+
-				// executed, and so we will be able to safely pass the return value of the relation method to the
+				if ($exists) {
-				// first argument of setValue
+					$val = $data->__get($name);
-				$val = $data->__get($name);
+				}
-				$hasObjectValue = true;
+			}
-			} else if(strpos($name,'[') && is_array($data) && !isset($data[$name])) {
+			else if(is_array($data)){
-				// if field is in array-notation, we need to resolve the array-structure PHP creates from query-strings
+				if(array_key_exists($name, $data)) {
-				preg_match('/' . addcslashes($name,'[]') . '=([^&]*)/', urldecode(http_build_query($data)), $matches);
+					$exists = true;
-				$val = isset($matches[1]) ? $matches[1] : null;
+					$val = $data[$name];
-			} elseif(is_array($data) && array_key_exists($name, $data)) {
+				}
-				// else we assume its a simple keyed array
+				// If field is in array-notation we need to access nested data
-				$val = $data[$name];
+				else if(strpos($name,'[')) {
-			} else {
+					// First encode data using PHP's method of converting nested arrays to form data
-				$val = null;
+					$flatData = urldecode(http_build_query($data));
+					// Then pull the value out from that flattened string
+					preg_match('/' . addcslashes($name,'[]') . '=([^&]*)/', $flatData, $matches);
+
+					if (isset($matches[1])) {
+						$exists = true;
+						$val = $matches[1];
+					}
+				}
 			}
 
 			// save to the field if either a value is given, or loading of blank/undefined values is forced
-			if(isset($val) || $hasObjectValue || $clearMissingFields) {
+			if($exists){
-				// pass original data as well so composite fields can act on the additional information
+				if ($val != false || ($mergeStrategy & self::MERGE_IGNORE_FALSEISH) != self::MERGE_IGNORE_FALSEISH){
+					// pass original data as well so composite fields can act on the additional information
+					$field->setValue($val, $data);
+				}
+			}
+			else if(($mergeStrategy & self::MERGE_CLEAR_MISSING) == self::MERGE_CLEAR_MISSING){
 				$field->setValue($val, $data);
 			}
 		}

tests/forms/FormTest.php

@@ -153,7 +153,7 @@ class FormTest extends FunctionalTest {
 		$captainWithDetails = $this->objFromFixture('FormTest_Player', 'captainNoDetails');
 		$team2 = $this->objFromFixture('FormTest_Team', 'team2');
 		$form->loadDataFrom($captainWithDetails);
-		$form->loadDataFrom($team2, true);
+		$form->loadDataFrom($team2, Form::MERGE_CLEAR_MISSING);
 		$this->assertEquals(
 			$form->getData(), 
 			array(
@@ -166,7 +166,35 @@ class FormTest extends FunctionalTest {
 			'LoadDataFrom() overwrites fields not found in the object with $clearMissingFields=true'
 		);
 	}
-	
+
+	public function testLoadDataFromIgnoreFalseish() {
+		$form = new Form(
+			new Controller(),
+			'Form',
+			new FieldList(
+				new TextField('Biography', 'Biography', 'Custom Default')
+			),
+			new FieldList()
+		);
+
+		$captainNoDetails = $this->objFromFixture('FormTest_Player', 'captainNoDetails');
+		$captainWithDetails = $this->objFromFixture('FormTest_Player', 'captainWithDetails');
+
+		$form->loadDataFrom($captainNoDetails, Form::MERGE_IGNORE_FALSEISH);
+		$this->assertEquals(
+			$form->getData(),
+			array('Biography' => 'Custom Default'),
+			'LoadDataFrom() doesn\'t overwrite fields when MERGE_IGNORE_FALSEISH set and values are false-ish'
+		);
+
+		$form->loadDataFrom($captainWithDetails, Form::MERGE_IGNORE_FALSEISH);
+		$this->assertEquals(
+			$form->getData(),
+			array('Biography' => 'Bio 1'),
+			'LoadDataFrom() does overwrite fields when MERGE_IGNORE_FALSEISH set and values arent false-ish'
+		);
+	}
+
 	public function testFormMethodOverride() {
 		$form = $this->getStubForm();
 		$form->setFormMethod('GET');