From d24095aba8de6e3328859953f00e1c6a4ac00db3 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Mon, 24 Jul 2023 16:10:47 +1200 Subject: [PATCH] FIX Image in summaryfields breaks search --- src/Forms/GridField/GridFieldFilterHeader.php | 2 +- src/ORM/DataObject.php | 63 ++++++++++++++----- tests/php/ORM/DataObjectTest.php | 45 +++++++++++++ tests/php/ORM/DataObjectTest/Player.php | 5 ++ tests/php/ORM/Search/SearchContextTest.php | 11 +++- .../SearchContextTest/NoSearchableFields.php | 23 +++++++ 6 files changed, 133 insertions(+), 16 deletions(-) diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index a0fff8038b0..e16e870a271 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -269,7 +269,7 @@ public function canFilterAnyColumns($gridField) sort($searchableFields); sort($summaryFields); // 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; } // we have fallen back to summary_fields, check they are filterable diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index cdb804206e2..f17c5d4873a 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -3724,6 +3724,52 @@ public function 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 * $searchable_fields list. If searchable fields are not defined on the @@ -3742,21 +3788,10 @@ public function searchableFields() $summaryFields = array_keys($this->summaryFields() ?? []); $fields = []; - // remove the custom getters as the search should not include them - $schema = static::getSchema(); if ($summaryFields) { - foreach ($summaryFields as $key => $name) { - $spec = $name; - - // 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; + foreach ($summaryFields as $name) { + if ($field = $this->getDatabaseBackedField($name)) { + $fields[] = $field; } } } diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 9ec9e162990..ed6c6a199d8 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -25,6 +25,7 @@ use SilverStripe\ORM\ValidationException; use SilverStripe\Security\Member; use SilverStripe\View\ViewableData; +use ReflectionMethod; use stdClass; class DataObjectTest extends SapphireTest @@ -2671,4 +2672,48 @@ public function testDBObjectEnum() $vals = ['25.25', '50.00', '75.00', '100.50']; $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); + } } diff --git a/tests/php/ORM/DataObjectTest/Player.php b/tests/php/ORM/DataObjectTest/Player.php index 0dc5a795f77..4aa2f69c791 100644 --- a/tests/php/ORM/DataObjectTest/Player.php +++ b/tests/php/ORM/DataObjectTest/Player.php @@ -37,4 +37,9 @@ class Player extends Member implements TestOnly 'IsRetired', 'ShirtNumber' ]; + + public function ReturnsNull() + { + return null; + } } diff --git a/tests/php/ORM/Search/SearchContextTest.php b/tests/php/ORM/Search/SearchContextTest.php index 8c80a44927c..7236709cbd4 100644 --- a/tests/php/ORM/Search/SearchContextTest.php +++ b/tests/php/ORM/Search/SearchContextTest.php @@ -58,13 +58,22 @@ public function testSearchableFieldsDefaultsToSummaryFields() $obj = new SearchContextTest\NoSearchableFields(); $summaryFields = $obj->summaryFields(); $expected = []; - foreach ($summaryFields as $field => $label) { + + $expectedSearchableFields = [ + 'Name', + 'Customer.FirstName', + 'HairColor', + 'EyeColor', + ]; + + foreach ($expectedSearchableFields as $field) { $expected[$field] = [ 'title' => $obj->fieldLabel($field), 'filter' => 'PartialMatchFilter', ]; } $this->assertEquals($expected, $obj->searchableFields()); + $this->assertNotEquals($summaryFields, $obj->searchableFields()); } public function testSummaryIncludesDefaultFieldsIfNotDefined() diff --git a/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php b/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php index 39ead853720..515300895a9 100644 --- a/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php +++ b/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php @@ -4,6 +4,7 @@ use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; +use SilverStripe\Assets\Image; class NoSearchableFields extends DataObject implements TestOnly { @@ -18,12 +19,34 @@ class NoSearchableFields extends DataObject implements TestOnly private static $has_one = [ 'Customer' => Customer::class, + 'Image' => Image::class, ]; private static $summary_fields = [ 'Name' => 'Custom Label', + 'Customer' => 'Customer', 'Customer.FirstName' => 'Customer', + 'Image.CMSThumbnail' => 'Image', + 'Image.BackLinks' => 'Backlinks', + 'Image.BackLinks.Count' => 'Backlinks', 'HairColor', 'EyeColor', + 'ReturnsNull', + 'DynamicField' ]; + + public function MyName() + { + return 'Class ' . $this->Name; + } + + public function getDynamicField() + { + return 'dynamicfield'; + } + + public function ReturnsNull() + { + return null; + } }