hotwired-laravel / turbo-laravel

This package gives you a set of conventions to make the most out of Hotwire in Laravel.
https://turbo-laravel.com
MIT License
793 stars 48 forks source link

Incorrect redirect after ValidationException? #104

Closed yttrian closed 1 year ago

yttrian commented 1 year ago

From my understanding, when request validation fails, the TurboMiddleware is supposed to redirect to the guessed route if it exists, but since ValidationException sets a redirectTo, it is used instead of trying to guessFormRedirectUrl.

https://github.com/hotwired-laravel/turbo-laravel/blob/300bacc76aa8181c51311560a652e3c45faa25f1/src/Http/Middleware/TurboMiddleware.php#L102-L109

Consider this example that models my situation.

// in web.php
Route::resource('something', SomeController::class); 
// somewhere in SomeController class
public function index() {
    return view('something.index');
}

public function create() {
    return view('something.create');
}

public function store(SomeFormRequest $request) {
    ...
}

The view for something.index contains a frame with the contents of something.create which is a form with action pointing to the store route and method POST.

<!-- somewhere in something.index's view -->
<turbo-frame id="create" src="{{ route('something.create') }}"></turbo-frame>
<!-- in something.create's view -->
<turbo-frame id="create">
   <form action="{{ route('something.store') }}" method="post">
       ...
   </form>
</turbo-frame>

When the form is submitted and validation fails it tries to handleRedirectInterally with something.index because that's what the ValidationException says to do when instead guessFormRedirectUrl would correctly say to use the content of something.create.

I'm basing this on comparing the debugger output of $formRedirectUrl versus $this->guessFormRedirectUrl($request) with a breakpoint at line 91 in TurboMiddleware.php

I am using Laravel 10 if that's relevant.

tonysm commented 1 year ago

@yttrian Interesting. I was going to say that Laravel doesn't set the redirectTo, but I found this in the source which indicates it sets it if you're using form requests, which seems to be the case here..

It looks like you may be able to override the getRedirectUrl in the form request in the meantime.

We only have tests using simple request validation (here) so that does look like a bug. I think we may just change the middleware to use the guessed route (if it exists) instead then... 🤔

tonysm commented 1 year ago

@yttrian a new version (1.12.0) was released with the fix for that. Thanks for reporting it.

yttrian commented 1 year ago

Thanks for getting this fixed!