Consolidate validation systems

[emteknetnz]

Currently there are two main types of validation within the CMS, which is fairly confusing:

• Model validation which is defined in DataObject::validate() and is called as part of DataObject::write(). This will always be called unless DataObject.validation_enabled is set to false. It IS called when updating data via an API system.
• Form validation, which is combination FormField::validate() and somewhat confusingly DataObject::getCMSCompositeValidator(), which is in most cases only uses the RequiredFields validator. Form field validation is called when a POST submission for a Form is processed. It IS NOT called when updating data via an API system.

Ideally we'd get rid of Form validation entirely and have all validation fire whenever DataObject::write() is called. This would have two major benefits:

• Reduce confusion to new users about how to add validation within a Silverstripe project
• Facilitate Silverstripe being fully API driven, meaning that data can be safety written via central API. The can be both within the Silverstripe CMS GUI (GraphQL was at one point supposed to do this, a future API system may also wish to do this), and from any third party system integration. If we're serious about making the CMS fully API driven I think consolidating validation is a requirement

One idea that could help greatly here would be to move validation logic from FormFields and on to their DBField counterparts, for instance move EmailField::validate() to a new class DBEmail and put the validation logic there. Projects would then need to change 'Email' => 'Varchar', to 'Email' => 'Email'. All DataObject->write()'s to this field would now have the Email field validated. This would also have an additional benefit where form auto-scaffolding would now automatically give you the correct EmailField instead of the default TextField.

We could mitigate the risk that projects forget to change Varchar to Email by putting the validation logic in a trait and just leaving it on the Email field and keeping Form field validation functional (so things double validate), though update our documentation to only give examples of the new best practice.

We'd also probably want to rename getCMSCompositeValidator() to just getCompositeValidator() and get it to run on DataObject::write(), rather than on Form submission, as RequiredField's is probably the main way that validation is added within projects. We could also just deprecate it and add a new private $static array $required_fields = []; or just $required on DataObject.

Related issues

• https://github.com/silverstripe/silverstripe-framework/issues/10967

Acceptance criteria

• [ ] Work out if it's feasible to move some or all Form validation to Model validation
• [ ] Work out mitigations to risks this would introduce to projects that are upgrading

[GuySartorelli]

Ideally we'd get rid of Form validation entirely

I assume you mean for data objects in the CMS specifically. We would obviously need to keep form validation for custom and front-end forms.

[emteknetnz]

Realistically we'd probably need to retain form validation as just removing it could cause significant upgrade pain. However I think it should be pushed to the background as much as possible and we ourselves ideally would upgrade things we manage to use model validation exclusively, and we encourage others to do the same.

We would obviously need to keep form validation for custom and front-end forms.

I'm not sure that's true, if the idea I raised in the original description of moving validation to DBField is viable, then projects could in theory create custom DBField's to correspond with any custom fields that have custom validation. That's probably annoying for people, though it doesn't mean it's not possible.

[GuySartorelli]

Not all forms are backed by a DataObject. Not all form fields would have a suitable DbField analogue.

I would be very resistant to moving form validation to non-form constructs. We don't want to make it unnecessarily difficult to make front-end forms, nor to create new form field types.

[emteknetnz]

Yes agree, as I said we'd probably need to retain Form validation, though encourage the use of model validation instead.

For an example of how this would look, assuming it all worked as envisioned, linkfields EmailLink would go from this:

class EmailLink extends Link
{
    // ...
    private static array $db = [
        'Email' => 'Varchar(255)',
    ];

    public function getCMSFields(): FieldList
    {
        $this->beforeUpdateCMSFields(function (FieldList $fields) {
            $fields->replaceField('Email', EmailField::create(
                'Email',
                _t(__CLASS__ . '.EMAIL_FIELD', 'Email address'),
            ));
            $fields->removeByName('OpenInNew');
        });
        return parent::getCMSFields();
    }

    public function getCMSCompositeValidator(): CompositeValidator
    {
        $validator = parent::getCMSCompositeValidator();
        $validator->addValidator(RequiredFields::create(['Email']));
        return $validator;
    }  
    // ...

to this

class EmailLink extends Link
{
    // ...
    private static array $db = [
        'Email' => 'Email',
    ];

    private static array $required = [
        'Email',
    ];

    public function getCMSFields(): FieldList
    {
        $this->beforeUpdateCMSFields(function (FieldList $fields) {
            $fields->removeByName('OpenInNew');
        });
        return parent::getCMSFields();
    }   
    // ...

[GuySartorelli]

That's fine, so long as someone can still create a contact form on the front end with an email field, and it gets validated correctly without needing a dbfield or data object connected to it 👍

That doesn't necessarily mean the validation for email address has to live inside the email field, but there needs to be an intuitive way to validate it with minimal additional boilerplate.