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

[FIX] website: language prefixed URL on website #31792

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@nle-odoo
Copy link
Contributor

commented Mar 12, 2019

In 10.0 and before, if we were eg. on website page /fr_FR/contact, if
fr_FR language (french) is installed and not the default website
language we would have all URL towards translated content (route with
multilang=True) prefixed with /fr_FR/ (the language code).

With 9cd982b some improvments were done to CDN but this also breaks
language prefixing, so this cause an unecessary redirect:

  • we are on /fr_FR/contact and click on "Shop"
  • we go to /shop
  • odoo redirects US /fr_FR/shop

With this PR, URLs are prefixed with language once again.

note: the added test is hackish but the function tested depends on a lot
of odoo.http things that are hard to use for test (request.render,
request.context, request.httpreqest).

opw-1922051

@nle-odoo nle-odoo added the OE label Mar 12, 2019

@nle-odoo nle-odoo force-pushed the odoo-dev:11.0-website-opw-1922051-nle branch Mar 12, 2019

@robodoo robodoo added the seen 🙂 label Mar 12, 2019

@nle-odoo nle-odoo force-pushed the odoo-dev:11.0-website-opw-1922051-nle branch to bb9100f Mar 13, 2019

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 13, 2019

[FIX] website: language in url like in 10.0 WIP
With 9cd982b some improvments were done to CDN but this also breaks
our automatic URL rewritting to include the language code.

This PR is a WIP (draft) to include the language code like before.

opw-1922051
closes odoo#31792

@robodoo robodoo added the CI 🤖 label Mar 13, 2019

@nle-odoo nle-odoo force-pushed the odoo-dev:11.0-website-opw-1922051-nle branch from bb9100f to 0abad90 Mar 22, 2019

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 22, 2019

[FIX] website: language in url like in 10.0 WIP
With 9cd982b some improvments were done to CDN but this also breaks
our automatic URL rewritting to include the language code.

This PR is a WIP (draft) to include the language code like before.

opw-1922051
closes odoo#31792

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 22, 2019

@nle-odoo nle-odoo force-pushed the odoo-dev:11.0-website-opw-1922051-nle branch from 0abad90 to 10e1b98 Mar 22, 2019

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 22, 2019

[FIX] website: language prefixed URL on website
In 10.0 and before, if we were eg. on website page /fr_FR/contact, if
`fr_FR` language (french) is installed and not the default website
language we would have all URL towards translated content (route with
multilang=True) prefixed with /fr_FR/ (the language code).

With 9cd982b some improvments were done to CDN but this also breaks
language prefixing, so this cause an unecessary redirect:

- we are on /fr_FR/contact and click on "Shop"
- we go to /shop
- odoo redirects US /fr_FR/shop

With this PR, URLs are prefixed with language once again.

note: the added test is hackish but the function tested depends on a lot
of odoo.http things that are hard to use for test (request.render,
request.context, request.httpreqest).

opw-1922051
closes odoo#31792

@nle-odoo nle-odoo changed the title [FIX] website: language in url like in 10.0 WIP [FIX] website: language prefixed URL on website Mar 22, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Mar 22, 2019

@nle-odoo nle-odoo requested a review from KangOl Mar 22, 2019

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Hi @KangOl

Do you think the test is okay for this PR, it is a lot hackish since I test low-level function and in is_multilang_url this is called and so should work for the test:

def is_multilang_url(local_url, langs=None):
        # ...
        router = request.httprequest.app.get_db_router(request.db).bind('')
        # Force to check method to POST. Odoo uses methods : ['POST'] and ['GET', 'POST']
        func = router.match(path, method='POST', query_args=query_string)[0]
        return (func.routing.get('website', False) and
                func.routing.get('multilang', func.routing['type'] == 'http'))
@KangOl

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

I would put the test(s) in its own class, set the initialization in setUp and create differents functions for each different cases (no request, not website, lang in context, monolang routes)

@nle-odoo nle-odoo force-pushed the odoo-dev:11.0-website-opw-1922051-nle branch from 10e1b98 to e67f4fd Mar 22, 2019

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 22, 2019

[FIX] website: language prefixed URL on website
In 10.0 and before, if we were eg. on website page /fr_FR/contact, if
`fr_FR` language (french) is installed and not the default website
language we would have all URL towards translated content (route with
multilang=True) prefixed with /fr_FR/ (the language code).

With 9cd982b some improvments were done to CDN but this also breaks
language prefixing, so this cause an unecessary redirect:

- we are on /fr_FR/contact and click on "Shop"
- we go to /shop
- odoo redirects US /fr_FR/shop

With this PR, URLs are prefixed with language once again.

note: the added test is hackish but the function tested depends on a lot
of odoo.http things that are hard to use for test (request.render,
request.context, request.httpreqest).

opw-1922051
closes odoo#31792
[FIX] website: language prefixed URL on website
In 10.0 and before, if we were eg. on website page /fr_FR/contact, if
`fr_FR` language (french) is installed and not the default website
language we would have all URL towards translated content (route with
multilang=True) prefixed with /fr_FR/ (the language code).

With 9cd982b some improvments were done to CDN but this also breaks
language prefixing, so this cause an unecessary redirect:

- we are on /fr_FR/contact and click on "Shop"
- we go to /shop
- odoo redirects US /fr_FR/shop

With this PR, URLs are prefixed with language once again.

note: the added test is hackish but the function tested depends on a lot
of odoo.http things that are hard to use for test (request.render,
request.context, request.httpreqest).

opw-1922051
closes #31792

@nle-odoo nle-odoo force-pushed the odoo-dev:11.0-website-opw-1922051-nle branch from e67f4fd to dcd114e Mar 22, 2019

@robodoo robodoo removed the CI 🤖 label Mar 22, 2019

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@KangOl ok, it is done

I will merge on monday if there is no other comment

@KangOl

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

Actually, trust robodoo
@robodoo r+

robodoo pushed a commit that referenced this pull request Mar 22, 2019

[FIX] website: language prefixed URL on website
In 10.0 and before, if we were eg. on website page /fr_FR/contact, if
`fr_FR` language (french) is installed and not the default website
language we would have all URL towards translated content (route with
multilang=True) prefixed with /fr_FR/ (the language code).

With 9cd982b some improvments were done to CDN but this also breaks
language prefixing, so this cause an unecessary redirect:

- we are on /fr_FR/contact and click on "Shop"
- we go to /shop
- odoo redirects US /fr_FR/shop

With this PR, URLs are prefixed with language once again.

note: the added test is hackish but the function tested depends on a lot
of odoo.http things that are hard to use for test (request.render,
request.context, request.httpreqest).

opw-1922051
closes #31792

Signed-off-by: Christophe Simonis <chs@odoo.com>

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 22, 2019

[FIX] http_routing: ignore # when checking multilang URL
With 953a693 when a route is not found we assume it is a website.page
that can be multilang.

But the fix of odoo#31792 we should once again test that all relative URL on
the website are multilang or not so an url like `/web#home` that was
considered not multilang before (because there is no route /web#home)
now is considered multilang.

With this fix, we only ignore the hash mark and URL fragment when
finding the URL's route (so we find that /web is not multilang).

related to odoo#31792
related to opw-1922051

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 22, 2019

[FIX] http_routing: ignore # when checking multilang URL
With 953a693 when a route is not found we assume it is a website.page
that can be multilang.

But the fix of odoo#31792 we should once again test that all relative URL on
the website are multilang or not so an url like `/web#home` that was
considered not multilang before (because there is no route /web#home)
now is considered multilang.

With this fix, we only ignore the hash mark and URL fragment when
finding the URL's route (so we find that /web is not multilang).

related to odoo#31792
related to opw-1922051
closes odoo#32059

nle-odoo added a commit to odoo-dev/odoo that referenced this pull request Mar 22, 2019

[FIX] http_routing: ignore # when checking multilang URL
With 953a693 when a route is not found we assume it is a website.page
that can be multilang.

But with odoo#31792 we should once again prefix URL by the language if
necessary.

The combination of the two cause issue when an url like `/web#home` is
tested since we do not take `#` into account, we check if the route is
multilang but no route `/web#home` is found => so we get
`/fr_FR/web#home`.

With this fix, `#fragment` is not taken into account when searching
route.

related to odoo#31792
related to opw-1922051
closes odoo#32059

robodoo pushed a commit that referenced this pull request Mar 22, 2019

[FIX] http_routing: ignore # when checking multilang URL
With 953a693 when a route is not found we assume it is a website.page
that can be multilang.

But with #31792 we should once again prefix URL by the language if
necessary.

The combination of the two cause issue when an url like `/web#home` is
tested since we do not take `#` into account, we check if the route is
multilang but no route `/web#home` is found => so we get
`/fr_FR/web#home`.

With this fix, `#fragment` is not taken into account when searching
route.

related to #31792
related to opw-1922051
closes #32059

Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com>

@robodoo robodoo added merged 🎉 and removed merging 👷 labels Mar 22, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 22, 2019

@nle-odoo nle-odoo deleted the odoo-dev:11.0-website-opw-1922051-nle branch Mar 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.