Skip to content
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

Replace RequireFields with DataObject.require config #11404

Open
7 tasks
emteknetnz opened this issue Sep 26, 2024 · 0 comments
Open
7 tasks

Replace RequireFields with DataObject.require config #11404

emteknetnz opened this issue Sep 26, 2024 · 0 comments
Assignees

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Sep 26, 2024

Update: need to this about this a bit more first - as a Form may require a Recaptcha field which isn't on the DataObject. It may make more sense to do something similar to #11402 where config on the DBField defines the FieldValidator's which can be used, likewise maybe it makes sense to add validators to DataObjects the same way instead of via getCMSCompositeValidator(). Other option is just call the validators in getCMSCompositeValidator() as part of DataObject::validate(), though this feels pretty inconsistent.

RequireFields is a commonly used form validator that is weirdly added via on DataObject::getCMSCompositeValidator(), despite DataObject not actually being a Form itself.

Form validation is not called as part of DataObject::write() / DataObject::validate(), meaning that the fields aren't actually required with programmatically updating the DataObject e.g. via an API call.

Instead we could enforce required fields by adding private static array $required = []; to DataObject where $db fields that are required are added. Those fields are then checked that they have values as part of DataObject::validate(), and if they are blank then ValidationResult::addFieldError($field, _t('KEY', 'Field is required')) is called.

$require should also enforce $has_one relations, though not any of the other relations, including belongs_to

Acceptance criteria

  • PRs targets CMS 5.4
  • DataObject fields are validated they are non-blank if they are defined in DataObject.required config
  • DataObject $has_one fields are validated they are non-zero
  • Validation messages show as appropriate in the CMS
  • RequiredFields is deprecated in CMS 5.4 and the message says to use DataObject.required instead
  • RequiredFields is removed from CMS 6
  • CMS 5 developer docs are updated to say use DataObject.required instead of RequireFields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant