From 0c31c3f2e93d5841119b84fd5233912d0a1f9225 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 31 Jan 2025 14:09:11 +1300 Subject: [PATCH] NEW Handle WithinRangeFilter in searchable_fields and SearchContext This is especially helpful with date ranges, e.g. all records edited within a certain range, but can be used out of the box with basically any numeric, date, or time-based fields. --- .../FieldValidation/BigIntFieldValidator.php | 2 + src/ORM/DataObject.php | 148 +++++++--- src/ORM/FieldType/DBDatetime.php | 1 + src/ORM/Filters/WithinRangeFilter.php | 19 +- src/ORM/Search/SearchContext.php | 59 +++- .../GridField/GridFieldFilterHeaderTest.php | 13 +- .../GridField/GridFieldFilterHeaderTest.yml | 4 + .../ModelWithBadSearchableFields.php | 34 +++ tests/php/ORM/DataObjectTest.php | 246 ++++++++++++++++ tests/php/ORM/DataObjectTest/Player.php | 10 + .../php/ORM/Search/BasicSearchContextTest.php | 22 ++ .../php/ORM/Search/BasicSearchContextTest.yml | 38 +++ tests/php/ORM/Search/SearchContextTest.php | 276 +++++++++++++++++- tests/php/ORM/Search/SearchContextTest.yml | 38 +++ .../SearchContextTest/AllFilterTypes.php | 3 + .../WithinRangeFilterModel.php | 63 ++++ 16 files changed, 919 insertions(+), 57 deletions(-) create mode 100644 tests/php/Forms/GridField/GridFieldFilterHeaderTest/ModelWithBadSearchableFields.php create mode 100644 tests/php/ORM/Search/SearchContextTest/WithinRangeFilterModel.php diff --git a/src/Core/Validation/FieldValidation/BigIntFieldValidator.php b/src/Core/Validation/FieldValidation/BigIntFieldValidator.php index 4e48ef15b6a..646caf1ed08 100644 --- a/src/Core/Validation/FieldValidation/BigIntFieldValidator.php +++ b/src/Core/Validation/FieldValidation/BigIntFieldValidator.php @@ -23,6 +23,8 @@ public function __construct( if ($bits === 32) { throw new RunTimeException('Cannot use BigIntFieldValidator on a 32-bit system'); } + $minValue ??= (int) DBBigInt::getMinValue(); + $maxValue ??= (int) DBBigInt::getMaxValue(); } $this->minValue = $minValue; $this->maxValue = $maxValue; diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 6fb7ad2d0e4..b6b2062a7c0 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -16,6 +16,7 @@ use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\Dev\Debug; use SilverStripe\Dev\Deprecation; +use SilverStripe\Forms\FieldGroup; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; @@ -46,6 +47,7 @@ use SilverStripe\Security\Security; use SilverStripe\View\SSViewer; use SilverStripe\Model\ModelData; +use SilverStripe\ORM\Filters\WithinRangeFilter; use stdClass; /** @@ -2438,40 +2440,7 @@ public function scaffoldSearchFields($_params = null) continue; } - // If a custom fieldclass is provided as a string, use it - $field = null; - if ($params['fieldClasses'] && isset($params['fieldClasses'][$fieldName])) { - $fieldClass = $params['fieldClasses'][$fieldName]; - $field = new $fieldClass($fieldName); - // If we explicitly set a field, then construct that - } elseif (isset($spec['field'])) { - // If it's a string, use it as a class name and construct - if (is_string($spec['field'])) { - $fieldClass = $spec['field']; - $field = new $fieldClass($fieldName); - - // If it's a FormField object, then just use that object directly. - } elseif ($spec['field'] instanceof FormField) { - $field = $spec['field']; - - // Otherwise we have a bug - } else { - user_error("Bad value for searchable_fields, 'field' value: " - . var_export($spec['field'], true), E_USER_WARNING); - } - - // Otherwise, use the database field's scaffolder - } elseif ($object = $this->relObject($fieldName)) { - if (is_object($object) && $object->hasMethod('scaffoldSearchField')) { - $field = $object->scaffoldSearchField(); - } else { - throw new Exception(sprintf( - "SearchField '%s' on '%s' does not return a valid DBField instance.", - $fieldName, - get_class($this) - )); - } - } + $field = $this->scaffoldSingleSearchField($fieldName, $spec, $params); // Allow fields to opt out of search if (!$field) { @@ -2483,6 +2452,24 @@ public function scaffoldSearchFields($_params = null) } $field->setTitle($spec['title']); + // If we're using a WithinRangeFilter, split the field into two separate fields (from and to) + if (is_a($spec['filter'] ?? '', WithinRangeFilter::class, true)) { + $fieldFrom = $field; + $fieldTo = clone $field; + $originalTitle = $field->Title(); + $originalName = $field->getName(); + + $fieldFrom->setName($originalName . '_SearchFrom'); + $fieldFrom->setTitle(_t(__CLASS__ . '.FILTER_WITHINRANGE_FROM', 'From')); + $fieldTo->setName($originalName . '_SearchTo'); + $fieldTo->setTitle(_t(__CLASS__ . '.FILTER_WITHINRANGE_TO', 'To')); + + $field = FieldGroup::create( + $originalTitle, + [$fieldFrom, $fieldTo] + )->setName($originalName)->addExtraClass('fieldgroup--fill-width'); + } + $fields->push($field); } @@ -2498,6 +2485,56 @@ public function scaffoldSearchFields($_params = null) return $fields; } + /** + * Scaffold a single form field for use by the search context form. + */ + private function scaffoldSingleSearchField(string $fieldName, array $spec, ?array $params): ?FormField + { + // If a custom fieldclass is provided as a string, use it + $field = null; + if ($params['fieldClasses'] && isset($params['fieldClasses'][$fieldName])) { + $fieldClass = $params['fieldClasses'][$fieldName]; + $field = new $fieldClass($fieldName); + // If we explicitly set a field, then construct that + } elseif (isset($spec['field'])) { + // If it's a string, use it as a class name and construct + if (is_string($spec['field'])) { + $fieldClass = $spec['field']; + $field = new $fieldClass($fieldName); + + // If it's a FormField object, then just use that object directly. + } elseif ($spec['field'] instanceof FormField) { + $field = $spec['field']; + + // Otherwise we have a bug + } else { + user_error("Bad value for searchable_fields, 'field' value: " + . var_export($spec['field'], true), E_USER_WARNING); + } + // Use the explicitly defined dataType if one was set + } elseif (isset($spec['dataType'])) { + $object = Injector::inst()->get($spec['dataType'], true); + $field = $this->scaffoldFieldFromObject($object, $fieldName); + $field->setName($fieldName); + // Otherwise, use the database field's scaffolder + } elseif ($object = $this->relObject($fieldName)) { + $field = $this->scaffoldFieldFromObject($object, $fieldName); + } + return $field; + } + + private function scaffoldFieldFromObject(mixed $object, string $fieldName): FormField + { + if (!is_object($object) || !$object->hasMethod('scaffoldSearchField')) { + throw new LogicException(sprintf( + "SearchField '%s' on '%s' does not return a valid DBField instance.", + $fieldName, + get_class($this) + )); + } + return $object->scaffoldSearchField(); + } + /** * Scaffold a simple edit form for all properties on this dataobject, * based on default {@link FormField} mapping in {@link DBField::scaffoldFormField()}. @@ -3896,6 +3933,7 @@ public function searchableFields() $rewrite = []; foreach ($fields as $name => $specOrName) { $identifier = (is_int($name)) ? $specOrName : $name; + $relObject = isset($specOrName['match_any']) ? null : $this->relObject($identifier); if (is_int($name)) { // Format: array('MyFieldName') @@ -3903,14 +3941,22 @@ public function searchableFields() } elseif (is_array($specOrName) && (isset($specOrName['match_any']))) { $rewrite[$identifier] = $fields[$identifier]; $rewrite[$identifier]['match_any'] = $specOrName['match_any']; - } elseif (is_array($specOrName) && ($relObject = $this->relObject($identifier))) { + } elseif (is_array($specOrName)) { // Format: array('MyFieldName' => array( // 'filter => 'ExactMatchFilter', // 'field' => 'NumericField', // optional // 'title' => 'My Title', // optional + // 'dataType' => DBInt::class // optional + // These two are only required if using WithinRangeFilter with a data type that doesn't + // inherently represent a date, time, or number + // 'rangeFromDefault' => PHP_INT_MIN + // 'rangeToDefault' => PHP_INT_MAX // )) $rewrite[$identifier] = array_merge( - ['filter' => $relObject->config()->get('default_search_filter_class')], + [ + 'filter' => $relObject?->config()->get('default_search_filter_class'), + 'dataType' => $relObject ? get_class($relObject) : null, + ], (array)$specOrName ); } else { @@ -3918,6 +3964,9 @@ public function searchableFields() $rewrite[$identifier] = [ 'filter' => $specOrName, ]; + if ($relObject !== null) { + $rewrite[$identifier]['dataType'] ??= get_class($relObject); + } } if (!isset($rewrite[$identifier]['title'])) { $rewrite[$identifier]['title'] = (isset($labels[$identifier])) @@ -3926,6 +3975,33 @@ public function searchableFields() if (!isset($rewrite[$identifier]['filter'])) { $rewrite[$identifier]['filter'] = 'PartialMatchFilter'; } + // When using a WithinRangeFilter we need to know what the default from and to values + // should be, so that if a user only enters one of the two fields the other can be + // populated appropriately within the filter. + if (is_a($rewrite[$identifier]['filter'], WithinRangeFilter::class, true)) { + // The dataType requirement here is explicitly for WithinRangeFilter. + // DO NOT make it mandatory for other filters without first checking if this breaks + // anything for filtering a relation, where the class on the other end of the relation + // implements scaffoldSearchField(). + $dataType = $rewrite[$identifier]['dataType'] ?? null; + if (!is_a($dataType ?? '', DBField::class, true)) { + throw new LogicException("dataType must be set to a DBField class for '$identifier'"); + } + if (!isset($rewrite[$identifier]['rangeFromDefault'])) { + $fromDefault = $dataType::getMinValue(); + if ($fromDefault === null) { + throw new LogicException("rangeFromDefault must be set for '$identifier'"); + } + $rewrite[$identifier]['rangeFromDefault'] = $fromDefault; + } + if (!isset($rewrite[$identifier]['rangeToDefault'])) { + $toDefault = $dataType::getMaxValue(); + if ($toDefault === null) { + throw new LogicException("rangeToDefault must be set for '$identifier'"); + } + $rewrite[$identifier]['rangeToDefault'] = $toDefault; + } + } } $fields = $rewrite; diff --git a/src/ORM/FieldType/DBDatetime.php b/src/ORM/FieldType/DBDatetime.php index 62f03e4913b..0ba47e72d9a 100644 --- a/src/ORM/FieldType/DBDatetime.php +++ b/src/ORM/FieldType/DBDatetime.php @@ -15,6 +15,7 @@ use SilverStripe\Model\ModelData; use SilverStripe\Core\Validation\FieldValidation\DateFieldValidator; use SilverStripe\Core\Validation\FieldValidation\DatetimeFieldValidator; +use SilverStripe\ORM\Tests\Search\SearchContextTest\WithinRangeFilterModel; /** * Represents a date-time field. diff --git a/src/ORM/Filters/WithinRangeFilter.php b/src/ORM/Filters/WithinRangeFilter.php index 55f943e334c..0dd15671486 100644 --- a/src/ORM/Filters/WithinRangeFilter.php +++ b/src/ORM/Filters/WithinRangeFilter.php @@ -6,20 +6,29 @@ class WithinRangeFilter extends SearchFilter { + private mixed $min = null; + private mixed $max = null; - private $min; - private $max; - - public function setMin($min) + public function setMin(mixed $min) { $this->min = $min; } - public function setMax($max) + public function getMin(): mixed + { + return $this->min; + } + + public function setMax(mixed $max) { $this->max = $max; } + public function getMax(): mixed + { + return $this->max; + } + protected function applyOne(DataQuery $query) { $this->model = $query->applyRelation($this->relation); diff --git a/src/ORM/Search/SearchContext.php b/src/ORM/Search/SearchContext.php index 7fd9da243b2..d360027cefd 100644 --- a/src/ORM/Search/SearchContext.php +++ b/src/ORM/Search/SearchContext.php @@ -19,7 +19,10 @@ use Exception; use LogicException; use SilverStripe\Core\Config\Config; +use SilverStripe\Forms\DateField; use SilverStripe\ORM\DataQuery; +use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\ORM\Filters\WithinRangeFilter; /** * Manages searching of properties on one or more {@link DataObject} @@ -78,6 +81,13 @@ class SearchContext */ protected $searchParams = []; + /** + * A list of fields that use WithinRangeFilter that have already been included in the query. + * This prevents the "from" and "to" fields from both independently affecting the query since they + * have to be paired together in the same filter. + */ + protected $withinRangeFieldsChecked = []; + /** * A key value pair of values that should be searched for. * The keys should match the field names specified in {@link SearchContext::$fields}. @@ -161,6 +171,7 @@ public function getQuery($searchParams, $sort = false, int|array|null $limit = n */ private function search(DataList $query): DataList { + $this->withinRangeFieldsChecked = []; /** @var DataObject $modelObj */ $modelObj = Injector::inst()->create($this->modelClass); $searchableFields = $modelObj->searchableFields(); @@ -296,8 +307,35 @@ private function individualFieldSearch(DataList $query, array $searchableFields, return $query; } $filter->setModel($this->modelClass); - $filter->setValue($searchPhrase); - $searchableFieldSpec = $searchableFields[$searchField] ?? []; + // WithinRangeFilter needs a bit of help knowing what its "from" and "to" values are + if ($filter instanceof WithinRangeFilter) { + $baseName = preg_replace('/_Search(From|To)$/', '', $searchField); + if (array_key_exists($baseName, $this->withinRangeFieldsChecked)) { + return $query; + } + $allSearchParams = $this->getSearchParams(); + $searchableFieldSpec = $searchableFields[$baseName] ?? []; + $from = $allSearchParams[$baseName . '_SearchFrom'] ?? $searchableFieldSpec['rangeFromDefault']; + $to = $allSearchParams[$baseName . '_SearchTo'] ?? $searchableFieldSpec['rangeToDefault']; + // If we're using DateField for a DBDateTime, set "from" to the start of the day, and "to" to the end of the day. + // Though if we're using the default values, we don't need to add this as it should already be there. + if (is_a($searchableFieldSpec['dataType'] ?? '', DBDatetime::class, true) + && is_a($searchableFieldSpec['field'] ?? '', DateField::class, true) + ) { + if ($from !== $searchableFieldSpec['rangeFromDefault']) { + $from .= ' 00:00:00'; + } + if ($to !== $searchableFieldSpec['rangeToDefault']) { + $to .= ' 23:59:59'; + } + } + $filter->setMin($from); + $filter->setMax($to); + $this->withinRangeFieldsChecked[$baseName] = true; + } else { + $filter->setValue($searchPhrase); + $searchableFieldSpec = $searchableFields[$searchField] ?? []; + } return $query->alterDataQuery(function ($dataQuery) use ($filter, $searchableFieldSpec) { $this->applyFilter($filter, $dataQuery, $searchableFieldSpec); }); @@ -318,9 +356,13 @@ private function applyFilter(SearchFilter $filter, DataQuery $dataQuery, array $ $modifiers = $filter->getModifiers(); $subGroup = $dataQuery->disjunctiveGroup(); foreach ($searchFields as $matchField) { - /** @var SearchFilter $filter */ - $filter = Injector::inst()->create($filterClass, $matchField, $value, $modifiers); - $filter->apply($subGroup); + /** @var SearchFilter $subFilter */ + $subFilter = Injector::inst()->create($filterClass, $matchField, $value, $modifiers); + if ($subFilter instanceof WithinRangeFilter) { + $subFilter->setMin($filter->getMin()); + $subFilter->setMax($filter->getMax()); + } + $subFilter->apply($subGroup); } } else { $filter->apply($dataQuery); @@ -364,11 +406,8 @@ public function clearEmptySearchFields($value) */ public function getFilter($name) { - if (isset($this->filters[$name])) { - return $this->filters[$name]; - } else { - return null; - } + $withinRangeFilterCheck = preg_replace('/_Search(From|To)$/', '', $name); + return $this->filters[$name] ?? $this->filters[$withinRangeFilterCheck] ?? null; } /** diff --git a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php index 40f76006b92..913ebdc46b3 100644 --- a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php +++ b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php @@ -15,6 +15,7 @@ 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\ModelWithBadSearchableFields; use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Mom; use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\NonDataObject; use SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Team; @@ -59,6 +60,7 @@ class GridFieldFilterHeaderTest extends SapphireTest Cheerleader::class, CheerleaderHat::class, Mom::class, + ModelWithBadSearchableFields::class, ]; protected function setUp(): void @@ -222,15 +224,16 @@ public function testCanFilterAnyColumns() 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)); + + // test that you can filterBy even if searchableFields() includes a non-db field + // this is because we're making a blind assumption it will be filterable in a custom SearchContext + $gridField->setList(ModelWithBadSearchableFields::get()); + $gridField->setModelClass(ModelWithBadSearchableFields::class); + $this->assertTrue($filterHeader->canFilterAnyColumns($gridField)); } public function testCanFilterAnyColumnsNonDataObject() diff --git a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.yml b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.yml index 47eeeb76528..d837d6dbe37 100644 --- a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.yml +++ b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.yml @@ -74,3 +74,7 @@ SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\TeamGroup: City: Melbourne Cheerleader: =>SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Cheerleader.cheerleader1 CheerleadersMom: =>SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\Mom.mom1 + +SilverStripe\Forms\Tests\GridField\GridFieldFilterHeaderTest\ModelWithBadSearchableFields: + record1: + Name: Record 1 diff --git a/tests/php/Forms/GridField/GridFieldFilterHeaderTest/ModelWithBadSearchableFields.php b/tests/php/Forms/GridField/GridFieldFilterHeaderTest/ModelWithBadSearchableFields.php new file mode 100644 index 00000000000..7412f343b95 --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldFilterHeaderTest/ModelWithBadSearchableFields.php @@ -0,0 +1,34 @@ + 'Varchar', + ]; + + private static $summary_fields = [ + 'Name' => 'Name', + ]; + + // Explicitly empty + private static $searchable_fields = []; + + public function searchableFields() + { + // Explicitly only include this custom field + return [ + 'WhatIsThis' => [ + 'field' => TextField::class, + 'title' => $this->fieldLabel('WhatIsThis'), + ], + ]; + } +} diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 7ed3106a0f0..7a8f4c0ed9a 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -27,6 +27,17 @@ use SilverStripe\Security\Member; use SilverStripe\Model\ModelData; use ReflectionMethod; +use SilverStripe\Forms\CheckboxField; +use SilverStripe\Forms\CompositeField; +use SilverStripe\Forms\DatalessField; +use SilverStripe\Forms\DropdownField; +use SilverStripe\Forms\HiddenField; +use SilverStripe\Forms\LiteralField; +use SilverStripe\Forms\NumericField; +use SilverStripe\Forms\TextareaField; +use SilverStripe\Forms\TextField; +use SilverStripe\ORM\FieldType\DBInt; +use SilverStripe\ORM\Filters\WithinRangeFilter; use stdClass; class DataObjectTest extends SapphireTest @@ -1419,6 +1430,241 @@ public function testSearchableFields() $this->assertEmpty($fields); } + public static function provideSearchableFieldsForWithinRangeFilter(): array + { + return [ + 'invalid field in general' => [ + 'config' => [ + 'NotARealField' => [ + 'filter' => WithinRangeFilter::class, + 'rangeFromDefault' => 'some value', + 'rangeToDefault' => 'another value', + ], + ], + 'exceptionMessage' => 'NotARealField is not a relation/field on ' . DataObjectTest\Team::class, + 'expected' => null, + ], + 'cannot filter by relation with WithinRangeFilter' => [ + 'config' => [ + 'Captain' => [ + 'filter' => WithinRangeFilter::class, + ], + ], + 'exceptionMessage' => "dataType must be set to a DBField class for 'Captain'", + 'expected' => null, + ], + 'missing default "from"' => [ + 'config' => [ + 'Title' => ['filter' => WithinRangeFilter::class], + ], + 'exceptionMessage' => "rangeFromDefault must be set for 'Title'", + 'expected' => null, + ], + 'missing default "to"' => [ + 'config' => [ + 'Title' => [ + 'filter' => WithinRangeFilter::class, + 'rangeFromDefault' => 'some value', + ], + ], + 'exceptionMessage' => "rangeToDefault must be set for 'Title'", + 'expected' => null, + ], + 'fully valid config' => [ + 'config' => [ + 'Title' => [ + 'filter' => WithinRangeFilter::class, + 'rangeFromDefault' => 'some value', + 'rangeToDefault' => 'another value', + ], + ], + 'exceptionMessage' => null, + 'expected' => [ + 'Title' => [ + 'filter' => WithinRangeFilter::class, + 'dataType' => DBVarchar::class, + 'rangeFromDefault' => 'some value', + 'rangeToDefault' => 'another value', + 'title' => 'Title', + ], + ], + ], + 'fully valid config inferred from datatype' => [ + 'config' => [ + 'Title' => [ + 'filter' => WithinRangeFilter::class, + 'dataType' => DBInt::class, + ], + ], + 'exceptionMessage' => null, + 'expected' => [ + 'Title' => [ + 'filter' => WithinRangeFilter::class, + 'dataType' => DBInt::class, + 'title' => 'Title', + 'rangeFromDefault' => DBInt::getMinValue(), + 'rangeToDefault' => DBInt::getMaxValue(), + ], + ], + ], + ]; + } + + #[DataProvider('provideSearchableFieldsForWithinRangeFilter')] + public function testSearchableFieldsForWithinRangeFilter(array $config, ?string $exceptionMessage, ?array $expected): void + { + DataObjectTest\Team::config()->set('searchable_fields', $config); + $team = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); + + if ($exceptionMessage !== null) { + $this->expectException(LogicException::class); + $this->expectExceptionMessage($exceptionMessage); + } + + $fields = $team->searchableFields(); + if ($expected !== null) { + $this->assertSame($expected, $fields); + } + } + + public static function provideScaffoldSearchFields(): array + { + return [ + 'inferred from simple config' => [ + 'config' => [ + 'Title', + 'DatabaseField', + 'NumericField', + 'Captain.IsRetired' => [ + 'Title' => 'Captain is retired', + ], + ], + 'generalSearchFieldName' => '', + 'expected' => [ + 'Title' => TextField::class, + 'DatabaseField' => TextField::class, + 'NumericField' => NumericField::class, + 'Captain__IsRetired' => DropdownField::class, + ], + ], + 'inferred from simple config (with general field)' => [ + 'config' => [ + 'Title', + 'DatabaseField', + 'NumericField', + 'Captain.IsRetired' => [ + 'Title' => 'Captain is retired', + ], + ], + 'generalSearchFieldName' => 'gen', + 'expected' => [ + 'gen' => HiddenField::class, + 'Title' => TextField::class, + 'DatabaseField' => TextField::class, + 'NumericField' => NumericField::class, + 'Captain__IsRetired' => DropdownField::class, + ], + ], + 'field specified in config' => [ + 'config' => [ + 'Title' => [ + 'field' => DatalessField::class, + ], + ], + 'generalSearchFieldName' => 'gen', + 'expected' => [ + 'gen' => HiddenField::class, + 'Title' => DatalessField::class, + ], + ], + 'no searchable fields' => [ + 'config' => [], + 'generalSearchFieldName' => 'gen', + 'expected' => [], + ], + 'within range duplicates field' => [ + 'config' => [ + 'NumericField' => WithinRangeFilter::class, + ], + 'generalSearchFieldName' => '', + 'expected' => [ + 'NumericField_SearchFrom' => NumericField::class, + 'NumericField_SearchTo' => NumericField::class, + ], + ], + 'search against relation (with method implemented)' => [ + 'config' => [ + 'Captain', + ], + 'generalSearchFieldName' => '', + 'expected' => [ + 'Captain.ID' => DropdownField::class, + ], + ], + ]; + } + + #[DataProvider('provideScaffoldSearchFields')] + public function testScaffoldSearchFields(array $config, string $generalSearchFieldName, array $expected): void + { + DataObjectTest\Team::config()->set('searchable_fields', $config); + DataObjectTest\Team::config()->set('general_search_field_name', $generalSearchFieldName); + $team = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); + + $fields = $team->scaffoldSearchFields(); + $fieldMap = []; + foreach ($fields as $field) { + if ($field instanceof CompositeField) { + foreach ($field->getChildren() as $childField) { + $fieldMap[$childField->getName()] = get_class($childField); + } + continue; + } + $fieldMap[$field->getName()] = get_class($field); + } + $this->assertSame($expected, $fieldMap); + } + + public function testScaffoldSearchFieldsWithArg(): void + { + $config = [ + 'Title', + 'DatabaseField' => [ + // This will get overridden by fieldClasses arg + 'field' => DatalessField::class, + ], + 'NumericField', + 'Captain.IsRetired' => [ + 'Title' => 'Captain is retired', + ], + ]; + $args = [ + 'fieldClasses' => [ + 'DatabaseField' => NumericField::class, + ], + 'restrictFields' => [ + 'DatabaseField', + 'Captain.IsRetired' + ], + ]; + $expected = [ + 'DatabaseField' => NumericField::class, + 'Captain__IsRetired' => DropdownField::class, + ]; + + + DataObjectTest\Team::config()->set('searchable_fields', $config); + DataObjectTest\Team::config()->set('general_search_field_name', ''); + $team = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); + + $fields = $team->scaffoldSearchFields($args); + $fieldMap = []; + foreach ($fields as $field) { + $fieldMap[$field->getName()] = get_class($field); + } + $this->assertSame($expected, $fieldMap); + } + public function testCastingHelper() { $team = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); diff --git a/tests/php/ORM/DataObjectTest/Player.php b/tests/php/ORM/DataObjectTest/Player.php index c780e92b6b5..17c3e4899a3 100644 --- a/tests/php/ORM/DataObjectTest/Player.php +++ b/tests/php/ORM/DataObjectTest/Player.php @@ -3,6 +3,8 @@ namespace SilverStripe\ORM\Tests\DataObjectTest; use SilverStripe\Dev\TestOnly; +use SilverStripe\Forms\DropdownField; +use SilverStripe\Forms\FormField; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\Tests\DataObjectTest; @@ -48,4 +50,12 @@ public function ReturnsNull() { return null; } + + public function scaffoldSearchField(): FormField + { + // This is a weird scenario, given you have to explicitly say the relation name here. + // This is just here to ensure we don't break this in a minor or patch. There's no + // reason not to break this in a major (or else improve it so the relation name is passed in) + return DropdownField::create('Captain.ID', null, Player::get()->map()); + } } diff --git a/tests/php/ORM/Search/BasicSearchContextTest.php b/tests/php/ORM/Search/BasicSearchContextTest.php index 9d0157de118..5e40d628cc6 100644 --- a/tests/php/ORM/Search/BasicSearchContextTest.php +++ b/tests/php/ORM/Search/BasicSearchContextTest.php @@ -15,6 +15,7 @@ use SilverStripe\ORM\Search\BasicSearchContext; use SilverStripe\Model\ArrayData; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\DataProviderExternal; class BasicSearchContextTest extends SapphireTest { @@ -22,6 +23,7 @@ class BasicSearchContextTest extends SapphireTest protected static $extra_dataobjects = [ SearchContextTest\GeneralSearch::class, + SearchContextTest\WithinRangeFilterModel::class, ]; private function getList(): ArrayList @@ -339,4 +341,24 @@ public function testSpecificFieldsCanBeSkipped() $this->assertNotEmpty($general1->ExcludeThisField); $this->assertCount(0, $results); } + + + #[DataProviderExternal(SearchContextTest::class, 'provideQueryWithinRangeFilter')] + public function testQueryWithinRangeFilter(array $params, array $expectedFixtureNames): void + { + $model = SearchContextTest\WithinRangeFilterModel::singleton(); + $context = $model->getDefaultSearchContext(); + $results = $context->getResults($params)->column('ID'); + + $found = []; + foreach ($expectedFixtureNames as $fixtureName) { + $id = $this->idFromFixture(SearchContextTest\WithinRangeFilterModel::class, $fixtureName); + if (in_array($id, $results)) { + $found[] = $fixtureName; + } + } + + $this->assertSame($expectedFixtureNames, $found); + $this->assertCount(count($expectedFixtureNames), $results, 'More results found than expected'); + } } diff --git a/tests/php/ORM/Search/BasicSearchContextTest.yml b/tests/php/ORM/Search/BasicSearchContextTest.yml index dfdd30f344f..0259f86c560 100644 --- a/tests/php/ORM/Search/BasicSearchContextTest.yml +++ b/tests/php/ORM/Search/BasicSearchContextTest.yml @@ -26,3 +26,41 @@ SilverStripe\ORM\Tests\Search\SearchContextTest\GeneralSearch: PartialMatchField: MatchNothing MatchAny1: MatchNothing MatchAny2: MatchNothing + +SilverStripe\ORM\Tests\Search\SearchContextTest\WithinRangeFilterModel: + lowrange: + DateOnly: '1912-05-03' + Datetime: '1912-05-03 01:23:45' + DatetimeWithDateField: '1912-05-03 01:23:45' + TimeOnly: '01:23:45' + IntRange: 13 + DecimalRange: 1.234 + FloatRange: 1.234 + PercentageRange: 0.05 + YearRange: 1912 + CurrencyRange: 1.23 + VarcharRangeWithConfig: 'c' + midrange: + DateOnly: '2005-04-06' + Datetime: '2005-04-06 12:34:56' + DatetimeWithDateField: '2005-04-06 12:34:56' + TimeOnly: '12:34:56' + IntRange: 500 + DecimalRange: 12.34 + FloatRange: 12.34 + PercentageRange: 0.53 + YearRange: 2005 + CurrencyRange: 12.34 + VarcharRangeWithConfig: 'l' + highrange: + DateOnly: '2123-03-07' + Datetime: '2123-03-07 23:45:47' + DatetimeWithDateField: '2123-03-07 23:45:47' + TimeOnly: '23:45:47' + IntRange: 1234 + DecimalRange: 123.4 + FloatRange: 123.4 + PercentageRange: 0.95 + YearRange: 2123 + CurrencyRange: 123.4 + VarcharRangeWithConfig: 'x' diff --git a/tests/php/ORM/Search/SearchContextTest.php b/tests/php/ORM/Search/SearchContextTest.php index ec475c3c299..b9ba52a149f 100644 --- a/tests/php/ORM/Search/SearchContextTest.php +++ b/tests/php/ORM/Search/SearchContextTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\ORM\Tests\Search; use LogicException; +use PHPUnit\Framework\Attributes\DataProvider; use ReflectionMethod; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; @@ -22,7 +23,6 @@ class SearchContextTest extends SapphireTest { - protected static $fixture_file = 'SearchContextTest.yml'; protected static $extra_dataobjects = [ @@ -34,6 +34,7 @@ class SearchContextTest extends SapphireTest SearchContextTest\Deadline::class, SearchContextTest\Action::class, SearchContextTest\AllFilterTypes::class, + SearchContextTest\WithinRangeFilterModel::class, SearchContextTest\Customer::class, SearchContextTest\Address::class, SearchContextTest\Order::class, @@ -178,6 +179,279 @@ public function testRelationshipObjectsLinkedInSearch() ); } + public static function provideQueryWithinRangeFilter(): array + { + return [ + // DBDate + 'date mid range' => [ + 'params' => [ + 'DateOnly_SearchFrom' => '1990-01-01', + 'DateOnly_SearchTo' => '2100-12-24', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'date from only' => [ + 'params' => [ + 'DateOnly_SearchFrom' => '1990-01-01', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'date to only' => [ + 'params' => [ + 'DateOnly_SearchTo' => '2100-12-24', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBDatetime + 'datetime mid range' => [ + 'params' => [ + 'Datetime_SearchFrom' => '1990-01-01 12:23:15', + 'Datetime_SearchTo' => '2100-12-24 12:23:15', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'datetime from only' => [ + 'params' => [ + 'Datetime_SearchFrom' => '1990-01-01 12:23:15', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'datetime to only' => [ + 'params' => [ + 'Datetime_SearchTo' => '2100-12-24 12:23:15', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + 'datetime mid range date only' => [ + 'params' => [ + 'DatetimeWithDateField_SearchFrom' => '1990-01-01', + 'DatetimeWithDateField_SearchTo' => '2100-12-24', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'datetime from only date only' => [ + 'params' => [ + 'DatetimeWithDateField_SearchFrom' => '1990-01-01', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'datetime to only date only' => [ + 'params' => [ + 'DatetimeWithDateField_SearchTo' => '2100-12-24', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + 'datetime mid range date only exact day' => [ + 'params' => [ + // The from time should end up being 00:00:00 + 'DatetimeWithDateField_SearchFrom' => '2005-04-06', + // The to time should end up being 24:59:59 + 'DatetimeWithDateField_SearchTo' => '2005-04-06', + ], + 'expectedFixtureNames' => ['midrange'], + ], + // DBTime + 'time mid range' => [ + 'params' => [ + 'TimeOnly_SearchFrom' => '05:23:45', + 'TimeOnly_SearchTo' => '17:01:23', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'time from only' => [ + 'params' => [ + 'TimeOnly_SearchFrom' => '05:23:45', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'time to only' => [ + 'params' => [ + 'TimeOnly_SearchTo' => '17:01:23', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBInt + 'int mid range' => [ + 'params' => [ + 'IntRange_SearchFrom' => '53', + 'IntRange_SearchTo' => '623', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'int from only' => [ + 'params' => [ + 'IntRange_SearchFrom' => '53', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'int to only' => [ + 'params' => [ + 'IntRange_SearchTo' => '623', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBDecimal + 'decimal mid range' => [ + 'params' => [ + 'DecimalRange_SearchFrom' => '2.0', + 'DecimalRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'decimal from only' => [ + 'params' => [ + 'DecimalRange_SearchFrom' => '2.0', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'decimal to only' => [ + 'params' => [ + 'DecimalRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBFloat + 'float mid range' => [ + 'params' => [ + 'FloatRange_SearchFrom' => '2.0', + 'FloatRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'float from only' => [ + 'params' => [ + 'FloatRange_SearchFrom' => '2.0', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'float to only' => [ + 'params' => [ + 'FloatRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBPercentage + 'percentage mid range' => [ + 'params' => [ + 'PercentageRange_SearchFrom' => '0.12', + 'PercentageRange_SearchTo' => '0.75', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'percentage from only' => [ + 'params' => [ + 'PercentageRange_SearchFrom' => '0.12', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'percentage to only' => [ + 'params' => [ + 'PercentageRange_SearchTo' => '0.75', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBYear + 'year mid range' => [ + 'params' => [ + 'YearRange_SearchFrom' => '1990', + 'YearRange_SearchTo' => '2100', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'year from only' => [ + 'params' => [ + 'YearRange_SearchFrom' => '1990', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'year to only' => [ + 'params' => [ + 'YearRange_SearchTo' => '2100', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBCurrency + 'currency mid range' => [ + 'params' => [ + 'CurrencyRange_SearchFrom' => '2.0', + 'CurrencyRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'currency from only' => [ + 'params' => [ + 'CurrencyRange_SearchFrom' => '2.0', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'currency to only' => [ + 'params' => [ + 'CurrencyRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // Special match_any config + 'match_any mid range' => [ + 'params' => [ + 'MatchAnyRange_SearchFrom' => '2.0', + 'MatchAnyRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'match_any from only' => [ + 'params' => [ + 'MatchAnyRange_SearchFrom' => '2.0', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'match_any to only' => [ + 'params' => [ + 'MatchAnyRange_SearchTo' => '63.125', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + // DBVarchar + 'varchar mid range' => [ + 'params' => [ + 'VarcharRangeWithConfig_SearchFrom' => 'e', + 'VarcharRangeWithConfig_SearchTo' => 's', + ], + 'expectedFixtureNames' => ['midrange'], + ], + 'varchar from only' => [ + 'params' => [ + 'VarcharRangeWithConfig_SearchFrom' => 'e', + ], + 'expectedFixtureNames' => ['midrange', 'highrange'], + ], + 'varchar to only' => [ + 'params' => [ + 'VarcharRangeWithConfig_SearchTo' => 's', + ], + 'expectedFixtureNames' => ['lowrange', 'midrange'], + ], + ]; + } + + #[DataProvider('provideQueryWithinRangeFilter')] + public function testQueryWithinRangeFilter(array $params, array $expectedFixtureNames): void + { + $model = SearchContextTest\WithinRangeFilterModel::singleton(); + $context = $model->getDefaultSearchContext(); + $results = $context->getResults($params)->column('ID'); + + $found = []; + foreach ($expectedFixtureNames as $fixtureName) { + $id = $this->idFromFixture(SearchContextTest\WithinRangeFilterModel::class, $fixtureName); + if (in_array($id, $results)) { + $found[] = $fixtureName; + } + } + + $this->assertSame($expectedFixtureNames, $found); + $this->assertCount(count($expectedFixtureNames), $results, 'More results found than expected'); + } + public function testCanGenerateQueryUsingAllFilterTypes() { $all = SearchContextTest\AllFilterTypes::singleton(); diff --git a/tests/php/ORM/Search/SearchContextTest.yml b/tests/php/ORM/Search/SearchContextTest.yml index ea297934b09..3d714c0f69f 100644 --- a/tests/php/ORM/Search/SearchContextTest.yml +++ b/tests/php/ORM/Search/SearchContextTest.yml @@ -71,6 +71,44 @@ SilverStripe\ORM\Tests\Search\SearchContextTest\AllFilterTypes: EndsWith: abcd-efgh-ijkl FulltextField: one two three +SilverStripe\ORM\Tests\Search\SearchContextTest\WithinRangeFilterModel: + lowrange: + DateOnly: '1912-05-03' + Datetime: '1912-05-03 01:23:45' + DatetimeWithDateField: '1912-05-03 01:23:45' + TimeOnly: '01:23:45' + IntRange: 13 + DecimalRange: 1.234 + FloatRange: 1.234 + PercentageRange: 0.05 + YearRange: 1912 + CurrencyRange: 1.23 + VarcharRangeWithConfig: 'c' + midrange: + DateOnly: '2005-04-06' + Datetime: '2005-04-06 12:34:56' + DatetimeWithDateField: '2005-04-06 12:34:56' + TimeOnly: '12:34:56' + IntRange: 500 + DecimalRange: 12.34 + FloatRange: 12.34 + PercentageRange: 0.53 + YearRange: 2005 + CurrencyRange: 12.34 + VarcharRangeWithConfig: 'l' + highrange: + DateOnly: '2123-03-07' + Datetime: '2123-03-07 23:45:47' + DatetimeWithDateField: '2123-03-07 23:45:47' + TimeOnly: '23:45:47' + IntRange: 1234 + DecimalRange: 123.4 + FloatRange: 123.4 + PercentageRange: 0.95 + YearRange: 2123 + CurrencyRange: 123.4 + VarcharRangeWithConfig: 'x' + SilverStripe\ORM\Tests\Search\SearchContextTest\Customer: customer1: FirstName: Bill diff --git a/tests/php/ORM/Search/SearchContextTest/AllFilterTypes.php b/tests/php/ORM/Search/SearchContextTest/AllFilterTypes.php index 0caad766a3d..5aa6ab11e98 100644 --- a/tests/php/ORM/Search/SearchContextTest/AllFilterTypes.php +++ b/tests/php/ORM/Search/SearchContextTest/AllFilterTypes.php @@ -5,6 +5,9 @@ use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; +/** + * Note this model intentionally omits WithinRangeFilter because that will be tested separately. + */ class AllFilterTypes extends DataObject implements TestOnly { private static $table_name = 'SearchContextTest_AllFilterTypes'; diff --git a/tests/php/ORM/Search/SearchContextTest/WithinRangeFilterModel.php b/tests/php/ORM/Search/SearchContextTest/WithinRangeFilterModel.php new file mode 100644 index 00000000000..f86329a5f83 --- /dev/null +++ b/tests/php/ORM/Search/SearchContextTest/WithinRangeFilterModel.php @@ -0,0 +1,63 @@ + 'Date', + 'Datetime' => 'Datetime', + 'DatetimeWithDateField' => 'Datetime', + 'TimeOnly' => 'Time', + 'IntRange' => 'Int', + 'DecimalRange' => 'Decimal', + 'FloatRange' => 'Float', + 'PercentageRange' => 'Percentage', + 'YearRange' => 'Year', + 'CurrencyRange' => 'Currency', + // Other field types require additional configuration but can work + 'VarcharRangeWithConfig' => 'Varchar', + ]; + + private static $searchable_fields = [ + 'DateOnly' => WithinRangeFilter::class, + 'Datetime' => WithinRangeFilter::class, + 'DatetimeWithDateField' => [ + 'filter' => WithinRangeFilter::class, + 'field' => DateField::class, + ], + 'TimeOnly' => WithinRangeFilter::class, + 'IntRange' => WithinRangeFilter::class, + 'DecimalRange' => WithinRangeFilter::class, + 'FloatRange' => WithinRangeFilter::class, + 'PercentageRange' => WithinRangeFilter::class, + 'YearRange' => WithinRangeFilter::class, + 'CurrencyRange' => WithinRangeFilter::class, + // Special "match_any" config can also work with this filter + 'MatchAnyRange' => [ + 'filter' => WithinRangeFilter::class, + 'dataType' => DBDecimal::class, + 'match_any' => [ + 'DecimalRange', + 'FloatRange', + 'CurrencyRange', + ], + ], + // @TODO also confirm exceptions thrown when either one of the range configs is missing + // Note the addition of rangeFromDefault and rangeToDefault here + 'VarcharRangeWithConfig' => [ + 'filter' => WithinRangeFilter::class, + 'rangeFromDefault' => 'a', + 'rangeToDefault' => 'z', + ], + ]; +}