Skip to content
Permalink
Browse files

[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>
  • Loading branch information...
nle-odoo committed Mar 22, 2019
1 parent 5324284 commit ad826f56ba49caba0881a40609083cf73250d610
Showing with 1 addition and 1 deletion.
  1. +1 −1 addons/http_routing/models/ir_http.py
@@ -164,7 +164,7 @@ def is_multilang_url(local_url, langs=None):
local_url = '/'.join(spath)
try:
# Try to match an endpoint in werkzeug's routing table
url = local_url.split('?')
url = local_url.partition('#')[0].split('?')
path = url[0]
query_string = url[1] if len(url) > 1 else None
router = request.httprequest.app.get_db_router(request.db).bind('')

10 comments on commit ad826f5

@wtaferner

This comment has been minimized.

Copy link
Contributor

wtaferner replied Mar 25, 2019

@nle-odoo @nim-odoo
Could it be that you have destroyed Stripe again? ;-)

loadJS is trying to load a multlang url now /de_DE/payment_stripe/static/src/js/stripe.js?_=1553506733734 which is not ending up well (HTTP 404). Basically it just works now on the second click only. Not sure, but maybe this and the other multilang fix caused this?

Please, try to fix this asap, we are currently investigating as well.

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

nle-odoo replied Mar 25, 2019

Hi, it looks like a combination of:

I will test and revert if reproduced.

@wtaferner

This comment has been minimized.

Copy link
Contributor

wtaferner replied Mar 25, 2019

@nle-odoo
Ok, the issue was/is that I did not update the module (Stripe) which is a bit cumbersome to be needed in stable, but of course this is the reason why it does not work from the start. Hmm, no idea if you have a solution for this.

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

nle-odoo replied Mar 25, 2019

Module update should not be necessary, since:

953a693

If a route is not found, we assume the URL is multilang and prefix with eg. /de_DE/

This was not an issue because in a previous commit, rewriting URL to prefix with /de_DE/ was lost.

The commit:
5324284

rewrite URL once again, so this explain that:

/de_DE/payment_stripe/static/src/js/stripe.js

is now considered multilang, but that should not cause an issue as far as I can tell, I am testing.

@wtaferner

This comment has been minimized.

Copy link
Contributor

wtaferner replied Mar 25, 2019

Hmm, our tests were only successful after changing the template code to load after ajax is loaded. The static sequential loading produced the non-existent language URL for the JS resource.

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

nle-odoo replied Mar 25, 2019

Ok, I have tested both, without stripe change:

with stripe change:

Can you give precision on how it is reproduced, when you are on the website you have /de_DE/ ? and do you have the stripe included on odoo or do you see the stripe payment popup like in this screenshot: https://i.imgur.com/UBPwxAU.png?

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

nle-odoo replied Mar 25, 2019

Hello,

I have been able to reproduce, I have created this revert PR:

and will check why this happen in between because there is a lot of other script link that do not have this issue.

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

nle-odoo replied Mar 25, 2019

Hi @wtaferner

It is reverted with 2a89f0c

From my tests this happened:

  • with the fix 5324284 applied and server restarted
  • without the fix 98fe505 applied and payment_stripe updated
  • with the XML cache cleared (I think it was why I could not reproduce)

I have a test PR (#32095) to reintroduce the reverted fix without introducing an error but it still needs:

  • tests
  • checking other possible use case
  • validation
@nhomar

This comment has been minimized.

Copy link
Contributor

nhomar replied Mar 25, 2019

dudes: I have a server down for this (I think) on odoo.sh, can you update que code on .sh servers ASAP please.!

@nle-odoo

This comment has been minimized.

Copy link
Contributor Author

nle-odoo replied Mar 26, 2019

Hello @nhomar,

Sorry for the late feedback, I am not sure this should apply. This fix was reverted yesterday because when visiting the website in a secondary language, we could get an issue when loading stripe.js file.

Please sign in to comment.
You can’t perform that action at this time.