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: handle versions in subpaths #15614

Merged
merged 1 commit into from
Nov 8, 2023
Merged

fix: handle versions in subpaths #15614

merged 1 commit into from
Nov 8, 2023

Conversation

mortenoh
Copy link
Member

@mortenoh mortenoh commented Nov 7, 2023

Summary

Fixes an issue with subpath enabling and versions (we only checked for without version), this fix fixes that and assumes first that a version exists (as its what we usually recommend)

Copy link

sonarcloud bot commented Nov 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@jenniferarnesen jenniferarnesen left a comment

Choose a reason for hiding this comment

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

Thanks for the quick work! Would it be possible to add a unit test for it?

Copy link
Contributor

@jbee jbee left a comment

Choose a reason for hiding this comment

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

A comment with examples of the cases would be nice.

@mortenoh
Copy link
Member Author

mortenoh commented Nov 7, 2023

A comment with examples of the cases would be nice.

Yeah, so for the Router we have this subpath ability, so if your url ends with /** it means subpaths are enabled, so if you go api/router/your-router/run/a/b/c the a/b/c will be added to the original router url, and this was working fine.

The problem was that the check was a bit naive, so if you had /api/41/router/your-router/a/b/c instead, it would not recognize if and think it was a non subpath enabled route.

@mortenoh mortenoh merged commit 367fde4 into master Nov 8, 2023
16 checks passed
@mortenoh mortenoh deleted the route-subpath-fix branch November 8, 2023 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants