From edce2fd6d09faee495c6278391f81c6fdf1e3be9 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Wed, 20 Feb 2019 14:31:02 +0000 Subject: [PATCH] [FIX] website: keep only specific views on _views_get recursive calls Before this commit: If a view A was doing a t-call on a view B and view B had view C as child. And view A had view D as child. And view D also t-call view B (that as mentionned above has view C as child). And view D was inactive. Then COWing C to set it as inactive would make `get_related_views()` on A to return both generic active C and COW inactive C. This rare case appeared for customize option that were enabled by default, like wishlist, compare.. Disabling this customize option would show both the generic customize view and the COW customize view. Resulting in the customize toggle being shown twice, one enabled and one disabled. Detailed explanation: 1. `website_sale.products` would call `_views_get()` with `options=True` for its t-call node `website_sale.products_item`, returning the full view tree, including COW wishlist. 2. `website_sale.products` would then call `_views_get()` on its child views with `options=extension.active`. It is the case for `website_sale.products_list_view`, which also has a t-call node `website_sale.products_item`. It would then perform the same `views_get()` as in step 1. but with `options` set to the view active state, in this case False. At that point, the `_view_get_inherited_children` method would filter by active before filter_duplicate, leaving the generic view over the specific one as it should. closes odoo/odoo#31286 --- addons/web_editor/models/ir_ui_view.py | 9 ++-- addons/website/tests/test_views.py | 70 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/addons/web_editor/models/ir_ui_view.py b/addons/web_editor/models/ir_ui_view.py index 24bb9884d1f..b870a57edf9 100644 --- a/addons/web_editor/models/ir_ui_view.py +++ b/addons/web_editor/models/ir_ui_view.py @@ -205,11 +205,7 @@ class IrUiView(models.Model): @api.model def _view_get_inherited_children(self, view, options): - extensions = view.inherit_children_ids - if not options: - # only active children - extensions = extensions.filtered(lambda view: view.active) - return extensions + return view.inherit_children_ids @api.model def _view_obj(self, view_id): @@ -257,6 +253,9 @@ class IrUiView(models.Model): views_to_return += self._views_get(called_view, options=options, bundles=bundles) extensions = self._view_get_inherited_children(view, options) + if not options: + # only active children + extensions = extensions.filtered(lambda view: view.active) # Keep options in a deterministic order regardless of their applicability for extension in extensions.sorted(key=lambda v: v.id): diff --git a/addons/website/tests/test_views.py b/addons/website/tests/test_views.py index 22e442448f9..0416bc72bf7 100644 --- a/addons/website/tests/test_views.py +++ b/addons/website/tests/test_views.py @@ -628,6 +628,76 @@ class TestCowViewSaving(common.TransactionCase): views = View.with_context(website_id=1).get_related_views('B') self.assertEqual(views.mapped('key'), ['B', 'I', 'II'], "Should only return the specific tree") + def test_get_related_views_tree_recursive_t_call_and_inherit_inactive(self): + """ If a view A was doing a t-call on a view B and view B had view C as child. + And view A had view D as child. + And view D also t-call view B (that as mentionned above has view C as child). + And view D was inactive (`d` in bellow schema). + + Then COWing C to set it as inactive would make `get_related_views()` on A to return + both generic active C and COW inactive C. + (Typically the case for Customize show on /shop for Wishlist, compare..) + See commit message for detailed explanation. + """ + # A -> B + # | ^ \ + # | | C + # d ___| + + View = self.env['ir.ui.view'] + Website = self.env['website'] + + products = View.create({ + 'name': 'Products', + 'type': 'qweb', + 'key': '_website_sale.products', + 'arch': ''' +
+ +
+ ''', + }) + + products_item = View.create({ + 'name': 'Products item', + 'type': 'qweb', + 'key': '_website_sale.products_item', + 'arch': ''' +
+ ''', + }) + + add_to_wishlist = View.create({ + 'name': 'Wishlist', + 'active': True, + 'customize_show': True, + 'inherit_id': products_item.id, + 'key': '_website_sale_wishlist.add_to_wishlist', + 'arch': ''' + + ''', + }) + + products_list_view = View.create({ + 'name': 'List View', + 'active': False, # <- That's the reason of why this behavior needed a fix + 'customize_show': True, + 'inherit_id': products.id, + 'key': '_website_sale.products_list_view', + 'arch': ''' +
+ +
+ ''', + }) + + views = View.with_context(website_id=1).get_related_views('_website_sale.products') + self.assertEqual(views, products + products_item + add_to_wishlist + products_list_view, "The four views should be returned.") + add_to_wishlist.with_context(website_id=1).write({'active': False}) # Trigger cow on hierarchy + add_to_wishlist_cow = Website.with_context(website_id=1).viewref(add_to_wishlist.key) + views = View.with_context(website_id=1).get_related_views('_website_sale.products') + self.assertEqual(views, products + products_item + add_to_wishlist_cow + products_list_view, "The generic wishlist view should have been replaced by the COW one.") + def test_cow_inherit_children_order(self): """ COW method should loop on inherit_children_ids in correct order when copying them on the new specific tree.