[OSPR-5796] [BB-2862] "LANGUAGE_CODE" site configuration option by 0x29a · Pull Request #27696 · openedx/openedx-platform

0x29a · 2021-05-21T12:33:26Z

One of our clients has an Arabic site and asked us if it's possible to show it in Arabic regardless of user's language settings. We discovered, that as is, it's impossible to do this in platform.

So, this PR is adding support of LANGUAGE_CODE site configuration option, which makes it possible to change translation of the entire site for all learners from Django admin panel.

JIRA tickets:

Sandbox URL:

https://pr27696.sandbox.opencraft.hosting/

Testing instructions:

https://pr27696.sandbox.opencraft.hosting/ has default configuration.

Open https://pr27696.sandbox.opencraft.hosting/admin/site_configuration/siteconfiguration/3/change/ (staff:edx).
Change LANGUAGE_CODE to ar and save.
Open https://pr27696.sandbox.opencraft.hosting in a new anonymous browser tab. Sign in (staff@example.com:edx).
Verify that language changed to Arabian.

Reviewers

openedx-webhooks · 2021-05-21T12:33:31Z

Thanks for the pull request, @0x29a! I've created OSPR-5796 to keep track of it in JIRA, where we prioritize reviews. 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:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

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.

natabene · 2021-05-24T14:24:11Z

@0x29a Thank you for your contribution. Please let me know once it is ready for our review.

arch-bom-gocd-alerts · 2021-06-11T18:01:53Z

📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build.

We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master.

This branch has been detected to not have commit 2e33565 as an ancestor. Here's how to see for yourself:

git merge-base --is-ancestor 2e335653 0x29a/bb2862/language_code_via_site_configuration && echo "You're all set" || echo "Please rebase onto master or merge master to your branch"

If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal).

viadanna

Good to go 👍

I tested this on the sandbox.
I read through the code.

0x29a · 2021-06-13T12:04:30Z

@natabene, this is ready for edX review.

DawoudSheraz · 2021-06-15T10:00:47Z

@0x29a Hello. I can't test it through the steps mentioned in the PR. The default language when accessing the sandbox is Arabic. I tried changing to English/Spanish and then creating a new session in the anonymous tab but I am still seeing Arabic text over the platform. Can you update the steps/add any missing context? Thank you

0x29a · 2021-06-22T21:25:31Z

Hi @DawoudSheraz. Sorry for the delay here. I've managed to reproduce the issue you've encountered.

The problem is that site configuration is cached by edx-django-sites-extensions module in django.contrib.sites.middleware.CurrentSiteMiddleware middleware. Default TTL is 5 minutes, so, after changing site configuration, you have to wait for cache invalidation or set SITE_CACHE_TTL to 0. Let me know if you'd like me to update sandbox configuration.

This behavior seems pretty confusing to me because site configuration helper method is already decorated in request_cache, so, in this case, site configuration is clearly being double cached, which looks like a bug in the platform.

DawoudSheraz · 2021-06-23T10:14:59Z

Hi @DawoudSheraz. Sorry for the delay here. I've managed to reproduce the issue you've encountered.

The problem is that site configuration is cached by edx-django-sites-extensions module in django.contrib.sites.middleware.CurrentSiteMiddleware middleware. Default TTL is 5 minutes, so, after changing site configuration, you have to wait for cache invalidation or set SITE_CACHE_TTL to 0. Let me know if you'd like me to update sandbox configuration.

This behavior seems pretty confusing to me because site configuration helper method is already decorated in request_cache, so, in this case, site configuration is clearly being double cached, which looks like a bug in the platform.

Hi @0x29a . Thanks for the detailed context. If you can change the configuration of the sandbox, it would be good and make it easier to review the changes. As for the double caching, I am afraid I am short on the context.

0x29a · 2021-06-23T10:55:52Z

@DawoudSheraz, I set SITE_CACHE_TTL to 0 on sandbox. Now language is changing immediately. Could you check?

DawoudSheraz · 2021-06-23T14:02:50Z

The changes are good. I don't have access to see BB tickets, so can you add some more context in PR description/link to any issue to explain why the addition of setting via site configuration is needed?

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

0x29a · 2021-06-23T16:07:59Z

@DawoudSheraz, I expanded PR description a bit. Does it look better now?

DawoudSheraz · 2021-06-23T16:30:13Z

@DawoudSheraz, I expanded PR description a bit. Does it look better now?

Yes, better. But one thing: adding the value in site configuration eventually changes the entire site(given the translations are present). The statement to enable some particular translation for all learners seems a bit confusing(unless there are additional things beyond scope of this PR that I am missing)

0x29a · 2021-06-23T16:35:21Z

@DawoudSheraz, I rephrased the sentence you mentioned.

Agrendalath · 2021-06-24T15:31:28Z

@marcotuts, I see that this PR is missing some context about what we want to achieve by doing this. We have some instances which have multi-site configurations to provide courses in different languages. For this example, let's assume that we have an instance offering courses in English and French. When users go to a courses.domain page, they see a list of English courses (filtered by organizations) and have a default (English) interface. When they visit the fr-courses.domain page, they see a list of French courses, and the Open edX interface is in French, to provide a consistent user experience.

1 - previous to this PR, the site was able to be in Arabic as its primary language (this is true for other open edx sites do I assume it is still true for the platform unless there is some sort of bug.)

This is still true.

2 - users can override the platform language using the site language dropdown in the account settings.

Can no other languages be instead configured within site language or this account settings field be marked as hidden?

Yes, we were considering this, but given that users can switch between these sites, we would need to add middleware, which:

Checks the current user's language.
Sends a POST to /i18n/setlang/ and reloads the page if a user does not have the current site's language set.

This makes the approach much more complex and time-consuming, and thus degrades the user experience.

3 - client wants to disallow switching off arabic (the site language) <-- is this the primary goal?

This is not a goal, but a consequence. If a site will have a LANGUAGE_CODE set in its SiteConfigurations, then this is going to take precedence over the user's preferred language. It's still possible to update a language for testing via a courses.domain/update_lang/ page (the preview functionality).

I'm curious if this adds another layer of language overriding / complexity that can already be achieved.

I've mentioned the language preview feature - this is something we could reuse for achieving this by updating user language preferences for the preview in the existing middleware, but this is going to break the language preview functionality for sites with LANGUAGE_CODE set. The proposed approach adds a layer for overriding the language, but is much simpler, as it transparently applies the specified language to all users visiting the site rather than altering the preview preferences of each user and adjusting them while switching between sites.

I hope this clarifies it a bit. Please ping me if you have any questions.

DawoudSheraz · 2021-06-30T17:04:22Z

@marcotuts Hello. Is this good to merge or are there still reservations? Thank you

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

* [SE-4101] fix: address VisibleBlocks caching race condition (openedx#27359) (#349) * fix: address VisibleBlocks caching race condition * sets visual block creation in an atomic transaction * refactor: add logging statement to bulk create Co-authored-by: Raul Gallegos <raul@opencraft.com> Co-authored-by: Raul Gallegos <raul@opencraft.com> * BB-3954 Add toggle for enrollment behavior (#351) Adds toggle REDIRECT_UNAUTHENTICATED_USER_TO_LOGIN_ON_ENROLL for enrollment behaviour for unauthenticated user. If true, the user will be redirected to 'signin_user' route. Co-authored-by: Arjun Singh Yadav <arjun@opencraft.com> * [BB-3622] feat:Restrict user create course (#319) (#352) * Add course creation condition for organization The condition added to ensure that if the feature is enabled user will not be able to create the course outside of the organization in which they belong. Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com> * fix: update the xblock-lti-consumer commit * Added new setting CERTIFICATE_DATE_FORMAT for easy customization of (#354) certificate issued date (cherry picked from commit 421e661) * fix: Produce grade report when subsections have future start dates When getting a subsection grade for a user, instead of failing if the user can't access that subsection, fallback to the collected structure. * Revert "Unhide student-generated certificates toggle" This reverts commit 8910ccf. Our clients are no longer using this feature, and edX won't accept this upstream as-is currently. See https://github.com/edx/edx-platform/pull/23735 for more information. Reverting to reduce code drift from upstream. * chore: update Arabic translations * fix:Fix function call to check MFE (#359) Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com> * fix: don't cache ajax request * feat: add site language config Cherry-picked from: https://github.com/edx/edx-platform/pull/27696 * Revert "[FAL-1813] fix: codejail issue when using matplotlib (#343)" (#365) This reverts commit 2143f84. * Update celery routing for celery 4+ (openedx#25567) * Update celery routing - Used routing function instead of class - Move task queues dictionary to Django settings - Removed routing_key parameter - Refactored routing for singleton celery instantiation Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com> (cherry picked from commit e3b4d23) * fix: add missing set_code_owner_attribute imports (cherry picked from commit d1060d2) * chore: bump edx-django-utils to 3.12.0 The required `set_code_owner_attribute` decorator was introduced in v3.12.0, therefore we need to bump the dependency. (cherry picked from commit f52b84e) * [SE-4482] Allow delete course content in Studio only for admin users (#360) Co-authored-by: Nizar Mahmoud <nizarmah@hotmail.com> * fix: Password reset page throwing not found error (#364) Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com> * fix: change buttons in wiki modal to anchor tags The action buttons in wiki modal have href attribute but does not have an event listener for click. This PR changes the buttons to anchor tags so that they work as expected when clicked. * fix: use high priority queue for celery heartbeat check Makes HIGH_PRIORITY_QUEUE a derived setting, which allows HEARTBEAT_CELERY_ROUTING_KEY to use the correct config variant default. (cherry picked from commit 2a9067a) * fix: prevent invalidation of allowlisted certificates The allowlisted certificates were getting invalidated upon visiting the Course Progress page by users. This is a rough backport of the Lilac fix (edx#26356). In Lilac, this is gated by the `certificates_revamp.use_allowlist` Waffle flag. In post-Lilac branches, this is working out of the box (the flag has been removed in edx#27576). Jira ticket: BB-4287 * feat: add celery beat configuration Co-authored-by: Raul Gallegos <raul@opencraft.com> Co-authored-by: Farhaan Bukhsh <farhaan@opencraft.com> Co-authored-by: Arjun Singh Yadav <arjun@opencraft.com> Co-authored-by: pkulkark <pooja@opencraft.com> Co-authored-by: Shimul Chowdhury <shimul@opencraft.com> Co-authored-by: João Cabrita <joao.cabrita@opencraft.com> Co-authored-by: Samuel Walladge <samuel@opencraft.com> Co-authored-by: Giovanni Cimolin da Silva <giovannicimolin@gmail.com> Co-authored-by: 0x29a <demid@opencraft.com> Co-authored-by: Dmitry Gamanenko <dmitry.gamanenko@raccoongang.com> Co-authored-by: Jillian Vogel <jill@opencraft.com> Co-authored-by: Muhammad Soban Javed <58461728+iamsobanjaved@users.noreply.github.com> Co-authored-by: Sandeep Choudhary <sandeep@opencraft.com> Co-authored-by: Nizar Mahmoud <nizarmah@hotmail.com> Co-authored-by: Agrendalath <piotr@surowiec.it>

natabene · 2021-07-19T15:45:31Z

Checking with product now about this.

marcotuts · 2021-07-19T16:34:35Z

The site configuration distinction and clarification above was helpful. No other concerns on me end

DawoudSheraz · 2021-07-26T05:35:42Z

@0x29a Hello. Can you rebase your PR, please? Once done, I will be merging this. Thank you

edx-status-bot · 2021-07-27T09:09:46Z

Your PR has finished running tests. There were no failures.

0x29a · 2021-07-27T09:10:33Z

Hello, @DawoudSheraz. Done.

openedx-webhooks · 2021-07-29T17:24:21Z

@0x29a 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

edx-pipeline-bot · 2021-07-29T18:27:36Z

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

edx-pipeline-bot · 2021-07-29T18:54:22Z

EdX Release Notice: This PR has been deployed to the production environment.

arbrandes · 2021-09-01T18:09:54Z

@0x29a, @Agrendalath: @cmltaWt0 identified a couple of issues with this over on the Lilac backport ticket. Do you mind creating an (internal) ticket to investigate as a follow-up?

0x29a · 2021-09-02T10:23:51Z

Done, @arbrandes.

…tion to Lilac # Description This PR contains changes from https://github.com/edx/edx-platform/pull/27696 PR cherry-picked to `open-release/lilac.master` branch.

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

openedx-webhooks added open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels May 21, 2021

0x29a changed the title ~~[WIP] [BB-2862] "LANGUAGE_CODE" site configuration option~~ [WIP] [OSPR-5796] [BB-2862] "LANGUAGE_CODE" site configuration option May 21, 2021

0x29a force-pushed the 0x29a/bb2862/language_code_via_site_configuration branch 2 times, most recently from aeaab6c to 879ab3c Compare June 9, 2021 11:15

viadanna approved these changes Jun 11, 2021

View reviewed changes

0x29a force-pushed the 0x29a/bb2862/language_code_via_site_configuration branch from 879ab3c to aa8e5bf Compare June 13, 2021 12:04

0x29a changed the title ~~[WIP] [OSPR-5796] [BB-2862] "LANGUAGE_CODE" site configuration option~~ [OSPR-5796] [BB-2862] "LANGUAGE_CODE" site configuration option Jun 13, 2021

openedx-webhooks added needs triage and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Jun 13, 2021

openedx-webhooks added awaiting prioritization and removed needs triage labels Jun 14, 2021

openedx-webhooks added changes requested and removed awaiting prioritization labels Jun 15, 2021

DawoudSheraz approved these changes Jun 23, 2021

View reviewed changes

0x29a referenced this pull request in open-craft/openedx-platform Jun 23, 2021

feat: add site language config

756a4d9

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

0x29a mentioned this pull request Jun 23, 2021

[BB-2862] "LANGUAGE_CODE" site configuration option (backport) open-craft/openedx-platform#366

Merged

Agrendalath referenced this pull request in open-craft/openedx-platform Jun 23, 2021

feat: add site language config

eba631e

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

dliberat referenced this pull request in open-craft/openedx-platform Jul 8, 2021

feat: add site language config

b42b5ea

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

feat: add site language config

b01544d

0x29a force-pushed the 0x29a/bb2862/language_code_via_site_configuration branch from aa8e5bf to b01544d Compare July 27, 2021 08:28

openedx-webhooks added ready to merge and removed product review PR requires product review before merging labels Jul 27, 2021

DawoudSheraz merged commit 9214e19 into openedx:master Jul 29, 2021

openedx-webhooks added merged and removed ready to merge labels Jul 29, 2021

Agrendalath deleted the 0x29a/bb2862/language_code_via_site_configuration branch July 30, 2021 11:25

0x29a mentioned this pull request Aug 20, 2021

[OSPR-5979] Port "LANGUAGE_CODE" site configuration option to Lilac #28502

Merged

johanseto referenced this pull request in eduNEXT/edx-platform Aug 2, 2022

feat: add site language config

26a1e09

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

johanseto referenced this pull request in eduNEXT/edx-platform Aug 3, 2022

feat: add site language config

8ab706e

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

Agrendalath mentioned this pull request Dec 6, 2022

fix: use language specified in SiteConfiguration #31408

Merged

Uh oh!

Conversation

0x29a commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

natabene commented May 24, 2021

Uh oh!

arch-bom-gocd-alerts commented Jun 11, 2021

Uh oh!

viadanna left a comment

Choose a reason for hiding this comment

Uh oh!

0x29a commented Jun 13, 2021

Uh oh!

DawoudSheraz commented Jun 15, 2021

Uh oh!

0x29a commented Jun 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DawoudSheraz commented Jun 23, 2021

Uh oh!

0x29a commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DawoudSheraz commented Jun 23, 2021

Uh oh!

0x29a commented Jun 23, 2021

Uh oh!

DawoudSheraz commented Jun 23, 2021

Uh oh!

0x29a commented Jun 23, 2021

Uh oh!

Agrendalath commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DawoudSheraz commented Jun 30, 2021

Uh oh!

natabene commented Jul 19, 2021

Uh oh!

marcotuts commented Jul 19, 2021

Uh oh!

DawoudSheraz commented Jul 26, 2021

Uh oh!

edx-status-bot commented Jul 27, 2021

Uh oh!

0x29a commented Jul 27, 2021

Uh oh!

openedx-webhooks commented Jul 29, 2021

Uh oh!

edx-pipeline-bot commented Jul 29, 2021

Uh oh!

edx-pipeline-bot commented Jul 29, 2021

Uh oh!

arbrandes commented Sep 1, 2021

Uh oh!

0x29a commented Sep 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

0x29a commented May 21, 2021 •

edited

Loading

openedx-webhooks commented May 21, 2021 •

edited

Loading

0x29a commented Jun 22, 2021 •

edited

Loading

0x29a commented Jun 23, 2021 •

edited

Loading

Agrendalath commented Jun 24, 2021 •

edited

Loading