From 68197fde9ac0e144bbae4210dcbdf4141abb06b6 Mon Sep 17 00:00:00 2001 From: "Julien (jula)" Date: Wed, 30 Aug 2023 12:33:02 +0200 Subject: [PATCH] [FIX] product: fix crash when ptav excluded __Current behavior before commit:__ `_get_own_attribute_exclusions()` returns the product template attribute value (ptav) even when `ptav_active` is **false**. This makes the frontend crash when we add a product that has such ptav to a sale order. __Description of the fix:__ Prevent ptavs with `ptav_active = false` to be in the result of `_get_own_attribute_exclusions()`. __To reproduce:__ 1. Create new Product template 1. Add 2 attributes with at least 2 values each 1. Click on the Configure button next to the first attribute line 1. Click on the first attribute value 1. Add an exclusion line for the newly created product template and one of the value from the other attribute 1. Put `ptav_active` of the ptav to `false` (not sure how to do it except from the ORM but one customer managed to do it) 1. Try to add the product to a sale order line 1. `TypeError: Cannot set properties of undefined (setting 'excluded')` opw-3466451 closes odoo/odoo#133808 X-original-commit: cf58369310f13377cd70794bf777f4e9198f643a Signed-off-by: Victor Feyens (vfe) --- addons/product/models/product_template.py | 6 +- .../tests/test_product_configurator_data.py | 156 +++++++++++------- 2 files changed, 102 insertions(+), 60 deletions(-) diff --git a/addons/product/models/product_template.py b/addons/product/models/product_template.py index 5a2d9f1ad60..2fa08a4f7b2 100644 --- a/addons/product/models/product_template.py +++ b/addons/product/models/product_template.py @@ -835,12 +835,12 @@ class ProductTemplate(models.Model): product_template_attribute_values = self.valid_product_template_attribute_line_ids.product_template_value_ids return { ptav.id: [ - value_id + value.id for filter_line in ptav.exclude_for.filtered( lambda filter_line: filter_line.product_tmpl_id == self - ) for value_id in filter_line.value_ids.ids + ) for value in filter_line.value_ids if value.ptav_active ] - for ptav in product_template_attribute_values + for ptav in product_template_attribute_values if ptav.ptav_active } def _get_parent_attribute_exclusions(self, parent_combination): diff --git a/addons/sale_product_configurator/tests/test_product_configurator_data.py b/addons/sale_product_configurator/tests/test_product_configurator_data.py index de370a4b7e2..bb997bc9f69 100644 --- a/addons/sale_product_configurator/tests/test_product_configurator_data.py +++ b/addons/sale_product_configurator/tests/test_product_configurator_data.py @@ -9,47 +9,28 @@ from odoo.addons.sale.tests.common import SaleCommon @tagged('post_install', '-at_install') class TestProductConfiguratorData(HttpCase, ProductVariantsCommon, SaleCommon): - def test_dropped_value_isnt_shown(self): - self.assertEqual(len(self.product_template_sofa.product_variant_ids), 3) - - # Use variants s.t. they are archived and not deleted when value is removed - self.empty_order.order_line = [ - Command.create({ - 'product_id': product.id - }) - for product in self.product_template_sofa.product_variant_ids - ] - self.empty_order.action_confirm() - - # Remove attribute value - self.product_template_sofa.attribute_line_ids.value_ids -= self.color_attribute_red - self.assertEqual(len(self.product_template_sofa.product_variant_ids.filtered('active')), 2) - - self.authenticate('demo', 'demo') - base_url = self.product_template_sofa.get_base_url() + def request_get_values(self, product_template): + base_url = product_template.get_base_url() response = self.opener.post( url=base_url + '/sale_product_configurator/get_values', json={ - 'params': dict( - product_template_id=self.product_template_sofa.id, - quantity=1.0, - currency_id=1, - so_date=str(self.env.cr.now()), - product_uom_id=None, - company_id=None, - pricelist_id=None, - ptav_ids=None, - only_main_product=False, - ), + 'params': { + 'product_template_id': product_template.id, + 'quantity': 1.0, + 'currency_id': 1, + 'so_date': str(self.env.cr.now()), + 'product_uom_id': None, + 'company_id': None, + 'pricelist_id': None, + 'ptav_ids': None, + 'only_main_product': False, + }, } ) - result = response.json()['result'] + return response.json()['result'] - # Make sure the inactive ptav was removed from the loaded attributes - self.assertEqual(len(result['products'][0]['attribute_lines'][0]['attribute_values']), 2) - - def test_dropped_attribute(self): - self.product_template_2_attribute_lines = self.env['product.template'].create({ + def create_product_template_with_2_attributes(self): + return self.env['product.template'].create({ 'name': 'Shirt', 'categ_id': self.product_category.id, 'attribute_line_ids': [ @@ -73,41 +54,102 @@ class TestProductConfiguratorData(HttpCase, ProductVariantsCommon, SaleCommon): }), ], }) - self.assertEqual(len(self.product_template_2_attribute_lines.product_variant_ids), 4) + + def test_dropped_value_isnt_shown(self): + self.assertEqual(len(self.product_template_sofa.product_variant_ids), 3) # Use variants s.t. they are archived and not deleted when value is removed self.empty_order.order_line = [ Command.create({ 'product_id': product.id }) - for product in self.product_template_2_attribute_lines.product_variant_ids + for product in self.product_template_sofa.product_variant_ids + ] + self.empty_order.action_confirm() + + # Remove attribute value + self.product_template_sofa.attribute_line_ids.value_ids -= self.color_attribute_red + self.assertEqual(len(self.product_template_sofa.product_variant_ids.filtered('active')), 2) + + self.authenticate('demo', 'demo') + result = self.request_get_values(self.product_template_sofa) + + # Make sure the inactive ptav was removed from the loaded attributes + self.assertEqual(len(result['products'][0]['attribute_lines'][0]['attribute_values']), 2) + + def test_dropped_attribute(self): + product_template = self.create_product_template_with_2_attributes() + self.assertEqual(len(product_template.product_variant_ids), 4) + + # Use variants s.t. they are archived and not deleted when value is removed + self.empty_order.order_line = [ + Command.create({ + 'product_id': product.id + }) + for product in product_template.product_variant_ids ] self.empty_order.action_confirm() # Remove attribute - self.product_template_2_attribute_lines.attribute_line_ids[0].unlink() - self.assertEqual(len(self.product_template_2_attribute_lines.product_variant_ids), 2) + product_template.attribute_line_ids[0].unlink() + self.assertEqual(len(product_template.product_variant_ids), 2) self.authenticate('demo', 'demo') - base_url = self.product_template_2_attribute_lines.get_base_url() - response = self.opener.post( - url=base_url + '/sale_product_configurator/get_values', - json={ - 'params': { - 'product_template_id': self.product_template_2_attribute_lines.id, - 'quantity': 1.0, - 'currency_id': 1, - 'so_date': str(self.env.cr.now()), - 'product_uom_id': None, - 'company_id': None, - 'pricelist_id': None, - 'ptav_ids': None, - 'only_main_product': False, - }, - } - ) - result = response.json()['result'] + result = self.request_get_values(product_template) # Make sure archived combinations with inactive ptav are not loaded as it's useless to # exclude combinations that are not even available self.assertFalse(result['products'][0]['archived_combinations']) + + def test_excluded_inactive_ptav(self): + product_template = self.create_product_template_with_2_attributes() + self.assertEqual(len(product_template.product_variant_ids), 4) + + ptav_with_exclusion = product_template.attribute_line_ids[0].product_template_value_ids[0] + ptav_excluded = product_template.attribute_line_ids[1].product_template_value_ids[0] + + # Add an exclusion + ptav_with_exclusion.write({ + 'exclude_for': [ + Command.create({ + 'product_tmpl_id': product_template.id, + 'value_ids': [ + Command.set([ + ptav_excluded.id, + ]), + ], + }), + ], + }) + self.assertEqual(len(product_template.product_variant_ids), 3) + + self.authenticate('demo', 'demo') + result = self.request_get_values(product_template) + # The PTAVs should be mutually excluded + self.assertEqual(result['products'][0]['exclusions'] + [str(ptav_with_exclusion.id)], [ptav_excluded.id]) + self.assertEqual(result['products'][0]['exclusions'] + [str(ptav_excluded.id)], [ptav_with_exclusion.id]) + + ptav_with_exclusion.write({'ptav_active': False}) + result = self.request_get_values(product_template) + # The inactive PTAV should not be in the product exclusions dict + self.assertFalse(str(ptav_with_exclusion.id) in result['products'][0]['exclusions']) + # The inactive PTAV should not be in the exclusions of the excluded PTAV + self.assertEqual(result['products'][0]['exclusions'][str(ptav_excluded.id)], []) + + ptav_with_exclusion.write({'ptav_active': True}) + ptav_excluded.write({'ptav_active': False}) + result = self.request_get_values(product_template) + # The excluded inactive PTAV should not be in the exclusions of the first PTAV + self.assertEqual(result['products'][0]['exclusions'][str(ptav_with_exclusion.id)], []) + # The excluded inactive PTAV should not be in the product exclusions dict + self.assertFalse(str(ptav_excluded.id) in result['products'][0]['exclusions']) + + ptav_with_exclusion.write({'ptav_active': False}) + ptav_excluded.write({'ptav_active': False}) + result = self.request_get_values(product_template) + + # The inactive PTAVs should not be in the product exclusions dict + self.assertFalse(str(ptav_with_exclusion.id) in result['products'][0]['exclusions']) + self.assertFalse(str(ptav_excluded.id) in result['products'][0]['exclusions'])