Skip to content

fix: remove language_session_key usage - #32578

Merged
edx-requirements-bot merged 3 commits into
masterfrom
usamasadiq/remove-language-session-key-usage
Aug 16, 2023
Merged

fix: remove language_session_key usage#32578
edx-requirements-bot merged 3 commits into
masterfrom
usamasadiq/remove-language-session-key-usage

Conversation

@UsamaSadiq

Copy link
Copy Markdown
Member

Description

This PR intends to resolve the issue #32388.

@UsamaSadiq
UsamaSadiq force-pushed the usamasadiq/remove-language-session-key-usage branch 2 times, most recently from e34641a to ed654b3 Compare June 27, 2023 16:31
@UsamaSadiq
UsamaSadiq marked this pull request as ready for review June 27, 2023 16:32
@UsamaSadiq
UsamaSadiq force-pushed the usamasadiq/remove-language-session-key-usage branch from ed654b3 to b63b6e1 Compare June 27, 2023 19:03

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A better PR description would have been helpful. It took me a while to realize LANGUAGE_COOKIE_NAME was not new in this PR. I thought we were transitioning from LANGUAGE_SESSION_KEY to LANGUAGE_COOKIE_NAME, and maybe we are, but just not all in this PR? Are all the test changes because we didn't have as much LANGUAGE_COOKIE_NAME test coverage as we'd like, and now we are adding it?

Note: I do not have expertise in this area, so this is just a general review. When my review is complete, you can determine if you want someone with more language-related expertise.

Comment thread cms/urls.py
Comment thread openedx/core/djangoapps/dark_lang/tests.py
Comment thread openedx/core/djangoapps/dark_lang/tests.py Outdated
Comment thread openedx/core/djangoapps/dark_lang/tests.py Outdated
Comment thread openedx/core/djangoapps/dark_lang/tests.py
Comment thread openedx/core/djangoapps/dark_lang/views.py
Comment thread openedx/core/djangoapps/lang_pref/middleware.py
Comment thread openedx/core/djangoapps/lang_pref/views.py Outdated
Comment thread cms/urls.py Outdated
@robrap

robrap commented Jul 26, 2023

Copy link
Copy Markdown
Contributor

@UsamaSadiq @iamsobanjaved: Please ping me when you want me to review again. Also, I'd appreciate quick replies to my comments (when you are ready), which might be "Done", or whatever is appropriate. Sometimes comments are marked "Outdated" due to changes, but the comment itself was never actually addressed. Thank you.

@robrap robrap added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jul 26, 2023

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure if you were ready for review, but I saw changes, so decided to take a look. Thanks.

Comment thread openedx/core/djangoapps/dark_lang/tests.py Outdated
Comment thread openedx/core/djangoapps/dark_lang/tests.py
Comment thread openedx/core/djangoapps/dark_lang/views.py
Comment thread openedx/core/djangoapps/lang_pref/middleware.py
Comment thread openedx/core/djangoapps/lang_pref/tests/test_middleware.py
@iamsobanjaved
iamsobanjaved force-pushed the usamasadiq/remove-language-session-key-usage branch from d64e47a to bff7f48 Compare August 7, 2023 14:08
@iamsobanjaved
iamsobanjaved force-pushed the usamasadiq/remove-language-session-key-usage branch from c11bfac to 27edbc6 Compare August 15, 2023 16:29
@iamsobanjaved

Copy link
Copy Markdown
Contributor

@robrap this is ready for review now

@robrap robrap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you. Looks good.

@robrap robrap added ready to merge and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Aug 15, 2023
@edx-requirements-bot
edx-requirements-bot merged commit aca521f into master Aug 16, 2023
@edx-requirements-bot
edx-requirements-bot deleted the usamasadiq/remove-language-session-key-usage branch August 16, 2023 10:05
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

shadinaif added a commit to zeit-labs/edx-platform that referenced this pull request Sep 13, 2023
This is a patch to pull request openedx#32578 because the `session_language` was renamed
to `update_language` except in three locations causing a 500 error:

Reverse for 'session_language' not found. 'session_language' is not a valid view function or pattern name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants