Merge pull request #10772 from creative-commoners/pulls/4.13/gridfieldfilter-regression

FIX Don't assume searchableFields() exists in gridfield filter
This commit is contained in:
Michal Kleiner 2023-05-09 20:29:24 +12:00 committed by GitHub
commit 218c9942bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 76 additions and 13 deletions

View File

@ -6,6 +6,7 @@ use LogicException;
use SilverStripe\Admin\LeftAndMain;
use SilverStripe\Control\Controller;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
use SilverStripe\Dev\Deprecation;
@ -258,20 +259,36 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi
return false;
}
$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)) {
$singleton = singleton($modelClass);
if (ClassInfo::hasMethod($singleton, 'summaryFields')
&& ClassInfo::hasMethod($singleton, 'searchableFields')
) {
// note: searchableFields() will return summary_fields if there are no searchable_fields on the model
$searchableFields = array_keys($singleton->searchableFields());
$summaryFields = array_keys($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;
}
}
} else {
// Allows non-DataObject classes to be used with this component
$columns = $gridField->getColumns();
foreach ($columns as $columnField) {
$metadata = $gridField->getColumnMetadata($columnField);
$title = $metadata['title'];
if ($title && $list->canFilterBy($columnField)) {
return true;
}
}
}
return false;
}

View File

@ -8,13 +8,16 @@ use SilverStripe\Dev\SapphireTest;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form;
use SilverStripe\Forms\GridField\GridField;
use SilverStripe\Forms\GridField\GridFieldConfig;
use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor;
use SilverStripe\Forms\GridField\GridFieldFilterHeader;
use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Cheerleader;
use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\CheerleaderHat;
use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Mom;
use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\NonDataObject;
use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Team;
use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\TeamGroup;
use SilverStripe\ORM\ArrayList;
use SilverStripe\ORM\DataList;
use SilverStripe\ORM\DataObject;
@ -187,7 +190,7 @@ class GridFieldFilterHeaderTest extends SapphireTest
{
$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
@ -208,4 +211,33 @@ class GridFieldFilterHeaderTest extends SapphireTest
Config::modify()->set(Team::class, 'summary_fields', ['MySummaryField']);
$this->assertFalse($filterHeader->canFilterAnyColumns($gridField));
}
public function testCanFilterAnyColumnsNonDataObject()
{
$list = new ArrayList([
new NonDataObject([]),
]);
$config = GridFieldConfig::create()->addComponent(new GridFieldFilterHeader());
$gridField = new GridField('testfield', 'testfield', $list, $config);
$form = new Form(null, 'Form', new FieldList([$gridField]), new FieldList());
/** @var GridFieldFilterHeader $component */
$component = $gridField->getConfig()->getComponentByType(GridFieldFilterHeader::class);
$this->assertFalse($component->canFilterAnyColumns($gridField));
}
public function testRenderHeadersNonDataObject()
{
$list = new ArrayList([
new NonDataObject([]),
]);
$config = GridFieldConfig::create()->addComponent(new GridFieldFilterHeader());
$gridField = new GridField('testfield', 'testfield', $list, $config);
$form = new Form(null, 'Form', new FieldList([$gridField]), new FieldList());
/** @var GridFieldFilterHeader $component */
$component = $gridField->getConfig()->getComponentByType(GridFieldFilterHeader::class);
$htmlFragment = $component->getHTMLFragments($gridField);
$this->assertNull($htmlFragment);
}
}

View File

@ -0,0 +1,14 @@
<?php
namespace SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest;
use SilverStripe\Dev\TestOnly;
use SilverStripe\View\ArrayData;
class NonDataObject extends ArrayData implements TestOnly
{
public function summaryFields()
{
return ['Title' => 'Title'];
}
}