From 21c25a7ccd0ba2d6574ddbcfbcf50dbbc03a1e6c Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 11 Jan 2024 14:42:25 +0000 Subject: [PATCH] [FIX] website: select correct website in page list/kanban view This commit was written for Odoo 16.4 and then backported. In Odoo 16, only point 2. described just below is not working. The 16.4 commit was thus retarget to Odoo 16 as it was also fixing the second point. ------------------ Before this commit: The website selected in the website selector of the search view in the website.page list/kanban view was always set to the first one found in the DB and was never: 1. the one you visited in the website preview 2. or the one matching the URL of your backend. But 1. was working fine before commit [1] from the frameworkjs which introduced a way to "reset" the screen between apps/menu switch. Despite 1. working before commit [1], it was not ideally coded and worked "by chance", see below for a few facts and explication. ------------------ Fact 1: When you have multiple websites and you are on a website, the website served is the result of 2 possibilities: 1. All websites have a specific domain, then you can only see a given website on its own domain. Attempting to select another website in the website switcher will redirect you to that other website domain. 2. You have one or more websites without any domain, then you can see those ones from any domain by using the website switcher. It will force the website in session, and despite being on a domain which should serve a given website (the one which has its domain set that domain if there is one), it will serve you the one you selected in the website switcher. Fact 2: - It is the `website_preview` which is setting the currentWebsite property of the `website_service`. - But the `website_service` can be used on its own, without any `website_preview` being involved in the process. - When the website preview is unmounted, the currentWebsite from the website service is reset to null. - The website page list component is reading the currentWebsite from the website service. - When switching from the website preview to another menu like the website pages list, the page list component (PageControllerMixin) is actually initialized/created before the website preview is unmounted. In the end, the following happen: 1. Go to website preview -> it sets the website service current website 2. Go to website page list 3. The website page list component reads the current website from the website service which is set 4. The website preview is unmounted, emptying the current website from the website service 5. The website page list is shown to the user 6. Any call from the page list component to the current website will now be "wrong" / not return the same as during its `onWillStart`, as website preview was unmounted just after that, emptying the website set in the website service. Luckily, there is none. Fact 3: Commit [1] changed the order of the steps listed above, now 4 occurs before 3, so when the page list component reads the website from the website service, the unmount of the website preview already kicked in, emptying that website service website. This is the source of the bug fixed here: the current website could never be found anymore. ----------------- This commit is simply finding the current website_id by asking it to the server. It will fix point 1. listed at the very beginning of the commit message, but will also make point 2. work. Another solution would have been to find a workaround to the frameworkjs change (like investigating the `noEmptyTransition` option) to restore what we had before, but it would have been as "fragile" as before and wouldn't have fixed other flows as the current fix does. ---------------- Steps to reproduce 1: - Without any domain set, go to your DB in the website preview of your website in the backend - Switch to the website 2 in the navbar website switcher - You are now viewing website 2 in the website preview - Click on "Pages" in the "Site" menu to go to the page list view - The website selected in the search view is the first one, not the website 2. Also, the page shown are from the website 1, not the website 2. - This was working before commit [1] and this commit is fixing that. But this commit is also fixing/improving flows which never worked: - Before commit [1] (or in Odoo 16 to be simpler), do the same 4 first steps as above. - You will see that the listed pages are the ones from website 2 and that the selected website is the second one, correct. - Now just reload the page, it will show website 1, despite the website 2 being forced (if you did reload the page on the preview, it would still show website 2). - This is because the website_preview was never involved after the reload since you reached the list view / website service without going through the website preview. - The same can be seen if you go to the Pages list view directly through CTRL+K, in which case you won't go through the preview. -------------------- Note that the chance is taken to reorder the wutils export, it was becoming a real mess and it was not possible to easily read through the method names to find/guess something. ------------------ [1]: https://github.com/odoo/odoo/commit/9f6ed9f6d1ef7ec1870980498480cae0ffc729d8 Related to task-3676124 opw-3658648 closes odoo/odoo#151269 X-original-commit: 0128beed551395b30f957a5edc1a23a6f3d0ba31 Signed-off-by: Quentin Smetz (qsm) Signed-off-by: Romain Derie (rde) --- .../static/src/components/views/page_list.js | 2 - .../src/components/views/page_search_model.js | 17 +++++++- .../src/components/views/page_views_mixin.js | 5 ++- .../website/static/src/js/tours/tour_utils.js | 40 ++++++++++++++---- .../static/tests/tours/page_manager.js | 41 +++++++++++++++++++ .../tours/snippet_cache_across_websites.js | 18 +------- addons/website/tests/test_page_manager.py | 12 +++++- 7 files changed, 104 insertions(+), 31 deletions(-) diff --git a/addons/website/static/src/components/views/page_list.js b/addons/website/static/src/components/views/page_list.js index 71ec551b867..722caf7f537 100644 --- a/addons/website/static/src/components/views/page_list.js +++ b/addons/website/static/src/components/views/page_list.js @@ -6,7 +6,6 @@ import {PageSearchModel} from "./page_search_model"; import {registry} from '@web/core/registry'; import {listView} from '@web/views/list/list_view'; import {ConfirmationDialog} from "@web/core/confirmation_dialog/confirmation_dialog"; -import {useService} from "@web/core/utils/hooks"; import {DeletePageDialog, DuplicatePageDialog} from '@website/components/dialog/page_properties'; import {CheckboxItem} from "@web/core/dropdown/checkbox_item"; @@ -17,7 +16,6 @@ export class PageListController extends PageControllerMixin(listView.Controller) */ setup() { super.setup(); - this.orm = useService('orm'); if (this.props.resModel === "website.page") { this.archiveEnabled = false; } diff --git a/addons/website/static/src/components/views/page_search_model.js b/addons/website/static/src/components/views/page_search_model.js index ddb03dfa456..0eba2fd565f 100644 --- a/addons/website/static/src/components/views/page_search_model.js +++ b/addons/website/static/src/components/views/page_search_model.js @@ -21,8 +21,8 @@ export class PageSearchModel extends SearchModel { // Before the searchModel performs its DB search call, append the // website domain to the search domain. await this.website.fetchWebsites(); - const website = this.website.currentWebsite || this.website.websites[0]; - this.notifyWebsiteChange(website.id); + const website = await this.getCurrentWebsite(); + await this.notifyWebsiteChange(website.id); }); } @@ -87,4 +87,17 @@ export class PageSearchModel extends SearchModel { this.pagesState.websiteDomain = websiteDomain; this._notify(); } + + /** + * Retrieves the current website. + * + * @returns {Object} The current website. + */ + async getCurrentWebsite() { + const currentWebsite = (await this.orm.call('website', 'get_current_website')).match(/\d+/); + if (currentWebsite) { + return this.website.websites.find(w => w.id === parseInt(currentWebsite[0])); + } + return this.website.websites[0]; + } } diff --git a/addons/website/static/src/components/views/page_views_mixin.js b/addons/website/static/src/components/views/page_views_mixin.js index cfcda5cce9d..3007fecc080 100644 --- a/addons/website/static/src/components/views/page_views_mixin.js +++ b/addons/website/static/src/components/views/page_views_mixin.js @@ -21,6 +21,7 @@ export const PageControllerMixin = (component) => class extends component { this.website = useService('website'); this.dialog = useService('dialog'); this.rpc = useService('rpc'); + this.orm = useService('orm'); this.websiteSelection = odoo.debug ? [{id: 0, name: _t("All Websites")}] : []; @@ -29,9 +30,9 @@ export const PageControllerMixin = (component) => class extends component { }); onWillStart(async () => { - await this.website.fetchWebsites(); + // `fetchWebsites()` already done by parent PageSearchModel this.websiteSelection.push(...this.website.websites); - this.state.activeWebsite = this.website.currentWebsite || this.website.websites[0]; + this.state.activeWebsite = await this.env.searchModel.getCurrentWebsite(); }); } diff --git a/addons/website/static/src/js/tours/tour_utils.js b/addons/website/static/src/js/tours/tour_utils.js index 6ad171b066b..19fe57e05fd 100644 --- a/addons/website/static/src/js/tours/tour_utils.js +++ b/addons/website/static/src/js/tours/tour_utils.js @@ -420,6 +420,31 @@ function selectElementInWeSelectWidget(widgetName, elementName, searchNeeded = f return steps; } +/** + * Switches to a different website by clicking on the website switcher. + * + * @param {number} websiteId - The ID of the website to switch to. + * @param {string} websiteName - The name of the website to switch to. + * @returns {Array} - The steps required to perform the website switch. + */ +function switchWebsite(websiteId, websiteName) { + return [{ + content: `Click on the website switch to switch to website '${websiteName}'`, + trigger: '.o_website_switcher_container button', + }, { + content: `Switch to website '${websiteName}'`, + extra_trigger: `iframe html:not([data-website-id="${websiteId}"])`, + trigger: `.o_website_switcher_container .dropdown-item:contains("${websiteName}")`, + }, { + content: "Wait for the iframe to be loaded", + // The page reload generates assets for the new website, it may take + // some time + timeout: 20000, + trigger: `iframe html[data-website-id="${websiteId}"]`, + isCheck: true, + }]; +} + export default { addMedia, assertCssVariable, @@ -431,22 +456,23 @@ export default { changeImage, changeOption, changePaddingSize, - clickOnElement, clickOnEditAndWaitEditMode, + clickOnElement, + clickOnExtraMenuItem, clickOnSave, clickOnSnippet, clickOnText, dragNDrop, + getClientActionUrl, goBackToBlocks, goToTheme, + registerBackendAndFrontendTour, + registerThemeHomepageTour, + registerWebsitePreviewTour, selectColorPalette, + selectElementInWeSelectWidget, selectHeader, selectNested, selectSnippetColumn, - getClientActionUrl, - registerThemeHomepageTour, - clickOnExtraMenuItem, - registerWebsitePreviewTour, - registerBackendAndFrontendTour, - selectElementInWeSelectWidget, + switchWebsite, }; diff --git a/addons/website/static/tests/tours/page_manager.js b/addons/website/static/tests/tours/page_manager.js index 7134c071fa4..ef86f6526b3 100644 --- a/addons/website/static/tests/tours/page_manager.js +++ b/addons/website/static/tests/tours/page_manager.js @@ -1,6 +1,7 @@ /** @odoo-module **/ import wTourUtils from '@website/js/tours/tour_utils'; +import { registry } from "@web/core/registry"; // TODO: This part should be moved in a QUnit test const checkKanbanGroupBy = [{ @@ -107,3 +108,43 @@ wTourUtils.registerWebsitePreviewTour('website_page_manager', { run: () => null, }, ]); + +wTourUtils.registerWebsitePreviewTour('website_page_manager_session_forced', { + test: true, + url: '/', +}, () => [...wTourUtils.switchWebsite(2, 'My Website 2'), { + content: "Click on Site", + trigger: 'button.dropdown-toggle[data-menu-xmlid="website.menu_site"]', +}, { + content: "Click on Pages", + trigger: 'a.dropdown-item[data-menu-xmlid="website.menu_website_pages_list"]', +}, { + content: "Check that the homepage is the one of My Website 2", + trigger: ".o_list_table .o_data_row .o_data_cell[name=name]:contains('Home') " + + "~ .o_data_cell[name=website_id]:contains('My Website 2')", + run: () => null, // it's a check +}, { + content: "Click on the search options", + trigger: ".o_searchview_dropdown_toggler", +}, { + content: "Check that the selected website is My Website 2", + trigger: ".o_dropdown_container.o_website_menu > .dropdown-item:contains('My Website 2')", + run: () => null, // it's a check +}]); + +registry.category("web_tour.tours").add('website_page_manager_direct_access', { + test: true, + url: '/web#action=website.action_website_pages_list', + steps: () => [{ + content: "Check that the homepage is the one of My Website 2", + trigger: ".o_list_table .o_data_row .o_data_cell[name=name]:contains('Home') " + + "~ .o_data_cell[name=website_id]:contains('My Website 2')", + run: () => null, // it's a check +}, { + content: "Click on the search options", + trigger: ".o_searchview_dropdown_toggler", +}, { + content: "Check that the selected website is My Website 2", + trigger: ".o_dropdown_container.o_website_menu > .dropdown-item:contains('My Website 2')", + run: () => null, // it's a check +}]}); diff --git a/addons/website/static/tests/tours/snippet_cache_across_websites.js b/addons/website/static/tests/tours/snippet_cache_across_websites.js index 097be7dbfbb..b4b5d4422c7 100644 --- a/addons/website/static/tests/tours/snippet_cache_across_websites.js +++ b/addons/website/static/tests/tours/snippet_cache_across_websites.js @@ -14,23 +14,7 @@ wTourUtils.registerWebsitePreviewTour('snippet_cache_across_websites', { }, // There's no need to save, but canceling might or might not show a popup... ...wTourUtils.clickOnSave(), - { - content: "Click on the website switch to switch to website 2", - trigger: '.o_website_switcher_container button', - }, - { - content: "Switch to website 2", - // Ensure data-website-id exists - extra_trigger: 'iframe html[data-website-id="1"]', - trigger: '.o_website_switcher_container .dropdown-item:contains("My Website 2")' - }, - { - content: "Wait for the iframe to be loaded", - // The page reload generates assets for website 2, it may take some time - timeout: 20000, - trigger: 'iframe html:not([data-website-id="1"])', - run: () => null, - }, + ...wTourUtils.switchWebsite(2, 'My Website 2'), ...wTourUtils.clickOnEditAndWaitEditMode(), { content: "Check that the custom snippet is not here", diff --git a/addons/website/tests/test_page_manager.py b/addons/website/tests/test_page_manager.py index 92e6e6da287..b34a5abd879 100644 --- a/addons/website/tests/test_page_manager.py +++ b/addons/website/tests/test_page_manager.py @@ -3,6 +3,10 @@ import odoo.tests +from odoo.tests.common import HOST +from odoo.tools import config + + @odoo.tests.common.tagged('post_install', '-at_install') class TestWebsitePageManager(odoo.tests.HttpCase): @@ -13,4 +17,10 @@ class TestWebsitePageManager(odoo.tests.HttpCase): 'domain': '', 'sequence': 20, }) - self.start_tour(self.env['website'].get_client_action_url('/'), 'website_page_manager', login="admin") + url = self.env['website'].get_client_action_url('/') + self.start_tour(url, 'website_page_manager', login="admin") + self.start_tour(url, 'website_page_manager_session_forced', login="admin") + + alternate_website = self.env['website'].search([], limit=2)[1] + alternate_website.domain = f'http://{HOST}:{config["http_port"]}' + self.start_tour('/web#action=website.action_website_pages_list', 'website_page_manager_direct_access', login='admin')