From 41e2c4859bb46190f1ebcdba9df950d8b2a282ca Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 5 Jan 2024 17:54:34 +0100 Subject: [PATCH] [FIX] website: properly filter website.page records on list/kanban view Note: This fw-port commit cherry-picked and squashed commit [2] directly as it was fixing this original commit before it had the chance to be forward ported. Since commit [1], which adapted the website pages list view to OWL, the records listed on screen are filtered according to the active website filter. However, the full list of records is still used behind the scenes for all potential actions. Steps to reproduce: 1. Go to the pages list view. 2. Select a specific website (if it's not already the case). => The total of records in the upper right corner does not match the number of pages on that specific website. 3. Click on the "Select all" checkbox. => All the pages are selected, including those that do not appear on screen. This is because the records were just visually hidden with a `t-if`. [1]: https://github.com/odoo/odoo/commit/940f4ee875332dafa1f379970a7683be6b3ee606 [2]: https://github.com/odoo/odoo/commit/db670f64f4c2190f1655f9077ea62885049a3c84 Courtesy of @robinlej and @detrouxdev Related to task-3676124 opw-3554064 opw-3658648 closes odoo/odoo#149547 X-original-commit: 8d78a916dc8c0eca82e8a6a2c6f5541938709930 Signed-off-by: Quentin Smetz (qsm) Signed-off-by: Romain Derie (rde) --- addons/test_website/__manifest__.py | 4 + .../test_website/data/test_website_data.xml | 8 ++ .../test_website/data/test_website_demo.xml | 9 ++ addons/test_website/models/model.py | 7 +- .../static/tests/tours/page_manager.js | 70 +++++++++++++++ addons/test_website/tests/__init__.py | 1 + .../test_website/tests/test_page_manager.py | 26 ++++++ .../test_website/views/test_model_views.xml | 81 +++++++++++++++++ addons/website/models/website.py | 19 ++++ .../src/components/views/page_kanban.js | 3 + .../src/components/views/page_kanban.xml | 4 + .../static/src/components/views/page_list.js | 3 + .../static/src/components/views/page_list.xml | 3 + .../src/components/views/page_search_model.js | 90 +++++++++++++++++++ .../src/components/views/page_views_mixin.js | 2 + addons/website/views/website_pages_views.xml | 3 +- .../website_forum/views/forum_post_views.xml | 2 +- 17 files changed, 332 insertions(+), 3 deletions(-) create mode 100644 addons/test_website/data/test_website_demo.xml create mode 100644 addons/test_website/static/tests/tours/page_manager.js create mode 100644 addons/test_website/tests/test_page_manager.py create mode 100644 addons/test_website/views/test_model_views.xml create mode 100644 addons/website/static/src/components/views/page_search_model.js diff --git a/addons/test_website/__manifest__.py b/addons/test_website/__manifest__.py index 01eeed34968..290caa3fbc0 100644 --- a/addons/test_website/__manifest__.py +++ b/addons/test_website/__manifest__.py @@ -17,8 +17,12 @@ models which only purpose is to run tests.""", 'website', 'theme_default', ], + 'demo': [ + 'data/test_website_demo.xml', + ], 'data': [ 'views/templates.xml', + 'views/test_model_views.xml', 'data/test_website_data.xml', 'security/ir.model.access.csv', ], diff --git a/addons/test_website/data/test_website_data.xml b/addons/test_website/data/test_website_data.xml index 3217fcf42f1..8fe80c15e40 100644 --- a/addons/test_website/data/test_website_data.xml +++ b/addons/test_website/data/test_website_data.xml @@ -10,6 +10,14 @@ + + + Test Model Generic + + + Test Model Website 1 + + diff --git a/addons/test_website/data/test_website_demo.xml b/addons/test_website/data/test_website_demo.xml new file mode 100644 index 00000000000..73e6a1d0287 --- /dev/null +++ b/addons/test_website/data/test_website_demo.xml @@ -0,0 +1,9 @@ + + + + + Test Model Website 2 + + + + diff --git a/addons/test_website/models/model.py b/addons/test_website/models/model.py index 3c7150f6448..2cd157a37a1 100644 --- a/addons/test_website/models/model.py +++ b/addons/test_website/models/model.py @@ -10,12 +10,17 @@ class TestModel(models.Model): _name = 'test.model' _inherit = [ 'website.seo.metadata', - 'website.published.mixin', + 'website.published.multi.mixin', 'website.searchable.mixin', ] _description = 'Website Model Test' name = fields.Char(required=True) + # `cascade` is needed as there is demo data for this model which are bound + # to website 2 (demo website). But some tests are unlinking the website 2, + # which would fail if the `cascade` is not set. Note that the website 2 is + # never set on any records in all other modules. + website_id = fields.Many2one('website', string='Website', ondelete='cascade') @api.model def _search_get_detail(self, website, order, options): diff --git a/addons/test_website/static/tests/tours/page_manager.js b/addons/test_website/static/tests/tours/page_manager.js new file mode 100644 index 00000000000..50d7856a05e --- /dev/null +++ b/addons/test_website/static/tests/tours/page_manager.js @@ -0,0 +1,70 @@ +/** @odoo-module **/ + +import { registry } from "@web/core/registry"; + +registry.category("web_tour.tours").add('test_website_page_manager', { + test: true, + url: '/web#action=test_website.action_test_model', + steps: () => [ +// Part 1: check that the website filter is working +{ + content: "Check that we see records from My Website", + trigger: ".o_list_table .o_data_row .o_data_cell[name=name]:contains('Test Model Website 1') " + + "~ .o_data_cell[name=website_id]:contains('My Website')", + run: () => null, // it's a check +}, { + content: "Check that there is only 2 records in the pager", + trigger: ".o_pager .o_pager_value:contains('1-2')", + run: () => null, // it's a check +}, { + content: "Click on the 'Select all records' checkbox", + trigger: "thead .o_list_record_selector", +}, { + content: "Check that there is only 2 records selected", + trigger: ".o_list_selection_box:contains('2 selected')", + run: () => null, // it's a check +}, { + content: "Click on the 'Select all records' checkbox again to unselect all records and see the search bar", + trigger: "thead .o_list_record_selector", +}, { + content: "Click on the search options", + trigger: ".o_searchview_dropdown_toggler", +}, { + content: "Select My Website 2", + trigger: ".o_dropdown_container.o_website_menu > .dropdown-item:contains('My Website 2')", +}, { + // This step is just here to ensure there is more records than the 2 + // available on website 1, to ensure the test is actually testing something. + content: "Check that we see records from My Website 2", + trigger: ".o_list_table .o_data_row .o_data_cell[name=name]:contains('Test Model Website 2') " + + "~ .o_data_cell[name=website_id]:contains('My Website 2')", + run: () => null, // it's a check +}, +// Part 2: ensure Kanban View is working / not crashing +{ + content: "Click on Kanban View", + trigger: '.o_cp_switch_buttons .o_kanban', +}, { + content: "Click on List View", + extra_trigger: '.o_kanban_renderer', + trigger: '.o_cp_switch_buttons .o_list', +}, { + content: "Wait for List View to be loaded", + trigger: '.o_list_renderer', + run: () => null, // it's a check +}] +}); + +registry.category("web_tour.tours").add('test_website_page_manager_js_class_bug', { + test: true, + url: '/web#action=test_website.action_test_model_js_class_bug', + steps: () => [ +{ + content: "Click on Kanban View", + trigger: '.o_cp_switch_buttons .o_kanban', +}, { + content: "Wait for Kanban View to be loaded", + trigger: '.o_kanban_renderer', + run: () => null, // it's a check +}] +}); diff --git a/addons/test_website/tests/__init__.py b/addons/test_website/tests/__init__.py index 977fcd95c7c..58eb63a6e22 100644 --- a/addons/test_website/tests/__init__.py +++ b/addons/test_website/tests/__init__.py @@ -9,6 +9,7 @@ from . import test_image_upload_progress from . import test_is_multilang from . import test_media from . import test_multi_company +from . import test_page_manager from . import test_page from . import test_performance from . import test_qweb diff --git a/addons/test_website/tests/test_page_manager.py b/addons/test_website/tests/test_page_manager.py new file mode 100644 index 00000000000..80d460f0924 --- /dev/null +++ b/addons/test_website/tests/test_page_manager.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import odoo.tests + + +@odoo.tests.common.tagged('post_install', '-at_install') +class TestWebsitePageManager(odoo.tests.HttpCase): + def test_page_manager_test_model(self): + if self.env['website'].search_count([]) == 1: + website2 = self.env['website'].create({ + 'name': 'My Website 2', + 'domain': '', + 'sequence': 20, + }) + else: + website2 = self.env['website'].search([], order='id desc', limit=1) + self.env['test.model'].create({'name': 'Test Model Website 2', 'website_id': website2.id}) + self.assertTrue( + len(set([t.website_id.id for t in self.env['test.model'].search([])])) >= 3, + "There should at least be one record without website_id and one for 2 different websites", + ) + self.start_tour('/web#action=test_website.action_test_model', 'test_website_page_manager', login="admin") + # This second test is about ensuring that you can switch from a list + # view which has no `website_pages_list` js_class to its kanban view + self.start_tour('/web#action=test_website.action_test_model_js_class_bug', 'test_website_page_manager_js_class_bug', login="admin") diff --git a/addons/test_website/views/test_model_views.xml b/addons/test_website/views/test_model_views.xml new file mode 100644 index 00000000000..0b3bebe3d2d --- /dev/null +++ b/addons/test_website/views/test_model_views.xml @@ -0,0 +1,81 @@ + + + + + + test.model.kanban + test.model + + + + + + +
+
+ + +
+ + +
+
+
+
+ + Published + Not Published +
+
+
+
+
+
+
+ + Test Model Pages Tree + test.model + 99 + + + + + + + + + + Test Model Pages + test.model + tree,kanban,form + + + + + + Test Model Pages Tree js_class bug + test.model + 99 + + + + + + + + + + + Test Model Pages js_class bug + test.model + tree,kanban,form + + + +
diff --git a/addons/website/models/website.py b/addons/website/models/website.py index e6f1e228f67..20b64f01ca1 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -1367,6 +1367,25 @@ class Website(models.Model): record['lastmod'] = page['write_date'].date() yield record + def get_website_page_ids(self): + if not self.env.user.has_group('website.group_website_restricted_editor'): + # Note that `website.pages` have `0,0,0,0` ACL rights by default for + # everyone except for the website designer which receive `1,0,0,0`. + # So the "Website/Site/Content/Pages" menu to reach the page manager + # is not shown to the restricted users, as the action linked model + # (website.page) can't be access. It's how the Odoo framework works. + # Still, we let the restricted editor access this resource for + # custos granting them read and/or write access on page. + raise AccessError(_("Access Denied")) + + domain = [('url', '!=', False)] + if self: + domain = AND([domain, self.website_domain()]) + pages = self.env['website.page'].sudo().search(domain) + if self: + pages = pages._get_most_specific_pages() + return pages.ids + def _get_website_pages(self, domain=None, order='name', limit=None): if domain is None: domain = [] diff --git a/addons/website/static/src/components/views/page_kanban.js b/addons/website/static/src/components/views/page_kanban.js index 1ae9ef607ba..7e346fa60de 100644 --- a/addons/website/static/src/components/views/page_kanban.js +++ b/addons/website/static/src/components/views/page_kanban.js @@ -1,6 +1,7 @@ /** @odoo-module **/ import {PageControllerMixin, PageRendererMixin} from "./page_views_mixin"; +import {PageSearchModel} from "./page_search_model"; import {registry} from '@web/core/registry'; import {kanbanView} from "@web/views/kanban/kanban_view"; import {CheckboxItem} from "@web/core/dropdown/checkbox_item"; @@ -19,6 +20,7 @@ PageKanbanController.components = { CheckboxItem, }; +// TODO master: remove `PageRendererMixin` extend, props override and template export class PageKanbanRenderer extends PageRendererMixin(kanbanView.Renderer) {} PageKanbanRenderer.props = [ ...kanbanView.Renderer.props, @@ -30,6 +32,7 @@ export const PageKanbanView = { ...kanbanView, Renderer: PageKanbanRenderer, Controller: PageKanbanController, + SearchModel: PageSearchModel, }; registry.category("views").add("website_pages_kanban", PageKanbanView); diff --git a/addons/website/static/src/components/views/page_kanban.xml b/addons/website/static/src/components/views/page_kanban.xml index 258145ce41f..63025ef7092 100644 --- a/addons/website/static/src/components/views/page_kanban.xml +++ b/addons/website/static/src/components/views/page_kanban.xml @@ -1,15 +1,19 @@ + recordFilter(record, props.list.records) + recordFilter(groupOrRecord.record, props.list.records) + + state.activeWebsite diff --git a/addons/website/static/src/components/views/page_list.js b/addons/website/static/src/components/views/page_list.js index 3ea9a8ec484..71ec551b867 100644 --- a/addons/website/static/src/components/views/page_list.js +++ b/addons/website/static/src/components/views/page_list.js @@ -2,6 +2,7 @@ import { _t } from "@web/core/l10n/translation"; import {PageControllerMixin, PageRendererMixin} from "./page_views_mixin"; +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"; @@ -94,6 +95,7 @@ PageListController.components = { CheckboxItem, }; +// TODO master: remove `PageRendererMixin` extend and props override export class PageListRenderer extends PageRendererMixin(listView.Renderer) {} PageListRenderer.props = [ ...listView.Renderer.props, @@ -105,6 +107,7 @@ export const PageListView = { ...listView, Renderer: PageListRenderer, Controller: PageListController, + SearchModel: PageSearchModel, }; registry.category("views").add("website_pages_list", PageListView); diff --git a/addons/website/static/src/components/views/page_list.xml b/addons/website/static/src/components/views/page_list.xml index d8d3d36f921..03c820406e0 100644 --- a/addons/website/static/src/components/views/page_list.xml +++ b/addons/website/static/src/components/views/page_list.xml @@ -7,12 +7,15 @@ + recordFilter(record, list.records) + + state.activeWebsite diff --git a/addons/website/static/src/components/views/page_search_model.js b/addons/website/static/src/components/views/page_search_model.js new file mode 100644 index 00000000000..ddb03dfa456 --- /dev/null +++ b/addons/website/static/src/components/views/page_search_model.js @@ -0,0 +1,90 @@ +/** @odoo-module */ + +import { useService } from "@web/core/utils/hooks"; +import { Domain } from '@web/core/domain'; +import { SearchModel } from '@web/search/search_model'; +import { onWillStart, useState } from "@odoo/owl"; + +export class PageSearchModel extends SearchModel { + /** + * @override + */ + setup() { + super.setup(...arguments); + this.website = useService('website'); + + this.rpc = useService('rpc'); + this.pagesState = useState({ + websiteDomain: false, + }); + onWillStart(async () => { + // 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); + }); + } + + /** + * @override + */ + exportState() { + const state = super.exportState(); + state.websiteDomain = this.pagesState.websiteDomain; + return state; + } + + /** + * @override + */ + _importState(state) { + super._importState(...arguments); + + if (state.websiteDomain) { + this.pagesState.websiteDomain = state.websiteDomain; + } + } + + /** + * @override + */ + _getDomain(params = {}) { + let domain = super._getDomain(params); + if (!this.pagesState.websiteDomain) { + return domain; + } + + domain = Domain.and([ + domain, + this.pagesState.websiteDomain, + ]); + return params.raw ? domain : domain.toList(); + } + + /** + * Updates the website domain state and notifies the change. That domain + * state will be appended to the base SearchModel domain. + * + * @param {number} websiteId - The ID of the website. + */ + async notifyWebsiteChange(websiteId) { + let websiteDomain = []; + if (websiteId) { + if (this.resModel === 'website.page') { + // In case of `website.page`, we can't find the website pages + // with a regular domain (because we need to filter duplicates). + const pageIds = await this.orm.call( + "website", + "get_website_page_ids", + [websiteId], + ); + websiteDomain = [['id', 'in', pageIds]]; + } else { + websiteDomain = [['website_id', 'in', [false, websiteId]]]; + } + } + this.pagesState.websiteDomain = websiteDomain; + this._notify(); + } +} 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 efac083c490..cfcda5cce9d 100644 --- a/addons/website/static/src/components/views/page_views_mixin.js +++ b/addons/website/static/src/components/views/page_views_mixin.js @@ -75,9 +75,11 @@ export const PageControllerMixin = (component) => class extends component { onSelectWebsite(website) { this.state.activeWebsite = website; + this.env.searchModel.notifyWebsiteChange(website.id); } }; +// TODO: Remove in master, records are not hidden through `t-if` anymore. export const PageRendererMixin = (component) => class extends component { /** * The goal here is to tweak the renderer to display records following some diff --git a/addons/website/views/website_pages_views.xml b/addons/website/views/website_pages_views.xml index 31e6237496f..5dfc91b049d 100644 --- a/addons/website/views/website_pages_views.xml +++ b/addons/website/views/website_pages_views.xml @@ -79,7 +79,8 @@ - diff --git a/addons/website_forum/views/forum_post_views.xml b/addons/website_forum/views/forum_post_views.xml index ccfcb052b89..98b26cf56ec 100644 --- a/addons/website_forum/views/forum_post_views.xml +++ b/addons/website_forum/views/forum_post_views.xml @@ -110,7 +110,7 @@ forum.post 99 - +