Skip to content

Add event to make model validation extendable#139

Merged
bennothommo merged 7 commits into
developfrom
extend-validation
Oct 16, 2023
Merged

Add event to make model validation extendable#139
bennothommo merged 7 commits into
developfrom
extend-validation

Conversation

@mjauvin

@mjauvin mjauvin commented Jan 28, 2023

Copy link
Copy Markdown
Member

Needed to add Validation to translatable Model attributes in Winter.Translate plugin.

Used by wintercms/wn-translate-plugin#52

@what-the-diff

what-the-diff Bot commented Jan 28, 2023

Copy link
Copy Markdown
  • Added a new event 'model.getValidationAttributes'
  • The model will fire this event when fetching the attributes to validate the model
  • If any listeners return an array, it will be used as validation attributes instead of $this->attributes (the default)

@mjauvin

mjauvin commented Jan 28, 2023

Copy link
Copy Markdown
Member Author

@bennothommo any idea what the error in code analysis is about?

@bennothommo

Copy link
Copy Markdown
Member

The static code analysis test has been a bit flaky recently. Feel free to ignore it for now.

Comment thread src/Database/Traits/Validation.php
@mjauvin mjauvin marked this pull request as draft January 30, 2023 15:12
@mjauvin mjauvin marked this pull request as ready for review January 30, 2023 15:13
@bennothommo

Copy link
Copy Markdown
Member

@mjauvin is it possible to just dynamically extend a model and add validation rules?

Model::extend(function ($model) {
    $model->rules['newField'] = 'required';
    // etc...
});

@mjauvin

mjauvin commented Jan 31, 2023

Copy link
Copy Markdown
Member Author

@bennothommo not sure I understand the relevance here

@bennothommo

Copy link
Copy Markdown
Member

@mjauvin I'm asking if there's any need to create a new event when a model extension can do it already.

If the Translatable behavior needs to implement validation on the fly, you can create a Rule class which handles the validation needed for the translation, and simply add that Rule class through the model extension.

@mjauvin

mjauvin commented Jan 31, 2023

Copy link
Copy Markdown
Member Author

@bennothommo but it doesn't need to create new rules. just apply the existing rules to the translations.

@mjauvin mjauvin added this to the v1.2.2 milestone Jan 31, 2023
@mjauvin

mjauvin commented Feb 28, 2023

Copy link
Copy Markdown
Member Author

Any objection on merging this?

Comment thread src/Database/Traits/Validation.php
@LukeTowers LukeTowers modified the milestones: v1.2.2, v1.2.3 Apr 20, 2023
@LukeTowers

Copy link
Copy Markdown
Member

@bennothommo any thoughts on this?

@bennothommo

Copy link
Copy Markdown
Member

Nope, no additional thoughts. If it works, it's all good by me.

@LukeTowers LukeTowers modified the milestones: v1.2.3, v1.2.4 Jul 7, 2023
@github-actions

github-actions Bot commented Sep 6, 2023

Copy link
Copy Markdown

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@mjauvin

mjauvin commented Oct 15, 2023

Copy link
Copy Markdown
Member Author

Can this be merged, it would allow me to continue work on the associated PR in Translate plugin (wintercms/wn-translate-plugin#52) ?

Comment thread src/Database/Traits/Validation.php
@bennothommo bennothommo merged commit c0acc2b into develop Oct 16, 2023
@bennothommo bennothommo deleted the extend-validation branch October 16, 2023 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants