lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.52k stars 124 forks source link

validateAttributes doesn't get prepareForValidation data #170

Closed davidjr82 closed 2 years ago

davidjr82 commented 2 years ago

Hi!

When prepareForValidation is used, validateAttributes method is not returning those changes:

public function prepareForValidation(ActionRequest $request): void
{
    $request->merge(['slug' => Str::slug($this->slug)]); // My slug => my-slug
}
public function handle( //...
    $validated_data = $this->validateAttributes(); // $validated_data contains 'slug' => 'My slug' instead of 'my-slug'
    // ...

Maybe this is how it should work (right now I don't remember how a regular FormRequest works), if so, forget and close this one. The easy workaround is to change again the value in the handle method, of course.

Thanks!

lorisleiva commented 2 years ago

Hi David! 👋

Thanks for raising this. There could be a little bug with the validation flow when validating attributes.

I don't have much time on my hands at the moment (just started a new job) but I'll take a look at it as soon as I can. I should hopefully be able to reproduce by creating a new regression test.

lorisleiva commented 2 years ago

Hey David, I had a look at your issue today and I know what's happening.

In the example you provided, the prepareForValidation method is actually being called but the $request->merge method will not affect the attributes of the action. That is on purpose because a request is only relevant for controllers whereas action attributes are decoupled from "how" the action was executed.

Therefore, a simple fix is to change the content of that prepareForValidation method so that it alters the attributes of the actions rather than the input of the request. See the example below.

public function prepareForValidation(): void
{
    $this->fill(['slug' => Str::slug($this->get('slug'))]);
}

I also created a regression test for you so you can see how this works with all supported action execution.

https://github.com/lorisleiva/laravel-actions/blob/4e60c9fbdfcea7d9977d882f3104c8996b43c169/tests/AsActionWithValidatedAttributesAndPrepareForValidationTest.php#L22-L25

davidjr82 commented 2 years ago

Hi Loris,

thanks for taking a look on this. I am also right now busy, but as soon as I can test it, I will write you back. Thanks again!

davidjr82 commented 2 years ago

Sorry for the delay. It works as expected, thanks a lot Loris.