feat: add i18n_js_namespace to support atlas - FC-0012 #452
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @shadinaif! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
|
please review @OmarIthawi |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #452 +/- ##
==========================================
- Coverage 97.85% 97.77% -0.08%
==========================================
Files 77 77
Lines 6705 6736 +31
==========================================
+ Hits 6561 6586 +25
- Misses 144 150 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks @shadinaif!!
Please remove non-essential mocking code. That should make the PR easier to review.
| @property | ||
| def i18n_service(self): | ||
| """ Obtains translation service """ | ||
| i18n_service = self.runtime.service(self, "i18n") | ||
| if i18n_service: | ||
| return i18n_service | ||
| else: | ||
| return DummyTranslationService() |
There was a problem hiding this comment.
Please revert this change. I don't think it's needed.
| """ | ||
| Return the JavaScript translation file provided by the XBlockI18NService. | ||
| """ | ||
| if url_getter_func := getattr(self.i18n_service, 'get_javascript_i18n_catalog_url', None): |
There was a problem hiding this comment.
| 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.
|
@shadinaif please mark this one as draft! |
|
being merged in #445 |
|
@shadinaif Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
Implement OEP-58 JavaScript translations
Details:
atlas pullfor the edx-platform and its plugins FC-0012 edx-platform#33166atlas pullfor XBlock translations | FC-0012 edx-platform#33698Testing
Local tutor 17.0.2
console.log(gettext('LTI Consumer'))References
This pull request is part of the FC-0012 project which implements the Translation Infrastructure update OEP-58.