Merge pull request #33698 from Zeit-Labs/i18n-service-oep58

feat: `atlas pull` for XBlock translations | FC-0012

.gitignore

@@ -61,6 +61,8 @@ conf/locale/fake*/LC_MESSAGES/*.po
 conf/locale/fake*/LC_MESSAGES/*.mo
 # this was a mistake in i18n_tools, now fixed.
 conf/locale/messages.mo
+conf/plugins-locale/
+/*/static/js/xblock.v1-i18n/
 
 ### Testing artifacts
 .testids/

Makefile

@@ -4,7 +4,8 @@
   docker_auth docker_build docker_tag_build_push_lms docker_tag_build_push_lms_dev \
   docker_tag_build_push_cms docker_tag_build_push_cms_dev docs extract_translations \
   guides help lint-imports local-requirements migrate migrate-lms migrate-cms \
-  pre-requirements pull pull_translations push_translations requirements shell swagger \
+  pre-requirements pull pull_xblock_translations pull_translations push_translations \
+  requirements shell swagger \
   technical-docs test-requirements ubuntu-requirements upgrade-package upgrade
 
 # Careful with mktemp syntax: it has to work on Mac and Ubuntu, which have differences.
@@ -55,7 +56,15 @@ endif
 push_translations: ## push source strings to Transifex for translation
 	i18n_tool transifex push
 
-pull_translations:  ## pull translations from Transifex
+pull_xblock_translations:  ## pull xblock translations via atlas
+	rm -rf conf/plugins-locale  # Clean up existing atlas translations
+	rm -rf lms/static/i18n/xblock.v1 cms/static/i18n/xblock.v1  # Clean up existing xblock compiled translations
+	mkdir -p conf/plugins-locale/xblock.v1/ lms/static/js/xblock.v1-i18n cms/static/js
+	python manage.py lms pull_xblock_translations --verbose $(ATLAS_OPTIONS)
+	python manage.py lms compile_xblock_translations
+	cp -r lms/static/js/xblock.v1-i18n cms/static/js
+
+pull_translations: ## pull translations from Transifex
 	git clean -fdX conf/locale
 ifeq ($(OPENEDX_ATLAS_PULL),)
 	i18n_tool transifex pull
@@ -65,13 +74,13 @@ ifeq ($(OPENEDX_ATLAS_PULL),)
 	git clean -fdX conf/locale/rtl
 	git clean -fdX conf/locale/eo
 	i18n_tool validate --verbose
-	paver i18n_compilejs
 else
+	make pull_xblock_translations
 	find conf/locale -mindepth 1 -maxdepth 1 -type d -exec rm -r {} \;
-	atlas pull $(OPENEDX_ATLAS_ARGS) translations/edx-platform/conf/locale:conf/locale
+	atlas pull $(ATLAS_OPTIONS) translations/edx-platform/conf/locale:conf/locale
 	i18n_tool generate
-	paver i18n_compilejs
 endif
+	paver i18n_compilejs
 
 
 detect_changed_source_translations: ## check if translation files are up-to-date

common/djangoapps/xblock_django/management/commands/compile_xblock_translations.py

@@ -0,0 +1,22 @@
+"""
+Compile the translation files for the XBlocks.
+"""
+
+from django.core.management.base import BaseCommand
+
+from xmodule.modulestore import api as xmodule_api
+
+from openedx.core.djangoapps.plugins.i18n_api import compile_po_files
+
+from ...translation import (
+    compile_xblock_js_messages,
+)
+
+
+class Command(BaseCommand):
+    """
+    Compile the translation files for the XBlocks.
+    """
+    def handle(self, *args, **options):
+        compile_po_files(xmodule_api.get_python_locale_root())
+        compile_xblock_js_messages()

common/djangoapps/xblock_django/management/commands/pull_xblock_translations.py

@@ -0,0 +1,51 @@
+"""
+Download the translations via atlas for the XBlocks.
+"""
+
+from django.core.management.base import BaseCommand, CommandError
+
+from openedx.core.djangoapps.plugins.i18n_api import ATLAS_ARGUMENTS
+from xmodule.modulestore import api as xmodule_api
+
+from ...translation import xblocks_atlas_pull
+
+
+class Command(BaseCommand):
+    """
+    Pull the XBlock translations via atlas for the XBlocks.
+
+    For detailed information about atlas pull options check the atlas documentation:
+
+     - https://github.com/openedx/openedx-atlas
+    """
+
+    def add_arguments(self, parser):
+        for argument in ATLAS_ARGUMENTS:
+            parser.add_argument(*argument.get_args(), **argument.get_kwargs())
+
+        parser.add_argument(
+            '--verbose|-v',
+            action='store_true',
+            default=False,
+            dest='verbose',
+            help='Verbose output using `--verbose` argument for `atlas pull`.',
+        )
+
+    def handle(self, *args, **options):
+        xblock_translations_root = xmodule_api.get_python_locale_root()
+        if list(xblock_translations_root.listdir()):
+            raise CommandError(f'"{xblock_translations_root}" should be empty before running atlas pull.')
+
+        atlas_pull_options = []
+
+        for argument in ATLAS_ARGUMENTS:
+            option_value = options.get(argument.dest)
+            if option_value is not None:
+                atlas_pull_options += [argument.flag, option_value]
+
+        if options['verbose']:
+            atlas_pull_options += ['--verbose']
+        else:
+            atlas_pull_options += ['--silent']
+
+        xblocks_atlas_pull(pull_options=atlas_pull_options)

common/djangoapps/xblock_django/tests/test_commands.py

@@ -0,0 +1,70 @@
+"""
+Tests for the pull_xblock_translations management command.
+"""
+
+from path import Path
+from unittest.mock import patch
+
+from django.core.management import call_command
+
+from done import DoneXBlock
+
+from xmodule.modulestore.api import (
+    get_python_locale_root,
+    get_javascript_i18n_file_path,
+)
+from xmodule.modulestore.tests.conftest import tmp_translations_dir
+
+
+def test_pull_xblock_translations(tmp_path):
+    """
+    Test the compile_xblock_translations management command.
+    """
+    temp_xblock_locale_path = Path(str(tmp_path))
+
+    with patch('common.djangoapps.xblock_django.translation.get_non_xmodule_xblocks') as mock_get_non_xmodule_xblocks:
+        with patch('xmodule.modulestore.api.get_python_locale_root') as mock_get_python_locale_root:
+            with patch('subprocess.run') as mock_run:
+                mock_get_python_locale_root.return_value = Path(str(temp_xblock_locale_path))
+                mock_get_non_xmodule_xblocks.return_value = [('done', DoneXBlock)]
+
+                call_command(
+                    'pull_xblock_translations',
+                    filter='ar,de_DE,jp',
+                    repository='openedx/custom-translations',
+                    branch='release/redwood',
+                )
+
+                assert mock_run.call_count == 1, 'Calls `subprocess.run`'
+                assert mock_run.call_args.kwargs['args'] == [
+                    'atlas', 'pull',
+                    '--expand-glob',
+                    '--filter', 'ar,de_DE,jp',
+                    '--repository', 'openedx/custom-translations',
+                    '--branch', 'release/redwood',
+                    '--silent',
+                    'translations/*/done/conf/locale:done',
+                ]
+
+
+def test_compile_xblock_translations(tmp_translations_dir):
+    """
+    Test the compile_xblock_translations management command.
+    """
+    # msgfmt isn't available in test environment, so we mock the `subprocess.run` and copy the django.mo file,
+    # it to ensure `compile_xblock_js_messages` can work.
+    with tmp_translations_dir(xblocks=[('done', DoneXBlock)], fixtures_to_copy=['django.po', 'django.mo']):
+        with patch.object(DoneXBlock, 'i18n_js_namespace', 'TestingDoneXBlockI18n'):
+            po_file = get_python_locale_root() / 'done/tr/LC_MESSAGES/django.po'
+
+            with patch('subprocess.run') as mock_run:
+                call_command('compile_xblock_translations')
+                assert mock_run.call_count == 1, 'Calls `subprocess.run`'
+                assert mock_run.call_args.kwargs['args'] == [
+                    'msgfmt', '--check-format', '-o', str(po_file.with_suffix('.mo')), str(po_file),
+                ], 'Compiles the .po files'
+
+            js_file_text = get_javascript_i18n_file_path('done', 'tr').text()
+            assert 'Merhaba' in js_file_text, 'Ensures the JavaScript catalog is compiled'
+            assert 'TestingDoneXBlockI18n' in js_file_text, 'Ensures the namespace is used'
+            assert 'gettext' in js_file_text, 'Ensures the gettext function is defined'

common/djangoapps/xblock_django/tests/test_translation.py

@@ -0,0 +1,26 @@
+"""
+Tests for the xblock_django.translation module.
+"""
+
+from done import DoneXBlock
+
+from ..translation import (
+    get_non_xmodule_xblock_module_names,
+    get_non_xmodule_xblocks,
+)
+
+
+def test_get_non_xmodule_xblock_module_names():
+    """
+    Ensure xmodule isn't returned but other default xblocks are.
+    """
+    assert 'xmodule' not in get_non_xmodule_xblock_module_names()
+    assert 'done' in get_non_xmodule_xblock_module_names()
+    assert 'lti_consumer' in get_non_xmodule_xblock_module_names()
+
+
+def test_get_non_xmodule_xblocks():
+    """
+    Ensures that default XBlocks are included.
+    """
+    assert ('done', DoneXBlock) in get_non_xmodule_xblocks()

common/djangoapps/xblock_django/translation.py

@@ -0,0 +1,156 @@
+"""
+XBlock translations pulling and compilation logic.
+"""
+
+import os
+import gettext
+
+from django.utils.encoding import force_str
+from django.views.i18n import JavaScriptCatalog
+from django.utils.translation import override, to_locale, get_language
+from statici18n.management.commands.compilejsi18n import Command as CompileI18NJSCommand
+from xblock.core import XBlock
+
+from openedx.core.djangoapps.plugins.i18n_api import atlas_pull_by_modules
+from xmodule.modulestore import api as xmodule_api
+
+
+class AtlasJavaScriptCatalog(JavaScriptCatalog):
+    """
+    View to return the selected language catalog as a JavaScript library.
+
+    This extends the JavaScriptCatalog class to allow custom domain and locale_dir.
+    """
+
+    translation = None
+
+    def get(self, request, *args, **kwargs):
+        """
+        Return the selected language catalog as a JavaScript library.
+
+        This overrides the JavaScriptCatalog.get() method class to allow custom locale_dir.
+        """
+        selected_language = get_language()
+        locale = to_locale(selected_language)
+        domain = kwargs['domain']
+        locale_dir = kwargs['locale_dir']
+        # Using GNUTranslations instead of DjangoTranslation to allow custom locale_dir without needing
+        # to use a custom `text.mo` translation domain.
+        self.translation = gettext.translation(domain, localedir=locale_dir, languages=[locale])
+        context = self.get_context_data(**kwargs)
+        return self.render_to_response(context)
+
+    @classmethod
+    def simulate_get_request(cls, locale, domain, locale_dir):
+        """
+        Simulate a GET request to the JavaScriptCatalog view.
+
+        Return:
+            str: The rendered JavaScript catalog.
+        """
+        with override(locale):
+            catalog_view = cls()
+            response = catalog_view.get(
+                request=None,  # we are passing None as the request, as the request
+                               # object is currently not used by django
+                domain=domain,
+                locale_dir=locale_dir,
+            )
+            return force_str(response.content)
+
+
+def mo_file_to_js_namespaced_catalog(xblock_conf_locale_dir, locale, domain, namespace):
+    """
+    Compile .mo to .js gettext catalog and wrap it in a namespace via the `compilejsi18n` command helpers.
+    """
+    rendered_js = AtlasJavaScriptCatalog.simulate_get_request(
+        locale=locale,
+        locale_dir=xblock_conf_locale_dir,
+        domain=domain,
+    )
+
+    # The `django-statici18n` package has a non-standard code license, therefore we're using its private API
+    # to avoid copying the code into this repository and running into licensing issues.
+    compile_i18n_js_command = CompileI18NJSCommand()
+    namespaced_catalog_js_code = compile_i18n_js_command._get_namespaced_catalog(  # pylint: disable=protected-access
+        rendered_js=rendered_js,
+        namespace=namespace,
+    )
+
+    return namespaced_catalog_js_code
+
+
+def xblocks_atlas_pull(pull_options):
+    """
+    Atlas pull the translations for the XBlocks that are installed.
+    """
+    xblock_module_names = get_non_xmodule_xblock_module_names()
+
+    atlas_pull_by_modules(
+        module_names=xblock_module_names,
+        locale_root=xmodule_api.get_python_locale_root(),
+        pull_options=pull_options,
+    )
+
+
+def compile_xblock_js_messages():
+    """
+    Compile the XBlock JavaScript messages from .mo file into .js files.
+    """
+    for xblock_module, xblock_class in get_non_xmodule_xblocks():
+        xblock_conf_locale_dir = xmodule_api.get_python_locale_root() / xblock_module
+        i18n_js_namespace = xblock_class.get_i18n_js_namespace()
+
+        for locale_dir in xblock_conf_locale_dir.listdir():
+            locale_code = str(locale_dir.basename())
+            locale_messages_dir = locale_dir / 'LC_MESSAGES'
+            js_translations_domain = None
+            for domain in ['djangojs', 'django']:
+                po_file_path = locale_messages_dir / f'{domain}.mo'
+                if po_file_path.exists():
+                    if not js_translations_domain:
+                        # Select which file to compile to `django.js`, while preferring `djangojs` over `django`
+                        js_translations_domain = domain
+
+            if js_translations_domain and i18n_js_namespace:
+                js_i18n_file_path = xmodule_api.get_javascript_i18n_file_path(xblock_module, locale_code)
+                os.makedirs(js_i18n_file_path.dirname(), exist_ok=True)
+                js_namespaced_catalog = mo_file_to_js_namespaced_catalog(
+                    xblock_conf_locale_dir=xblock_conf_locale_dir,
+                    locale=locale_code,
+                    domain=js_translations_domain,
+                    namespace=i18n_js_namespace,
+                )
+
+                with open(js_i18n_file_path, 'w', encoding='utf-8') as f:
+                    f.write(js_namespaced_catalog)
+
+
+def get_non_xmodule_xblocks():
+    """
+    Returns a list of XBlock classes with their module name excluding edx-platform/xmodule xblocks.
+    """
+    xblock_classes = []
+    for _xblock_tag, xblock_class in XBlock.load_classes():
+        xblock_module_name = xmodule_api.get_root_module_name(xblock_class)
+        if xblock_module_name != 'xmodule':
+            # XBlocks in edx-platform/xmodule are already translated in edx-platform/conf/locale
+            # So there is no need to add special handling for them.
+            xblock_classes.append(
+                (xblock_module_name, xblock_class),
+            )
+
+    return xblock_classes
+
+
+def get_non_xmodule_xblock_module_names():
+    """
+    Returns a list of module names for the plugins that supports translations excluding `xmodule`.
+    """
+    xblock_module_names = set(
+        xblock_module_name
+        for xblock_module_name, _xblock_class in get_non_xmodule_xblocks()
+    )
+
+    sorted_xblock_module_names = list(sorted(xblock_module_names))
+    return sorted_xblock_module_names