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: Query in path #19087

Merged
merged 3 commits into from Apr 4, 2024
Merged

fix: Query in path #19087

merged 3 commits into from Apr 4, 2024

Conversation

caalador
Copy link
Contributor

@caalador caalador commented Apr 3, 2024

Fix mistaken add of query
parameters into the route
breaking route resolution
when navigating thrhough link.

fixes #19080

Copy link

github-actions bot commented Apr 3, 2024

Test Results

1 099 files  ±0  1 099 suites  ±0   1h 25m 52s ⏱️ -43s
6 942 tests +4  6 893 ✅ +4  49 💤 ±0  0 ❌ ±0 
7 302 runs  +9  7 241 ✅ +9  61 💤 ±0  0 ❌ ±0 

Results for commit 3948f4f. ± Comparison against base commit 30f8aaf.

♻️ This comment has been updated with latest results.

Fix mistaken add of query
parameters into the route
breaking route resolution
when navigating thrhough link.

part of #19080
@caalador caalador force-pushed the issues/19080-route-not-ofund branch from 22c8027 to 2f678b1 Compare April 3, 2024 10:05
@mshabarov mshabarov requested a review from tltv April 3, 2024 10:05
Copy link
Member

@tltv tltv left a comment

Choose a reason for hiding this comment

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

Makes sense. Create also new IT for the main issue (can be in separated PR).
Shouldn't this also remove hash from the baseURI?

@caalador
Copy link
Contributor Author

caalador commented Apr 3, 2024

Actually this might be a problem in the server side route resolver instead of a problem getting the query. I can only see a couple tests that test the href for routerlink, but nothing that would test actually navigating with the query and that is what fails now.

@caalador
Copy link
Contributor Author

caalador commented Apr 3, 2024

Now it should be ok and there are tests in place for the navigation.

Copy link

sonarcloud bot commented Apr 4, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@tltv tltv merged commit adbf6a3 into main Apr 4, 2024
26 checks passed
@tltv tltv deleted the issues/19080-route-not-ofund branch April 4, 2024 07:22
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.0.alpha21 and is also targeting the upcoming stable 24.4.0 version.

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.

In Vaadin 24.4.0.alpha18, RouterLink does not update the browser URL
3 participants