mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
FIX Image in summaryfields breaks search
This commit is contained in:
parent
8c3ba81052
commit
d24095aba8
@ -269,7 +269,7 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi
|
|||||||
sort($searchableFields);
|
sort($searchableFields);
|
||||||
sort($summaryFields);
|
sort($summaryFields);
|
||||||
// searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields
|
// searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields
|
||||||
if ($searchableFields !== $summaryFields) {
|
if (!empty($searchableFields) && ($searchableFields !== $summaryFields)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
// we have fallen back to summary_fields, check they are filterable
|
// we have fallen back to summary_fields, check they are filterable
|
||||||
|
@ -3724,6 +3724,52 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
|
|||||||
$this->extend('onAfterBuild');
|
$this->extend('onAfterBuild');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function getDatabaseBackedField(string $fieldPath): ?string
|
||||||
|
{
|
||||||
|
$component = $this;
|
||||||
|
$fieldParts = [];
|
||||||
|
$parts = explode('.', $fieldPath ?? '');
|
||||||
|
|
||||||
|
foreach ($parts as $nextPart) {
|
||||||
|
if (!$component) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
$fieldParts[] = $nextPart;
|
||||||
|
|
||||||
|
if ($component instanceof Relation || $component instanceof DataList) {
|
||||||
|
if ($component->hasMethod($nextPart)) {
|
||||||
|
// If the next part is a method, we don't have a database-backed field.
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
// The next part could either be a field, or another relation
|
||||||
|
$singleton = DataObject::singleton($component->dataClass());
|
||||||
|
if ($singleton->dbObject($nextPart) instanceof DBField) {
|
||||||
|
// If the next part is a DBField, we've found the database-backed field.
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
$component = $component->relation($nextPart);
|
||||||
|
array_shift($parts);
|
||||||
|
} elseif ($component instanceof DataObject && ($component->dbObject($nextPart) instanceof DBField)) {
|
||||||
|
// If the next part is a DBField, we've found the database-backed field.
|
||||||
|
break;
|
||||||
|
} elseif ($component instanceof DataObject && $component->getRelationType($nextPart) !== null) {
|
||||||
|
// If it's a last part or only one elemnt of a relation, we don't have a database-backed field.
|
||||||
|
if (count($parts) === 1) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
$component = $component->$nextPart();
|
||||||
|
array_shift($parts);
|
||||||
|
} elseif (ClassInfo::hasMethod($component, $nextPart)) {
|
||||||
|
// If the next part is a method, we don't have a database-backed field.
|
||||||
|
return null;
|
||||||
|
} else {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return implode('.', $fieldParts) ?: null;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get the default searchable fields for this object, as defined in the
|
* Get the default searchable fields for this object, as defined in the
|
||||||
* $searchable_fields list. If searchable fields are not defined on the
|
* $searchable_fields list. If searchable fields are not defined on the
|
||||||
@ -3742,21 +3788,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
|
|||||||
$summaryFields = array_keys($this->summaryFields() ?? []);
|
$summaryFields = array_keys($this->summaryFields() ?? []);
|
||||||
$fields = [];
|
$fields = [];
|
||||||
|
|
||||||
// remove the custom getters as the search should not include them
|
|
||||||
$schema = static::getSchema();
|
|
||||||
if ($summaryFields) {
|
if ($summaryFields) {
|
||||||
foreach ($summaryFields as $key => $name) {
|
foreach ($summaryFields as $name) {
|
||||||
$spec = $name;
|
if ($field = $this->getDatabaseBackedField($name)) {
|
||||||
|
$fields[] = $field;
|
||||||
// Extract field name in case this is a method called on a field (e.g. "Date.Nice")
|
|
||||||
if (($fieldPos = strpos($name ?? '', '.')) !== false) {
|
|
||||||
$name = substr($name ?? '', 0, $fieldPos);
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($schema->fieldSpec($this, $name)) {
|
|
||||||
$fields[] = $name;
|
|
||||||
} elseif ($this->relObject($spec)) {
|
|
||||||
$fields[] = $spec;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -25,6 +25,7 @@ use SilverStripe\ORM\Tests\DataObjectTest\TreeNode;
|
|||||||
use SilverStripe\ORM\ValidationException;
|
use SilverStripe\ORM\ValidationException;
|
||||||
use SilverStripe\Security\Member;
|
use SilverStripe\Security\Member;
|
||||||
use SilverStripe\View\ViewableData;
|
use SilverStripe\View\ViewableData;
|
||||||
|
use ReflectionMethod;
|
||||||
use stdClass;
|
use stdClass;
|
||||||
|
|
||||||
class DataObjectTest extends SapphireTest
|
class DataObjectTest extends SapphireTest
|
||||||
@ -2671,4 +2672,48 @@ class DataObjectTest extends SapphireTest
|
|||||||
$vals = ['25.25', '50.00', '75.00', '100.50'];
|
$vals = ['25.25', '50.00', '75.00', '100.50'];
|
||||||
$this->assertSame(array_combine($vals ?? [], $vals ?? []), $obj->dbObject('MyEnumWithDots')->enumValues());
|
$this->assertSame(array_combine($vals ?? [], $vals ?? []), $obj->dbObject('MyEnumWithDots')->enumValues());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function provideTestGetDatabaseBackedField()
|
||||||
|
{
|
||||||
|
return [
|
||||||
|
['Captain.IsRetired', 'Captain.IsRetired'],
|
||||||
|
['Captain.ShirtNumber', 'Captain.ShirtNumber'],
|
||||||
|
['Captain.FavouriteTeam', null],
|
||||||
|
['Captain.FavouriteTeam.Fans', null],
|
||||||
|
['Captain.FavouriteTeam.Fans.Count', null],
|
||||||
|
['Captain.FavouriteTeam.Title', 'Captain.FavouriteTeam.Title'],
|
||||||
|
['Captain.FavouriteTeam.Title.Plain', 'Captain.FavouriteTeam.Title'],
|
||||||
|
['Captain.FavouriteTeam.ReturnsNull', null],
|
||||||
|
['Captain.FavouriteTeam.MethodDoesNotExist', null],
|
||||||
|
['Captain.ReturnsNull', null],
|
||||||
|
['Founder.FavouriteTeam.Captain.ShirtNumber', 'Founder.FavouriteTeam.Captain.ShirtNumber'],
|
||||||
|
['Founder.FavouriteTeam.Captain.Fans', null],
|
||||||
|
['Founder.FavouriteTeam.Captain.Fans.Name.Plain', 'Founder.FavouriteTeam.Captain.Fans.Name'],
|
||||||
|
['Founder.FavouriteTeam.Captain.ReturnsNull', null],
|
||||||
|
['HasOneRelationship.FavouriteTeam.MyTitle', null],
|
||||||
|
['SubTeams.Comments.Name.Plain', 'SubTeams.Comments.Name'],
|
||||||
|
['Title', 'Title'],
|
||||||
|
['Title.Plain', 'Title'],
|
||||||
|
['DatabaseField', 'DatabaseField'],
|
||||||
|
['DatabaseField.MethodDoesNotExist', 'DatabaseField'],
|
||||||
|
['ReturnsNull', null],
|
||||||
|
['DynamicField', null],
|
||||||
|
['SubTeams.ParentTeam.Fans', null],
|
||||||
|
['SubTeams.ParentTeam.Founder.FoundingTeams', null],
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider provideTestGetDatabaseBackedField
|
||||||
|
*/
|
||||||
|
public function testGetDatabaseBackedField(string $fieldPath, $expected)
|
||||||
|
{
|
||||||
|
$dataObjectClass = new DataObject();
|
||||||
|
$method = new ReflectionMethod($dataObjectClass, 'getDatabaseBackedField');
|
||||||
|
$method->setAccessible(true);
|
||||||
|
$class = new Team([]);
|
||||||
|
|
||||||
|
$databaseBackedField = $method->invokeArgs($class, [$fieldPath]);
|
||||||
|
$this->assertSame($expected, $databaseBackedField);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -37,4 +37,9 @@ class Player extends Member implements TestOnly
|
|||||||
'IsRetired',
|
'IsRetired',
|
||||||
'ShirtNumber'
|
'ShirtNumber'
|
||||||
];
|
];
|
||||||
|
|
||||||
|
public function ReturnsNull()
|
||||||
|
{
|
||||||
|
return null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -58,13 +58,22 @@ class SearchContextTest extends SapphireTest
|
|||||||
$obj = new SearchContextTest\NoSearchableFields();
|
$obj = new SearchContextTest\NoSearchableFields();
|
||||||
$summaryFields = $obj->summaryFields();
|
$summaryFields = $obj->summaryFields();
|
||||||
$expected = [];
|
$expected = [];
|
||||||
foreach ($summaryFields as $field => $label) {
|
|
||||||
|
$expectedSearchableFields = [
|
||||||
|
'Name',
|
||||||
|
'Customer.FirstName',
|
||||||
|
'HairColor',
|
||||||
|
'EyeColor',
|
||||||
|
];
|
||||||
|
|
||||||
|
foreach ($expectedSearchableFields as $field) {
|
||||||
$expected[$field] = [
|
$expected[$field] = [
|
||||||
'title' => $obj->fieldLabel($field),
|
'title' => $obj->fieldLabel($field),
|
||||||
'filter' => 'PartialMatchFilter',
|
'filter' => 'PartialMatchFilter',
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
$this->assertEquals($expected, $obj->searchableFields());
|
$this->assertEquals($expected, $obj->searchableFields());
|
||||||
|
$this->assertNotEquals($summaryFields, $obj->searchableFields());
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testSummaryIncludesDefaultFieldsIfNotDefined()
|
public function testSummaryIncludesDefaultFieldsIfNotDefined()
|
||||||
|
@ -4,6 +4,7 @@ namespace SilverStripe\ORM\Tests\Search\SearchContextTest;
|
|||||||
|
|
||||||
use SilverStripe\Dev\TestOnly;
|
use SilverStripe\Dev\TestOnly;
|
||||||
use SilverStripe\ORM\DataObject;
|
use SilverStripe\ORM\DataObject;
|
||||||
|
use SilverStripe\Assets\Image;
|
||||||
|
|
||||||
class NoSearchableFields extends DataObject implements TestOnly
|
class NoSearchableFields extends DataObject implements TestOnly
|
||||||
{
|
{
|
||||||
@ -18,12 +19,34 @@ class NoSearchableFields extends DataObject implements TestOnly
|
|||||||
|
|
||||||
private static $has_one = [
|
private static $has_one = [
|
||||||
'Customer' => Customer::class,
|
'Customer' => Customer::class,
|
||||||
|
'Image' => Image::class,
|
||||||
];
|
];
|
||||||
|
|
||||||
private static $summary_fields = [
|
private static $summary_fields = [
|
||||||
'Name' => 'Custom Label',
|
'Name' => 'Custom Label',
|
||||||
|
'Customer' => 'Customer',
|
||||||
'Customer.FirstName' => 'Customer',
|
'Customer.FirstName' => 'Customer',
|
||||||
|
'Image.CMSThumbnail' => 'Image',
|
||||||
|
'Image.BackLinks' => 'Backlinks',
|
||||||
|
'Image.BackLinks.Count' => 'Backlinks',
|
||||||
'HairColor',
|
'HairColor',
|
||||||
'EyeColor',
|
'EyeColor',
|
||||||
|
'ReturnsNull',
|
||||||
|
'DynamicField'
|
||||||
];
|
];
|
||||||
|
|
||||||
|
public function MyName()
|
||||||
|
{
|
||||||
|
return 'Class ' . $this->Name;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getDynamicField()
|
||||||
|
{
|
||||||
|
return 'dynamicfield';
|
||||||
|
}
|
||||||
|
|
||||||
|
public function ReturnsNull()
|
||||||
|
{
|
||||||
|
return null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user