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

Add event to make model validation extendable #139

Merged
merged 7 commits into from
Oct 16, 2023
Merged

Conversation

mjauvin
Copy link
Member

@mjauvin mjauvin commented Jan 28, 2023

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

Used by wintercms/wn-translate-plugin#52

@what-the-diff
Copy link

what-the-diff bot commented Jan 28, 2023

  • 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
Copy link
Member Author

mjauvin commented Jan 28, 2023

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

@bennothommo
Copy link
Member

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

@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
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
Copy link
Member Author

mjauvin commented Jan 31, 2023

@bennothommo not sure I understand the relevance here

@bennothommo
Copy link
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
Copy link
Member Author

mjauvin commented Jan 31, 2023

@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
Copy link
Member Author

mjauvin commented Feb 28, 2023

Any objection on merging this?

@LukeTowers LukeTowers modified the milestones: v1.2.2, v1.2.3 Apr 20, 2023
@LukeTowers
Copy link
Member

@bennothommo any thoughts on this?

@bennothommo
Copy link
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
Copy link

github-actions bot commented Sep 6, 2023

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
Copy link
Member Author

mjauvin commented Oct 15, 2023

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

@bennothommo bennothommo merged commit c0acc2b into develop Oct 16, 2023
8 checks passed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants