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

Refactor legacy, custom i18n mechanism to use core django i18n #14256

Closed
stevejalim opened this issue Feb 27, 2024 · 0 comments · Fixed by #14327
Closed

Refactor legacy, custom i18n mechanism to use core django i18n #14256

stevejalim opened this issue Feb 27, 2024 · 0 comments · Fixed by #14327
Assignees
Labels
Backend Server stuff yo

Comments

@stevejalim
Copy link
Collaborator

stevejalim commented Feb 27, 2024

In preparation for Wagtail, switch from our current custom i18n code to Django's i18n code.

This will draw heavy, erm, inspiration from @escattone's work in mozilla/kitsune#5653

One thing to note there is that they added...

a custom "shim" that ensures uppercase country codes within locales that include them (for example, en-US instead of en-us).

which we will also need to ensure behaviour remains the same in Bedrock. However, further work on that shim (or the default set of settings.LANGUAGES will be needed over on the Wagtail-in-Bedrock branch because the default language codes are all lowercase, and Wagtail will not boot if you try to use uppercase ones for Wagtail (so that they look the same in URLs) while settings.LANGUAGES is still all lower case. The work to deal with that will have to land in the wagtail-specific branch

  • Understand all our locale-related settings

    • LOCALES_BY_REGION
    • PROD_LANGUAGES - fed by LOCALES_BY_REGION
    • DEV_LANGUAGES - based on whatever's in the fluent cloned repo
    • FALLBACK_LOCALES - 'redirects' from one lang code to another, rather than fall back to en-US
    • CANONICAL_LOCALES - 'redirects' for lang codes within the same lang - eg en->en-US, ja-jp-mac->jp, zh-hk->zh-TW
    • lazy_lang_group - Groups languages with a common prefix into a map keyed on said prefix (eg: 'en')
    • lazy_lang_url_map - dict of k,v pairs like 'en-us':'en-US' - this may be how we map django langs to ours
    • lazy_langs - Override Django's built-in with our native names (but in Django lowercase format)
    • LANG_GROUPS -> lazy-eval version of lazy_lang_group
    • LANGUAGE_URL_MAP -> lazy eval of the case-changing lazy_lang_url_map used for URLs
    • LANGUAGES -> lazy eval of our override of available language codes, all lowercased
    • SUPPORTED_NONLOCALES -> paths that should not have a locale prepended to them
    • SUPPORTED_LOCALE_IGNORE -> paths that can or cannot have a locale prepended - rare
    • NOINDEX_URLS -> paths we don't want indexed by a search engine
    • EXTRA_INDEX_URLS - Pages we do want indexed but don't show up in automated URL discovery or are only available in a non-default locale - eg /privacy/firefox-klar
  • Understand legacy resolvers.py

    • get/set_url_prefix -> accessors for thread-local language prefix
    • reverse is a custom reverse() func that adds the prefix after reversing a view to get its path
    • _get_language_map() - dict of lang code -> URL-lang code mappings. Uses CANONICAL_LOCALES + LANGUAGE_URL_MAP / lazy_lang_url_map from settings
    • FULL_LANGUAGE_MAP -> uses _get_language_map
    • find_supported - look for URL-mode lang code (eg fr-CA) or prefix/partial lang code (fr) for a given lang.
    • split_path - helper to split a path into locale, remaiing path if possible, else returns "" and the full path
    • Prefixer - class that generates a lang prefix for a URL. Tries to extract locale info from the path, never the session
    • Prefixer.get_language tries toget a lang code from the header, sanitising/falling back via get_best_language
    • Prefixer.fix -> see if there's a lang code on there and if not add the best fit
  • Where is urlresolvers used? ie, where are we likely to need changes

    • all reversing in bedrock is using urlresolvers.reverse - lots of calls
    • lib/l10n_utils/__init__.py is using get_best_translation
    • url templatetag
    • LocaleURLMiddleware, mainly.
    • tests
  • Understand kitsune work in switch to django's i18n kitsune#5653

  • Refactor

    • To remove

      • LocaleURLMiddleware
    • To change

      • See if we can switch to multi-case in settings

        • switch the casing in settings

        • Add in django's LocaleMiddleware

        • Update URLconf with i18n_patterns

          • Mozorg
            • Split some sub-confs into two parts, one with and one without i18npattern
          • Pocket
          • ensure SUPPORTED_NONLOCALES and SUPPORTED_LOCALE_IGNORE are handled in new approach - look at how it's used
        • make the urlresolvers.reverse() simpler and not use the Prexfixer (https://github.com/mozilla/kitsune/blob/main/kitsune/sumo/urlresolvers.py)

          • IMPORTANT: looks like we still need to prefix the locale on to URLs
            • Do with custom i18n_patterns for reversing
            • Handle with middleware for unprefixed locales:
        • get another locale working - fr, en-GB, etc

      • Ensure default locale fallbacks are working

        • en should go to en-US not en-CA
        • Add tests
      • Explore what we need to do with settings.FALLBACK_LOCALES Looks like we stopped supporting them. Make a new ticket as a follow up
        ```

      • Rebase on main

      • might be good to drop them into LANG_INFO as fallback langs

      • FIX SUPPORT FOR THREE-DIGIT LOCALES:

      • Rebase on main

      • Respect territory-specific languages as the default - eg en-gb should give us /en-GB/

      • Seek more redirect( ) calls outside of the redirects app and add tests for them

        • mozorg
        • firefox
        • products
        • newsletters
        • careers
        • etc
      • Fix up redirects in app urls with locale_prefix=True, prepend_locale=False ?

      • Ensure that redirects respect Accept-Language headers after the redirect, with as short a path as possible

      • update tests

        • swap HTTP_ACCEPT_LANGUAGE in self.client.get

        • Remove Prefixer and its methods

          • .fix
        • Move mozorg.localed_urls back to just mozorg.urls

        • Deal with lib.l10n_utils

          • Confirm whether we're entirely removing our l10n_utils (I don't recommend so as our fluent render is in there, etc) or "just" using Django for translation activation/management
            • HOW TO DECIDE? -> Look at how django.utils.translation works - is it very similar to ours? It used to be.
            • What about the functions we have that django i18n does not?
            • If the latter, generally change from lib.l10n_utils import translation to core django translation
              • KEEP references to request.locale
                • ensure we populate it
              • seek getattrr(request, "locale"
                • refactor
            • Clean up lib.l10n_utils.translation
          • CHECK IF WE REALLY NEED THE MONKEYPATCHING - IS IT WORKING?
            • adjust calls to it to be translation.get_language()
          • Check if we can update get_best_translation - bear in mind it's used for that 404-locale fallback behaviour and has a strict mode
            • is there overlap between what's in bedrock.base.i18n ?
            • What about an overlap with django.utils.translation.get_supported_language_variant ??
          • Fix infinite redirects for es-MX, es-ES etc
        • Ensure the language switcher works / that we're getting the lang code from the URL

        • Check support for re prefixed lang codes

          • NOINDEX_URLS
          • EXTRA_INDEX_URLS
        • Make sure sitemaps are working

          • top-level
          • locale-level
        • Merge bedrock.base.i18n into l10n_utils - seems silly to keep them separate

        • Can we refactor path_needs_lang_code to not need settings and instead infer it from bedrock.*.nonlocale_urls?

        • update calls to translation.activate in tests

          • remove custom context manager from test setup
      • Ensure we cover these test equalivents https://github.com/mozilla/bedrock/blob/335f99f5f8121b00fe326ac19233c403fefaa5bb/bedrock/base/tests/test_urlresolvers.py

        • REVIEW

          • RATIONALISE
          • JUSTIFY
          • SPECIAL THINGS
            • watch for changes to get_locale(request) to take out the param. REVERT so it always has it.
        • Rebase on main

        • Do we need to track all locale redirects with Markus? if so, add to the fixup middleware

        • remove the redirect from l10n_render unless it breaks a zillion tests

        • Rebase on main

  • Add a test to protect against redirect() usage in *urls.py ?

  • Do we need to add tests for redirects following the appropriate locale?

  • ensure correct behaviour for locale redirects from LOCALE_VARIANTS

    • zh-Hant-TW ends up on zh-CN which is WRONG
  • Rebase on main before QA

  • QA

    • Unit tests all passing

      • Redirect tests all passing
    • Integration tests all passing

    • Confirm languages available with DEV=True and DEV=False

    • Confirm we still retain the behaviour where the URL lang code wins over the Accept-language header

    • DB update works

    • Test with DEV=True and DEV=False

    • Ensure three-letter languages like "dsp" are working

    • Test drive locally

      • Mozorg mode
        • Check special 404 Locale picker page works
        • check all sensitive lang-based redirects work as on mozorg
        • Check newsletters are ok
          • Check locale switching in NewsletterForm
          • check updating prefs in a non-en-US local works ok
        • Check VPN signup is ok
        • Check Contentful-sourced pages are OK
        • Check WP-blog-sourced pages are OK
        • Check FF download is ok
          • ensure the right lang is reflected in the page and the URL
        • Check sitemap gen is happy
        • Test Contentful Preview still works
    • Pocket mode

      • Test all product marketing pages
      • check/compare locale fixups - eg es-es
        • if we end up with mixed-case paths, check if the proxy will break
        • zh-cn / zh-tw lang codes
    • Test drive on a demo instance

      • Mozorg mode
      • Pocket mode
    • Compare with switch to Django's i18n sumo#1469 (comment)

  • test 404s with DEBUG=False

  • PR notes

    • settings.LANGUAGE_URL_MAP has changed to have mixed-case keys
    • [ ]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Server stuff yo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant