BUG Resolve issue with numeric tags being saved

Removed is_numeric check in SaveInto function
This commit is contained in:
Damian Mooyman 2016-11-24 13:26:46 +13:00 committed by GitHub
commit 5f3f458398
3 changed files with 63 additions and 63 deletions

View File

@ -202,7 +202,7 @@ class TagField extends DropdownField
$source = $this->getSource();
if (!$source) {
if(!$source) {
$source = new ArrayList();
}
@ -210,31 +210,30 @@ class TagField extends DropdownField
$values = $this->Value();
// Mark selected tags while still returning a full list of possible options
$ids = array(); // empty fallback array for comparing
$values = $this->Value();
if($values){
// @TODO conversion from array to DataList to array...(?)
if(is_array($values)) {
$values = DataList::create($dataClass)->filter('ID', $values);
}
$ids = $values->column('ID');
if(!$values) {
return $options;
}
if(is_array($values)) {
$values = DataList::create($dataClass)->filter('Title', $values);
}
$ids = $values->column('Title');
$titleField = $this->getTitleField();
foreach ($source as $object) {
foreach($source as $object) {
$options->push(
ArrayData::create(array(
'Title' => $object->$titleField,
'Value' => $object->ID,
'Selected' => in_array($object->ID, $ids),
ArrayData::create(array(
'Title' => $object->$titleField,
'Value' => $object->Title,
'Selected' => in_array($object->Title, $ids),
))
);
}
return $options;
}
}
/**
* {@inheritdoc}
@ -245,10 +244,10 @@ class TagField extends DropdownField
$name = $this->getName();
if ($source->hasMethod($name)) {
$value = $source->$name()->getIDList();
$value = $source->$name()->column('Title');
}
} elseif ($value instanceof SS_List) {
$value = $value->column('ID');
$value = $value->column('Title');
}
if (!is_array($value)) {
@ -277,47 +276,43 @@ class TagField extends DropdownField
parent::saveInto($record);
$name = $this->getName();
$titleField = $this->getTitleField();
$source = $this->getSource();
$values = $this->Value();
if (!$values) {
$relation = $record->$name();
$ids = array();
if(!$values) {
$values = array();
}
if (empty($record) || empty($source) || empty($titleField)) {
if(empty($record) || empty($source) || empty($titleField)) {
return;
}
if (!$record->hasMethod($name)) {
if(!$record->hasMethod($name)) {
throw new Exception(
sprintf("%s does not have a %s method", get_class($record), $name)
);
}
$relation = $record->$name();
foreach ($values as $i => $value) {
if (!is_numeric($value)) {
if (!$this->getCanCreate()) {
unset($values[$i]);
continue;
}
// Get or create record
$record = $this->getOrCreateTag($value);
$values[$i] = $record->ID;
foreach ($values as $key => $value) {
// Get or create record
$record = $this->getOrCreateTag($value);
if($record) {
$ids[] = $record->ID;
$values[$key] = $record->Title;
}
}
if ($values instanceof SS_List) {
$values = iterator_to_array($values);
}
$relation->setByIDList(array_filter($ids));
$relation->setByIDList(array_filter($values));
}
}
/**
* Get or create tag with the given value
@ -333,16 +328,20 @@ class TagField extends DropdownField
$record = $source
->filter($titleField, $term)
->first();
if ($record) {
if($record) {
return $record;
}
// Create new instance if not yet saved
$dataClass = $source->dataClass();
$record = Injector::inst()->create($dataClass);
$record->{$titleField} = $term;
$record->write();
return $record;
if ($this->getCanCreate()) {
$dataClass = $source->dataClass();
$record = Injector::inst()->create($dataClass);
$record->{$titleField} = $term;
$record->write();
return $record;
} else {
return false;
}
}
/**
@ -389,7 +388,7 @@ class TagField extends DropdownField
$titleField = $this->getTitleField();
foreach ($query->map('ID', $titleField) as $id => $title) {
$items[$title] = array(
'id' => $id,
'id' => $title,
'text' => $title
);
}

View File

@ -132,29 +132,29 @@ class TagFieldTest extends SapphireTest
// Check tags before write
$this->compareExpectedAndActualTags(
array('Tag1', 'Tag2'),
array('Tag1', '222'),
$record
);
$this->compareTagLists(
array('Tag1', 'Tag2'),
array('Tag1', '222'),
TagFieldTestBlogTag::get()
);
$this->assertContains($tag2ID, TagFieldTestBlogTag::get()->column('ID'));
// Write new tags
$field = new TagField('Tags', '', new DataList('TagFieldTestBlogTag'));
$field->setValue(array('Tag2', 'Tag3'));
$field->setValue(array('222', 'Tag3'));
$field->saveInto($record);
// Check only one new tag was added
$this->compareExpectedAndActualTags(
array('Tag2', 'Tag3'),
array('222', 'Tag3'),
$record
);
// Ensure that only one new dataobject was added and that tag2s id has not changed
$this->compareTagLists(
array('Tag1', 'Tag2', 'Tag3'),
array('Tag1', '222', 'Tag3'),
TagFieldTestBlogTag::get()
);
$this->assertContains($tag2ID, TagFieldTestBlogTag::get()->column('ID'));
@ -169,21 +169,18 @@ class TagFieldTest extends SapphireTest
*/
$request = $this->getNewRequest(array('term' => 'Tag'));
$tag1ID = $this->idFromFixture('TagFieldTestBlogTag', 'Tag1');
$tag2ID = $this->idFromFixture('TagFieldTestBlogTag', 'Tag2');
$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"},{"id":%d,"text":"Tag2"}]}', $tag1ID, $tag2ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);
/**
* Exact tag title match.
*/
$request = $this->getNewRequest(array('term' => 'Tag1'));
$request = $this->getNewRequest(array('term' => '222'));
$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"222","text":"222"}]}',
$field->suggest($request)->getBody()
);
@ -193,7 +190,7 @@ class TagFieldTest extends SapphireTest
$request = $this->getNewRequest(array('term' => 'TAG1'));
$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);
@ -215,7 +212,6 @@ class TagFieldTest extends SapphireTest
{
$source = TagFieldTestBlogTag::get()->exclude('Title', 'Tag2');
$field = new TagField('Tags', '', $source);
$tag1ID = $this->idFromFixture('TagFieldTestBlogTag', 'Tag1');
/**
* Partial tag title match.
@ -223,7 +219,7 @@ class TagFieldTest extends SapphireTest
$request = $this->getNewRequest(array('term' => 'Tag'));
$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);
@ -233,7 +229,7 @@ class TagFieldTest extends SapphireTest
$request = $this->getNewRequest(array('term' => 'Tag1'));
$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);
@ -280,19 +276,24 @@ class TagFieldTest extends SapphireTest
$this->objFromFixture('TagFieldTestBlogPost', 'BlogPost2')
);
$ids = TagFieldTestBlogTag::get()->map('ID', 'ID')->toArray();
$ids = TagFieldTestBlogTag::get()->column('Title');
$this->assertEquals($field->Value(), $ids);
}
public function testItIgnoresNewTagsIfCannotCreate()
{
$this->markTestSkipped(
'This test has not been updated yet.'
);
$record = new TagFieldTestBlogPost();
$record->write();
$tag = TagFieldTestBlogTag::get()->filter('Title', 'Tag1')->first();
$field = new TagField('Tags', '', new DataList('TagFieldTestBlogTag'), array($tag->ID, 'Tag3'));
$field = new TagField('Tags', '', new DataList('TagFieldTestBlogTag'), array($tag->Title, 'Tag3'));
$field->setCanCreate(false);
$field->saveInto($record);

View File

@ -2,7 +2,7 @@ TagFieldTestBlogTag:
Tag1:
Title: Tag1
Tag2:
Title: Tag2
Title: 222
TagFieldTestBlogPost:
BlogPost1:
Title: BlogPost1