Skip to content

Commit

Permalink
BUG Resolve issue with numeric tags being saved
Browse files Browse the repository at this point in the history 
Removed is_numeric check in SaveInto function
  • Loading branch information
Damian Mooyman committed Nov 24, 2016
2 parents 00c950e + a75eab8 commit 5f3f458
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 63 deletions.
89 changes: 44 additions & 45 deletions code/TagField.php
View file
Expand Up @@ -202,39 +202,38 @@ protected function getOptions()

$source = $this->getSource();

if (!$source) {
if(!$source) {
$source = new ArrayList();
}

$dataClass = $source->dataClass();

$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}
Expand All @@ -245,10 +244,10 @@ public function setValue($value, $source = null)
$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)) {
Expand Down Expand Up @@ -277,47 +276,43 @@ public function saveInto(DataObjectInterface $record)
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
Expand All @@ -333,16 +328,20 @@ protected function getOrCreateTag($term)
$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;
}
}

/**
Expand Down Expand Up @@ -389,7 +388,7 @@ protected function getTags($term)
$titleField = $this->getTitleField();
foreach ($query->map('ID', $titleField) as $id => $title) {
$items[$title] = array(
'id' => $id,
'id' => $title,
'text' => $title
);
}
Expand Down
35 changes: 18 additions & 17 deletions tests/TagFieldTest.php
View file
Expand Up @@ -132,29 +132,29 @@ public function testSaveDuplicateTags()

// 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'));
Expand All @@ -169,21 +169,18 @@ public function testItSuggestsTags()
*/
$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()
);

Expand All @@ -193,7 +190,7 @@ public function testItSuggestsTags()
$request = $this->getNewRequest(array('term' => 'TAG1'));

$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);

Expand All @@ -215,15 +212,14 @@ public function testRestrictedSuggestions()
{
$source = TagFieldTestBlogTag::get()->exclude('Title', 'Tag2');
$field = new TagField('Tags', '', $source);
$tag1ID = $this->idFromFixture('TagFieldTestBlogTag', 'Tag1');

/**
* Partial tag title match.
*/
$request = $this->getNewRequest(array('term' => 'Tag'));

$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);

Expand All @@ -233,7 +229,7 @@ public function testRestrictedSuggestions()
$request = $this->getNewRequest(array('term' => 'Tag1'));

$this->assertEquals(
sprintf('{"items":[{"id":%d,"text":"Tag1"}]}', $tag1ID),
'{"items":[{"id":"Tag1","text":"Tag1"}]}',
$field->suggest($request)->getBody()
);

Expand Down Expand Up @@ -280,19 +276,24 @@ public function testItDisplaysValuesFromRelations()
$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);

Expand Down
2 changes: 1 addition & 1 deletion tests/TagFieldTest.yml
View file
Expand Up @@ -2,7 +2,7 @@ TagFieldTestBlogTag:
Tag1:
Title: Tag1
Tag2:
Title: Tag2
Title: 222
TagFieldTestBlogPost:
BlogPost1:
Title: BlogPost1
Expand Down

0 comments on commit 5f3f458

Please sign in to comment.