Allow configuration options for the prefix pattern

Created on 8 November 2024, 4 months ago

Problem/Motivation

At the moment this module's language-country-url negotiation method hard-codes an expectation that url prefixes will be of the form /LANGCODE-COUNTRYCODE/. The COUNTRYCODE part takes its value from the country_code base field on the respective lcn_country entity. The LANGCODE part takes its value from the core's language-url negotiation method configuration.

The latter is confusing because, in the admin UI, language-url appears to be an unrelated negotiation method. Moreover, a site builder may wish to use multiple negotiation methods in a fallback arrangement, and LCN currently prevents the possibility of configuring language-url differently to language-country-url, which seems a shame.

Proposed resolution

  1. Duplicate the Path prefix configuration section from language-url to language-country-url so LCM holds the equivalent config for customising the language prefixes but under its own namespace (call it Language path prefix configuration). These should default to the language codes, but are available for customisation.
  2. Add a similar Country path prefix configuration section. These should default to their country codes, but are available for customisation.
  3. Keep the current behaviour of using prefixes of the form /LANGCODE-COUNTRYCODE/ (but honour the above configuration so each part may be customized within the negotiation method). I suppose this format could be exposed as configuration--a simple string with tokens in (e.g. :language-:country), but that feels like overkill--this is a standard format for regional language variations.
  4. Extra fun: introduce configuration to allow a custom prefix to be optionally specified for any language-country pair. Its hard to imagine a use case that isn't covered by this degree of flexibility. Rather than having a textfield for every language-country permutation, could use the same kind of form structure as e.g. core option's "Add another item" table-y interface
✨ Feature request
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡¬πŸ‡§United Kingdom jamsilver West Midlands, UK

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jamsilver
  • πŸ‡¬πŸ‡§United Kingdom jamsilver West Midlands, UK
  • πŸ‡¬πŸ‡§United Kingdom jamsilver West Midlands, UK
  • Assigned to Anna D
  • Status changed to Needs work about 1 month ago
  • 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)

  • πŸ‡ΊπŸ‡¦Ukraine Anna D
Production build 0.71.5 2024