CMS 6 - changes to validators

[GuySartorelli]

Follow up from #10907

Acceptance criteria

• Rename FieldsValidator to FormFieldsValidator
• RequiredFields should be renamed RequiredFieldsValidator to match the naming convention of other validators
• Anywhere that just adds RequiredFields to a form should instead add CompositeValidator with both a FormFieldsValidator and (if necessary) RequiredFieldsValidator validator
  • Form::__construct() and SilverStripe\AssetAdmin\Forms\RemoteFileFormFactory::getForm() should explicitly exclude the RequiredFieldsValidator validator
• FieldsValidator is removed
• Form loops its form fields and calls validate on each of them
• RequiredFieldsValidator should not call the validate() method on form fields, as that is the responsibility of the FormFieldsValidator.
• Anywhere that previously added RequiredFields to a form just to get FormField validation to work no longer adds a validator
• RequiredFieldsValidator can be configured to not consider whitespace only a value
  • RequiredFieldsValidator should be configurable to consider whitespace only values to not constitute the presence of a value
  • This should be configurable globally via YAML, and configurable for individual RequiredFieldsValidator instances via a new setter method.
  • It should be false by default e.g. mimic existing behaviour

Note about PRs

I've basically treated this like 3 separate issues and split the PRs

• Allow whitespace changes
• Rename classes
• Call form field validate with form

CMS 5 PRs - allow whitespace only

• ENH Config to toggle allowing whitespace only in Requiredfields #11491
• DOC RequiredFields allow_whitespace_only config developer-docs#647

CMS 5 PRs - rename classes

• DOC Deprecations for validators developer-docs#646
• API Deprecate renamed validators #11490
• API Deprecate renamed validator symbiote/silverstripe-advancedworkflow#565
• API Deprecate renamed validator silverstripe-userforms#1354

After merging all the above reassign to Steve to merge-up and rebase

Kitchen sink CI CM6 - rename classes

• https://github.com/creative-commoners/recipe-kitchen-sink/actions/runs/12245293053

^ Unit test failure is existing

CMS 6 PRs - rename classes

• API Rename validator classes #11486
• API Rename validator classes silverstripe-userforms#1353
• API Rename validator classes symbiote/silverstripe-advancedworkflow#564
• API Rename validator classes silverstripe-admin#1864
• API Rename validator classes silverstripe-asset-admin#1519
• API Rename validator classes silverstripe-cms#3033
• API Rename validator classes silverstripe-elemental#1286
• API Rename validator classes silverstripe-linkfield#355
• API Rename validator classes silverstripe-mfa#578
• API Rename validator classes silverstripe-spamprotection#129
• API Rename validator classes silverstripe-frameworktest#214
• DOC Rename validator classes developer-docs#645

Kitchen sink CI - call form field validate within form

• https://github.com/creative-commoners/recipe-kitchen-sink/actions/runs/12268735349

^ Unit test failure is existing

CMS 6 PRs - call form field validate within form

• ENH Call validate on form fields within form #11506
• ENH Do not call form field validate in validator silverstripe-userforms#1358
• DOC Remove references to FieldsValidator developer-docs#652
• MNT Update inline comment silverstripe-frameworktest#217
• MNT Update unit test silverstripe-spamprotection#130
• ENH Do not pass in redundant validator silverstripe-asset-admin#1522

[emteknetnz]

@GuySartorelli Having a second look at these ACs it seems like they may cause a fair bit of upgrade pain as RequiredFields is a very commonly used class, and it's very likely that projects will forgot to "upgrade" them to be CompositeValidators that includes both a FieldsValidator and a RequiredFields, and now field validation will silently stop working

Even when they do remember to add it, it honestly sounds like really annoying boilerplate. People will wonder why they need to add a FieldsValidator since that should really just be there by default

I'm keen not to go with the original solution, I can think of a few alternative solutions instead:

Option A

• Do nothing, or something minimal such as make RequiredFields extend FieldsValidator

Option B

• Update the logic in Form so that no matter whatever Form::setValidator() is called with, when calling Form::getValidator() you always end up with a CompositeValidator with a FieldsValidator along with what the argument passed to setValidator() was if it wasn't a CompositeValidator or a FieldsValidator

Option C

• Set the Form validator to a CompositeValidator with a FieldsValidator in Form::__construct()
• Remove setValidator() and getValidator()
• Add addValidator() which adds to the Forms CompositeValidator()
• Add setCompositeValidator() and getCompositeValidator()

Option D

• Get rid of FieldsValidator
• Move it logic to Form::validate() to happen before calling the equivalent of $this->getValidator()->validate()

My recommendation is Option D as it seems like it should have always been doing this - can you think of any reason to not do this?

[GuySartorelli]

D sounds fine.

[GuySartorelli]

@emteknetnz It's not clear why this is assigned to me - there's no PRs that aren't in draft.

[emteknetnz]

Sorry forgot to mark PRs as ready for review

[GuySartorelli]

PRs merged. Reassigning to Steve for the next round

[emteknetnz]

No more rounds, we're done :-)