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

feat: atlas pull for XBlock translations | FC-0012 #33698

Merged
merged 2 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down
19 changes: 14 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @feanil for the quick triage.

Conflicts has been resolved and I've tested it again and it seems to be playing well.

I've changed this line to match the Tutor option to have one less surprise for operators.

i18n_tool generate
paver i18n_compilejs
endif
paver i18n_compilejs


detect_changed_source_translations: ## check if translation files are up-to-date
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
Original file line number Diff line number Diff line change
@@ -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)
70 changes: 70 additions & 0 deletions common/djangoapps/xblock_django/tests/test_commands.py
Original file line number Diff line number Diff line change
@@ -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'
26 changes: 26 additions & 0 deletions common/djangoapps/xblock_django/tests/test_translation.py
Original file line number Diff line number Diff line change
@@ -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()
156 changes: 156 additions & 0 deletions common/djangoapps/xblock_django/translation.py
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading