From 0a7cca5312aee23b3503ca5750fa8e603043a7a0 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 7 Sep 2023 14:44:23 +1200 Subject: [PATCH] ENH Use the core ArrayList Searchfilter functionality No need for the extra dependency after 5.1.0 --- CONTRIBUTING.md | 2 +- README.md | 2 +- composer.json | 3 +- docs/en/01-validators.md | 8 ++-- .../DependentRequiredFieldsValidator.php | 46 +++++++++++++++++-- .../DependentRequiredFieldsValidatorTest.php | 12 ++--- 6 files changed, 53 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4e30bdc..25ccd50 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,7 +8,7 @@ try your hand at fixing it via [a pull request](https://github.com/guysartorelli Any feature requests or recommendations should be raised as [an issue](https://github.com/guysartorelli/silverstripe-composable-validators/issues). -This module adheres to [the Silverstripe module standards](https://docs.silverstripe.org/en/4/developer_guides/extending/modules/#module-standard) +This module adheres to [the Silverstripe module standards](https://docs.silverstripe.org/en/developer_guides/extending/modules/#module-standard) Commit messages should be [semantic](https://seesparkbox.com/foundry/semantic_commit_messages), though the style used for this module differs from that reference in the following ways: - The first letter after the commit type is capitalized. diff --git a/README.md b/README.md index 1f8c035..4f73196 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ Like `ValidatesMultipleFields` but requires a configuration array for each field [10]: https://api.silverstripe.org/4/SilverStripe/Forms/RequiredFields.html [11]: docs/en/01-validators.md#warningfieldsvalidator [12]: docs/en/01-validators.md#dependentrequiredfieldsvalidator -[13]: https://docs.silverstripe.org/en/4/developer_guides/model/searchfilters/ +[13]: https://docs.silverstripe.org/en/developer_guides/model/searchfilters/ [14]: docs/en/01-validators.md#requiredblocksvalidator [15]: https://github.com/silverstripe/silverstripe-elemental [16]: docs/en/01-validators.md#regexfieldsvalidator diff --git a/composer.json b/composer.json index eca3f5f..d0f16d2 100644 --- a/composer.json +++ b/composer.json @@ -38,8 +38,7 @@ }, "require": { "php": "^8.1", - "silverstripe/framework": "^5.0.17", - "signify-nz/silverstripe-searchfilter-arraylist": "^1.1.0" + "silverstripe/framework": "^5.1.0" }, "require-dev": { "silverstripe/cms": "^5", diff --git a/docs/en/01-validators.md b/docs/en/01-validators.md index 740c320..fba6549 100644 --- a/docs/en/01-validators.md +++ b/docs/en/01-validators.md @@ -20,7 +20,7 @@ This validator is also extremely useful for front-end forms, as it provides clie If the [`DataObjectDefaultAjaxExtension`](./02-extensions.md#dataobjectdefaultajaxextension) extension has been applied, calling `parent::getCMSCompositeValidator()` inside the `getCMSCompositeValidator()` method will return an `AjaxCompositeValidator`, which can then be manipulated. -You can also opt to just return a new `AjaxCompositeValidator` from that method - and in front-end situations, you can instantiate a new `AjaxCompositeValidator` (preferably [via injection](https://docs.silverstripe.org/en/4/developer_guides/extending/injector/) i.e. `AjaxCompositeValidator::create()`). +You can also opt to just return a new `AjaxCompositeValidator` from that method - and in front-end situations, you can instantiate a new `AjaxCompositeValidator` (preferably [via injection](https://docs.silverstripe.org/en/developer_guides/extending/injector/) i.e. `AjaxCompositeValidator::create()`). Ajax validation can also be disabled at any stage, if there is a cause for doing so. @@ -123,7 +123,7 @@ Displays a validation warning if the field(s) has no value. ## DependentRequiredFieldsValidator -Allows you to define fields as being required conditionally based on the values of other fields. It uses [`SearchFilters`](https://docs.silverstripe.org/en/4/developer_guides/model/searchfilters/) to provide a variety of ways to compare values, depending on what causes the fields to be required. It uses (so has all of the functionality and methods of) the [`ValidatesMultipleFieldsWithConfig`](#validatesmultiplefieldswithconfig) trait. +Allows you to define fields as being required conditionally based on the values of other fields. It uses [`SearchFilters`](https://docs.silverstripe.org/en/developer_guides/model/searchfilters/) to provide a variety of ways to compare values, depending on what causes the fields to be required. It uses (so has all of the functionality and methods of) the [`ValidatesMultipleFieldsWithConfig`](#validatesmultiplefieldswithconfig) trait. In the below example, we have fields with various levels of dependency on whether they are required or not. @@ -142,9 +142,7 @@ DependentRequiredFieldsValidator::create([ **Note:** All of the dependencies must be met for a field to be considered required. So in the example above, if `DependencyField` had the value "someValues" the `StartsEndsWithField` would not be marked required, because only one of its dependencies is met. -The conditional checking functionality is powered by [signify-nz/silverstripe-searchfilter-arraylist](https://github.com/signify-nz/silverstripe-searchfilter-arraylist), which does provide some extensibility. You may want to check the documentation of that module. - -All of the `SearchFilter`s and modifiers documented in [Silverstripe's SearchFilter documentation](https://docs.silverstripe.org/en/4/developer_guides/model/searchfilters/) should be supported - if you find that isn't the case, please [raise an issue](https://github.com/signify-nz/silverstripe-searchfilter-arraylist/issues) (or, better yet, a pull request) against the `signify-nz/silverstripe-searchfilter-arraylist` module. +All of the `SearchFilter`s and modifiers documented in [Silverstripe's SearchFilter documentation](https://docs.silverstripe.org/en/developer_guides/model/searchfilters/) should be supported - if you find that isn't the case, please [raise an issue](https://github.com/silverstripe/silverstripe-framework/issues) (or, better yet, a pull request) against the `siglverstripe/silverstripe-framework` module. ## RequiredBlocksValidator diff --git a/src/Validators/DependentRequiredFieldsValidator.php b/src/Validators/DependentRequiredFieldsValidator.php index 085bb6f..5701316 100644 --- a/src/Validators/DependentRequiredFieldsValidator.php +++ b/src/Validators/DependentRequiredFieldsValidator.php @@ -3,10 +3,11 @@ namespace Signify\ComposableValidators\Validators; use Signify\ComposableValidators\Traits\ValidatesMultipleFieldsWithConfig; -use Signify\SearchFilterArrayList\SearchFilterableArrayList; use SilverStripe\Core\ClassInfo; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\Filters\SearchFilter; /** @@ -49,7 +50,7 @@ public function php($data) $dependencyValue = $data[$dependencyFieldName]; $tempObj = new \stdClass(); $tempObj->$dependencyFieldName = $dependencyValue; - $filterList = SearchFilterableArrayList::create([$tempObj]); + $filterList = ArrayList::create([$tempObj]); $isRequired = $filterList->filter($filterKey, $filterValue)->count() !== 0; // If field is not required, we can stop processing it. if (!$isRequired) { @@ -103,6 +104,42 @@ protected function validateField($data, FieldList $fields, string $fieldName, ar return true; } + /** + * Ugly copypasta from SearchFilterable trait 'cause that's just easier. + * Don't use the trait directly 'cause if new methods are added to that trait, we don't want them here. + */ + private function createSearchFilter($filter, $value) + { + // Field name is always the first component + $fieldArgs = explode(':', $filter); + $fieldName = array_shift($fieldArgs); + $default = 'DataListFilter.default'; + + // Inspect type of second argument to determine context + $secondArg = array_shift($fieldArgs); + $modifiers = $fieldArgs; + if (!$secondArg) { + // Use default SearchFilter if none specified. E.g. `->filter(['Name' => $myname])` + $filterServiceName = $default; + } else { + // The presence of a second argument is by default ambiguous; We need to query + // Whether this is a valid modifier on the default filter, or a filter itself. + /** @var SearchFilter $defaultFilterInstance */ + $defaultFilterInstance = Injector::inst()->get($default); + if (in_array(strtolower($secondArg), $defaultFilterInstance->getSupportedModifiers() ?? [])) { + // Treat second (and any subsequent) argument as modifiers, using default filter + $filterServiceName = $default; + array_unshift($modifiers, $secondArg); + } else { + // Second argument isn't a valid modifier, so assume is filter identifier + $filterServiceName = "DataListFilter.{$secondArg}"; + } + } + + // Build instance + return Injector::inst()->create($filterServiceName, $fieldName, $value, $modifiers); + } + /** * Build the validation error message for a field based on its dependency filter. * @@ -113,10 +150,9 @@ protected function validateField($data, FieldList $fields, string $fieldName, ar */ protected function buildValidationMessage(FieldList $fields, string $title, array $filter): string { - $arrayList = SearchFilterableArrayList::create(); $dependencies = []; foreach ($filter as $filterKey => $filterValue) { - $filter = $arrayList->createSearchFilter($filterKey, $filterValue); + $filter = $this->createSearchFilter($filterKey, $filterValue); $filterClass = get_class($filter); $dependencyField = $this->getFormField($fields, $filter->getName()); $negated = in_array('not', $filter->getModifiers()) ? '_NEGATED' : ''; @@ -143,7 +179,7 @@ protected function buildValidationMessage(FieldList $fields, string $title, arra } /** - * Make a containing all values for a given dependency filter. + * Make a string containing all values for a given dependency filter. * * @param SearchFilter $filter * @return string diff --git a/tests/php/ValidatorTests/DependentRequiredFieldsValidatorTest.php b/tests/php/ValidatorTests/DependentRequiredFieldsValidatorTest.php index 2690c56..b126769 100644 --- a/tests/php/ValidatorTests/DependentRequiredFieldsValidatorTest.php +++ b/tests/php/ValidatorTests/DependentRequiredFieldsValidatorTest.php @@ -16,8 +16,8 @@ class DependentRequiredFieldsValidatorTest extends SapphireTest * If the dependency is not met, the field should not be required. * * Note we do not need to test all SearchFilter dependency combinations, as - * that functionality comes from the signify-nz/silverstripe-searchfilter-arraylist - * module which has unit tests for that. + * that functionality comes from silverstripe/framework which has its own testing + * for that. */ public function testFieldNotRequiredIfDependencyNotMet(): void { @@ -38,8 +38,8 @@ public function testFieldNotRequiredIfDependencyNotMet(): void * If the dependency is met, the field should be required. * * Note we do not need to test all SearchFilter dependency combinations, as - * that functionality comes from the signify-nz/silverstripe-searchfilter-arraylist - * module which has unit tests for that. + * that functionality comes from silverstripe/framework which has its own testing + * for that. */ public function testFieldRequiredIfDependencyMet(): void { @@ -70,8 +70,8 @@ public function testFieldRequiredIfDependencyMet(): void * If the nullish dependency is met, the field should be required. * * Note we do not need to test all SearchFilter dependency combinations, as - * that functionality comes from the signify-nz/silverstripe-searchfilter-arraylist - * module which has unit tests for that. + * that functionality comes from silverstripe/framework which has its own testing + * for that. */ public function testFieldRequiredIfNullishDependencyMet(): void {