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 language switcher "eating" part of vocids that end in a langcode (e.g. "interessen" and "en") #1234

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

schlawiner
Copy link
Contributor

I happend to have a vocabulary named (vocid) "interessen", which ends in "en", one of the available ui languages.
When switching the UI to German, the "en" ending in the url (/interessen/en/) was replaced with the "de" code, as well as appending another /de. So en was (re)placed twice, leading to broken urls such as /interessde/de/.

I fixed this for now by making the "| replace" filter that rewrites the url not as greedy: instead of any langcode-looking text, it now looks for a slash preceding it (/en instead of just en). This will probably not fix instances where a vocid is exactly the same as one of the langcodes (vocabularies named "en", "de", "fr" etc.) but is good enough for me at the moment.

@sonarcloud
Copy link

sonarcloud bot commented Nov 5, 2021

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@osma osma added the bug label Nov 5, 2021
@osma
Copy link
Member

osma commented Nov 5, 2021

Thanks! Looks good but needs to be tested.

You are right that's it's possible to end up in trouble by having a vocabulary id that is identical to a language code. The URL pattern for Skosmos doesn't support that case very well.

@osma
Copy link
Member

osma commented Nov 9, 2021

I tested this both with and without the dropdown-style UI language selector. It does exactly what it should, so I'm merging it now. Thanks a lot @schlawiner !

@osma osma merged commit 30ce5d0 into NatLibFi:master Nov 9, 2021
@osma osma added this to the 2.13 milestone Nov 9, 2021
osma added a commit that referenced this pull request Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants