- Issue created by @jamsilver
- Assigned to Anna D
- Status changed to Needs work
about 1 month ago 3:14pm 4 February 2025 - Merge request !13#3486455: Default Prefix Pattern && custom prefixes for each pair β (Open) created by Unnamed author
- First commit to issue fork.
- π©πͺGermany simonbaese Berlin
It still requires some more work. We do not yet handle partial matches in the path prefix. Therefore, the fallback mechanism can not work. This is also why the tests are failing at the moment.
- πΊπ¦Ukraine Anna D
The fallback mechanism has been fixed. However, the redirect to the international page has been removed when a country is not allowed, because it is unclear whether the issue is a wrong prefix or just a path (e.g., ./foo-boo/test).
Notes:
Partial matching should not be applied in the getCountryCodeFromPath or getLangcodeFromPath methods as it was done before. If partial matching is used, it would be difficult to determine if the language-country combination is valid. This could lead to an additional check that we would like to avoid.I am leaning toward having a single method to retrieve both the $langcode and $country_code from the countryRepository prefixes, as they are interdependent. Both should either be valid or both should be NULL.
Partial matching should be implemented as a separate method that would be used exclusively for fallback purposes and for detecting the current country.
- π©πͺGermany simonbaese Berlin
I agree with the first paragraph.
I want to maintain a clean interface for the path utility service. Essentially, other services should not make assumptions or have any logic to handle the prefix in the path. I know that the path handling could be more efficient. But having a clear separation here is more important than a couple of nanoseconds.
Please also stay "close" to the original implementation in the scope of this issue. The behaviour in the fallback mechanism changes quite a bit. All the changes were correct semantically, though. Further, as I understand the logic now, we should have a valid (matches custom prefix or default pattern) once we arrive in the
CurrentCountry
service.Needs work because we need to test more methods in the
PathUtility
service. - πΊπ¦Ukraine Anna D
Partially agree.
I'd be more strict with country-language negotiation. Usually, if the prefix is wrong, it leads to a 404 error. What we do with partial matching and fallback is more experimental, and we try to guess the country-language pair.
- π©πͺGermany simonbaese Berlin
Making the fallback mechanism responsible for the partial matching is a good idea. Also, it cleans up the interfaces for the other services. The issue still needs work for testing the path utility service.
- πΊπ¦Ukraine Anna D
A little more work is needed:
1. With the current setup, absolute paths (e.g., 'https://example...') are incorrectly processed in the getPrefixFromPath() and removePrefixFromPath() methods.
2. The use of strtolower($prefix) in getPrefixFromPath() reduces flexibility for prefixes like en-GB (at least for custom prefixes, since auto-replaced country codes are always forced to lowercase).
3. removePrefixFromPath() should reuse getPrefixFromPath(), but it wonβt work as expected if the prefix contains uppercase letters.I wouldn't touch lowercase in this issue; we need to think a little more about it. (maybe allow to use case in the pattern? COUNTRY_CODE vs county_code)