diff --git a/model/DataObject.php b/model/DataObject.php index d778a6994..7cc3e4c73 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -1362,12 +1362,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if (!isset($tableManipulation['fields'])) { continue; } - foreach ($tableManipulation['fields'] as $fieldValue) { + foreach ($tableManipulation['fields'] as $fieldName => $fieldValue) { if (is_array($fieldValue)) { - user_error( - 'DataObject::writeManipulation: parameterised field assignments are disallowed', - E_USER_ERROR - ); + $dbObject = $this->dbObject($fieldName); + // If the field allows non-scalar values we'll let it do dynamic assignments + if ($dbObject && $dbObject->scalarValueOnly()) { + user_error( + 'DataObject::writeManipulation: parameterised field assignments are disallowed', + E_USER_ERROR + ); + } } } } diff --git a/model/ManyManyList.php b/model/ManyManyList.php index a64d00c34..bbf024c47 100644 --- a/model/ManyManyList.php +++ b/model/ManyManyList.php @@ -254,8 +254,10 @@ class ManyManyList extends RelationList { ); } + /** @var DBField[] $fieldObjects */ + $fieldObjects = array(); if($extraFields && $this->extraFields) { - // Write extra field to manipluation in the same way + // Write extra field to manipulation in the same way // that DataObject::prepareManipulationTable writes fields foreach($this->extraFields as $fieldName => $fieldSpec) { // Skip fields without an assignment @@ -263,6 +265,7 @@ class ManyManyList extends RelationList { $fieldObject = SS_Object::create_from_string($fieldSpec, $fieldName); $fieldObject->setValue($extraFields[$fieldName]); $fieldObject->writeToManipulation($manipulation[$this->joinTable]); + $fieldObjects[$fieldName] = $fieldObject; } } } @@ -275,12 +278,15 @@ class ManyManyList extends RelationList { if (!isset($tableManipulation['fields'])) { continue; } - foreach ($tableManipulation['fields'] as $fieldValue) { + foreach ($tableManipulation['fields'] as $fieldName => $fieldValue) { if (is_array($fieldValue)) { - user_error( - 'ManyManyList::add: parameterised field assignments are disallowed', - E_USER_ERROR - ); + // If the field allows non-scalar values we'll let it do dynamic assignments + if (isset($fieldObjects[$fieldName]) && $fieldObjects[$fieldName]->scalarValueOnly()) { + user_error( + 'ManyManyList::add: parameterised field assignments are disallowed', + E_USER_ERROR + ); + } } } } diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 83e9c6bd5..501044ee1 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -32,6 +32,7 @@ class DataObjectTest extends SapphireTest { 'DataObjectTest_Sortable', 'ManyManyListTest_Product', 'ManyManyListTest_Category', + 'MockDynamicAssignmentDataObject' ); /** @@ -1773,6 +1774,35 @@ class DataObjectTest extends SapphireTest { $this->assertNotEmpty($do->CompositeMoneyField); } + + public function testWriteManipulationWithNonScalarValuesAllowed() + { + $do = MockDynamicAssignmentDataObject::create(); + $do->write(); + $do->StaticScalarOnlyField = true; + $do->DynamicScalarOnlyField = false; + $do->DynamicField = true; + $do->write(); + $this->assertEquals(1, $do->StaticScalarOnlyField); + $this->assertEquals(0, $do->DynamicScalarOnlyField); + $this->assertEquals(1, $do->DynamicField); + } + + /** + * @expectedException PHPUnit_Framework_Error + * @expectedExceptionMessageRegExp /parameterised field assignments are disallowed/ + */ + public function testWriteManipulationWithNonScalarValuesDisallowed() + { + + $do = MockDynamicAssignmentDataObject::create(); + $do->write(); + $do->StaticScalarOnlyField = false; + $do->DynamicScalarOnlyField = true; + $do->DynamicField = false; + + $do->write(); + } } class DataObjectTest_Sortable extends DataObject implements TestOnly { diff --git a/tests/model/ManyManyListTest.php b/tests/model/ManyManyListTest.php index 7990a8b51..a75c92cc2 100644 --- a/tests/model/ManyManyListTest.php +++ b/tests/model/ManyManyListTest.php @@ -21,6 +21,7 @@ class ManyManyListTest extends SapphireTest { 'ManyManyListTest_ExtraFields', 'ManyManyListTest_Product', 'ManyManyListTest_Category', + 'MockDynamicAssignmentDataObject', ); @@ -306,7 +307,40 @@ class ManyManyListTest extends SapphireTest { $this->assertEquals(1, $productsRelatedToProductB->count()); } + public function testWriteManipulationWithNonScalarValuesAllowed() + { + $left = MockDynamicAssignmentDataObject::create(); + $left->write(); + $right = MockDynamicAssignmentDataObject::create(); + $right->write(); + $left->MockManyMany()->add($right, array( + 'ManyManyStaticScalarOnlyField' => true, + 'ManyManyDynamicScalarOnlyField' => false, + 'ManyManyDynamicField' => true + )); + $pivot = $left->MockManyMany()->first(); + $this->assertEquals(1, $pivot->ManyManyStaticScalarOnlyField); + $this->assertEquals(0, $pivot->ManyManyDynamicScalarOnlyField); + $this->assertEquals(1, $pivot->ManyManyDynamicField); + } + /** + * @expectedException PHPUnit_Framework_Error + * @expectedExceptionMessageRegExp /parameterised field assignments are disallowed/ + */ + public function testWriteManipulationWithNonScalarValuesDisallowed() + { + $left = MockDynamicAssignmentDataObject::create(); + $left->write(); + $right = MockDynamicAssignmentDataObject::create(); + $right->write(); + + $left->MockManyMany()->add($right, array( + 'ManyManyStaticScalarOnlyField' => false, + 'ManyManyDynamicScalarOnlyField' => true, + 'ManyManyDynamicField' => false + )); + } } /** diff --git a/tests/model/Mock/MockDynamicAssignmentDBField.php b/tests/model/Mock/MockDynamicAssignmentDBField.php new file mode 100644 index 000000000..4a09c4a29 --- /dev/null +++ b/tests/model/Mock/MockDynamicAssignmentDBField.php @@ -0,0 +1,49 @@ +scalarOnly = $scalarOnly; + $this->dynamicAssignment = $dynamicAssignment; + parent::__construct($name); + } + + /** + * If the field value and dynamicAssignment are true, we'll try to do a dynamic assignment + * @param mixed $value + * @return array|int|mixed + */ + public function prepValueForDB($value) + { + if ($value) { + return $this->dynamicAssignment + ? array('ABS(?)' => array(1)) + : 1; + } + + return 0; + } + + public function scalarValueOnly() + { + return $this->scalarOnly; + } +} diff --git a/tests/model/Mock/MockDynamicAssignmentDataObject.php b/tests/model/Mock/MockDynamicAssignmentDataObject.php new file mode 100644 index 000000000..5363de1fa --- /dev/null +++ b/tests/model/Mock/MockDynamicAssignmentDataObject.php @@ -0,0 +1,44 @@ + 'MockDynamicAssignmentDBField(1,0)', + + // This field tries to emit dynamic assignment but will fail because of scalar only + 'DynamicScalarOnlyField' => 'MockDynamicAssignmentDBField(1,1)', + + // This field does dynamic assignment and will pass + 'DynamicField' => 'MockDynamicAssignmentDBField(0,1)', + ); + + private static $many_many = array( + "MockManyMany" => 'MockDynamicAssignmentDataObject' + ); + + private static $belongs_many_many = array( + "MockBelongsManyMany" => 'MockDynamicAssignmentDataObject' + ); + + private static $many_many_extraFields = array( + 'MockManyMany' => array( + // This field only emits scalar value and will save + 'ManyManyStaticScalarOnlyField' => 'MockDynamicAssignmentDBField(1,0)', + + // This field tries to emit dynamic assignment but will fail because of scalar only + 'ManyManyDynamicScalarOnlyField' => 'MockDynamicAssignmentDBField(1,1)', + + // This field does dynamic assignment and will pass + 'ManyManyDynamicField' => 'MockDynamicAssignmentDBField(0,1)', + ) + ); +}