From 0f40cc38ecf17f572610efdc0e3e1edf253ce0a8 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 23 Mar 2023 10:57:03 +1300 Subject: [PATCH] FIX Respect searchable_fields --- src/Forms/GridField/GridFieldFilterHeader.php | 24 ++++++++++------- .../GridField/GridFieldFilterHeaderTest.php | 26 +++++++++++++++++++ .../GridFieldFilterHeaderTest/Team.php | 5 ++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 58599b2fe..6292985be 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -254,21 +254,25 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi public function canFilterAnyColumns($gridField) { $list = $gridField->getList(); - - if (!$this->checkDataType($list)) { + if (!($list instanceof Filterable) || !$this->checkDataType($list)) { return false; } - - $columns = $gridField->getColumns(); - foreach ($columns as $columnField) { - $metadata = $gridField->getColumnMetadata($columnField); - $title = $metadata['title']; - - if ($title && $list->canFilterBy($columnField)) { + $modelClass = $gridField->getModelClass(); + // note: searchableFields() will return summary_fields if there are no searchable_fields on the model + $searchableFields = array_keys($modelClass::singleton()->searchableFields()); + $summaryFields = array_keys($modelClass::singleton()->summaryFields()); + sort($searchableFields); + sort($summaryFields); + // searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields + if ($searchableFields !== $summaryFields) { + return true; + } + // we have fallen back to summary_fields, check they are filterable + foreach ($searchableFields as $searchableField) { + if ($list->canFilterBy($searchableField)) { return true; } } - return false; } diff --git a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php index db70ddddb..bf6c95b8c 100644 --- a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php +++ b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php @@ -182,4 +182,30 @@ class GridFieldFilterHeaderTest extends SapphireTest $this->assertEquals('ReallyCustomSearch', $this->component->getSearchField()); } + + public function testCanFilterAnyColumns() + { + $gridField = $this->gridField; + $filterHeader = $gridField->getConfig()->getComponentByType(GridFieldFilterHeader::class); + + // test that you can filter by something if searchable_fields is not defined + // silverstripe will scaffold db columns that are in the gridfield to be + // searchable by default + Config::modify()->remove(Team::class, 'searchable_fields'); + $this->assertTrue($filterHeader->canFilterAnyColumns($gridField)); + + // test that you can filterBy if searchable_fields is defined + Config::modify()->set(Team::class, 'searchable_fields', ['Name']); + $this->assertTrue($filterHeader->canFilterAnyColumns($gridField)); + + // test that you can filterBy if searchable_fields even if it is not a legit field + // this is because we're making a blind assumption it will be filterable later in a SearchContext + Config::modify()->set(Team::class, 'searchable_fields', ['WhatIsThis']); + $this->assertTrue($filterHeader->canFilterAnyColumns($gridField)); + + // test that you cannot filter by non-db field when it falls back to summary_fields + Config::modify()->remove(Team::class, 'searchable_fields'); + Config::modify()->set(Team::class, 'summary_fields', ['MySummaryField']); + $this->assertFalse($filterHeader->canFilterAnyColumns($gridField)); + } } diff --git a/tests/php/Forms/GridField/GridFieldFilterHeaderTest/Team.php b/tests/php/Forms/GridField/GridFieldFilterHeaderTest/Team.php index 98d587975..b39eefe56 100644 --- a/tests/php/Forms/GridField/GridFieldFilterHeaderTest/Team.php +++ b/tests/php/Forms/GridField/GridFieldFilterHeaderTest/Team.php @@ -24,4 +24,9 @@ class Team extends DataObject implements TestOnly 'Cheerleader' => Cheerleader::class, 'CheerleadersMom' => Mom::class ]; + + public function getMySummaryField() + { + return 'MY SUMMARY FIELD'; + } }