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

Translate parameters - urlFromPattern bug #1104

Open
Jk1979 opened this issue Apr 18, 2024 · 1 comment
Open

Translate parameters - urlFromPattern bug #1104

Jk1979 opened this issue Apr 18, 2024 · 1 comment

Comments

@Jk1979
Copy link

Jk1979 commented Apr 18, 2024

Winter CMS Build

dev-develop

PHP Version

8.1

Database engine

MySQL/MariaDB

Plugins installed

No response

Issue description

I think we have wrong behaviour in winter/storm/src/Router/Helper.php getParameterName function.
In old october cms version (1.x) this function looked like this -
image

but now it is like this -

image

and if we have url pattern like this -

url = "/some/:dest|^destinations$|^world$|^countries$/:region?*"

when we run $this->getRouter()->getParameters(); for url "some/destinations/central-america/belize/"
we get such array -

array:2 [
"dest" => "destinations"
"region?" => "central-america/belize"
]

but then in function urlFromPattern in Router.php we have following situation -

here

image

for parameter "region?", function Helper::getParameterName returns "region", instead of "region?" like it was in previous version (see above) , this is due to this condition -

image

and then, in this condition -
$parameterExists = array_key_exists($paramName, $parameters) &&

we have false, because "region" key does not exist in params array, and we have wrong url like -
"some/destinations"

instead of

"some/destinations/central-america/belize"

In order to solve this, I had to remove "?" from params keys, before passing it to urlFromPattern($pattern, $parameters = [])

Steps to replicate

see above

Workaround

No response

@LukeTowers
Copy link
Member

@Jk1979 can you submit a test case for this issue in a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants