Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use middleware for setting language #1633

Conversation

niklasmohrin
Copy link
Member

Fixes #1631

Instead of setting session and user settings for language when they log in, it is now updated on every request. Now, when a logged in user visits the site with a fresh session, the preference from the database is used.

I couldn't figure out another way to do this easily. An answer to this question also suggests using a third party middleware, https://pypi.python.org/pypi/django-user-language-middleware/. It does the same as this.

One test broke - it used the HTTP_ACCEPT_LANGUAGE header. I believe that it was handled by the django locale middleware.

Is this a problem? Other thoughts?

evap/middleware.py Outdated Show resolved Hide resolved
@janno42 janno42 self-requested a review October 15, 2021 19:45
@niklasmohrin niklasmohrin force-pushed the uselanguagemiddlewaretoshowusersthatwecare branch from 331d2b6 to 9f95a36 Compare October 18, 2021 16:35
evap/middleware.py Show resolved Hide resolved
@@ -93,7 +93,9 @@ def test_order(self):
page = self.app.get(self.url, user=student).body.decode()
self.assertLess(page.index(evaluation1.name_en), page.index(evaluation2.name_en))

page = self.app.get(self.url, user=student, extra_environ={"HTTP_ACCEPT_LANGUAGE": "de"}).body.decode()
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you know why this doesn't work anymore? shouldn't this run into the "else" branch in your middleware and use what the LocaleMiddleware chose?

If this does not work, does that mean that first-time users will get the default language from evap's settings (and not their browser language?

Copy link
Member

Choose a reason for hiding this comment

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

line 93 is the first request for this user, so after that, they would have the langauge from that request (english) stored in the database and used, or am I missing something?

First time users should still have their browser language accepted.

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems you're right!

@karyon
Copy link
Collaborator

karyon commented Oct 24, 2021

btw, this also removes evap's usage of LANGUAGE_SESSION_KEY, which will be removed in django 4.0, so that's nice.

@niklasmohrin niklasmohrin merged commit 0a5b980 into e-valuation:master Oct 25, 2021
@niklasmohrin niklasmohrin deleted the uselanguagemiddlewaretoshowusersthatwecare branch October 25, 2021 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Lost in translation: Language preference dropped when browser is closed
4 participants