From ceb1bebc0b2e54a3dbbeaecdc93a7cb534d85ad0 Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Mon, 5 Jun 2023 14:44:32 +0000 Subject: [PATCH] [REV] website_sale: stop handling exclusions in /shop Commit 91d0e9c645abdc0333c850a9fb9336272db02dd3 made sure that excluded combination did not appear in /shop search results, but it significantly slowed down searches, even in databases without exclusions. Since the excluded combinations cannot be added to the cart (and the original feedback did not come from an effective ticket), we believe the gain is not worth the cost. This commit reverts that change. Task-3326948 closes odoo/odoo#125722 X-original-commit: 3082ac99dc195a89e1299be73ef149f40231f86e Signed-off-by: Antoine Vandevenne (anv) Signed-off-by: Victor Feyens (vfe) --- addons/website_sale/controllers/main.py | 48 +--------- .../static/src/js/website_sale.js | 53 ++--------- .../website_sale_shop_variant_exclusion.js | 38 -------- addons/website_sale/tests/__init__.py | 1 - ...est_website_sale_shop_variant_exclusion.py | 93 ------------------- 5 files changed, 8 insertions(+), 225 deletions(-) delete mode 100644 addons/website_sale/static/tests/tours/website_sale_shop_variant_exclusion.js delete mode 100644 addons/website_sale/tests/test_website_sale_shop_variant_exclusion.py diff --git a/addons/website_sale/controllers/main.py b/addons/website_sale/controllers/main.py index 090c1a41f39..ebca503df00 100644 --- a/addons/website_sale/controllers/main.py +++ b/addons/website_sale/controllers/main.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from collections import defaultdict -from itertools import product as cartesian_product + import json import logging @@ -249,52 +248,7 @@ class WebsiteSale(http.Controller): order=self._get_search_order(post), options=options) search_result = details[0].get('results', request.env['product.template']).with_context(bin_size=True) - if attrib_set: - # Attributes value per attribute - attribute_values = request.env['product.attribute.value'].browse(attrib_set) - values_per_attribute = defaultdict(lambda: request.env['product.attribute.value']) - # In case we have only one value per attribute we can check for a combination using those attributes directly - multi_value_attribute = False - for value in attribute_values: - values_per_attribute[value.attribute_id] |= value - if len(values_per_attribute[value.attribute_id]) > 1: - multi_value_attribute = True - def filter_template(template, attribute_values_list): - # Transform product.attribute.value to product.template.attribute.value - attribute_value_to_ptav = dict() - for ptav in template.attribute_line_ids.product_template_value_ids: - attribute_value_to_ptav[ptav.product_attribute_value_id] = ptav.id - possible_combinations = False - for attribute_values in attribute_values_list: - ptavs = request.env['product.template.attribute.value'].browse( - [attribute_value_to_ptav[val] for val in attribute_values if val in attribute_value_to_ptav] - ) - if len(ptavs) < len(attribute_values): - # In this case the template is not compatible with this specific combination - continue - if len(ptavs) == len(template.attribute_line_ids): - if template._is_combination_possible(ptavs): - return True - elif len(ptavs) < len(template.attribute_line_ids): - if len(attribute_values_list) == 1: - if any(template._get_possible_combinations(necessary_values=ptavs)): - return True - if not possible_combinations: - possible_combinations = template._get_possible_combinations() - if any(len(ptavs & combination) == len(ptavs) for combination in possible_combinations): - return True - return False - - # If multi_value_attribute is False we know that we have our final combination (or at least a subset of it) - if not multi_value_attribute: - possible_attrib_values_list = [attribute_values] - else: - # Cartesian product from dict keys and values - possible_attrib_values_list = [request.env['product.attribute.value'].browse([v.id for v in values]) for - values in cartesian_product(*values_per_attribute.values())] - - search_result = search_result.filtered(lambda tmpl: filter_template(tmpl, possible_attrib_values_list)) return fuzzy_search_term, product_count, search_result def _shop_get_query_url_kwargs(self, category, search, min_price, max_price, attrib=None, order=None, **post): diff --git a/addons/website_sale/static/src/js/website_sale.js b/addons/website_sale/static/src/js/website_sale.js index 746d8dfbdcc..b868ad0099d 100644 --- a/addons/website_sale/static/src/js/website_sale.js +++ b/addons/website_sale/static/src/js/website_sale.js @@ -9,7 +9,6 @@ const cartHandlerMixin = wSaleUtils.cartHandlerMixin; import "web.zoomodoo"; import {extraMenuUpdateCallbacks} from "website.content.menu"; import dom from "web.dom"; -import { cartesian } from "@web/core/utils/arrays"; import { ComponentWrapper } from "web.OwlCompatibility"; import { ProductImageViewerWrapper } from "@website_sale/js/components/website_sale_image_viewer"; import { debounce, throttleForAnimation } from "@web/core/utils/timing"; @@ -694,61 +693,23 @@ publicWidget.registry.WebsiteSale = publicWidget.Widget.extend(VariantMixin, car $('.toggle_summary_div').toggleClass('d-none'); $('.toggle_summary_div').removeClass('d-xl-block'); }, - /** - * @private - */ - _isValidCombination(combination, attributeExclusions) { - if (attributeExclusions.exclusions) { - for (const attribute of combination) { - if (!attributeExclusions.exclusions.hasOwnProperty(attribute)) { - continue; - } - for (const excludedAttribute of attributeExclusions.exclusions[attribute]) { - if (combination.includes(excludedAttribute)) { - return false; - } - } - } - } - if (attributeExclusions.archived_combination) { - for (const archivedCombination of attributeExclusions.archived_combination) { - if (archivedCombination.length !== combination.length) { - continue; - } - if (archivedCombination.filter((attr) => combination.includes(attr)).length === combination.length) { - return false; - } - } - } - return true; - }, /** * @private */ _applyHashFromSearch() { const params = $.deparam(window.location.search.slice(1)); if (params.attrib) { - const attributeValuesPerAttribute = {}; - for (const attrib of params.attrib) { - const [ptalId, ptavId] = attrib.split('-'); - const attribValueSelector = `.js_variant_change[name="ptal-${ptalId}"][value="${ptavId}"]`; + const dataValueIds = []; + for (const attrib of [].concat(params.attrib)) { + const attribSplit = attrib.split('-'); + const attribValueSelector = `.js_variant_change[name="ptal-${attribSplit[0]}"][value="${attribSplit[1]}"]`; const attribValue = this.el.querySelector(attribValueSelector); if (attribValue !== null) { - if (!attributeValuesPerAttribute[ptalId]) { - attributeValuesPerAttribute[ptalId] = []; - } - attributeValuesPerAttribute[ptalId].push(ptavId); + dataValueIds.push(attribValue.dataset.value_id); } } - const attributeSelection = this.el.querySelector('.js_add_cart_variants'); - const attributeExclusions = attributeSelection && JSON.parse(attributeSelection.dataset.attribute_exclusions); - if (attributeExclusions && Object.values(attributeValuesPerAttribute).length > 1) { - const allCombinations = cartesian(...Object.values(attributeValuesPerAttribute)); - const selectedCombination = allCombinations.find(c => this._isValidCombination(c, attributeExclusions)); - - if (selectedCombination && selectedCombination.length) { - window.location.replace('#attr=' + selectedCombination.join(',')); - } + if (dataValueIds.length) { + window.location.hash = `attr=${dataValueIds.join(',')}`; } } this._applyHash(); diff --git a/addons/website_sale/static/tests/tours/website_sale_shop_variant_exclusion.js b/addons/website_sale/static/tests/tours/website_sale_shop_variant_exclusion.js deleted file mode 100644 index 1560cbc2a42..00000000000 --- a/addons/website_sale/static/tests/tours/website_sale_shop_variant_exclusion.js +++ /dev/null @@ -1,38 +0,0 @@ -/** @odoo-module **/ - -import { registry } from "@web/core/registry"; - -registry.category("web_tour.tours").add('shop_variant_exclusion', { - url: '/shop', - test: true, - steps: [ - { - content: "select product attribute First Attribute - Value 1", - trigger: 'form.js_attributes input:not(:checked) + label:contains(First Attribute - Value 1)', - }, - { - content: "select product attribute Second Attribute - Value 2", - trigger: 'form.js_attributes input:not(:checked) + label:contains(Second Attribute - Value 2)', - }, - { - content: "check for product template", - trigger: '[data-oe-expression="product.name"]:contains(Test Product)', - run: () => {} - }, - { - content: "deselect product attribute Second Attribute - Value 2", - trigger: 'form.js_attributes input:checked + label:contains(Second Attribute - Value 2)', - }, - { - content: "select product attribute Second Attribute - Value 1", - extra_trigger: 'body:not(:has(#customize-menu:visible .dropdown-menu:visible))', - trigger: 'form.js_attributes input:not(:checked) + label:contains(Second Attribute - Value 1)', - }, - { - content: "check for no product defined", - trigger: "h3:contains(No product defined)", - run: () => {} - }, - ] -}); - diff --git a/addons/website_sale/tests/__init__.py b/addons/website_sale/tests/__init__.py index d5671ee7747..5829d2d36d3 100644 --- a/addons/website_sale/tests/__init__.py +++ b/addons/website_sale/tests/__init__.py @@ -22,7 +22,6 @@ from . import test_website_sequence from . import test_website_sale_show_compare_list_price from . import test_website_sale_visitor from . import test_website_sale_product -from . import test_website_sale_shop_variant_exclusion from . import test_website_editor from . import test_website_sale_reorder_from_portal from . import test_website_sale_snippets diff --git a/addons/website_sale/tests/test_website_sale_shop_variant_exclusion.py b/addons/website_sale/tests/test_website_sale_shop_variant_exclusion.py deleted file mode 100644 index dffd57ce3f9..00000000000 --- a/addons/website_sale/tests/test_website_sale_shop_variant_exclusion.py +++ /dev/null @@ -1,93 +0,0 @@ -# Part of Odoo. See LICENSE file for full copyright and licensing details. - -from odoo.addons.base.tests.common import HttpCaseWithUserDemo, HttpCaseWithUserPortal -from odoo.tests import tagged - -@tagged('post_install', '-at_install') -class TestShopVariantExclusion(HttpCaseWithUserDemo, HttpCaseWithUserPortal): - - @classmethod - def setUpClass(cls): - super(TestShopVariantExclusion, cls).setUpClass() - # create a template - cls.product_template = cls.env['product.template'].create({ - 'name': 'Test Product', - 'is_published': True, - 'list_price': 750, - }) - - cls.first_product_attribute = cls.env['product.attribute'].create({ - 'name': 'First Attribute', - 'visibility': 'visible', - 'sequence': 10, - }) - - cls.second_product_attribute = cls.env['product.attribute'].create({ - 'name': 'Second Attribute', - 'visibility': 'visible', - }) - - cls.product_attribute_value_1 = cls.env['product.attribute.value'].create({ - 'name': 'First Attribute - Value 1', - 'attribute_id': cls.first_product_attribute.id, - 'sequence': 1, - }) - cls.product_attribute_value_2 = cls.env['product.attribute.value'].create({ - 'name': 'First Attribute - Value 2', - 'attribute_id': cls.first_product_attribute.id, - }) - cls.product_attribute_value_3 = cls.env['product.attribute.value'].create({ - 'name': 'Second Attribute - Value 1', - 'attribute_id': cls.second_product_attribute.id, - }) - cls.product_attribute_value_4 = cls.env['product.attribute.value'].create({ - 'name': 'Second Attribute - Value 2', - 'attribute_id': cls.second_product_attribute.id, - }) - - # set attribute and attribute values on the template - cls.env['product.template.attribute.line'].create([{ - 'attribute_id': cls.first_product_attribute.id, - 'product_tmpl_id': cls.product_template.id, - 'value_ids': [(6, 0, [cls.product_attribute_value_1.id, cls.product_attribute_value_2.id])] - }]) - - cls.env['product.template.attribute.line'].create([{ - 'attribute_id': cls.second_product_attribute.id, - 'product_tmpl_id': cls.product_template.id, - 'value_ids': [(6, 0, [cls.product_attribute_value_3.id, cls.product_attribute_value_4.id])] - }]) - - def _add_exclude(self, ptav1, ptav2, product_template): - ptav1.update({ - 'exclude_for': [(0, 0, { - 'product_tmpl_id': product_template.id, - 'value_ids': [(6, 0, [ptav2.id])] - })] - }) - - def _get_product_template_attribute_value(self, product_attribute_value, model): - """ - Return the `product.template.attribute.value` matching - `product_attribute_value` for self. - - :param: recordset of one product.attribute.value - :return: recordset of one product.template.attribute.value if found - else empty - """ - return model.valid_product_template_attribute_line_ids.filtered( - lambda l: l.attribute_id == product_attribute_value.attribute_id - ).product_template_value_ids.filtered( - lambda v: v.product_attribute_value_id == product_attribute_value - ) - - def test_admin_shop_variant_exclusion(self): - # Enable Variant Group - self.env.ref('product.group_product_variant').write({'users': [(4, self.env.ref('base.user_admin').id)]}) - ptav1 = self._get_product_template_attribute_value(self.product_attribute_value_1, self.product_template) - ptav2 = self._get_product_template_attribute_value(self.product_attribute_value_3, self.product_template) - self._add_exclude(ptav1, ptav2, self.product_template) - # Enable Product Attributes Left Panel - self.env['ir.ui.view'].with_context(active_test=False).search( - [('key', '=', 'website_sale.products_attributes')]).write({'active': True}) - self.start_tour("/", 'shop_variant_exclusion', login="admin")