Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

bug 1443586: Make lang preference cookie persistent #4718

Merged
merged 1 commit into from Mar 29, 2018

Conversation

MatonAnthony
Copy link
Contributor

Increase the max-age of the language preference cookie to one
year instead of only the current session.

This avoids to ask if people wants to keep their current locale
at each of their visits.

Copy link
Contributor

@escattone escattone left a comment

Choose a reason for hiding this comment

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

Thanks @MatonAnthony! This looks good except the test (see comment below).

@@ -152,6 +152,15 @@ def test_not_possible_to_set_non_locale_cookie(self):
# No language cookie should be saved as `foo` is not a supported locale
assert not language_cookie

def test_setting_language_cookie_age(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

@MatonAnthony I think you'll first have to rebase your branch on master, as the LanguageCookieTest class no longer exists. Once you rebase on the latest master, you could fold this check:

assert language_cookie['max-age'] == settings.LANGUAGE_COOKIE_AGE

into the existing test test_setting_language_cookie_working.

@escattone escattone self-assigned this Mar 27, 2018
@MatonAnthony MatonAnthony force-pushed the persistent-lang-cookie-1443586 branch from 7d15ae9 to aeead8c Compare March 29, 2018 15:35
@codecov-io
Copy link

Codecov Report

Merging #4718 into master will decrease coverage by 0.47%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4718      +/-   ##
==========================================
- Coverage   95.94%   95.47%   -0.48%     
==========================================
  Files         270      263       -7     
  Lines       25352    23872    -1480     
  Branches     1743     1712      -31     
==========================================
- Hits        24325    22791    -1534     
- Misses        817      871      +54     
  Partials      210      210
Impacted Files Coverage Δ
kuma/core/tests/test_views.py 100% <100%> (ø) ⬆️
kuma/settings/common.py 92.18% <100%> (+0.03%) ⬆️
kuma/wiki/views/delete.py 62.29% <0%> (-34.68%) ⬇️
kuma/feeder/models.py 71.42% <0%> (-28.58%) ⬇️
kuma/attachments/views.py 84.74% <0%> (-6%) ⬇️
kuma/wiki/admin.py 76.23% <0%> (-3.22%) ⬇️
kuma/core/middleware.py 86.29% <0%> (-1.62%) ⬇️
kuma/wiki/views/document.py 90.31% <0%> (-1.16%) ⬇️
kuma/wiki/models.py 91.82% <0%> (-0.72%) ⬇️
kuma/wiki/views/revision.py 92.55% <0%> (-0.38%) ⬇️
... and 39 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 192b366...aeead8c. Read the comment docs.

Increase the max-age of the language preference cookie to one
year instead of only the current session.

This avoids to ask if people wants to keep their current locale
at each of their visits.
@MatonAnthony MatonAnthony force-pushed the persistent-lang-cookie-1443586 branch from aeead8c to e9eca98 Compare March 29, 2018 15:46
Copy link
Contributor

@escattone escattone left a comment

Choose a reason for hiding this comment

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

Just to be thorough, I verified this using my local dev environment, and it worked great. Thanks so much @MatonAnthony!

@escattone escattone merged commit 335f5e0 into mdn:master Mar 29, 2018
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.

None yet

3 participants