Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Keep query params on redirect#27482
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…) and redirectToRoute() methodsI liked the addition of $keepQueryParams to the redirectAction() method,but I thought it would be an even more useful addition to theControllerTraits redirect() and redirectToRoute() methods.
| 4.1.0 | ||
| ----- | ||
| * Added optional $keepQueryParams argument to redirect() and redirectToRoute() methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should be moved in the 4.2 section
| * @final | ||
| */ | ||
| protectedfunctionredirect(string$url,int$status =302):RedirectResponse | ||
| protectedfunctionredirect(string$url,int$status =302,bool$keepQueryParams =false):RedirectResponse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As this function accepts a URL and not a route, I don't see the usefulness of this feature to be honest. When generating the URL, you can already use the url generator to add the current query string based on$request->query->all();.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
redirect() is called by redirectToRoute() and I wanted to keep things consistent with both functions and with fix#26281. Also, the way I did things it will append the old query params to the redirect url (which might have its own query params). I basically borrowed the code from urlRedirectAction() and gave these functions the same $keepQueryParams option.
I had a bunch of code where I was appending query params before calling redirect() or redirectToRoute() and it just made sense to move the logic into the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
if you callredirectToRoute, you can pass extra params to the url generator, and it will put them in the query string.
So IMO, this is not needed (the redirect controller does not give you the control of the arguments which is why it needed to add this feature)
nicolas-grekas commentedJun 19, 2018
@lfjeff OK to close? If you think the doc could be improved, PR welcome on it of course. |
lfjeff commentedJun 19, 2018
OK with me to close. This is my first Symfony contribution, so I'm not sure where the relevant doc for this feature is located. |
| protectedfunctionredirect(string$url,int$status =302,bool$keepQueryParams =false):RedirectResponse | ||
| { | ||
| if ($keepQueryParams) { | ||
| $qs =$this->container->get('request_stack')->getCurrentRequest()->getQueryString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
maybe you can rename $qs to $queryString ?
| if ($keepQueryParams) { | ||
| $qs =$this->container->get('request_stack')->getCurrentRequest()->getQueryString(); | ||
| if ($qs) { | ||
| if (false ===strpos($url,'?')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it will be more readable with a ternary
javiereguiluz commentedSep 21, 2018
Closing as explained in favor ofsymfony/symfony-docs#10373. Thanks! |
…(javiereguiluz)This PR was merged into the 2.8 branch.Discussion----------Mentioned how to redirect and maintain the query stringThisfixessymfony/symfony#27482.Commits-------71d6ab1 Mentioned how to redirect and maintain the query string
Uh oh!
There was an error while loading.Please reload this page.
Added optional $keepQueryParams to redirect() and redirectToRoute() methods.