Skip to content

fix: use language specified in SiteConfiguration [BB-6930]#516

Merged
Agrendalath merged 2 commits intoopencraft-release/nutmeg.2from
agrendalath/bb-6930-fix_site_language_nutmeg
Dec 9, 2022
Merged

fix: use language specified in SiteConfiguration [BB-6930]#516
Agrendalath merged 2 commits intoopencraft-release/nutmeg.2from
agrendalath/bb-6930-fix_site_language_nutmeg

Conversation

@Agrendalath
Copy link
Member

This backports openedx#31408 to the common Nutmeg branch.

This feature was implemented in b01544d to replace the session's language
in the request. 44ddbdf moved the process from the request to the response,
which made this feature unusable (because the request was already processed).
44ddbdf also made this feature set the language cookie. However, it is
overwritten by user preferences.
To fix this, we could overwrite the cookie of the response after it's set from
user preferences. However, it is not an ideal solution because when users
switch between Sites with different languages, the first response will use the
language of the previous page. Therefore, this ignores user preferences and
alters the cookie of a request instead.
This test stopped throwing the `TransactionManagementError` once we added the
`site_configuration.get_value()` call to the language preferences middleware.
@Agrendalath Agrendalath force-pushed the agrendalath/bb-6930-fix_site_language_nutmeg branch from e361442 to a3f869a Compare December 8, 2022 12:50
Copy link
Member

@pkulkark pkulkark left a comment

Choose a reason for hiding this comment

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

LGTM 👍

  • I tested this: Tested the upstream PR and ensured the changes are same.
  • I read through the code
  • I checked for accessibility issues N/A
  • Includes documentation N/A

@Agrendalath Agrendalath merged commit 611859c into opencraft-release/nutmeg.2 Dec 9, 2022
@Agrendalath Agrendalath deleted the agrendalath/bb-6930-fix_site_language_nutmeg branch December 9, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants