Skip to content

Commit

Permalink
Merge pull request #10772 from creative-commoners/pulls/4.13/gridfiel…
Browse files Browse the repository at this point in the history
…dfilter-regression

FIX Don't assume searchableFields() exists in gridfield filter
  • Loading branch information
michalkleiner authored May 9, 2023
2 parents 908b6f2 + 01808a8 commit 218c994
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 13 deletions.
41 changes: 29 additions & 12 deletions src/Forms/GridField/GridFieldFilterHeader.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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;
Expand Down Expand Up @@ -258,20 +259,36 @@ public function canFilterAnyColumns($gridField)
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;
}
Expand Down
34 changes: 33 additions & 1 deletion tests/php/Forms/GridField/GridFieldFilterHeaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
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;

Expand Down Expand Up @@ -187,7 +190,7 @@ 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
Expand All @@ -208,4 +211,33 @@ public function testCanFilterAnyColumns()
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);
}
}
Original file line number Diff line number Diff line change
@@ -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'];
}
}

0 comments on commit 218c994

Please sign in to comment.