BUGFIX Added support for array values in LookupField, to ensure it works correctly when used as a readonly representation of ListboxField (AIR-39)

forms/LookupField.php

@@ -13,29 +13,40 @@ class LookupField extends DropdownField {
 	 * Returns a readonly span containing the correct value.
 	 */
 	function Field() {
-		if(trim($this->value) || $this->value === '0') {
+		$source = $this->getSource();
-			$this->value = trim($this->value);
-			$source = $this->getSource();
-			if(is_array($source)) {
-				$mappedValue = isset($source[$this->value]) ? $source[$this->value] : null;
-			} elseif($source instanceof SQLMap) {
-				$mappedValue = $source->getItem($this->value);
-			}
-		}
 		
-		if(!isset($mappedValue)) $mappedValue = $this->value ? $this->value : "<i>(none)</i>";
 		
-		if($this->value) {
+		// Normalize value to array to simplify further processing
-			$val = $this->dontEscape ? $this->value : Convert::raw2xml($this->value);
+		$values = (is_array($this->value)) ? $this->value : array(trim($this->value));
+
+		$mapped = array();
+		if($source instanceof SQLMap) {
+			foreach($values as $value) $mapped[] = $source->getItem($value);
+		} elseif(is_array($source)) {
+			$mapped = array_intersect_key($source, array_combine($values, $values));
 		} else {
-			$val = '<i>(none)</i>';
+			$mapped = array();
 		}
 
-		$valforInput = $this->value ? Convert::raw2att($val) : "";
+		// Don't check if string arguments are matching against the source,
+		// as they might be generated HTML diff views instead of the actual values
+		if($this->value && !$mapped) {
+			$mapped = array(trim($this->value));
+			$values = array();
+		}
+		
+		if($mapped) {
+			$attrValue = implode(', ', array_values($mapped));
+			if(!$this->dontEscape) $attrValue = Convert::raw2xml($attrValue);
+			$inputValue = implode(', ', array_values($values)); 
+		} else {
+			$attrValue = "<i>(none)</i>";
+			$inputValue = '';
+		}
 
 		return "<span class=\"readonly\" id=\"" . $this->id() .
-			"\">$mappedValue</span><input type=\"hidden\" name=\"" . $this->name .
+			"\">$attrValue</span><input type=\"hidden\" name=\"" . $this->name .
-			"\" value=\"" . $valforInput . "\" />";
+			"\" value=\"" . $inputValue . "\" />";
 	}
 	
 	function performReadonlyTransformation() {

tests/forms/LookupFieldTest.php

@@ -0,0 +1,82 @@
+<?php
+/**
+ * @package sapphire
+ * @subpackage tests
+ */
+
+class LookupFieldTest extends SapphireTest {
+	
+	static $fixture_file = 'sapphire/tests/forms/LookupFieldTest.yml';
+
+	function testNullValueWithNumericArraySource() {
+		$source = array(1 => 'one', 2 => 'two', 3 => 'three');
+		$f = new LookupField('test', 'test', $source);
+		$f->setValue(null);
+		$this->assertEquals(
+			'<span class="readonly" id="test"><i>(none)</i></span><input type="hidden" name="test" value="" />', 
+			$f->Field()
+		);
+	}
+
+	function testStringValueWithNumericArraySource() {
+		$source = array(1 => 'one', 2 => 'two', 3 => 'three');
+		$f = new LookupField('test', 'test', $source);
+		$f->setValue(1);
+		$this->assertEquals(
+			'<span class="readonly" id="test">one</span><input type="hidden" name="test" value="1" />', 
+			$f->Field()
+		);
+	}
+	
+	function testUnknownStringValueWithNumericArraySource() {
+		$source = array(1 => 'one', 2 => 'two', 3 => 'three');
+		$f = new LookupField('test', 'test', $source);
+		$f->setValue('<ins>w00t</ins>');
+		$f->dontEscape = true; // simulates CMSMain->compareversions()
+		$this->assertEquals(
+			'<span class="readonly" id="test"><ins>w00t</ins></span><input type="hidden" name="test" value="" />', 
+			$f->Field()
+		);
+	}
+
+	function testArrayValueWithAssociativeArraySource() {
+		// Array values (= multiple selections) might be set e.g. from ListboxField
+		$source = array('one' => 'one val', 'two' => 'two val', 'three' => 'three val');
+		$f = new LookupField('test', 'test', $source);
+		$f->setValue(array('one','two'));
+		$this->assertEquals(
+			'<span class="readonly" id="test">one val, two val</span><input type="hidden" name="test" value="one, two" />', 
+			$f->Field()
+		);
+	}
+	
+	function testArrayValueWithNumericArraySource() {
+		// Array values (= multiple selections) might be set e.g. from ListboxField
+		$source = array(1 => 'one', 2 => 'two', 3 => 'three');
+		$f = new LookupField('test', 'test', $source);
+		$f->setValue(array(1,2));
+		$this->assertEquals(
+			'<span class="readonly" id="test">one, two</span><input type="hidden" name="test" value="1, 2" />', 
+			$f->Field()
+		);
+	}
+	
+	function testArrayValueWithSqlMapSource() {
+		$member1 = $this->objFromFixture('Member', 'member1');
+		$member2 = $this->objFromFixture('Member', 'member2');
+		$member3 = $this->objFromFixture('Member', 'member3');
+		
+		$source = new SQLMap(singleton('Member')->buildSQL());
+		$f = new LookupField('test', 'test', $source);
+		$f->setValue(array($member1->ID, $member2->ID));
+		$this->assertEquals(
+			sprintf(
+				'<span class="readonly" id="test">member1, member2</span><input type="hidden" name="test" value="%s, %s" />', 
+				$member1->ID,
+				$member2->ID
+			),
+			$f->Field()
+		);
+	}
+	
+}

tests/forms/LookupFieldTest.yml

@@ -0,0 +1,7 @@
+Member:
+   member1:
+      FirstName: member1
+   member2:
+      FirstName: member2
+   member3:
+      FirstName: member3