Merge query params instead of just appending them #197
Conversation
There was a problem hiding this comment.
Thank you for working on this! I've taken a look it, but it's not really solving the issue yet. Can you take another look at it?
Besides, I am wondering about the priority of the query paramers if there are query parameters both in the destination URL and the request URL. Which one takes precedence and overrides the other one?
I do not have an opinion or scenario in mind right now, so maybe it can stay like it is. Or maybe it would be good to have it configurable 🤔 Anyway, just wanted to share this thought.
| $query = array_merge($destination_query, $request->query()); | ||
|
|
||
| if (count($query)) { | ||
| $destination .= '?' . http_build_query($query); |
There was a problem hiding this comment.
This can still lead to invalid URLs with duplicate question marks when the destination URL already contains a query string. The query string needs to be removed first before appending the merged query string.
In the test below you are setting a query string on the source URL but not on the destination URL, which is a different scenario.
There was a problem hiding this comment.
I fixed this issue, please see #202
The solution gives precedence to the destination's query parameters, as they have to be stated explicitly when defining a redirect.
I am also thinking about a solution to redirect URLs independently of their query parameters (and if the preserve_query_parameters config is set, to forward all query parameters also to the destination URL). But as this comment suggests, the regex type shall be used for this use case: #12
Fixes #192