Issue 4822862858813440: Issue 2472 - Have https://eyeo.com/jobs redirect properly (Closed)
DescriptionIssue 2472 - Have https://eyeo.com/jobs redirect properly
Patch Set 1 #
Total comments: 2
Patch Set 2 : Second attempt #MessagesTotal messages: 10
I have tested this and I think it works fine, I would appreciate a sanity check though as I have not played with the infrastructure stuff for a while.
On 2015/05/07 19:56:05, kzar wrote: > I have tested this and I think it works fine, I would appreciate a sanity check > though as I have not played with the infrastructure stuff for a while. How does that fix */jobs redirects? I see what you do there, and it looks quite valid, but I cannot find any */en/jobs/index files...
There is a file web.eyeo.com/pages/jobs/index.tmpl which renders to en/jobs/index. To quote Wladimir: 14:39 <palant> kzar: it's eyeo.com/jobs/index right now- 14:40 <palant> kzar: canonical URLs work normally but that's a directory..."
On 2015/05/08 11:47:07, kzar wrote: > There is a file web.eyeo.com/pages/jobs/index.tmpl which renders to > en/jobs/index. > > To quote Wladimir: > > 14:39 <palant> kzar: it's eyeo.com/jobs/index right now- > 14:40 <palant> kzar: canonical URLs work normally but that's a directory..." LGTM.
LGTM - I've put Wladimir in CC since he was also involved here.
http://codereview.adblockplus.org/4822862858813440/diff/5629499534213120/modu... File modules/web/templates/site.conf.erb (right): http://codereview.adblockplus.org/4822862858813440/diff/5629499534213120/modu... modules/web/templates/site.conf.erb:41: rewrite ^(.*) /en$1 last; So people will always be redirected to the English version of the page in case of a directory? That seems wrong. In order to avoid duplicating code, why not just check for existance? if (-e "$document_root/${preferredLang}_$preferredRegion$uri") This will redirect for both files and directories if they exist. We will only need the three checks we already have. Note that looking for $index_page explicitly isn't necessary - if it doesn't exist then it is 404 one way or another.
Patch Set 2 : Second attempt http://codereview.adblockplus.org/4822862858813440/diff/5629499534213120/modu... File modules/web/templates/site.conf.erb (right): http://codereview.adblockplus.org/4822862858813440/diff/5629499534213120/modu... modules/web/templates/site.conf.erb:41: rewrite ^(.*) /en$1 last; On 2015/05/08 21:06:05, Wladimir Palant wrote: > So people will always be redirected to the English version of the page in case > of a directory? That seems wrong. > > In order to avoid duplicating code, why not just check for existance? > > if (-e "$document_root/${preferredLang}_$preferredRegion$uri") > > This will redirect for both files and directories if they exist. We will only > need the three checks we already have. Note that looking for $index_page > explicitly isn't necessary - if it doesn't exist then it is 404 one way or > another. Shucks, good point. Done.
LGTM
LGTM
LGTM. 
|
