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: add i18n_js_namespace to support atlas - FC-0012 #452

Closed
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
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Please See the `releases tab <https://github.com/openedx/xblock-lti-consumer/rel

Unreleased
~~~~~~~~~~
9.9.0 (2024-01-24)
---------------------------
* XBlockI18NService js translations support

9.8.3 - 2024-01-23
------------------
* Additional NewRelic traces to functions suspected of causing performance issues.
Expand Down
2 changes: 1 addition & 1 deletion lti_consumer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
from .apps import LTIConsumerApp
from .lti_xblock import LtiConsumerXBlock

__version__ = '9.8.3'
__version__ = '9.9.0'
33 changes: 30 additions & 3 deletions lti_consumer/lti_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
external_user_id_1p1_launches_enabled,
database_config_enabled,
EXTERNAL_ID_REGEX,
DummyTranslationService,
)

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -257,6 +258,7 @@
"""

block_settings_key = 'lti_consumer'
i18n_js_namespace = 'XBlockLtiConsumerI18N'

display_name = String(
display_name=_("Display Name"),
Expand Down Expand Up @@ -663,10 +665,13 @@
return scenarios

@staticmethod
def _get_statici18n_js_url(loader): # pragma: no cover
def _get_deprecated_i18n_js_url(loader):
"""
Returns the Javascript translation file for the currently selected language, if any found by
Returns the deprecated JavaScript translation file for the currently selected language, if any found by
`pkg_resources`

This method returns pre-OEP-58 i18n files and is deprecated in favor
of `get_javascript_i18n_catalog_url`.
"""
lang_code = translation.get_language()
if not lang_code:
Expand All @@ -678,6 +683,19 @@
return text_js.format(lang_code=code)
return None

def _get_statici18n_js_url(self, loader):
"""
Return the JavaScript translation file provided by the XBlockI18NService.
"""
if url_getter_func := getattr(self.i18n_service, 'get_javascript_i18n_catalog_url', None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if url_getter_func := getattr(self.i18n_service, 'get_javascript_i18n_catalog_url', None):
if i18n_service := self.runtime.service(self, "i18n"):
if url_getter_func := getattr(i18n_service, 'get_javascript_i18n_catalog_url', None):

No need for the DummyTranslationService imho. Keeping the code simpler is better.

if javascript_url := url_getter_func(self):
return javascript_url

if deprecated_url := self._get_deprecated_i18n_js_url(loader):
return self.runtime.local_resource_url(self, deprecated_url)

return None

Check warning on line 697 in lti_consumer/lti_xblock.py

View check run for this annotation

Codecov / codecov/patch

lti_consumer/lti_xblock.py#L697

Added line #L697 was not covered by tests

def validate_field_data(self, validation, data):
# Validate custom parameters is a list.
if not isinstance(data.custom_parameters, list):
Expand Down Expand Up @@ -776,6 +794,15 @@
# because the service is not defined in those contexts.
return True

@property
def i18n_service(self):
""" Obtains translation service """
i18n_service = self.runtime.service(self, "i18n")
if i18n_service:
return i18n_service
else:
return DummyTranslationService()
Comment on lines +797 to +804
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this change. I don't think it's needed.


@property
def editable_fields(self):
"""
Expand Down Expand Up @@ -1226,7 +1253,7 @@
fragment.add_javascript(loader.load_unicode('static/js/xblock_lti_consumer.js'))
statici18n_js_url = self._get_statici18n_js_url(loader)
if statici18n_js_url:
fragment.add_javascript_url(self.runtime.local_resource_url(self, statici18n_js_url))
fragment.add_javascript_url(statici18n_js_url)
fragment.initialize_js('LtiConsumerXBlock')
return fragment

Expand Down
16 changes: 16 additions & 0 deletions lti_consumer/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@
return text


def ngettext_fallback(text_singular, text_plural, number):
""" Dummy `ngettext` replacement to make string extraction tools scrape strings marked for translation """
if number == 1:
return text_singular

Check warning on line 37 in lti_consumer/utils.py

View check run for this annotation

Codecov / codecov/patch

lti_consumer/utils.py#L36-L37

Added lines #L36 - L37 were not covered by tests
else:
return text_plural

Check warning on line 39 in lti_consumer/utils.py

View check run for this annotation

Codecov / codecov/patch

lti_consumer/utils.py#L39

Added line #L39 was not covered by tests


def get_lti_api_base():
"""
Returns base url to be used as issuer on OAuth2 flows
Expand Down Expand Up @@ -361,3 +369,11 @@
return object_fields
except (AttributeError, TypeError):
return {}


class DummyTranslationService:
"""
Dummy drop-in replacement for i18n XBlock service
"""
gettext = _
ngettext = ngettext_fallback
Loading