BUG Fix false values for many_many_ExtraFields not being saved

Fixes #4067
This commit is contained in:
Damian Mooyman 2015-06-08 15:44:27 +12:00
parent f21e59585e
commit acf19b72e2
3 changed files with 103 additions and 50 deletions

View File

@ -495,21 +495,39 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
return $backlink;
}
/**
* Get the list of extra data from the $record as saved into it by
* {@see Form::saveInto()}
*
* Handles detection of falsey values explicitly saved into the
* DataObject by formfields
*
* @param DataObject $record
* @param SS_List $list
* @return array List of data to write to the relation
*/
protected function getExtraSavedData($record, $list) {
// Skip extra data if not ManyManyList
if(!($list instanceof ManyManyList)) {
return null;
}
$data = array();
foreach($list->getExtraFields() as $field => $dbSpec) {
$savedField = "ManyMany[{$field}]";
if($record->hasField($savedField)) {
$data[$field] = $record->getField($savedField);
}
}
return $data;
}
public function doSave($data, $form) {
$new_record = $this->record->ID == 0;
$controller = $this->getToplevelController();
$list = $this->gridField->getList();
if($list instanceof ManyManyList) {
// Data is escaped in ManyManyList->add()
$extraData = (isset($data['ManyMany'])) ? $data['ManyMany'] : null;
} else {
$extraData = null;
}
if(!$this->record->canEdit()) {
return $controller->httpError(403);
}
@ -527,6 +545,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler {
try {
$form->saveInto($this->record);
$this->record->write();
$extraData = $this->getExtraSavedData($this->record, $list);
$list->add($this->record, $extraData);
} catch(ValidationException $e) {
$form->sessionMessage($e->getResult()->message(), 'bad', false);

View File

@ -230,37 +230,29 @@ class ManyManyList extends RelationList {
$hasExisting = false;
}
$manipulation = array();
// Blank manipulation
$manipulation = array(
$this->joinTable => array(
'command' => $hasExisting ? 'update' : 'insert',
'fields' => array()
)
);
if($hasExisting) {
$manipulation[$this->joinTable]['command'] = 'update';
$manipulation[$this->joinTable]['where'] = array(
"\"{$this->joinTable}\".\"{$this->foreignKey}\"" => $foreignID,
"\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID
);
} else {
$manipulation[$this->joinTable]['command'] = 'insert';
}
if($extraFields) {
foreach($extraFields as $fieldName => $fieldValue) {
if(is_null($fieldValue)) {
$manipulation[$this->joinTable]['fields'][$fieldName] = null;
} elseif($fieldValue instanceof DBField) {
// rely on writeToManipulation to manage the changes
// required for this field.
$working = array('fields' => array());
// create a new instance of the field so we can
// modify the field name to the correct version.
$field = DBField::create_field(get_class($fieldValue), $fieldValue);
$field->setName($fieldName);
$field->writeToManipulation($working);
foreach($working['fields'] as $extraName => $extraValue) {
$manipulation[$this->joinTable]['fields'][$extraName] = $extraValue;
}
} else {
$manipulation[$this->joinTable]['fields'][$fieldName] = $fieldValue;
if($extraFields && $this->extraFields) {
// Write extra field to manipluation 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]);
}
}
}
@ -357,24 +349,32 @@ class ManyManyList extends RelationList {
*/
public function getExtraData($componentName, $itemID) {
$result = array();
// Skip if no extrafields or unsaved record
if(empty($this->extraFields) || empty($itemID)) {
return $result;
}
if(!is_numeric($itemID)) {
user_error('ComponentSet::getExtraData() passed a non-numeric child ID', E_USER_ERROR);
}
if($this->extraFields) {
$cleanExtraFields = array();
foreach ($this->extraFields as $fieldName => $dbFieldSpec) {
$cleanExtraFields[] = "\"{$fieldName}\"";
}
$query = new SQLQuery($cleanExtraFields, "\"{$this->joinTable}\"");
if($filter = $this->foreignIDWriteFilter($this->getForeignID())) {
$query->setWhere($filter);
} else {
user_error("Can't call ManyManyList::getExtraData() until a foreign ID is set", E_USER_WARNING);
}
$query->addWhere("\"{$this->localKey}\" = {$itemID}");
$queryResult = $query->execute()->current();
$cleanExtraFields = array();
foreach ($this->extraFields as $fieldName => $dbFieldSpec) {
$cleanExtraFields[] = "\"{$fieldName}\"";
}
$query = new SQLQuery($cleanExtraFields, "\"{$this->joinTable}\"");
$filter = $this->foreignIDWriteFilter($this->getForeignID());
if($filter) {
$query->setWhere($filter);
} else {
user_error("Can't call ManyManyList::getExtraData() until a foreign ID is set", E_USER_WARNING);
}
$query->addWhere(array(
"\"{$this->localKey}\"" => $itemID
));
$queryResult = $query->execute()->current();
if ($queryResult) {
foreach ($queryResult as $fieldName => $value) {
$result[$fieldName] = $value;
}

View File

@ -174,11 +174,15 @@ class GridFieldDetailFormTest extends FunctionalTest {
$manyManyField = $parser->getByXpath('//*[@id="Form_ItemEditForm"]//input[@name="ManyMany[IsPublished]"]');
$this->assertTrue((bool)$manyManyField);
// Test save of IsPublished field
$response = $this->post(
$editformurl,
array(
'Name' => 'Updated Category',
'ManyMany' => array('IsPublished' => 1),
'ManyMany' => array(
'IsPublished' => 1,
'PublishedBy' => 'Richard'
),
'action_doSave' => 1
)
);
@ -187,7 +191,33 @@ class GridFieldDetailFormTest extends FunctionalTest {
$person = GridFieldDetailFormTest_Person::get()->sort('FirstName')->First();
$category = $person->Categories()->filter(array('Name' => 'Updated Category'))->First();
$this->assertEquals(
array('IsPublished' => 1),
array(
'IsPublished' => 1,
'PublishedBy' => 'Richard'
),
$person->Categories()->getExtraData('', $category->ID)
);
// Test update of value with falsey value
$response = $this->post(
$editformurl,
array(
'Name' => 'Updated Category',
'ManyMany' => array(
'PublishedBy' => ''
),
'action_doSave' => 1
)
);
$this->assertFalse($response->isError());
$person = GridFieldDetailFormTest_Person::get()->sort('FirstName')->First();
$category = $person->Categories()->filter(array('Name' => 'Updated Category'))->First();
$this->assertEquals(
array(
'IsPublished' => 0,
'PublishedBy' => ''
),
$person->Categories()->getExtraData('', $category->ID)
);
}
@ -316,7 +346,8 @@ class GridFieldDetailFormTest_Person extends DataObject implements TestOnly {
private static $many_many_extraFields = array(
'Categories' => array(
'IsPublished' => 'Boolean'
'IsPublished' => 'Boolean',
'PublishedBy' => 'Varchar'
)
);
@ -464,7 +495,10 @@ class GridFieldDetailFormTest_CategoryController extends Controller implements T
// GridField lists categories for a specific person
$person = GridFieldDetailFormTest_Person::get()->sort('FirstName')->First();
$detailFields = singleton('GridFieldDetailFormTest_Category')->getCMSFields();
$detailFields->addFieldToTab('Root.Main', new CheckboxField('ManyMany[IsPublished]'));
$detailFields->addFieldsToTab('Root.Main', array(
new CheckboxField('ManyMany[IsPublished]'),
new TextField('ManyMany[PublishedBy]'))
);
$field = new GridField('testfield', 'testfield', $person->Categories());
$field->getConfig()->addComponent($gridFieldForm = new GridFieldDetailForm($this, 'Form'));
$gridFieldForm->setFields($detailFields);