Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG Renable the ability to do dynamic assignment with DBField #8821

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions model/DataObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -1362,12 +1362,16 @@ protected function writeManipulation($baseTable, $now, $isNewRecord) {
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
);
}
}
}
}
Expand Down
18 changes: 12 additions & 6 deletions model/ManyManyList.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,15 +254,18 @@ public function add($item, $extraFields = array()) {
);
}

/** @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
if(array_key_exists($fieldName, $extraFields)) {
$fieldObject = Object::create_from_string($fieldSpec, $fieldName);
$fieldObject->setValue($extraFields[$fieldName]);
$fieldObject->writeToManipulation($manipulation[$this->joinTable]);
$fieldObjects[$fieldName] = $fieldObject;
}
}
}
Expand All @@ -275,12 +278,15 @@ public function add($item, $extraFields = array()) {
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
);
}
}
}
}
Expand Down
30 changes: 30 additions & 0 deletions tests/model/DataObjectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class DataObjectTest extends SapphireTest {
'DataObjectTest_Sortable',
'ManyManyListTest_Product',
'ManyManyListTest_Category',
'MockDynamicAssignmentDataObject'
);

/**
Expand Down Expand Up @@ -1773,6 +1774,35 @@ public function testSetFieldWithArrayOnCompositeField()
$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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah - user errors 😆

* @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 {
Expand Down
36 changes: 35 additions & 1 deletion tests/model/ManyManyListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ManyManyListTest extends SapphireTest {
'ManyManyListTest_ExtraFields',
'ManyManyListTest_Product',
'ManyManyListTest_Category',
'MockDynamicAssignmentDataObject',
);


Expand Down Expand Up @@ -306,7 +307,40 @@ public function testFilteringOnPreviouslyJoinedTable() {
$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
));
}
}

/**
Expand Down
49 changes: 49 additions & 0 deletions tests/model/Mock/MockDynamicAssignmentDBField.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

/**
* This is a fake DB field specifically design to test dynamic value assignment. You can set `scalarValueOnly` in
* the constructor. You can control whetever the field will try to do a dynamic assignment by specifing
* `$dynamicAssignment` in nthe consturctor.
*
* If the field is set to false, it will try to do a plain assignment. This is so you can save the initial value no
* matter what. If the field is set to true, it will try to do a dynamic assignment.
*/
class MockDynamicAssignmentDBField extends Boolean
{

private $scalarOnly;
private $dynamicAssignment;

/**
* @param string $name
* @param boolean $scalarOnly Whether our fake field should be scalar only.
* @param boolean $dynamicAssignment Whether our fake field will try to do a dynamic assignment.
*/
public function __construct($name = '', $scalarOnly = false, $dynamicAssignment = false)
{
$this->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;
}
}
44 changes: 44 additions & 0 deletions tests/model/Mock/MockDynamicAssignmentDataObject.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

/**
* This is a fake DB field specifically design to test dynamic value assignment
* @property boolean $StaticScalarOnlyField
* @property boolean $DynamicScalarOnlyField
* @property boolean $DynamicField
* @method ManyManyList MockManyMany
*/
class MockDynamicAssignmentDataObject extends DataObject implements TestOnly
{

private static $db = array(
// This field only emits scalar value and will save
'StaticScalarOnlyField' => '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)',
)
);
}