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

Fix repeating of persistent parameter in url #1055

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Rixafy
Copy link
Contributor

@Rixafy Rixafy commented Dec 14, 2022

When loading clean page with datagrid and persistent parameter included in router url, datagrid js always updated url and added redundant persistent parameter, for isntance:

url where example.com is persistent parameter domainName => http://localhost/domain/example.com/cron-task
would turn into http://localhost/domain/example.com/cron-task?domainName=example.com
because datagrid js has no way of knowing that there is already that persistent parameter.

In that case, my solution checks for /example.com in url, and if it's there, it skips that parameter while building url query, I think it's good enough when there is persistent parameter standalone somewhere in url.

@radimvaculik
Copy link
Member

@Rixafy Have you tested with multiple parameters with same value, but different key? For example domainName1=example.com&domainName2=example.com

@Rixafy
Copy link
Contributor Author

Rixafy commented Dec 28, 2022

@radimvaculik I cannot replicate that, when I try to accept $test, in actionDefault(string $test), and then I add ?test=example.com in url, datagrid will ignore that test parameter for some reason, and it will apply only domainName persistent parameter and grid-* parameters.

Anyway, I improved the checking if this happened, after every check it will remove that /example.com from checked url, and next parameter will be checked on modified url string, this will work when persistent parameter will be placed before the get parameters in that map, but I have no way of testing if it's the case, since it ignored my ?test=example.com completely and it modified url without that parameter, so when I reloaded the page, I got 404.

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

Successfully merging this pull request may close these issues.

2 participants