feat: atlas pull for XBlock translations

xmodule/modulestore/api.py

@@ -0,0 +1,46 @@
+"""
+Python APIs for the xmodule.modulestore module.
+"""
+
+from django.conf import settings
+
+
+def get_root_module_name(class_or_function):
+    """
+    Return the root module name for the given class or function.
+    """
+    module_path = class_or_function.__module__
+    return module_path.split('.')[0]
+
+
+def get_xblock_root_module_name(block):
+    """
+    Return the XBlock Python module name.
+    """
+    # `xblock.unmixed_class` is a property added by the XBlock library to add mixins to the class which conceals
+    # the original class properties.
+    xblock_original_class = getattr(block, 'unmixed_class', block.__class__)
+    return get_root_module_name(xblock_original_class)
+
+
+def get_python_locale_root():
+    """
+    Return the XBlock locale root directory for OEP-58 translations.
+    """
+    return settings.REPO_ROOT / 'conf/plugins-locale/xblock.v1'
+
+
+def get_javascript_i18n_file_name(xblock_module, locale):
+    """
+    Return the relative path to the JavaScript i18n file.
+
+    Relative to the /static/ directory.
+    """
+    return f'js/xblock.v1-i18n/{xblock_module}/{locale}.js'
+
+
+def get_javascript_i18n_file_path(xblock_module, locale):
+    """
+    Return the absolute path to the JavaScript i18n file.
+    """
+    return settings.STATICI18N_ROOT / get_javascript_i18n_file_name(xblock_module, locale)

xmodule/modulestore/django.py

@@ -19,6 +19,7 @@ from django.conf import settings
 if not settings.configured:
     settings.configure()
 
+from django.contrib.staticfiles.storage import staticfiles_storage  # lint-amnesty, pylint: disable=wrong-import-position
 from django.core.cache import caches, InvalidCacheBackendError  # lint-amnesty, pylint: disable=wrong-import-position
 import django.dispatch  # lint-amnesty, pylint: disable=wrong-import-position
 import django.utils  # lint-amnesty, pylint: disable=wrong-import-position
@@ -30,6 +31,13 @@ from xmodule.modulestore.draft_and_published import BranchSettingMixin  # lint-a
 from xmodule.modulestore.mixed import MixedModuleStore  # lint-amnesty, pylint: disable=wrong-import-position
 from xmodule.util.xmodule_django import get_current_request_hostname  # lint-amnesty, pylint: disable=wrong-import-position
 
+from .api import (  # lint-amnesty, pylint: disable=wrong-import-position
+    get_javascript_i18n_file_name,
+    get_javascript_i18n_file_path,
+    get_python_locale_root,
+    get_xblock_root_module_name,
+)
+
 # We also may not always have the current request user (crum) module available
 try:
     from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
@@ -365,12 +373,13 @@ class XBlockI18nService:
     has ugettext, ungettext, etc), so we can use it directly as the runtime
     i18n service.
 
+    This service supports OEP-58 translations (https://docs.openedx.org/en/latest/developers/concepts/oep58.html)
+    that are pulled via atlas.
     """
     def __init__(self, block=None):
         """
-        Attempt to load an XBlock-specific GNU gettext translator using the XBlock's own domain
-        translation catalog, currently expected to be found at:
-            <xblock_root>/conf/locale/<language>/LC_MESSAGES/<domain>.po|mo
+        Attempt to load an XBlock-specific GNU gettext translation using the XBlock's own domain
+        translation catalog.
         If we can't locate the domain translation catalog then we fall-back onto
         django.utils.translation, which will point to the system's own domain translation catalog
         This effectively achieves translations by coincidence for an XBlock which does not provide
@@ -378,21 +387,60 @@ class XBlockI18nService:
         """
         self.translator = django.utils.translation
         if block:
-            xblock_class = getattr(block, 'unmixed_class', block.__class__)
-            xblock_resource = xblock_class.__module__
-            xblock_locale_dir = 'translations'
-            xblock_locale_path = resource_filename(xblock_resource, xblock_locale_dir)
-            xblock_domain = 'text'
+            xblock_locale_domain, xblock_locale_dir = self.get_python_locale(block)
             selected_language = get_language()
-            try:
-                self.translator = gettext.translation(
-                    xblock_domain,
-                    xblock_locale_path,
-                    [to_locale(selected_language if selected_language else settings.LANGUAGE_CODE)]
-                )
-            except OSError:
-                # Fall back to the default Django translator if the XBlock translator is not found.
-                pass
+
+            if xblock_locale_dir:
+                try:
+                    self.translator = gettext.translation(
+                        xblock_locale_domain,
+                        xblock_locale_dir,
+                        [to_locale(selected_language if selected_language else settings.LANGUAGE_CODE)]
+                    )
+                except OSError:
+                    # Fall back to the default Django translator if the XBlock translator is not found.
+                    pass
+
+    def get_python_locale(self, block):
+        """
+        Return the XBlock locale directory with the domain name.
+
+        Return:
+            (domain, locale_path): A tuple of the domain name and the XBlock locale directory.
+
+        This method looks for translations in two locations:
+            - First it looks for `atlas` translations in get_python_locale_root().
+            - Alternatively, it looks for bundled translations in the XBlock pip package which are
+              found at <python_environment_xblock_root>/conf/locale/<language>/LC_MESSAGES/<domain>.po|mo
+        """
+        xblock_module_name = get_xblock_root_module_name(block)
+        xblock_locale_path = get_python_locale_root() / xblock_module_name
+
+        # OEP-58 translations are pulled via atlas and takes precedence if exists.
+        if xblock_locale_path.isdir():
+            # The `django` domain is used for XBlocks consistent with the other repositories.
+            return 'django', xblock_locale_path
+
+        # Pre-OEP-58 translations within the XBlock pip packages are deprecated but supported.
+        deprecated_xblock_locale_path = resource_filename(xblock_module_name, 'translations')
+        # The `text` domain was used for XBlocks pre-OEP-58.
+        return 'text', deprecated_xblock_locale_path
+
+    def get_javascript_i18n_catalog_url(self, block):
+        """
+        Return the XBlock compiled JavaScript translations catalog static url.
+
+        Return:
+            str: The static url to the JavaScript translations catalog, otherwise None.
+        """
+        xblock_module_name = get_xblock_root_module_name(block)
+        language_name = get_language()  # Returns language name e.g. `de` or `de-de`.
+        locale = to_locale(language_name)  # Use the `de` or `de_DE` format for the locale directory.
+
+        if get_javascript_i18n_file_path(xblock_module_name, locale).exists():
+            relative_file_path = get_javascript_i18n_file_name(xblock_module_name, locale)
+            return staticfiles_storage.url(relative_file_path)
+        return None
 
     def __getattr__(self, name):
         name = 'gettext' if name == 'ugettext' else name

xmodule/modulestore/tests/conftest.py

@@ -0,0 +1,107 @@
+"""
+Test fixture for the `xmodule.modulestore` module.
+"""
+
+import shutil
+from contextlib import contextmanager
+from unittest.mock import Mock, patch
+
+import pytest
+from path import Path
+
+from django.test.utils import override_settings
+
+from xmodule.modulestore.api import get_python_locale_root
+
+
+@pytest.fixture
+def tmp_translations_dir(tmp_path, settings):
+    """
+    Pytest fixture to create a temporary directory for translations.
+
+    Returns:
+        (function): Context manager to be used with the `with tmp_translations_dir(...):` statement.
+    """
+
+    @contextmanager
+    def _tmp_translations_dir(xblocks, fixtures_to_copy=None):
+        """
+        Context manager to create temporary directory for translations.
+
+        Args:
+            xblocks: A list of tuples of (module_name, xblock_class) to patch `get_non_xmodule_xblocks` for consistent
+                     test runs.
+
+            fixtures_to_copy: A list of `modulestore/tests/fixtures` file names to copy to the XBlocks directory.
+
+        Yields:
+            Path: The temporary edx-platform directory path.
+
+        The temp directory will have the following structure:
+
+        edx-platform/
+          ├── conf
+          │ └── plugins-locale
+          │     └── xblock.v1
+          │         └── done
+          │             └── tr
+          │                 └── LC_MESSAGES
+          │                     └── django.po
+          └── lms
+              └── static
+        """
+        # tmp_path represents settings.REPO_ROOT
+        # Converting to `path.path()` to be compatible with the `settings.REPO_ROOT` type.
+        original_repo_root = settings.REPO_ROOT
+        repo_root = Path(str(tmp_path / 'edx-platform'))
+
+        project_dir_name = settings.PROJECT_ROOT.basename()  # lms or cms
+        static_i18n_root = repo_root / f'{project_dir_name}/static'
+
+        with override_settings(REPO_ROOT=repo_root, STATICI18N_ROOT=static_i18n_root):
+            gettext_fixtures = original_repo_root / 'xmodule/modulestore/tests/fixtures'
+
+            python_root = get_python_locale_root()
+            python_root.makedirs_p()
+            static_i18n_root.makedirs_p()
+
+            if fixtures_to_copy:
+                for module_name, _xblock in xblocks:
+                    for fixture in fixtures_to_copy:
+                        dest_dir = python_root / module_name / 'tr/LC_MESSAGES'
+                        dest_dir.makedirs_p()
+                        shutil.copyfile(gettext_fixtures / fixture, dest_dir / fixture)
+
+            with patch('common.djangoapps.xblock_django.translation.get_non_xmodule_xblocks', return_value=xblocks):
+                yield repo_root
+
+    return _tmp_translations_dir
+
+
+def create_mock_xblock(module_name):
+    """
+    Create a mocked XBlock with the given module name.
+    """
+    block = Mock()
+    block.unmixed_class.__module__ = module_name
+    return block
+
+
+@pytest.fixture
+def mock_modern_xblock(tmp_translations_dir):
+    """
+    Mocks a successful `atlas pull` for `my_modern_xblock` xblock.
+
+    Yields:
+        dict: A dictionary of mocked XBlocks:
+            - modern_xblock: A mocked XBlock atlas translations.
+            - legacy_xblock: A mocked XBlock without atlas translations.
+    """
+    with tmp_translations_dir(
+        xblocks=[('my_modern_xblock', Mock())],
+        fixtures_to_copy=['django.po', 'django.mo'],
+    ):
+        yield {
+            'legacy_xblock': create_mock_xblock('my_legacy_xblock'),
+            'modern_xblock': create_mock_xblock('my_modern_xblock'),
+        }

xmodule/modulestore/tests/fixtures/django.po

@@ -0,0 +1,22 @@
+# Minimal po file for testing.
+# Compile to mo file:
+#
+#  $ msgfmt -o django.mo django.po
+#
+
+#, fuzzy
+msgid ""
+msgstr ""
+"Project-Id-Version: 0.1\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2023-12-20 23:07+0300\n"
+"PO-Revision-Date: 2023-12-20 23:07+0300\n"
+"Last-Translator: Omar <omar@somewhere>\n"
+"Language-Team: Tr <omar@somewhere>\n"
+"Language: tr\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=utf-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+
+msgid "Hello"
+msgstr "Merhaba"

xmodule/modulestore/tests/test_api.py

@@ -0,0 +1,75 @@
+"""
+Tests for the modulestore and XBlock python APIs.
+"""
+from unittest.mock import Mock
+
+from django.conf import settings
+
+from lti_consumer.lti_xblock import LtiConsumerXBlock
+from done import DoneXBlock
+from xblock.field_data import DictFieldData
+
+from xblock.test.tools import TestRuntime
+from xblock.test.test_runtime import TestSimpleMixin
+from xmodule.video_block import VideoBlock
+from xmodule.modulestore.api import (
+    get_javascript_i18n_file_name,
+    get_javascript_i18n_file_path,
+    get_python_locale_root,
+    get_root_module_name,
+    get_xblock_root_module_name,
+)
+
+
+def test_get_root_module_name():
+    """
+    Ensure the module name function works with different xblocks.
+    """
+    assert get_root_module_name(LtiConsumerXBlock) == 'lti_consumer'
+    assert get_root_module_name(VideoBlock) == 'xmodule'
+    assert get_root_module_name(DoneXBlock) == 'done'
+
+
+def test_get_xblock_root_module_name():
+    """
+    Ensure the get_root_module_name works with mixed XBlocks.
+
+    The XBlock uses a little-known Mixologist class which changes the final
+    XBlock object class. See the XBlock.construct_xblock_from_class method
+    for more information about this behavior.
+    """
+    field_data = DictFieldData({
+        'field_a': 5,
+        'field_x': [1, 2, 3],
+    })
+    runtime = TestRuntime(Mock(), mixins=[TestSimpleMixin], services={'field-data': field_data})
+
+    mixed_done_xblock = runtime.construct_xblock_from_class(DoneXBlock, Mock())
+
+    assert mixed_done_xblock.__module__ == 'xblock.internal'  # Mixed classes has a runtime generated module name.
+    assert mixed_done_xblock.unmixed_class == DoneXBlock, 'The unmixed_class property retains the original property.'
+
+    assert get_xblock_root_module_name(mixed_done_xblock) == 'done'
+
+
+def test_file_paths_api():
+    """
+    Test the `get_python_locale_root` returned path.
+    """
+    root = get_python_locale_root()
+    assert root.endswith('edx-platform/conf/plugins-locale/xblock.v1'), 'Needs to match Makefile and other code'
+
+
+def test_get_javascript_i18n_file_name():
+    """
+    Test get_javascript_i18n_file_name relative path to `/static` URL.
+    """
+    assert get_javascript_i18n_file_name('lti_consumer', 'ar') == 'js/xblock.v1-i18n/lti_consumer/ar.js'
+
+
+def test_get_javascript_i18n_file_path():
+    """
+    Test get_javascript_i18n_file_path absolute file path.
+    """
+    path = str(get_javascript_i18n_file_path('done', 'eo'))
+    assert path.endswith(f'{settings.PROJECT_ROOT}/static/js/xblock.v1-i18n/done/eo.js')

xmodule/modulestore/tests/test_django_utils.py

@@ -0,0 +1,54 @@
+"""
+Tests for the modulestore.django module
+"""
+
+from unittest.mock import patch
+
+import django.utils.translation
+
+from xmodule.modulestore.django import XBlockI18nService
+
+
+def test_get_python_locale_with_atlas_oep58_translations(mock_modern_xblock):
+    """
+    Test that the XBlockI18nService.get_python_locale() method finds the atlas locale if it exists.
+
+    More on OEP-58 and atlas pull: https://docs.openedx.org/en/latest/developers/concepts/oep58.html.
+    """
+    i18n_service = XBlockI18nService()
+    block = mock_modern_xblock['modern_xblock']
+    domain, locale_path = i18n_service.get_python_locale(block)
+
+    assert locale_path.endswith('conf/plugins-locale/xblock.v1/my_modern_xblock'), 'Uses atlas locale if found.'
+    assert domain == 'django', 'Uses django domain when atlas locale is found.'
+
+
+@patch('xmodule.modulestore.django.resource_filename', return_value='/lib/my_legacy_xblock/translations')
+def test_get_python_locale_with_bundled_translations(mock_modern_xblock):
+    """
+    Ensure that get_python_locale() falls back to XBlock internal translations if atlas translations weren't pulled.
+
+    Pre-OEP-58 translations were stored in the `translations` directory of the XBlock which is
+    accessible via the `pkg_resources.resource_filename` function.
+    """
+    i18n_service = XBlockI18nService()
+    block = mock_modern_xblock['legacy_xblock']
+    domain, path = i18n_service.get_python_locale(block)
+
+    assert path == '/lib/my_legacy_xblock/translations', 'Backward compatible with pe-OEP-58.'
+    assert domain == 'text', 'Use the legacy `text` domain for backward compatibility with old XBlocks.'
+
+
+def test_i18n_service_translator_with_modern_xblock(mock_modern_xblock):
+    """
+    Ensure the XBlockI18nService uses the atlas translations if found.
+    """
+    block = mock_modern_xblock['modern_xblock']
+
+    with django.utils.translation.override('fr'):
+        i18n_service = XBlockI18nService(block)
+        assert i18n_service.translator is django.utils.translation, 'French is not pulled by `mock_modern_xblock`.'
+
+    with django.utils.translation.override('tr'):
+        i18n_service = XBlockI18nService(block)
+        assert i18n_service.translator is not django.utils.translation, 'Turkish is pulled by `mock_modern_xblock`.'