fix: fix issues with LANGUAGE_CODE [BB-4823] [OSPR-6178] - #29122
Closed
Cup0fCoffee wants to merge 2 commits into
Closed
fix: fix issues with LANGUAGE_CODE [BB-4823] [OSPR-6178]#29122Cup0fCoffee wants to merge 2 commits into
Cup0fCoffee wants to merge 2 commits into
Conversation
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @Cup0fCoffee! I've created OSPR-6178 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 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. |
Cup0fCoffee
force-pushed
the
maxim/bb-4823-fix-issues-with-LANGUAGE-CODE
branch
from
October 27, 2021 18:09
10e2c94 to
45e1fec
Compare
When side-wide language is set via LANGUAGE_CODE site configuration, some elements of legacy UI still use the language set in user preferences (or None, which fallsback to 'en' if nothing is set in preferences). The expectation is that everything should be translated to the language which is set in the site configuration. In legacy UI (i.e. templates) the code of the current language sometimes is needed, and in these cases `user_language` variable from the context is used. The value for that variable is inserted via context processor, which takes the value from the user preferences. The solution modifies the context processor to check if there is a language set in site configuration, and if there is, send that value, instead of whatever value there was in user peferences.
When a site-wide language is set in site configurations, the desired result is that MFEs use that language over the one which is set in user preferences (or if None is set, it would fallback to 'en'). Since the MFEs use language value from cookies to determine which language to use, and the existing solution was only modifying the language in the session, and not overwrite the value in the cookies, the MFEs would use language set in users' preferences (or 'en' if nothing was set). This commit removes the existing solution and implements a new one in the middleware that sets language values in cookies based on the values set in users' preferences. The modified middleware is going to check if there is a site-wide language set in site configurations, and if it is, will use that value over the one which is set in users' preferences. Additionally, if site-wide language is set, the middleware won't save the language which is passed in cookies as the user preference, to not overwrite the user preference for other sites, where this site configuration doesn't apply.
Cup0fCoffee
force-pushed
the
maxim/bb-4823-fix-issues-with-LANGUAGE-CODE
branch
from
October 27, 2021 19:26
45e1fec to
1a93925
Compare
Contributor
|
@Cup0fCoffee Thank you for your contribution. Is this ready for our review? |
Cup0fCoffee
changed the title
openedx-webhooks
added
needs triage
and removed
waiting on author
Contributor
Author
|
Closing this PR in favor of creating two separate and smaller PRs. |
|
@Cup0fCoffee 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the issues of #28502, which are described this comment.
#28502 introduced the change that allows to set
LANGUAGE_CODEin site configurations, which made it possible to change translation of the entire site for all learners from Django admin panel. However, the implemented solution didn't work in all cases:Before (the date is not in the correct format):
After (the date is now in the correct format):
Supporting information
BB-4823
OSPR-6178
#28502
Testing instructions
LANGUAGE_CODEsite config toarat http://localhost:18000/admin/site_configuration/siteconfiguration/ (it might take some time to take effect because of caching; you can addSITE_CACHE_TTL = 0tolms/envs/private.pyto remove caching)frontend-app-account) and check that it's displayed in Arabic; check that setting the language (second dropdown from the bottom) to English doesn't change the languageDeadline
"None"
Concerns
After setting
LANGUAGE_CODEfor a site, users who visited that site will have language cookies with the same value asLANGUAGE_CODEin site configuration, which might create the following problems:LANGUAGE_CODEis removed after, the middleware will use the values from cookies to update users' preferences, which might end up messing up preferences for users that don't want that language be saved to their preferences.LANGUAGE_CODEset) the language in users' preferences will be overwritten. Since edX Sites are normally hosted on different domains and default value forSESSION_COOKIE_DOMAINsettings is an empty string, this shouldn't be a problem, but in certain situations it can cause problems.