Merge pull request #1871 from wilr/open6132

FIX: Add support for multi dimensional source arrays in LookupField

core/ArrayLib.php

@@ -1,6 +1,8 @@
 <?php
+
 /**
  * Library of static methods for manipulating arrays.
+ *
  * @package framework
  * @subpackage misc
  */
@@ -43,7 +45,10 @@ class ArrayLib {
 	 * @return array
 	 */
 	public static function invert($arr) {
-		if(!$arr) return false;
+		if(!$arr) {
+			return false;
+		}
+
 		$result = array();
 		
 		foreach($arr as $columnName => $column) {
@@ -56,7 +61,7 @@ class ArrayLib {
 	}
 	
 	/**
-	 * Return an array where the keys are all equal to the values
+	 * Return an array where the keys are all equal to the values.
 	 * 
 	 * @param $arr array
 	 * @return array
@@ -70,7 +75,8 @@ class ArrayLib {
 	 */
 	public static function array_values_recursive($arr) {
 		$lst = array();
-		foreach(array_keys($arr) as $k){
+
+		foreach(array_keys($arr) as $k) {
 			$v = $arr[$k];
 			if (is_scalar($v)) {
 				$lst[] = $v;
@@ -80,92 +86,115 @@ class ArrayLib {
 				);
 			}
 		}
+
 		return $lst;
 	}
 	
 	/**
-	 * Filter an array by keys (useful for only allowing certain form-input to be saved).
+	 * Filter an array by keys (useful for only allowing certain form-input to 
+	 * be saved).
 	 * 
 	 * @param $arr array
 	 * @param $keys array
+	 *
 	 * @return array
 	 */
-	public static function filter_keys($arr, $keys)
-	{
-		foreach ($arr as $key => $v) {
-			if (!in_array($key, $keys)) {
+	public static function filter_keys($arr, $keys) {
+		foreach($arr as $key => $v) {
+			if(!in_array($key, $keys)) {
 				unset($arr[$key]);
 			}
 		}
+
 		return $arr;
 	}
 	
 	/**
-	 * Determines if an array is associative by checking
-	 * for existing keys via array_key_exists().
+	 * Determines if an array is associative by checking for existing keys via 
+	 * array_key_exists().
+	 *
 	 * @see http://nz.php.net/manual/en/function.is-array.php#76188
 	 *
 	 * @param array $arr
+	 *
 	 * @return boolean
 	 */
 	public static function is_associative($arr) {
 		if(is_array($arr) && ! empty($arr)) {
 			for($iterator = count($arr) - 1; $iterator; $iterator--) {
-				if (!array_key_exists($iterator, $arr)) return true;
+				if (!array_key_exists($iterator, $arr)) {
+					return true;
+				}
 			}
+
 			return !array_key_exists(0, $arr);
 		}
+
 		return false;
 	}
 
 	/**
 	 * Recursively searches an array $haystack for the value(s) $needle.
+	 *
 	 * Assumes that all values in $needle (if $needle is an array) are at 
 	 * the SAME level, not spread across multiple dimensions of the $haystack.
 	 *
 	 * @param mixed $needle
 	 * @param array $haystack
 	 * @param boolean $strict
+	 *
 	 * @return boolean
 	 */
 	public static function in_array_recursive($needle, $haystack, $strict = false) {
-		if(!is_array($haystack)) return false; // Not an array, we've gone as far as we can down this branch
+		if(!is_array($haystack)) {
+			return false; 
+		}
 
-		if(in_array($needle, $haystack, $strict)) return true; // Is it in this level of the array?
-		else {
-			foreach($haystack as $obj) { // It's not, loop over the rest of this array
-				if(self::in_array_recursive($needle, $obj, $strict)) return true; 
+		if(in_array($needle, $haystack, $strict)) {
+			return true;
+		} else {
+			foreach($haystack as $obj) {
+				if(self::in_array_recursive($needle, $obj, $strict)) {
+					return true; 
+				}
 			}
 		}
 		
-		return false; // Never found $needle :(
+		return false; 
 	}
 	
 	/**
 	 * Recursively merges two or more arrays.
 	 *
-	 * Behaves similar to array_merge_recursive(), however it only merges values when both are arrays
-	 * rather than creating a new array with both values, as the PHP version does. The same behaviour
-	 * also occurs with numeric keys, to match that of what PHP does to generate $_REQUEST.
+	 * Behaves similar to array_merge_recursive(), however it only merges 
+	 * values when both are arrays rather than creating a new array with 
+	 * both values, as the PHP version does. The same behaviour also occurs 
+	 * with numeric keys, to match that of what PHP does to generate $_REQUEST.
+	 *
+	 * @param array $array
 	 *
-	 * @param array $array, ...
 	 * @return array
 	 */
 	public static function array_merge_recursive($array) {
 		$arrays = func_get_args();
 		$merged = array();
+		
 		if(count($arrays) == 1) {
 			return $array;
 		}
+
 		while ($arrays) {
 			$array = array_shift($arrays);
+			
 			if (!is_array($array)) {
 				trigger_error('ArrayLib::array_merge_recursive() encountered a non array argument', E_USER_WARNING);
 				return;
 			}
+
 			if (!$array) {
 				continue;
 			}
+			
 			foreach ($array as $key => $value) {
 				if (is_array($value) && array_key_exists($key, $merged) && is_array($merged[$key])) {
 					$merged[$key] = ArrayLib::array_merge_recursive($merged[$key], $value);
@@ -174,6 +203,30 @@ class ArrayLib {
 				}
 			}
 		}
+
 		return $merged;
 	}
+
+	/** 
+	 * Takes an multi dimension array and returns the flattened version.
+	 *
+	 * @param array $array
+	 * @param boolean $preserveKeys
+	 *
+	 * @return array
+	 */ 
+	public static function flatten($array, $preserveKeys = true, &$out = array()) { 
+		foreach($array as $key => $child) {
+			if(is_array($child)) {
+				$out = self::flatten($child, $preserveKeys, $out);
+			} else if($preserveKeys) {
+				$out[$key] = $child;
+			} else {
+				$out[] = $child;
+			}
+		}
+
+		return $out;
+	}
 }
+	

forms/LookupField.php

@@ -1,30 +1,51 @@
 <?php
+
 /**
  * Read-only complement of {@link DropdownField}.
- * Shows the "human value" of the dropdown field for the currently selected value.
+ *
+ * Shows the "human value" of the dropdown field for the currently selected 
+ * value.
+ *
  * @package forms
  * @subpackage fields-basic
  */
 class LookupField extends DropdownField {
 
+	/**
+	 * @var boolean $readonly
+	 */
 	protected $readonly = true;
 	
 	/**
 	 * Returns a readonly span containing the correct value.
+	 *
+	 * @param array $properties
+	 *
+	 * @return string
 	 */
 	public function Field($properties = array()) {
 		$source = $this->getSource();
 		
-		
 		// Normalize value to array to simplify further processing
-		$values = (is_array($this->value) || is_object($this->value)) ? $this->value : array(trim($this->value));
+		if(is_array($this->value) || is_object($this->value)) {
+			$values = $this->value;
+		} else {
+			$values = array(trim($this->value));
+		}
 
 		$mapped = array();
+
 		if($source instanceof SQLMap) {
-			foreach($values as $value) $mapped[] = $source->getItem($value);
-		} else if($source instanceof ArrayAccess || is_array($source)) {
 			foreach($values as $value) {
-				if(isset($source[$value])) $mapped[] = $source[$value];
+				$mapped[] = $source->getItem($value);
+			}
+		} else if($source instanceof ArrayAccess || is_array($source)) {
+			$source = ArrayLib::flatten($source);
+			
+			foreach($values as $value) {
+				if(isset($source[$value])) {
+					$mapped[] = $source[$value];
+				}
 			}
 		} else {
 			$mapped = array();
@@ -39,7 +60,11 @@ class LookupField extends DropdownField {
 		
 		if($mapped) {
 			$attrValue = implode(', ', array_values($mapped));
-			if(!$this->dontEscape) $attrValue = Convert::raw2xml($attrValue);
+			
+			if(!$this->dontEscape) {
+				$attrValue = Convert::raw2xml($attrValue);
+			}
+
 			$inputValue = implode(', ', array_values($values)); 
 		} else {
 			$attrValue = "<i>(none)</i>";
@@ -51,17 +76,26 @@ class LookupField extends DropdownField {
 			"\" value=\"" . $inputValue . "\" />";
 	}
 	
+	/**
+	 * @return LookupField
+	 */
 	public function performReadonlyTransformation() {
 		$clone = clone $this;
+
 		return $clone;
 	}
 
+	/**
+	 * @return string
+	 */
 	public function Type() {
 		return "lookup readonly";
 	}
 	
 	/**
-	 * Override parent behaviour by not merging arrays.
+	 * Override parent behavior by not merging arrays.
+	 *
+	 * @return array
 	 */
 	public function getSource() {
 		return $this->source;

tests/core/ArrayLibTest.php

@@ -1,9 +1,11 @@
 <?php
+
 /**
  * @package framework
  * @subpackage tests
  */
 class ArrayLibTest extends SapphireTest {
+
 	public function testInvert() {
 		$arr = array(
 			'row1' => array(
@@ -187,4 +189,31 @@ class ArrayLibTest extends SapphireTest {
 			'Numeric keys should behave like string keys'
 		);
 	}
+
+	public function testFlatten() {
+		$options = array(
+			'1' => 'one',
+			'2' => 'two'
+		);
+
+		$expected = $options;
+
+		$this->assertEquals($expected, ArrayLib::flatten($options));
+
+		$options = array(
+			'1' => array(
+				'2' => 'two',
+				'3' => 'three'
+			),
+			'4' => 'four'
+		);
+
+		$expected = array(
+			'2' => 'two',
+			'3' => 'three',
+			'4' => 'four'
+		);
+
+		$this->assertEquals($expected, ArrayLib::flatten($options));
+	}
 }

tests/forms/LookupFieldTest.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @package framework
  * @subpackage tests
@@ -80,4 +81,34 @@ class LookupFieldTest extends SapphireTest {
 		);
 	}
 
+	public function testWithMultiDimensionalSource() {
+		$choices = array(
+			"Non-vegetarian" => array(
+				0 => 'Carnivore',
+			),
+			"Vegetarian" => array(
+				3 => 'Carrots', 
+			),
+			"Other" => array(
+				9 => 'Vegan'
+			)
+		);
+
+		$f = new LookupField('test', 'test', $choices);
+		$f->setValue(3);
+
+		$this->assertEquals(
+			'<span class="readonly" id="test">Carrots</span><input type="hidden" name="test" value="3" />', 
+			$f->Field()
+		);
+
+		$f->setValue(array(
+			3, 9
+		));
+
+		$this->assertEquals(
+			'<span class="readonly" id="test">Carrots, Vegan</span><input type="hidden" name="test" value="3, 9" />', 
+			$f->Field()
+		);
+	}
 }