Skip to content

[FrameworkBundle] feature: redirect query params too in RedirectController #9314

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

Merged
merged 3 commits into from
Apr 26, 2018

Conversation

Simperfit
Copy link
Contributor

Adding documentation for the redirectAction improvement.

This needs the feature to be merged first

@javiereguiluz javiereguiluz added the Waiting Code Merge Docs for features pending to be merged label Feb 23, 2018
@@ -155,3 +158,4 @@ action:
Because you are redirecting to a route instead of a path, the required
option is called ``route`` in the ``redirect()`` action, instead of ``path``
in the ``urlRedirect()`` action.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be reverted

@xabbuh xabbuh added this to the 4.1 milestone Apr 20, 2018
@xabbuh xabbuh removed the Waiting Code Merge Docs for features pending to be merged label Apr 20, 2018
@@ -146,6 +148,7 @@ action:
'_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction',
'route' => 'sonata_admin_dashboard',
'permanent' => true,
'keepQueryParams' => true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing comma in the last element of the arrays is missing, look #8325

Adding documentation for the redirectAction improvement
@Simperfit
Copy link
Contributor Author

@xabbuh @DylanDelobel done

@@ -96,7 +96,7 @@ Redirecting Using a Route

Assume you are migrating your website from WordPress to Symfony, you want to
redirect ``/wp-admin`` to the route ``sonata_admin_dashboard``. You don't know
the path, only the route name. This can be achieved using the
the path, only the route name, if you pass query parameters to this route and active the ``keepQueryParams``, it will be redirected too. This can be achieved using the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually split lines after the first word that crosses the 72nd character. But we can make that tweak while merging. :)

@javiereguiluz
Copy link
Member

Thanks for documenting this new 4.1 feature ... and for implementing the feature too!

@javiereguiluz javiereguiluz merged commit ba5c343 into symfony:master Apr 26, 2018
javiereguiluz added a commit that referenced this pull request Apr 26, 2018
…edirectController (Simperfit, javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

[FrameworkBundle] feature: redirect query params too in RedirectController

Adding documentation for the redirectAction improvement.

This needs the feature to be merged first

Commits
-------

ba5c343 Restored a removed character
6c92c05 Reworded the new option explanation
7210586 feature: redirect query params too in RedirectController
javiereguiluz added a commit that referenced this pull request May 28, 2018
This PR was merged into the 4.1 branch.

Discussion
----------

Added a missing versionadded directive

We forgot to add this in #9314.

Commits
-------

f7d96ea Added a missing versionadded directive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants