From 842d328353c97b028c6a04ae39fdfd7ebf637c71 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 31 Jan 2025 14:06:23 +1300 Subject: [PATCH 1/4] API Provide minimum and maximum comparison values for numeric DBFields This will be used with handling WithinRangeFilter in searchable fields so that if only one of the two ends of a range is filled in, the other can be automatically populated. --- .../FieldValidation/BigIntFieldValidator.php | 20 +++-------------- .../FieldValidation/IntFieldValidator.php | 17 +++----------- src/ORM/FieldType/DBBigInt.php | 15 +++++++++++++ src/ORM/FieldType/DBDate.php | 10 +++++++++ src/ORM/FieldType/DBDatetime.php | 10 +++++++++ src/ORM/FieldType/DBDecimal.php | 10 +++++++++ src/ORM/FieldType/DBField.php | 16 ++++++++++++++ src/ORM/FieldType/DBFloat.php | 10 +++++++++ src/ORM/FieldType/DBForeignKey.php | 8 +------ src/ORM/FieldType/DBInt.php | 22 +++++++++++++++++++ src/ORM/FieldType/DBPercentage.php | 4 ++-- src/ORM/FieldType/DBTime.php | 10 +++++++++ src/ORM/FieldType/DBYear.php | 8 +++---- 13 files changed, 116 insertions(+), 44 deletions(-) diff --git a/src/Core/Validation/FieldValidation/BigIntFieldValidator.php b/src/Core/Validation/FieldValidation/BigIntFieldValidator.php index ee9d8c5b12b..4e48ef15b6a 100644 --- a/src/Core/Validation/FieldValidation/BigIntFieldValidator.php +++ b/src/Core/Validation/FieldValidation/BigIntFieldValidator.php @@ -4,6 +4,7 @@ use RunTimeException; use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\ORM\FieldType\DBBigInt; /** * A field validator for 64-bit integers @@ -11,21 +12,6 @@ */ class BigIntFieldValidator extends IntFieldValidator { - /** - * The minimum value for a signed 64-bit integer. - * Defined as string instead of int otherwise will end up as a float - * on 64-bit systems - * - * When this is cast to an int in IntFieldValidator::__construct() - * it will be properly cast to an int - */ - protected const MIN_INT = '-9223372036854775808'; - - /** - * The maximum value for a signed 64-bit integer. - */ - protected const MAX_INT = '9223372036854775807'; - public function __construct( string $name, mixed $value, @@ -51,10 +37,10 @@ protected function validateValue(): ValidationResult // int values that are too large or too small will be cast to float // on 64-bit systems and will fail the validation in IntFieldValidator if (is_string($this->value)) { - if (!is_null($this->minValue) && bccomp($this->value, static::MIN_INT) === -1) { + if (!is_null($this->minValue) && bccomp($this->value, DBBigInt::getMinValue()) === -1) { $result->addFieldError($this->name, $this->getTooSmallMessage()); } - if (!is_null($this->maxValue) && bccomp($this->value, static::MAX_INT) === 1) { + if (!is_null($this->maxValue) && bccomp($this->value, DBBigInt::getMaxValue()) === 1) { $result->addFieldError($this->name, $this->getTooLargeMessage()); } } diff --git a/src/Core/Validation/FieldValidation/IntFieldValidator.php b/src/Core/Validation/FieldValidation/IntFieldValidator.php index 28c677b036d..95701d03b7d 100644 --- a/src/Core/Validation/FieldValidation/IntFieldValidator.php +++ b/src/Core/Validation/FieldValidation/IntFieldValidator.php @@ -3,24 +3,13 @@ namespace SilverStripe\Core\Validation\FieldValidation; use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\ORM\FieldType\DBInt; /** * Validates that a value is a 32-bit signed integer */ class IntFieldValidator extends NumericNonStringFieldValidator { - /** - * The minimum value for a signed 32-bit integer. - * Defined as string instead of int because be cast to a float - * on 32-bit systems if defined as an int - */ - protected const MIN_INT = '-2147483648'; - - /** - * The maximum value for a signed 32-bit integer. - */ - protected const MAX_INT = '2147483647'; - public function __construct( string $name, mixed $value, @@ -28,10 +17,10 @@ public function __construct( ?int $maxValue = null ) { if (is_null($minValue)) { - $minValue = (int) static::MIN_INT; + $minValue = (int) DBInt::getMinValue(); } if (is_null($maxValue)) { - $maxValue = (int) static::MAX_INT; + $maxValue = (int) DBInt::getMaxValue(); } parent::__construct($name, $value, $minValue, $maxValue); } diff --git a/src/ORM/FieldType/DBBigInt.php b/src/ORM/FieldType/DBBigInt.php index af888e8f0bf..c40b80399a3 100644 --- a/src/ORM/FieldType/DBBigInt.php +++ b/src/ORM/FieldType/DBBigInt.php @@ -14,6 +14,21 @@ */ class DBBigInt extends DBInt { + /** + * The minimum value for a signed 64-bit integer. + * Defined as string instead of int otherwise will end up as a float + * on 64-bit systems + * + * When this is cast to an int in IntFieldValidator::__construct() + * it will be properly cast to an int + */ + protected const MIN_INT = '-9223372036854775808'; + + /** + * The maximum value for a signed 64-bit integer. + */ + protected const MAX_INT = '9223372036854775807'; + private static array $field_validators = [ // Remove parent validator and add BigIntValidator instead IntFieldValidator::class => null, diff --git a/src/ORM/FieldType/DBDate.php b/src/ORM/FieldType/DBDate.php index 39b84e7b01b..e8eb1a1000c 100644 --- a/src/ORM/FieldType/DBDate.php +++ b/src/ORM/FieldType/DBDate.php @@ -619,4 +619,14 @@ protected function explodeDateString($value) $parts[] = $matches['time']; return $parts; } + + public static function getMinValue(): string + { + return '0000-00-00'; + } + + public static function getMaxValue(): string + { + return '9999-12-31'; + } } diff --git a/src/ORM/FieldType/DBDatetime.php b/src/ORM/FieldType/DBDatetime.php index 0ba5a201b03..62f03e4913b 100644 --- a/src/ORM/FieldType/DBDatetime.php +++ b/src/ORM/FieldType/DBDatetime.php @@ -369,4 +369,14 @@ public function getISOFormat(): string { return DBDatetime::ISO_DATETIME; } + + public static function getMinValue(): string + { + return '0000-00-00 00:00:00'; + } + + public static function getMaxValue(): string + { + return '9999-12-31 23:59:59'; + } } diff --git a/src/ORM/FieldType/DBDecimal.php b/src/ORM/FieldType/DBDecimal.php index 0a32e0aeaef..a07049109a9 100644 --- a/src/ORM/FieldType/DBDecimal.php +++ b/src/ORM/FieldType/DBDecimal.php @@ -135,4 +135,14 @@ public function prepValueForDB(mixed $value): array|float|int|null return (float) $value; } + + public static function getMinValue(): float + { + return PHP_FLOAT_MIN; + } + + public static function getMaxValue(): float + { + return PHP_FLOAT_MAX; + } } diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index 29e326c41c0..64cec20fa97 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -571,4 +571,20 @@ public function scalarValueOnly(): bool { return true; } + + /** + * @return mixed The minimum value for comparisons with this field - or null if that's not determinable. + */ + public static function getMinValue(): mixed + { + return null; + } + + /** + * @return mixed The maximum value for comparisons with this field - or null if that's not determinable. + */ + public static function getMaxValue(): mixed + { + return null; + } } diff --git a/src/ORM/FieldType/DBFloat.php b/src/ORM/FieldType/DBFloat.php index 367957a4242..5764dff8528 100644 --- a/src/ORM/FieldType/DBFloat.php +++ b/src/ORM/FieldType/DBFloat.php @@ -89,4 +89,14 @@ public function prepValueForDB(mixed $value): array|float|int|null return $value; } + + public static function getMinValue(): float + { + return PHP_FLOAT_MIN; + } + + public static function getMaxValue(): float + { + return PHP_FLOAT_MAX; + } } diff --git a/src/ORM/FieldType/DBForeignKey.php b/src/ORM/FieldType/DBForeignKey.php index 6f30348688f..1d6dceb2b90 100644 --- a/src/ORM/FieldType/DBForeignKey.php +++ b/src/ORM/FieldType/DBForeignKey.php @@ -2,14 +2,8 @@ namespace SilverStripe\ORM\FieldType; -use SilverStripe\Assets\File; -use SilverStripe\Assets\Image; -use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Validation\FieldValidation\IntFieldValidator; -use SilverStripe\Forms\FileHandleField; use SilverStripe\Forms\FormField; -use SilverStripe\Forms\SearchableDropdownField; -use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; use SilverStripe\Model\ModelData; @@ -78,7 +72,7 @@ public function setValue(mixed $value, null|array|ModelData $record = null, bool return parent::setValue($value, $record, $markChanged); } - public function getMinValue(): int + public static function getMinValue(): int { return 0; } diff --git a/src/ORM/FieldType/DBInt.php b/src/ORM/FieldType/DBInt.php index 3acaf0e70b1..c10751c75e1 100644 --- a/src/ORM/FieldType/DBInt.php +++ b/src/ORM/FieldType/DBInt.php @@ -13,6 +13,18 @@ */ class DBInt extends DBField { + /** + * The minimum value for a signed 32-bit integer. + * Defined as string instead of int because be cast to a float + * on 32-bit systems if defined as an int + */ + protected const MIN_INT = '-2147483648'; + + /** + * The maximum value for a signed 32-bit integer. + */ + protected const MAX_INT = '2147483647'; + private static array $field_validators = [ IntFieldValidator::class ]; @@ -89,4 +101,14 @@ public function prepValueForDB(mixed $value): array|int|null return (int)$value; } + + public static function getMinValue(): string|int + { + return static::MIN_INT; + } + + public static function getMaxValue(): string|int + { + return static::MAX_INT; + } } diff --git a/src/ORM/FieldType/DBPercentage.php b/src/ORM/FieldType/DBPercentage.php index 1cac39e38c1..b1e6b173775 100644 --- a/src/ORM/FieldType/DBPercentage.php +++ b/src/ORM/FieldType/DBPercentage.php @@ -39,12 +39,12 @@ public function __construct(?string $name = null, int $precision = 4) parent::__construct($name, $precision + 1, $precision); } - public function getMinValue(): float + public static function getMinValue(): float { return 0.0; } - public function getMaxValue(): float + public static function getMaxValue(): float { return 1.0; } diff --git a/src/ORM/FieldType/DBTime.php b/src/ORM/FieldType/DBTime.php index 5b44cc6fac3..476038f2083 100644 --- a/src/ORM/FieldType/DBTime.php +++ b/src/ORM/FieldType/DBTime.php @@ -181,4 +181,14 @@ public function getTimestamp(): int } return 0; } + + public static function getMinValue(): string + { + return '00:00:00'; + } + + public static function getMaxValue(): string + { + return '23:59:59'; + } } diff --git a/src/ORM/FieldType/DBYear.php b/src/ORM/FieldType/DBYear.php index 04c26c09dc4..0f05443b528 100644 --- a/src/ORM/FieldType/DBYear.php +++ b/src/ORM/FieldType/DBYear.php @@ -21,8 +21,8 @@ class DBYear extends DBField private static $field_validators = [ YearFieldValidator::class => [ - 'minValue' => 'getMinYear', - 'maxValue' => 'getMaxYear' + 'minValue' => 'getMinValue', + 'maxValue' => 'getMaxValue' ], ]; @@ -70,12 +70,12 @@ public function setValue(mixed $value, null|array|ModelData $record = null, bool return $this; } - public function getMinYear(): int + public static function getMinValue(): int { return DBYear::MIN_YEAR; } - public function getMaxYear(): int + public static function getMaxValue(): int { return DBYear::MAX_YEAR; } From d6751ae0d10d72d57cdb3b5856f19d8dbb175c50 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Fri, 31 Jan 2025 14:09:11 +1300 Subject: [PATCH 2/4] 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 | 62 ++++ 16 files changed, 918 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..b1835740b5d --- /dev/null +++ b/tests/php/ORM/Search/SearchContextTest/WithinRangeFilterModel.php @@ -0,0 +1,62 @@ + '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', + ], + ], + // Note the addition of rangeFromDefault and rangeToDefault here + 'VarcharRangeWithConfig' => [ + 'filter' => WithinRangeFilter::class, + 'rangeFromDefault' => 'a', + 'rangeToDefault' => 'z', + ], + ]; +} From c0880fa60ccd506681c0d9155f3a3b92cb425d3d Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Tue, 18 Feb 2025 10:57:44 +1300 Subject: [PATCH 3/4] FIX Pass filter fields through the form before using them. Some form fields such as CurrencyField manipulate the value to give more pure values, e.g. removing a currency symbol before the actual value. --- src/Forms/CurrencyField.php | 1 + src/Forms/GridField/GridFieldFilterHeader.php | 20 ++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Forms/CurrencyField.php b/src/Forms/CurrencyField.php index 4546075dff3..5a743b3cd1f 100644 --- a/src/Forms/CurrencyField.php +++ b/src/Forms/CurrencyField.php @@ -30,6 +30,7 @@ public function setValue($value, $data = null) . number_format((double)preg_replace('/[^0-9.\-]/', '', $value ?? ''), 2); return $this; } + /** * Overwrite the datavalue before saving to the db ;-) * return 0.00 if no value, or value is non-numeric diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 0cd2908a180..e84f1f34e2d 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -116,9 +116,13 @@ public function handleAction(GridField $gridField, $actionName, $arguments, $dat $state->Columns = []; if ($actionName === 'filter') { - if (isset($data['filter'][$gridField->getName()])) { - foreach ($data['filter'][$gridField->getName()] as $key => $filter) { - $state->Columns->$key = $filter; + $filterValues = $data['filter'][$gridField->getName()] ?? null; + if ($filterValues !== null) { + $form = $this->getSearchForm($gridField); + $this->removeSearchPrefixFromFields($form->Fields()); + $form->loadDataFrom($filterValues); + foreach ($filterValues as $fieldName => $rawFilterValue) { + $state->Columns->$fieldName = $form->Fields()->dataFieldByName($fieldName)?->dataValue() ?? $rawFilterValue; } } } @@ -498,6 +502,16 @@ private function addSearchPrefixToFields(FieldList $fields): void } } + private function removeSearchPrefixFromFields(FieldList $fields): void + { + foreach ($fields as $field) { + $field->setName(str_replace('Search__', '', $field->getName())); + if ($field instanceof CompositeField) { + $this->removeSearchPrefixFromFields($field->getChildren()); + } + } + } + /** * Update CSS classes for form fields, including nested inside composite fields */ From d047b6c09c240734380f1d39082da1357b62f5f2 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Tue, 18 Feb 2025 12:54:53 +1300 Subject: [PATCH 4/4] ENH Update scaffolded search fields to be more sensible --- src/ORM/FieldType/DBCurrency.php | 6 ++++++ src/ORM/FieldType/DBEmail.php | 6 ++++++ src/ORM/FieldType/DBHTMLText.php | 2 +- src/ORM/FieldType/DBUrl.php | 6 ++++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/ORM/FieldType/DBCurrency.php b/src/ORM/FieldType/DBCurrency.php index aea5d3b3e39..e5b8ee647ec 100644 --- a/src/ORM/FieldType/DBCurrency.php +++ b/src/ORM/FieldType/DBCurrency.php @@ -4,6 +4,7 @@ use SilverStripe\Forms\CurrencyField; use SilverStripe\Forms\FormField; +use SilverStripe\Forms\NumericField; use SilverStripe\Model\ModelData; /** @@ -68,4 +69,9 @@ public function scaffoldFormField(?string $title = null, array $params = []): ?F { return CurrencyField::create($this->getName(), $title); } + + public function scaffoldSearchField(?string $title = null): ?FormField + { + return NumericField::create($this->getName(), $title); + } } diff --git a/src/ORM/FieldType/DBEmail.php b/src/ORM/FieldType/DBEmail.php index 2c16c8ab6ba..d266d2d9584 100644 --- a/src/ORM/FieldType/DBEmail.php +++ b/src/ORM/FieldType/DBEmail.php @@ -6,6 +6,7 @@ use SilverStripe\ORM\FieldType\DBVarchar; use SilverStripe\Core\Validation\FieldValidation\EmailFieldValidator; use SilverStripe\Forms\FormField; +use SilverStripe\Forms\TextField; class DBEmail extends DBVarchar { @@ -19,4 +20,9 @@ public function scaffoldFormField(?string $title = null, array $params = []): ?F $field->setMaxLength($this->getSize()); return $field; } + + public function scaffoldSearchField(?string $title = null): ?FormField + { + return TextField::create($this->getName(), $title); + } } diff --git a/src/ORM/FieldType/DBHTMLText.php b/src/ORM/FieldType/DBHTMLText.php index fb82d2f95e8..81de75e3182 100644 --- a/src/ORM/FieldType/DBHTMLText.php +++ b/src/ORM/FieldType/DBHTMLText.php @@ -186,7 +186,7 @@ public function scaffoldFormField(?string $title = null, array $params = []): ?F public function scaffoldSearchField(?string $title = null): ?FormField { - return new TextField($this->name, $title); + return TextField::create($this->name, $title); } /** diff --git a/src/ORM/FieldType/DBUrl.php b/src/ORM/FieldType/DBUrl.php index ab9435c6555..cac84e4f2a6 100644 --- a/src/ORM/FieldType/DBUrl.php +++ b/src/ORM/FieldType/DBUrl.php @@ -5,6 +5,7 @@ use SilverStripe\ORM\FieldType\DBVarchar; use SilverStripe\Core\Validation\FieldValidation\UrlFieldValidator; use SilverStripe\Forms\FormField; +use SilverStripe\Forms\TextField; use SilverStripe\Forms\UrlField; class DBUrl extends DBVarchar @@ -19,4 +20,9 @@ public function scaffoldFormField(?string $title = null, array $params = []): ?F $field->setMaxLength($this->getSize()); return $field; } + + public function scaffoldSearchField(?string $title = null): ?FormField + { + return TextField::create($this->getName(), $title); + } }