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

[10.x] use the same criteria StartSession uses on previous URL #46390

Closed
wants to merge 1 commit into from
Closed

[10.x] use the same criteria StartSession uses on previous URL #46390

wants to merge 1 commit into from

Conversation

rodrigopedra
Copy link
Contributor

Closes #46388

PR #46234 introduced a change on how the previous URL is calculated when the previous URL matches the current URL.

Unfortunately, it breaks redirecting back to the same URL, which is a common pattern on PATCH/PUT, as the previous implementation didn't consider which request method is being used.

This PR

  • Changes the previous URL calculation to use the same criteria StartSession middleware uses to check if it should update the previous URL in session

@rodrigopedra
Copy link
Contributor Author

I guess we could extract this logic to the request object. I wanted to propose a quick fix for the introduced error

@rodrigopedra
Copy link
Contributor Author

Not sure why just one set of tests, failed, from the running time it seems it got stuck somewhere and ended up failing.

StyleCI cloudflare human verification keeps me on a verification loop, so I couldn't see what is wrong.

@taylorotwell
Copy link
Member

I had to revert the other PR and tag a patch, so I'm guessing this PR isn't complete anymore.

@taylorotwell
Copy link
Member

I'm also hesitant to keep chasing down this road.

@taylorotwell taylorotwell marked this pull request as draft March 8, 2023 16:53
@rodrigopedra
Copy link
Contributor Author

@taylorotwell sorry for the inconvenience caused. This PR was more like a hot-fix attempt.

When I saw the related issue, I just wondered to share what I use, which doesn't break as I only use it at blade templates. I didn't think on the broad usage.

I guess it is better to close this by now, and then propose a more thoughtful solution, considering those other caveats. Perhaps closer to 11.x.

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.

return back() is broken in laravel 10.3 version
2 participants