-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ValidationInterface
to the DataObject
class
#11457
Comments
ValidationInterface
to the DataObject
class
Can you please explain what you mean by "the other method validate() methods with bool and void returns types" and your reasoning for why they're not worth the effort? |
@emteknetnz please respond to the above comment |
PRs merged. Reassigning to steve to respond to the above and to do B2 |
In the "Notes" section in the description I listed what I think are all off the other DataObject::validate() and Form::validationResult() both return ValidationResult's already, so the amount of refactoring required in pretty minimal. However all the other validate methods that don't return a ValidationResult require significantly more refactoring, and IMO it's not worth the effort at this stage given all the other things that need doing |
https://github.com/silverstripe/silverstripe-framework/blob/6/src/Core/Validation/ValidationInterface.php provides a standardised method signature for objects with a validate() method
So far this had been added to
At very least it should be added to DataObject which already effectively implements it, just with loose typing.
If feasible within a reasonable amount of it, it should also be added to as many other objects that currently have a validate($validator) or validate(): bool, which will involve some level of refactoring
Acceptance criteria
Notes
Other validate() methods:
Two lots of PRs:
A) Add ValidationInterface to DataObject
B) Rename Form::validationResult() to validate()
I've deemed changing the other method validate() methods with bool and void returns types as not worth the effort
A) Add ValidationInterface to DataObject - CM5 6.0
Kitchen Sink CI
^ test failures are existing - elemental - framework
PRs
B1) Rename Form::validationResult() to validate() - CMS 5
Kitchen Sink CI
PRs
Assign back to Steve to do merge-up and make PRs for CMS 6.0
B2) Rename Form::validationResult() to validate() - CMS 6.0
Kitchen Sink CI
PRs
The text was updated successfully, but these errors were encountered: