Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29537689: #3299 - Add URI redirect for missing translations (Closed)

Created:
Sept. 6, 2017, 10:54 p.m. by f.nicolaisen
Modified:
Sept. 11, 2017, 4:12 p.m.
Reviewers:
mathias
CC:
f.lopez, Fred
Base URL:
https://hg1/infrastructure
Visibility:
Public.

Description

#3299 - Add URI redirect for missing translations This change is related to the previous commit for #3421. After this change, If someone has manually chosen Mexican (es-MX), and they surf to a page that does not have a Mexican, but a Spanish (es) version available, they will be flipped to Spanish and not return when later surfing to a page available in Mexican. The existing behavior was to just show 404s in these cases, which would be terrible experience with all those half-complete translation sets we have currently. We could check for an available translation on every request, and flip them back to Mexican when possible, but that will be counter-intuitive to those who have manually chosen a different language in the top-right selector on the page. The optimal solution would be to keep their last chosen language stored in session and say "You're currently surfing the Mexican site but this page only has Spanish translation available", but that would mean revisiting privacy policy and server mechanics, which is out of scope for this commit.

Patch Set 1 #

Total comments: 2

Patch Set 2 : try language if no translation for region #

Patch Set 3 : fix comments and whitespace #

Total comments: 6

Patch Set 4 : fix brackets and improve commit message #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -0 lines) Patch
M modules/web/templates/site.conf.erb View 1 2 3 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 6
f.nicolaisen
Sept. 6, 2017, 10:54 p.m. (2017-09-06 22:54:57 UTC) #1
f.nicolaisen
https://codereview.adblockplus.org/29537689/diff/29537690/modules/web/templates/site.conf.erb File modules/web/templates/site.conf.erb (right): https://codereview.adblockplus.org/29537689/diff/29537690/modules/web/templates/site.conf.erb#newcode66 modules/web/templates/site.conf.erb:66: location ~ ^/([a-z][a-z]\_[A-Z][A-Z])(/.+) { It would be cool if ...
Sept. 6, 2017, 11 p.m. (2017-09-06 23:00:19 UTC) #2
f.nicolaisen
Note that the PS2 patch was hotfixed to production yesterday. In PS3, I haven't changed ...
Sept. 8, 2017, 1:51 p.m. (2017-09-08 13:51:11 UTC) #3
mathias
(Wrong reviewer addresses here again, fixed for now.) https://codereview.adblockplus.org/29537689/diff/29539748/modules/web/templates/site.conf.erb File modules/web/templates/site.conf.erb (right): https://codereview.adblockplus.org/29537689/diff/29539748/modules/web/templates/site.conf.erb#newcode60 modules/web/templates/site.conf.erb:60: location ...
Sept. 8, 2017, 7:20 p.m. (2017-09-08 19:20:21 UTC) #4
f.nicolaisen
https://codereview.adblockplus.org/29537689/diff/29539748/modules/web/templates/site.conf.erb File modules/web/templates/site.conf.erb (right): https://codereview.adblockplus.org/29537689/diff/29539748/modules/web/templates/site.conf.erb#newcode60 modules/web/templates/site.conf.erb:60: location ~ ^/([a-z][a-z]\_[A-Z][A-Z])(/.+) { On 2017/09/08 19:20:20, mathias wrote: ...
Sept. 11, 2017, 7:33 a.m. (2017-09-11 07:33:14 UTC) #5
mathias
Sept. 11, 2017, 12:52 p.m. (2017-09-11 12:52:52 UTC) #6
LGTM. Please improve on the commit message so it becomes a sentence, i.e. "#0000
- Add redirect ..." or similar.

Powered by Google App Engine
This is Rietveld