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

Fixes #16758: Create language cookie if required #16764

Merged

Conversation

m2martin
Copy link
Contributor

Fixes: #16758

If a new browser session has no language cookie set, check if it should be set and redirect to the requested page.

@m2martin
Copy link
Contributor Author

I'd like to get rid of the redirection, but django.utils.translation.activate() seems worthless here. Does someone have a better idea?

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

This probably isn't the best approach to ensure translation is active. Instead, we should update the calls to set_cookie() in LoginView and UserConfigView to set an expiration time on the language cookie (which should presumably match the expiration time of the session cookie, which was derived from LOGIN_TIMEOUT).

@m2martin
Copy link
Contributor Author

I totally agree that this approach is not very elegant. After looking into the way how Django handles cookie renewals, I found a very simple way to accomplish the same for our language cookie. Please have a look at the new commit.

@dreng
Copy link
Contributor

dreng commented Jul 4, 2024

IMO, instead of dealing with cookies, those kind of user preferences should be stored in the database. Otherwise you will have to set the language preference in every browser on every computer again (not even mentioning private browsing). In our case you have to set it each day actually, because cookies will be automatically removed by a group policy. That can be really annoying.

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

Thanks @m2martin, great work!

@jeremystretch jeremystretch merged commit 6a1245c into netbox-community:develop Jul 9, 2024
3 checks passed
@m2martin m2martin deleted the 16758-language-cookie branch August 4, 2024 10:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Language setting is lost for new browser sessions
3 participants