epartment / nova-dependency-container

A Laravel Nova field container allowing to depend on other fields values
MIT License
382 stars 163 forks source link

Fixes validation for Nova Actions #179

Closed subzero10 closed 3 years ago

subzero10 commented 3 years ago

Even when actions use the trait ActionHasDependencies, the validateFields() of the trait is never called, because the validateFields() of the ActionRequest never calls it. I also fixed a minor bug where fieldsForValidation() would not return the resulting array.

ragingdave commented 3 years ago

You are incorrect on validateFields in the ActionRequest not being hit. I can say for certain (as I just ran a test) that validateFields method is hit for an Action that uses the trait ActionHasDependencies.

I would suggest in the future that bug fixes aren't combined as part of this PR might have been accepted but as of now it will simply be closed.

ragingdave commented 3 years ago

If you feel that this resolution is in error please submit an issue so it can be further investigated.

subzero10 commented 3 years ago

You are incorrect on validateFields in the ActionRequest not being hit. I can say for certain (as I just ran a test) that validateFields method is hit for an Action that uses the trait ActionHasDependencies.

I would suggest in the future that bug fixes aren't combined as part of this PR might have been accepted but as of now it will simply be closed.

Can you please point me to that test? Thanks.

ragingdave commented 3 years ago

You can throw a dump statement in this packages code in your own environment in the ActionRequest validateFields method and see that upon submission that code is hit. Specifically anywhere in the function itself after https://github.com/epartment/nova-dependency-container/blob/master/src/Http/Requests/ActionRequest.php#L18