BUG Fix issue with SS_List as datasource for dropdown field

BUG Fix validation issue with CheckboxSetField
Fixes #6166

forms/CheckboxSetField.php

@@ -333,21 +333,22 @@ class CheckboxSetField extends OptionsetField {
 			return true;
 		}
 		$sourceArray = $this->getSourceAsArray();
+		$validValues = array_keys($sourceArray);
 		if (is_array($values)) {
-			if (!array_intersect_key($sourceArray, $values)) {
+			if (!array_intersect($validValues, $values)) {
 				$validator->validationError(
 					$this->name,
 					_t(
 						'CheckboxSetField.SOURCE_VALIDATION',
 						"Please select a value within the list provided. '{value}' is not a valid option",
-						array('value' => implode(' and ', array_diff($sourceArray, $values)))
+						array('value' => implode(' and ', $values))
 					),
 					"validation"
 				);
 				return false;
 			}
 		} else {
-			if (!in_array($this->value, $sourceArray)) {
+			if (!in_array($this->value, $validValues)) {
 				$validator->validationError(
 					$this->name,
 					_t(

forms/DropdownField.php

@@ -318,14 +318,23 @@ class DropdownField extends FormField {
 	public function getSourceAsArray()
 	{
 		$source = $this->getSource();
+
+		// Simplify source if presented as dataobject list
+		if ($source instanceof SS_List) {
+			$source = $source->map();
+		}
+		if ($source instanceof SS_Map) {
+			$source = $source->toArray();
+		}
+
 		if (is_array($source)) {
 			return $source;
-		} else {
+		}
+
 		$sourceArray = array();
 		foreach ($source as $key => $value) {
 			$sourceArray[$key] = $value;
 		}
-		}
 		return $sourceArray;
 	}
 

tests/forms/CheckboxSetFieldTest.php

@@ -177,28 +177,65 @@ class CheckboxSetFieldTest extends SapphireTest {
 		$tag1 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag1');
 		$tag2 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag2');
 		$tag3 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag3');
-		$field = CheckboxSetField::create('Test', 'Testing', $checkboxTestArticle->Tags()	->map());
+		$field = CheckboxSetField::create('Test', 'Testing', $checkboxTestArticle->Tags());
 		$validator = new RequiredFields();
-		$field->setValue(array(
+		$field->setValue(array( $tag1->ID, $tag2->ID ));
-			$tag1->ID => $tag1->ID,
+		$isValid = $field->validate($validator);
-			$tag2->ID => $tag2->ID
-		));
 		$this->assertTrue(
-			$field->validate($validator),
+			$isValid,
 			'Validates values in source map'
 		);
-		//invalid value should fail
+
+		// Invalid value should fail
+		$validator = new RequiredFields();
 		$fakeID = CheckboxSetFieldTest_Tag::get()->max('ID') + 1;
-		$field->setValue(array($fakeID => $fakeID));
+		$field->setValue(array($fakeID));
 		$this->assertFalse(
 			$field->validate($validator),
 			'Field does not valid values outside of source map'
 		);
+		$errors = $validator->getErrors();
+		$error = reset($errors);
+		$this->assertEquals(
+			"Please select a value within the list provided. '$fakeID' is not a valid option",
+			$error['message']
+		);
+
+		// Multiple invalid values should fail
+		$validator = new RequiredFields();
+		$fakeID = CheckboxSetFieldTest_Tag::get()->max('ID') + 1;
+		$field->setValue(array($fakeID, $tag3->ID));
+		$this->assertFalse(
+			$field->validate($validator),
+			'Field does not valid values outside of source map'
+		);
+		$errors = $validator->getErrors();
+		$error = reset($errors);
+		$this->assertEquals(
+			"Please select a value within the list provided. '{$fakeID} and {$tag3->ID}' is not a valid option",
+			$error['message']
+		);
+
+		// Invalid value with non-array value
+		$validator = new RequiredFields();
+		$field->setValue($fakeID);
+		$this->assertFalse(
+			$field->validate($validator),
+			'Field does not valid values outside of source map'
+		);
+		$errors = $validator->getErrors();
+		$error = reset($errors);
+		$this->assertEquals(
+			"Please select a value within the list provided. '{$fakeID}' is not a valid option",
+			$error['message']
+		);
+
 		// non valid value included with valid options should succeed
+		$validator = new RequiredFields();
 		$field->setValue(array(
-			$tag1->ID => $tag1->ID,
+			$tag1->ID,
-			$tag2->ID => $tag2->ID,
+			$tag2->ID,
-			$tag3->ID => $tag3->ID
+			$tag3->ID
 		));
 		$this->assertTrue(
 			$field->validate($validator),

tests/forms/DropdownFieldTest.php

@@ -6,12 +6,37 @@
 class DropdownFieldTest extends SapphireTest {
 
 	public function testGetSource() {
-		$source = array(1=>'one');
+		$source = array(1=>'one', 2 => 'two');
 		$field = new DropdownField('Field', null, $source);
 		$this->assertEquals(
-			$field->getSource(),
+			$source,
+			$field->getSource()
+		);
+		$this->assertEquals(
+			$source,
+			$field->getSourceAsArray()
+		);
+
+		$items = new ArrayList([
+			[ 'ID' => 1, 'Title' => 'ichi', 'OtherField' => 'notone' ],
+			[ 'ID' => 2, 'Title' => 'ni', 'OtherField' => 'nottwo' ],
+		]);
+		$field->setSource($items);
+		$this->assertEquals(
+			$field->getSourceAsArray(),
 			array(
-				1 => 'one'
+				1 => 'ichi',
+				2 => 'ni',
+			)
+		);
+
+		$map = new SS_Map($items, 'ID', 'OtherField');
+		$field->setSource($map);
+		$this->assertEquals(
+			$field->getSourceAsArray(),
+			array(
+				1 => 'notone',
+				2 => 'nottwo',
 			)
 		);
 	}