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

Saas 11.5 imp kwargs rde #27125

Closed
wants to merge 2 commits into
base: saas-11.5
from

Conversation

Projects
None yet
2 participants
@rdeodoo
Contributor

rdeodoo commented Sep 20, 2018

task-32836 + website selector fix

@rdeodoo rdeodoo requested a review from JKE-be Sep 20, 2018

@C3POdoo C3POdoo added the RD label Sep 20, 2018

rdeodoo added some commits Sep 19, 2018

[FIX] website: website forced on wrong session with website selector
Forcing website on session before navigating to the other website would not
work as expected if the other website has a different URL (hostname).
It would force the website on session on current website, then navigate to
second website which could still had another website in session.

Now, we force the website in session once we land on the website in the
dispatch() method.

Step to reproduce:
  - Set website1 to 127.0.0.10
  - Set website2 to 127.0.0.20
  - From website1 click on website2 in website selector
  - Website2 is forced on website1 session (wrong)
  - You land correctly on website2 and see website2
  - From there (website2) click on website1 in website selector
  - Website1 is forced on website2 session (wrong)
  - You land on website1 but see website2 since website2 was forced on
    website1 session at steps 3/4.

This is even more trickier when there is a third website without a domain in
the equation (meaning this website can be seen from website1 and website2
without changing URL (hostname)).
@rdeodoo

This comment has been minimized.

Show comment
Hide comment
@rdeodoo

rdeodoo Sep 25, 2018

Contributor

Improvement for later: Do not use fw in URL when domain does not change.
Working solution:

        var website_domain = ev.currentTarget.getAttribute('domain');
        if (website_domain && window.location.hostname !== website_domain) {
            // if domain unchanged, this line will do a nop while we need to refresh
            // the page to load the new forced website.
            var url = new URL($.param.querystring(window.location.href, {'fw': website_id_to_switch_to}));
            url.hostname = website_domain;
            window.location.href = url;
        }
        else {
            // Avoid showing `fw` in URL to force website as we won't change domain
            this._rpc({
                route: '/website/force_website',
                params: {
                    website_id: website_id_to_switch_to || false,
                },
            }).then(function () {
                // Remove `fw` from URL or it will force it back after the _rpc
                // Eg: Site1/dom1 -> Go to site2/dom2 -> Select site3/nodom -> still fw=2 in url that show site2
                var query_string = $.deparam.querystring();
                if (query_string.hasOwnProperty('fw')) {
                    delete query_string.fw;
                    var url = $.param.querystring(window.location.href.split('?')[0], query_string);
                    window.location.href = url;
                }
                else {
                    window.location.reload(true);
                }
            });
        }
Contributor

rdeodoo commented Sep 25, 2018

Improvement for later: Do not use fw in URL when domain does not change.
Working solution:

        var website_domain = ev.currentTarget.getAttribute('domain');
        if (website_domain && window.location.hostname !== website_domain) {
            // if domain unchanged, this line will do a nop while we need to refresh
            // the page to load the new forced website.
            var url = new URL($.param.querystring(window.location.href, {'fw': website_id_to_switch_to}));
            url.hostname = website_domain;
            window.location.href = url;
        }
        else {
            // Avoid showing `fw` in URL to force website as we won't change domain
            this._rpc({
                route: '/website/force_website',
                params: {
                    website_id: website_id_to_switch_to || false,
                },
            }).then(function () {
                // Remove `fw` from URL or it will force it back after the _rpc
                // Eg: Site1/dom1 -> Go to site2/dom2 -> Select site3/nodom -> still fw=2 in url that show site2
                var query_string = $.deparam.querystring();
                if (query_string.hasOwnProperty('fw')) {
                    delete query_string.fw;
                    var url = $.param.querystring(window.location.href.split('?')[0], query_string);
                    window.location.href = url;
                }
                else {
                    window.location.reload(true);
                }
            });
        }
@rdeodoo

This comment has been minimized.

Show comment
Hide comment
@rdeodoo

rdeodoo Sep 25, 2018

Contributor

Landed in saas-11.5 at 569554b and 4daceed

Contributor

rdeodoo commented Sep 25, 2018

Landed in saas-11.5 at 569554b and 4daceed

@rdeodoo rdeodoo closed this Sep 25, 2018

@rdeodoo rdeodoo deleted the odoo-dev:saas-11.5-imp-kwargs-rde branch Sep 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment