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

Add reuse_query_params to UrlHelper to allow reuse of existing request query params #7

Merged
merged 7 commits into from
Apr 20, 2021

Conversation

sheridans
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

Description

It's currently possible to reuse route parameters (by default), however it is not possible to reuse existing query parameters when generating routes.

For example if the existing url is "/?filter=name" and you use the UrlHelper class to regenerate the route with using: $this-url('home', [], ['page' => 2]);then the generated URL would be "/?page=2" removing existing parameters. This PR changes that so the generated route would automatically be "/?filter=name&page=2" by using existing parameters.

Any parameters generated via UrlHelper will override existing parameters.

This feature adds the ability to reuse existing request query parameters (enabled by default) when generating a URL for the same route. Similar to the way reusing route parameters can be disabled, this adds the functionality to disable reuse of query parameters using 'reuse_query_params' option in a similar way.

This works by injecting the ServerRequestInterface instance into the UrlHelper via the UrlHelperMiddleware and is enabled by fault.

The existing tests have been updating to support the ServerRequestInterface injection.

@sheridans sheridans changed the title Add reuse_use_query to UrlHelper to allow reuse of existing request query params Add reuse_query_params to UrlHelper to allow reuse of existing request query params Apr 9, 2021
@@ -102,6 +108,13 @@ public function __invoke(
$routeParams = $this->mergeParams($routeName, $result, $routeParams);
}

$reuseQueryParams = ! isset($options['reuse_query_params']) || (bool) $options['reuse_query_params'];
Copy link

@kynx kynx Apr 9, 2021

Choose a reason for hiding this comment

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

Yikes! That's quite a BC break.

Am I reading this right? If I hit /a?foo=bar and then generate a URL for b, it'll end up /b?foo=bar by default? If so, that's not a behaviour I'd want 99.9999% of the time.

EDIT: Ah, just spotted the if ($result->getMatchedRouteName() !== $route) - so it only applies to the current route 😅

Copy link
Contributor Author

@sheridans sheridans Apr 10, 2021

Choose a reason for hiding this comment

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

Sorry should have mentioned that, yes it only applies to routes generated for current route, similar functionality to reuse_route_params. It will not reuse query params for different routes.

@weierophinney weierophinney added this to the 5.5.0 milestone Apr 20, 2021
@weierophinney weierophinney added the Enhancement New feature or request label Apr 20, 2021
@weierophinney
Copy link
Contributor

I rebased on current 5.5.x, as that brings in the GHA workflow and a number of other changes. Everything is passing, so I'm going to go ahead and merge.

@sheridans Please create a PR against the mezzio/mezzio repository with updates to the documentation to demonstrate the new feature. (Documentation is in the docs/book/v3/features/helpers/url-helper.md file of that repo.)

Thanks!

@weierophinney weierophinney merged commit 3ceaaa5 into mezzio:5.5.x Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants