From f8dad1bb4b712d8f34035420ef1f6e484c638f5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Tue, 18 Dec 2018 16:21:32 +0000 Subject: [PATCH] [FIX] product,(website_)sale(*): fix product configurator This commit has several parts: - fix ordering of attributes and values - fix/create python logic (notably for checking the possibility of a combination and for price computing) - fix currency - display correct info on the views without the need of JS (prevent flickering) - fix various issues with website and SO - add a lot of tests (both unit and tour) ================================================================================ *** Make consistent the filtering and ordering of product variants *** Filter & Check possibility ========================== The first variant shown on the website should always be possible. When activating the customize "list view of variants", we don't want to show impossible combination on the list. We also don't want the user to be able to add to cart an impossible variant. Show appropriate messages when no combination is possible. Order ===== We want the variant order to be consistent with what the user selected with the sequence fields. The display order on the website should be the same than the display on the various backend views. This implies also fixing the order of: - product.attribute - product.attribute.value - product.template.attribute.line - product.template.attribute.value The general order is now: (parent,) sequence, id. Before, it was already a mix of those, but not always in the same order, and not every key for every model. The name has been removed from the order: either we sequence them correctly, or we want them in the creation order. On the website, the first combination shown by the python should be the same as the one that will be computed by the JS to prevent flickering. Ideally there should be no RPC at page loading if the python has computed everything correctly, but it is currently not possible to achieve because the view logic for website_sale_stock is only in JS. ================================================================================ *** Fix product template and variant price computation *** Move and improve the get_combination_info method: - take into account tax included/excluded settings (B2B/B2C) - take into account pricelist discount policy (striked price) - fix inconsistencies when price after pricelist would be 0 - use proper rounding and discount compute with the currency helper methods - fix an issue where context "partner" would have a mix of id and recordset ================================================================================ *** Fix currency conversions *** The list_price and lst_price of a product are always encoded using the currency of the product. The results of "combination info" are always returned using the currency of the pricelist if given, or the currency of the product. The prices on an order (line) are encoded with the currency of its pricelist. ================================================================================ *** Fix dynamic variants access rights *** Public users on the e-commerce should be able to create product variants if the configuration of the attributes of the product template is set as "dynamic". This uses "sudo" to bypass create security rules for public users. Multiple checks are done to ensure that it creates a valid product variant. ================================================================================ *** Miscellaneous bugs, tracebacks and necessary improvements *** In general: - correctly show warning when no variant is possible - don't let user add to cart an impossible variant (from optional product modal or from any RPC), both in interface and controller logic - fix as much flickering as possible by correctly pre-computing in python/views - move business logic from controllers to models - correctly add no_variant and is_custom values on the SO line description Sale order: - fix traceback when emptying the product_id field from configurator website_sale: - correctly compute first image / carousel, and correctly swap images when changing variants, without duplicate image or zoom problems - don't use list view of variants when it doesn't make sense - added missing product info when adding a no_variant attribute to cart - fix cart accessory product computation website_sale_wishlist: - fix it with dynamic variants, no_variant and is_custom website_sale_comparison: - fix it with dynamic variants (comparison and alternative products) website_sale_stock: - fix it in general (some flickering still present) ================================================================================ Authored by @seb-odoo with contribution from @awa-odoo task-1910821 closes odoo/odoo#28079 --- addons/event_sale/models/sale_order.py | 2 +- addons/mrp/models/mrp_bom.py | 2 +- addons/product/data/product_demo.xml | 6 + addons/product/i18n/product.pot | 6 + addons/product/models/product.py | 65 ++- addons/product/models/product_attribute.py | 68 ++- addons/product/models/product_template.py | 506 +++++++++++++++--- addons/product/tests/common.py | 1 + .../test_product_attribute_value_config.py | 431 +++++++++++---- .../product/views/product_attribute_views.xml | 4 +- .../sale/controllers/product_configurator.py | 218 +++----- addons/sale/data/sale_demo.xml | 1 + addons/sale/models/analytic.py | 2 +- addons/sale/models/product_product.py | 8 + addons/sale/models/product_template.py | 220 +++++++- addons/sale/models/sale.py | 92 +++- .../src/js/product_configurator_controller.js | 14 +- .../src/js/product_configurator_mixin.js | 197 ++++--- .../src/js/product_configurator_modal.js | 24 +- addons/sale/static/src/js/tour.js | 2 +- .../static/src/scss/product_configurator.scss | 18 +- addons/sale/tests/__init__.py | 1 + ...est_sale_product_attribute_value_config.py | 343 ++++++++++++ .../sale_product_configurator_templates.xml | 166 +++--- .../sale/wizard/sale_product_configurator.py | 2 +- .../product_configurator_advanced_test_ui.js | 4 +- .../product_configurator_pricelist_test_ui.js | 100 ++++ .../tests/product_configurator_test_ui.js | 25 +- addons/sale_management/tests/test_sale_ui.py | 41 ++ addons/sale_management/views/assets.xml | 3 +- addons/sale_stock/static/src/js/tour.js | 14 + addons/website_sale/controllers/main.py | 89 ++- addons/website_sale/i18n/website_sale.pot | 12 + addons/website_sale/models/product.py | 199 +++++-- addons/website_sale/models/sale_order.py | 105 +++- .../static/src/js/website_sale.js | 168 ++++-- .../static/src/js/website_sale_options.js | 3 +- .../static/src/js/website_sale_tour_buy.js | 6 +- ...e_sale_tour_shop_custom_attribute_value.js | 4 +- .../js/website_sale_tour_shop_customize.js | 97 +--- ...ale_tour_shop_deleted_archived_variants.js | 51 ++ ...website_sale_tour_shop_dynamic_variants.js | 36 ++ ...ite_sale_tour_shop_no_variant_attribute.js | 39 ++ .../static/src/scss/website_sale.scss | 5 + addons/website_sale/tests/__init__.py | 2 + addons/website_sale/tests/test_customize.py | 198 +++++++ .../tests/test_website_sale_pricelist.py | 6 +- ...ite_sale_product_attribute_value_config.py | 68 +++ .../tests/test_website_sale_zoom.py | 3 + addons/website_sale/views/templates.xml | 184 +++++-- .../controllers/main.py | 7 +- .../models/website_sale_comparison.py | 2 +- .../website_sale_comparison_template.xml | 39 +- addons/website_sale_stock/controllers/main.py | 31 +- .../models/product_template.py | 32 +- .../src/js/product_configurator_mixin.js | 9 +- .../website_sale_wishlist/controllers/main.py | 4 +- .../models/product_wishlist.py | 9 + .../static/src/js/website_sale_wishlist.js | 16 +- .../views/website_sale_wishlist_template.xml | 13 +- 60 files changed, 3135 insertions(+), 888 deletions(-) create mode 100644 addons/sale/tests/test_sale_product_attribute_value_config.py create mode 100644 addons/sale_management/static/tests/product_configurator_pricelist_test_ui.js create mode 100644 addons/website_sale/static/src/js/website_sale_tour_shop_deleted_archived_variants.js create mode 100644 addons/website_sale/static/src/js/website_sale_tour_shop_dynamic_variants.js create mode 100644 addons/website_sale/static/src/js/website_sale_tour_shop_no_variant_attribute.js create mode 100644 addons/website_sale/tests/test_website_sale_product_attribute_value_config.py diff --git a/addons/event_sale/models/sale_order.py b/addons/event_sale/models/sale_order.py index c41a6218f10..b749f615293 100644 --- a/addons/event_sale/models/sale_order.py +++ b/addons/event_sale/models/sale_order.py @@ -86,6 +86,6 @@ class SaleOrderLine(models.Model): lang=self.order_id.partner_id.lang, ) - return ticket.get_ticket_multiline_description_sale() + return ticket.get_ticket_multiline_description_sale() + self._get_sale_order_line_multiline_description_variants() else: return super(SaleOrderLine, self).get_sale_order_line_multiline_description_sale(product) diff --git a/addons/mrp/models/mrp_bom.py b/addons/mrp/models/mrp_bom.py index ea084066f48..a7c18ef1117 100644 --- a/addons/mrp/models/mrp_bom.py +++ b/addons/mrp/models/mrp_bom.py @@ -291,7 +291,7 @@ class MrpBomLine(models.Model): @api.onchange('parent_product_tmpl_id') def onchange_parent_product(self): return {'domain': {'attribute_value_ids': [ - ('id', 'in', self.parent_product_tmpl_id.mapped('attribute_line_ids.value_ids.id')), + ('id', 'in', self.parent_product_tmpl_id._get_valid_product_attribute_values().ids), ('attribute_id.create_variant', '!=', 'no_variant') ]}} diff --git a/addons/product/data/product_demo.xml b/addons/product/data/product_demo.xml index d4299a2dd51..c1b75e8290e 100644 --- a/addons/product/data/product_demo.xml +++ b/addons/product/data/product_demo.xml @@ -128,26 +128,32 @@ Legs + 1 Steel + 1 Aluminium + 2 Color + 2 White + 1 Black + 2 diff --git a/addons/product/i18n/product.pot b/addons/product/i18n/product.pot index c2a2400986e..28b00fb9384 100644 --- a/addons/product/i18n/product.pot +++ b/addons/product/i18n/product.pot @@ -2008,6 +2008,12 @@ msgstr "" msgid "The number of variants to generate is too high. You should either not generate variants for each combination or generate them on demand from the sales order. To do so, open the form view of attributes and change the mode of *Create Variants*." msgstr "" +#. module: product +#: code:addons/product/models/product_attribute.py +#, python-format +msgid "You are trying to change the type of an attribute value still referenced on at least one product template: %s" +msgstr "" + #. module: product #: code:addons/product/models/product_attribute.py:56 #, python-format diff --git a/addons/product/models/product.py b/addons/product/models/product.py index dadb60c4ce6..c0c4dde774f 100644 --- a/addons/product/models/product.py +++ b/addons/product/models/product.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import logging import re from odoo import api, fields, models, tools, _ @@ -11,6 +12,8 @@ from odoo.addons import decimal_precision as dp from odoo.tools import float_compare, pycompat +_logger = logging.getLogger(__name__) + class ProductCategory(models.Model): _name = "product.category" @@ -83,13 +86,16 @@ class ProductProduct(models.Model): _inherit = ['mail.thread', 'mail.activity.mixin'] _order = 'default_code, name, id' + # price: total price, context dependent (partner, pricelist, quantity) price = fields.Float( 'Price', compute='_compute_product_price', digits=dp.get_precision('Product Price'), inverse='_set_product_price') + # price_extra: catalog extra value only, sum of variant extra attributes price_extra = fields.Float( 'Variant Price Extra', compute='_compute_product_price_extra', digits=dp.get_precision('Product Price'), help="This is the sum of the extra price of all attributes") + # lst_price: catalog value + extra, context dependent (uom) lst_price = fields.Float( 'Sale Price', compute='_compute_product_lst_price', digits=dp.get_precision('Product Price'), inverse='_set_product_lst_price', @@ -163,8 +169,8 @@ class ProductProduct(models.Model): pricelist_id_or_name = self._context.get('pricelist') if pricelist_id_or_name: pricelist = None - partner = self._context.get('partner', False) - quantity = self._context.get('quantity', 1.0) + partner = self.env.context.get('partner', False) + quantity = self.env.context.get('quantity', 1.0) # Support context pricelists specified as display_name or ID for compatibility if isinstance(pricelist_id_or_name, pycompat.string_types): @@ -278,11 +284,29 @@ class ProductProduct(models.Model): else: self.product_tmpl_id.image = image + @api.depends('product_tmpl_id', 'attribute_value_ids') def _compute_product_template_attribute_value_ids(self): + # Fetch and pre-map the values first for performance. It assumes there + # won't be too many values, but there might be a lot of products. + values = self.env['product.template.attribute.value'].search([ + ('product_tmpl_id', 'in', self.mapped('product_tmpl_id').ids), + ('product_attribute_value_id', 'in', self.mapped('attribute_value_ids').ids), + ]) + + values_per_template = {} + for ptav in values: + pt_id = ptav.product_tmpl_id.id + if pt_id not in values_per_template: + values_per_template[pt_id] = {} + values_per_template[pt_id][ptav.product_attribute_value_id.id] = ptav + for product in self: - product.product_template_attribute_value_ids = self.env['product.template.attribute.value']._search([ - ('product_tmpl_id', '=', product.product_tmpl_id.id), - ('product_attribute_value_id', 'in', product.attribute_value_ids.ids)]) + product.product_template_attribute_value_ids = self.env['product.template.attribute.value'] + for pav in product.attribute_value_ids: + if product.product_tmpl_id.id not in values_per_template or pav.id not in values_per_template[product.product_tmpl_id.id]: + _logger.warning("A matching product.template.attribute.value was not found for the product.attribute.value #%s on the template #%s" % (pav.id, product.product_tmpl_id.id)) + else: + product.product_template_attribute_value_ids += values_per_template[product.product_tmpl_id.id][pav.id] @api.one def _get_pricelist_items(self): @@ -390,7 +414,7 @@ class ProductProduct(models.Model): result = [] for product in self.sudo(): # display only the attributes with multiple possible values on the template - variable_attributes = product.attribute_line_ids.filtered(lambda l: len(l.value_ids) > 1).mapped('attribute_id') + variable_attributes = product.product_tmpl_id._get_valid_product_template_attribute_lines().filtered(lambda l: len(l.value_ids) > 1).mapped('attribute_id') variant = product.attribute_value_ids._variant_name(variable_attributes) name = variant and "%s (%s)" % (product.name, variant) or product.name @@ -610,9 +634,34 @@ class ProductProduct(models.Model): :return: True if the attibutes and values are correct, False instead """ self.ensure_one() - values = self.attribute_value_ids.filtered(lambda v: v.attribute_id.create_variant != 'no_variant') + values = self.attribute_value_ids attributes = values.mapped('attribute_id') - return attributes == valid_attributes and values <= valid_values + if attributes != valid_attributes: + return False + for value in values: + if value not in valid_values: + return False + return True + + @api.multi + def _is_variant_possible(self, parent_combination=None): + """Return whether the variant is possible based on its own combination, + and optionally a parent combination. + + See `_is_combination_possible` for more information. + + This will always exclude variants for templates that have `no_variant` + attributes because the variant itself will not be the full combination. + + :param parent_combination: combination from which `self` is an + optional or accessory product. + :type parent_combination: recordset `product.template.attribute.value` + + :return: ẁhether the variant is possible based on its own combination + :rtype: bool + """ + self.ensure_one() + return self.product_tmpl_id._is_combination_possible(self.product_template_attribute_value_ids, parent_combination=parent_combination) class ProductPackaging(models.Model): diff --git a/addons/product/models/product_attribute.py b/addons/product/models/product_attribute.py index 7551e57f61f..ed1f70b4bc4 100644 --- a/addons/product/models/product_attribute.py +++ b/addons/product/models/product_attribute.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo.osv import expression from odoo import api, fields, models, _ from odoo.addons import decimal_precision as dp from odoo.exceptions import UserError, ValidationError @@ -11,11 +10,13 @@ from odoo.osv import expression class ProductAttribute(models.Model): _name = "product.attribute" _description = "Product Attribute" - _order = 'sequence, name' + # if you change this _order, keep it in sync with the method + # `_sort_key_attribute_value` in `product.template` + _order = 'sequence, id' name = fields.Char('Attribute', required=True, translate=True) value_ids = fields.One2many('product.attribute.value', 'attribute_id', 'Values', copy=True) - sequence = fields.Integer('Sequence', help="Determine the display order") + sequence = fields.Integer('Sequence', help="Determine the display order", index=True) attribute_line_ids = fields.One2many('product.template.attribute.line', 'attribute_id', 'Lines') create_variant = fields.Selection([ ('no_variant', 'Never'), @@ -25,15 +26,43 @@ class ProductAttribute(models.Model): string="Create Variants", help="Check this if you want to create multiple variants for this attribute.", required=True) + @api.multi + def _without_no_variant_attributes(self): + return self.filtered(lambda pa: pa.create_variant != 'no_variant') + + @api.multi + def write(self, vals): + """Override to make sure attribute type can't be changed if it's used on + a product template. + + This is important to prevent because changing the type would make + existing combinations invalid without recomputing them, and recomputing + them might take too long and we don't want to change products without + the user knowing about it.""" + if 'create_variant' in vals: + products = self._get_related_product_templates() + if products: + message = ', '.join(products.mapped('name')) + raise UserError(_('You are trying to change the type of an attribute value still referenced on at least one product template: %s') % message) + return super(ProductAttribute, self).write(vals) + + @api.multi + def _get_related_product_templates(self): + return self.env['product.template'].with_context(active_test=False).search([ + ('attribute_line_ids.attribute_id', 'in', self.ids), + ]) + class ProductAttributeValue(models.Model): _name = "product.attribute.value" + # if you change this _order, keep it in sync with the method + # `_sort_key_variant` in `product.template' _order = 'attribute_id, sequence, id' _description = 'Attribute Value' name = fields.Char(string='Value', required=True, translate=True) - sequence = fields.Integer(string='Sequence', help="Determine the display order") - attribute_id = fields.Many2one('product.attribute', string='Attribute', ondelete='cascade', required=True) + sequence = fields.Integer(string='Sequence', help="Determine the display order", index=True) + attribute_id = fields.Many2one('product.attribute', string='Attribute', ondelete='cascade', required=True, index=True) _sql_constraints = [ ('value_company_uniq', 'unique (name, attribute_id)', 'This attribute value already exists !') @@ -51,11 +80,21 @@ class ProductAttributeValue(models.Model): @api.multi def unlink(self): - linked_products = self.env['product.product'].with_context(active_test=False).search([('attribute_value_ids', 'in', self.ids)]) + linked_products = self._get_related_product_templates() if linked_products: raise UserError(_('The operation cannot be completed:\nYou are trying to delete an attribute value with a reference on a product variant.')) return super(ProductAttributeValue, self).unlink() + @api.multi + def _without_no_variant_attributes(self): + return self.filtered(lambda pav: pav.attribute_id.create_variant != 'no_variant') + + @api.multi + def _get_related_product_templates(self): + return self.env['product.template'].with_context(active_test=False).search([ + ('attribute_line_ids.value_ids', 'in', self.ids), + ]) + class ProductTemplateAttributeLine(models.Model): """Attributes available on product.template with their selected values in a m2m. @@ -64,9 +103,10 @@ class ProductTemplateAttributeLine(models.Model): _name = "product.template.attribute.line" _rec_name = 'attribute_id' _description = 'Product Template Attribute Line' + _order = 'attribute_id, id' - product_tmpl_id = fields.Many2one('product.template', string='Product Template', ondelete='cascade', required=True) - attribute_id = fields.Many2one('product.attribute', string='Attribute', ondelete='restrict', required=True) + product_tmpl_id = fields.Many2one('product.template', string='Product Template', ondelete='cascade', required=True, index=True) + attribute_id = fields.Many2one('product.attribute', string='Attribute', ondelete='restrict', required=True, index=True) value_ids = fields.Many2many('product.attribute.value', string='Attribute Values') product_template_value_ids = fields.Many2many( 'product.template.attribute.value', @@ -76,7 +116,7 @@ class ProductTemplateAttributeLine(models.Model): @api.constrains('value_ids', 'attribute_id') def _check_valid_attribute(self): - if any(line.value_ids > line.attribute_id.value_ids for line in self): + if any(not line.value_ids or line.value_ids > line.attribute_id.value_ids for line in self): raise ValidationError(_('You cannot use this attribute with the following value.')) return True @@ -152,13 +192,17 @@ class ProductTemplateAttributeLine(models.Model): return self.browse(attribute_ids).name_get() return super(ProductTemplateAttributeLine, self)._name_search(name=name, args=args, operator=operator, limit=limit, name_get_uid=name_get_uid) + @api.multi + def _without_no_variant_attributes(self): + return self.filtered(lambda ptal: ptal.attribute_id.create_variant != 'no_variant') + class ProductTemplateAttributeValue(models.Model): """Materialized relationship between attribute values and product template generated by the product.template.attribute.line""" _name = "product.template.attribute.value" - _order = 'sequence, attribute_id, id' + _order = 'product_attribute_value_id, id' _description = 'Product Attribute Value' name = fields.Char('Value', related="product_attribute_value_id.name") @@ -192,6 +236,10 @@ class ProductTemplateAttributeValue(models.Model): return super(ProductTemplateAttributeValue, self).name_get() return [(value.id, "%s: %s" % (value.attribute_id.name, value.name)) for value in self] + @api.multi + def _without_no_variant_attributes(self): + return self.filtered(lambda ptav: ptav.attribute_id.create_variant != 'no_variant') + class ProductTemplateAttributeExclusion(models.Model): _name = "product.template.attribute.exclusion" diff --git a/addons/product/models/product_template.py b/addons/product/models/product_template.py index 1ccf742bd9a..36e35f10e66 100644 --- a/addons/product/models/product_template.py +++ b/addons/product/models/product_template.py @@ -2,15 +2,13 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import itertools -import operator -import psycopg2 from odoo.addons import decimal_precision as dp from odoo import api, fields, models, tools, _ -from odoo.exceptions import ValidationError, RedirectWarning, except_orm +from odoo.exceptions import ValidationError, RedirectWarning, UserError +from odoo.osv import expression from odoo.tools import pycompat -from odoo.exceptions import UserError class ProductTemplate(models.Model): @@ -61,13 +59,16 @@ class ProductTemplate(models.Model): 'res.currency', 'Currency', compute='_compute_currency_id') # price fields + # price: total template price, context dependent (partner, pricelist, quantity) price = fields.Float( 'Price', compute='_compute_template_price', inverse='_set_template_price', digits=dp.get_precision('Product Price')) + # list_price: catalog price, user defined list_price = fields.Float( 'Sales Price', default=1.0, digits=dp.get_precision('Product Price'), help="Price at which the product is sold to customers.") + # lst_price: catalog price for template, but including extra for variants lst_price = fields.Float( 'Public Price', related='list_price', readonly=False, digits=dp.get_precision('Product Price')) @@ -174,8 +175,8 @@ class ProductTemplate(models.Model): pricelist_id_or_name = self._context.get('pricelist') if pricelist_id_or_name: pricelist = None - partner = self._context.get('partner') - quantity = self._context.get('quantity', 1.0) + partner = self.env.context.get('partner') + quantity = self.env.context.get('quantity', 1.0) # Support context pricelists specified as display_name or ID for compatibility if isinstance(pricelist_id_or_name, pycompat.string_types): @@ -435,14 +436,16 @@ class ProductTemplate(models.Model): def create_variant_ids(self): Product = self.env["product.product"] - variants_to_create = [] - variants_to_activate = [] - variants_to_unlink = [] - for tmpl_id in self.with_context(active_test=False): + # Handle the variants for each template separately. This will be + # less efficient when called on a lot of products with few variants + # but it is better when there's a lot of variants on one template. + variants_to_create = [] + variants_to_activate = self.env['product.product'] + variants_to_unlink = self.env['product.product'] # adding an attribute with only one value should not recreate product # write this attribute on every product to make sure we don't lose them - variant_alone = tmpl_id.attribute_line_ids.filtered(lambda line: line.attribute_id.create_variant == 'always' and len(line.value_ids) == 1).mapped('value_ids') + variant_alone = tmpl_id._get_valid_product_template_attribute_lines().filtered(lambda line: line.attribute_id.create_variant == 'always' and len(line.value_ids) == 1).mapped('value_ids') for value_id in variant_alone: updated_products = tmpl_id.product_variant_ids.filtered(lambda product: value_id.attribute_id not in product.mapped('attribute_value_ids.attribute_id')) updated_products.write({'attribute_value_ids': [(4, value_id.id)]}) @@ -452,15 +455,15 @@ class ProductTemplate(models.Model): # process. # Technical note: if there is no attribute, a variant is still created because # 'not any([])' and 'set([]) not in set([])' are True. - if not any(attrib.create_variant == 'dynamic' for attrib in tmpl_id.mapped('attribute_line_ids.attribute_id')): - # Iterator containing all possible attribute values combination + if not tmpl_id.has_dynamic_attributes(): + # Iterator containing all possible `product.attribute.value` combination # The iterator is used to avoid MemoryError in case of a huge number of combination. all_variants = itertools.product(*( - line.value_ids.ids for line in tmpl_id.attribute_line_ids if line.value_ids[:1].attribute_id.create_variant != 'no_variant' + line.value_ids.ids for line in tmpl_id._get_valid_product_template_attribute_lines()._without_no_variant_attributes() )) - # Set containing existing attribute values combination + # Set containing existing `product.attribute.value` combination existing_variants = { - frozenset(variant.attribute_value_ids.filtered(lambda r: r.attribute_id.create_variant != 'no_variant').ids) + frozenset(variant.attribute_value_ids.ids) for variant in tmpl_id.product_variant_ids } # For each possible variant, create if it doesn't exist yet. @@ -473,107 +476,436 @@ class ProductTemplate(models.Model): }) if len(variants_to_create) > 1000: raise UserError(_( - 'The number of variants to generate is too high. ' - 'You should either not generate variants for each combination or generate them on demand from the sales order. ' - 'To do so, open the form view of attributes and change the mode of *Create Variants*.')) + 'The number of variants to generate is too high. ' + 'You should either not generate variants for each combination or generate them on demand from the sales order. ' + 'To do so, open the form view of attributes and change the mode of *Create Variants*.')) # Check existing variants if any needs to be activated or unlinked. # - if the product is not active and has valid attributes and attribute values, it # should be activated # - if the product does not have valid attributes or attribute values, it should be # deleted - valid_value_ids = tmpl_id.mapped('attribute_line_ids.value_ids').filtered( - lambda v: v.attribute_id.create_variant != 'no_variant' - ) - valid_attribute_ids = valid_value_ids.mapped('attribute_id') + valid_value_ids = tmpl_id._get_valid_product_attribute_values()._without_no_variant_attributes() + valid_attribute_ids = tmpl_id._get_valid_product_attributes()._without_no_variant_attributes() for product_id in tmpl_id.product_variant_ids: if product_id._has_valid_attributes(valid_attribute_ids, valid_value_ids): if not product_id.active: - variants_to_activate.append(product_id) + variants_to_activate += product_id else: - variants_to_unlink.append(product_id) + variants_to_unlink += product_id - if variants_to_activate: - Product.concat(*variants_to_activate).write({'active': True}) + if variants_to_activate: + variants_to_activate.write({'active': True}) - # create new products - if variants_to_create: - Product.create(variants_to_create) + # create new products + if variants_to_create: + Product.create(variants_to_create) - # unlink or inactive product - for variant in variants_to_unlink: + # unlink or inactive product + # try in batch first because it is much faster try: with self._cr.savepoint(), tools.mute_logger('odoo.sql_db'): - variant.unlink() - # We catch all kind of exception to be sure that the operation doesn't fail. - except (psycopg2.Error, except_orm): - variant.write({'active': False}) - pass + variants_to_unlink.unlink() + except Exception: + # fall back to one by one if batch is not possible + for variant in variants_to_unlink: + try: + with self._cr.savepoint(), tools.mute_logger('odoo.sql_db'): + variant.unlink() + # We catch all kind of exception to be sure that the operation doesn't fail. + except Exception: + # Note: this can still fail if something is preventing from archiving. + # This is the case from existing stock reordering rules. + variant.write({'active': False}) return True def has_dynamic_attributes(self): - return self.attribute_line_ids and \ - any(create_variant == 'dynamic' - for create_variant in self.mapped('attribute_line_ids.value_ids.attribute_id.create_variant')) + """Return whether this `product.template` has at least one dynamic + attribute. + + :return: True if at least one dynamic attribute, False otherwise + :rtype: bool + """ + self.ensure_one() + return any(a.create_variant == 'dynamic' for a in self._get_valid_product_attributes()) + + @api.multi + def _get_valid_product_template_attribute_lines(self): + """A product template attribute line is considered valid if it has at + least one possible value. + + Those with only one value are considered valid, even though they should + not appear on the configurator itself (unless they have an is_custom + value to input), indeed single value attributes can be used to filter + products among others based on that attribute/value. + + This method is necessary because it was previously possible to save a + line without any value on it, so the database might not be consistent in + that regard. + + :return: all the valid product template attribute lines of this template + :rtype: recordset `product.template.attribute.line` + """ + self.ensure_one() + return self.attribute_line_ids.filtered(lambda ptal: ptal.value_ids) + + @api.multi + def _get_valid_product_attributes(self): + """A product attribute is considered valid for a template if it + has at least one possible value set on the template. + + See `_get_valid_product_template_attribute_lines`. + + :return: all the valid product attributes of this template + :rtype: recordset `product.attribute` + """ + self.ensure_one() + product_attributes = self.env['product.attribute'] + for ptal in self._get_valid_product_template_attribute_lines(): + product_attributes |= ptal.attribute_id + return product_attributes + + @api.multi + def _get_valid_product_attribute_values(self): + """A product attribute value is considered valid for a template if it is + defined on a product template attribute line. + + :return: all the valid product attribute values of this template + :rtype: recordset `product.attribute.value` + """ + self.ensure_one() + return self._get_valid_product_template_attribute_lines().mapped('value_ids') + + @api.multi + def _get_possible_variants(self, parent_combination=None): + """Return the existing variants that are possible. + + For dynamic attributes, it will only return the variants that have been + created already. For no_variant attributes, it will return an empty + recordset because the variants themselves are not a full combination. + + :param parent_combination: combination from which `self` is an + optional or accessory product. + :type parent_combination: recordset `product.template.attribute.value` + + :return: the existing variants that are possible. + :rtype: recordset of `product.product` + """ + self.ensure_one() + return self.product_variant_ids.filtered(lambda p: p._is_variant_possible(parent_combination)) @api.multi def get_filtered_variants(self, reference_product=None): - """ - Will filter availability (excluded) for the product - combinations (ex: color: white excludes size: large). - - Will also filter availability (excluded) for the parent - product if specified (meaning that this product is a an optionnal or - accessory product of the reference_product). - Args: - reference_product (product.product): The reference product that has - the current product as an option or accessory product. - Returns: - The filtered list of product variants - """ + """deprecated, use _get_possible_variants instead""" self.ensure_one() - product_template_attribute_values = self.env['product.template.attribute.value'].search([('product_tmpl_id', '=', self.id)]) + + parent_combination = self.env['product.template.attribute.value'] + if reference_product: # append the reference_product if provided - product_template_attribute_values |= reference_product.product_template_attribute_value_ids - if reference_product._context.get('no_variant_attribute_values'): + parent_combination |= reference_product.product_template_attribute_value_ids + if reference_product.env.context.get('no_variant_attribute_values'): # Add "no_variant" attribute values' exclusions # They are kept in the context since they are not linked to this product variant - product_template_attribute_values |= reference_product._context.get('no_variant_attribute_values') + parent_combination |= reference_product.env.context.get('no_variant_attribute_values') + return self._get_possible_variants(parent_combination) - product_variants = self.product_variant_ids - for product_template_attribute_value in product_template_attribute_values: - # CASE 1: The whole product is excluded when no attribute values are selected in the parent product - # returns empty recordset of product.product if so. What is checked is: - # If the product_attribute value doesn't belong to self (i.e. belongs to the reference product) - # and self is the excluded product template on the exclusion lines - # and the exclusions is on the product without specified product attribute values (i.e. the whole product is excluded) - if product_template_attribute_value.product_tmpl_id != self \ - and self in product_template_attribute_value.exclude_for.mapped('product_tmpl_id') \ - and any(not exclude_for.value_ids - for exclude_for in product_template_attribute_value.exclude_for.filtered( - lambda excluded_product_attribute_value: excluded_product_attribute_value.product_tmpl_id == self)): - return self.env['product.product'] + @api.multi + def _get_attribute_exclusions(self, parent_combination=None): + """Return the list of attribute exclusions of a product. - # CASE 2: Check if some of the product.template.attribute.value of the product are excluded - # for this prodcut. A variant could be excluded: - # - Either by itself (eg: The office chair with iron legs excludes the color white) - # - Or by the reference product (eg: The customizable desk with iron legs excludes the office chair with aluminium legs) - for excluded in product_template_attribute_value.exclude_for.filtered( - lambda excluded_product_attribute_value: excluded_product_attribute_value.product_tmpl_id == self): - product_variants -= product_variants.filtered( - lambda variant: - # 1/ Check the applicability of the exclusion - # i.e: the restriction comes from the parent - # OR the restriction is on a product_attribute_value that this variant has, eg: - # if the office chair with iron legs excludes the color white, we must check - # that this variant has iron legs to check the exclusion - (product_template_attribute_value.product_tmpl_id != self or product_template_attribute_value in variant.product_template_attribute_value_ids) and - # 2/ Check the variant has one of the excluded attribute values - any(attribute_value in excluded.value_ids for attribute_value in variant.product_template_attribute_value_ids)) + :param parent_combination: the combination from which + `self` is an optional or accessory product. Indeed exclusions + rules on one product can concern another product. + :type parent_combination: recordset `product.template.attribute.value` - return product_variants + :return: dict of exclusions + - exclusions: from this product itself + - parent_exclusions: from the parent_combination + - archived_combinations: variants that are archived + - existing_combinations: variants that are existing (as opposed to + deleted, because deleted need to be considered impossible if + there are no dynamic attributes). + - has_dynamic_attributes: whether there is a dynamic attribute + - no_variant_product_template_attribute_value_ids: values that are + no_variant + """ + self.ensure_one() + return { + 'exclusions': self._get_own_attribute_exclusions(), + 'parent_exclusions': self._get_parent_attribute_exclusions(parent_combination), + 'archived_combinations': self._get_archived_combinations(), + 'has_dynamic_attributes': self.has_dynamic_attributes(), + 'existing_combinations': self._get_existing_combinations(), + 'no_variant_product_template_attribute_value_ids': self._get_no_variant_product_template_attribute_values(), + } + + @api.multi + def _get_own_attribute_exclusions(self): + """Get exclusions coming from the current template. + + Dictionnary, each ptav is a key, and for each of them the value is + an array with the other ptav that they exclude (empty if no exclusion). + """ + self.ensure_one() + product_template_attribute_values = self._get_valid_product_template_attribute_lines().mapped('product_template_value_ids') + return { + ptav.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 ptav in product_template_attribute_values + } + + @api.multi + def _get_parent_attribute_exclusions(self, parent_combination): + """Get exclusions coming from the parent combination. + + Array, each element is a ptav that is excluded because of the parent. + """ + self.ensure_one() + if not parent_combination: + return [] + return [ + value_id + for filter_line in parent_combination.mapped('exclude_for').filtered( + lambda filter_line: filter_line.product_tmpl_id == self + ) for value_id in filter_line.value_ids.ids + ] + + @api.multi + def _get_archived_combinations(self): + self.ensure_one() + """Get archived combinations. + + Array, each element is an array with ids of an archived combination. + """ + valid_value_ids = self._get_valid_product_attribute_values()._without_no_variant_attributes() + valid_attribute_ids = self._get_valid_product_attributes()._without_no_variant_attributes() + + # Search only among those having the right set of attributes. + domain = [('product_tmpl_id', '=', self.id), ('active', '=', False)] + for pa in valid_attribute_ids: + domain = expression.AND([[('attribute_value_ids.attribute_id.id', '=', pa.id)], domain]) + archived_variants = self.env['product.product'].search(domain) + + archived_variants = archived_variants.filtered(lambda v: v._has_valid_attributes(valid_attribute_ids, valid_value_ids)) + + return [archived_variant.product_template_attribute_value_ids.ids + for archived_variant in archived_variants] + + @api.multi + def _get_existing_combinations(self): + self.ensure_one() + """Get existing combinations. + + Needed because when not using dynamic attributes, the combination is + not ok if it doesn't exist (= if the variant has been deleted). + + Array, each element is an array with ids of an existing combination. + """ + valid_value_ids = self._get_valid_product_attribute_values()._without_no_variant_attributes() + valid_attribute_ids = self._get_valid_product_attributes()._without_no_variant_attributes() + + # Search only among those having the right set of attributes. + domain = [('product_tmpl_id', '=', self.id), ('active', '=', True)] + for pa in valid_attribute_ids: + domain = expression.AND([[('attribute_value_ids.attribute_id.id', '=', pa.id)], domain]) + existing_variants = self.env['product.product'].search(domain) + + existing_variants = existing_variants.filtered(lambda v: v._has_valid_attributes(valid_attribute_ids, valid_value_ids)) + + return [variant.product_template_attribute_value_ids.ids + for variant in existing_variants] + + @api.multi + def _get_no_variant_product_template_attribute_values(self): + self.ensure_one() + product_template_attribute_values = self._get_valid_product_template_attribute_lines().mapped('product_template_value_ids') + return product_template_attribute_values.filtered( + lambda v: v.attribute_id.create_variant == 'no_variant' + ).ids + + @api.multi + def _is_combination_possible(self, combination, parent_combination=None): + """ + The combination is possible if it is not excluded by any rule + coming from the current template, not excluded by any rule from the + parent_combination (if given), and there should not be any archived + variant with the exact same combination. + + If the template does not have any dynamic attribute, the combination + is also not possible if the matching variant has been deleted. + + Moreover the attributes of the combination must excatly match the + attributes allowed on the template. + + :param combination: the combination to check for possibility + :type combination: recordset `product.template.attribute.value` + + :param parent_combination: combination from which `self` is an + optional or accessory product. + :type parent_combination: recordset `product.template.attribute.value` + + :return: whether the combination is possible + :rtype: bool + """ + self.ensure_one() + + if len(combination) != len(self._get_valid_product_template_attribute_lines()): + # number of attribute values passed is different than the + # configuration of attributes on the template + return False + + if self._get_valid_product_attributes() != combination.mapped('attribute_id'): + # combination has different attributes than the ones configured on the template + return False + + variant = self._get_variant_for_combination(combination) + if not self.has_dynamic_attributes() and not variant: + # the variant has been deleted + return False + + exclusions = self._get_own_attribute_exclusions() + if exclusions: + # exclude if the current value is in an exclusion, + # and the value excluding it is also in the combination + for ptav in combination: + for exclusion in exclusions.get(ptav.id): + if exclusion in combination.ids: + return False + + parent_exclusions = self._get_parent_attribute_exclusions(parent_combination) + if parent_exclusions: + for exclusion in parent_exclusions: + if exclusion in combination.ids: + return False + + filtered_combination = combination._without_no_variant_attributes() + archived_combinations = self._get_archived_combinations() + if archived_combinations and filtered_combination.ids in archived_combinations: + return False + + return True + + @api.multi + def _get_variant_for_combination(self, combination): + """Get the variant matching the combination. + + All of the values in combination must be present in the variant, and the + variant should not have more attributes. Ignore the attributes that are + not supposed to create variants. + + :param combination: recordset of `product.template.attribute.value` + + :return: the variant if found, else empty + :rtype: recordset `product.product` + """ + self.ensure_one() + + filtered_combination = combination._without_no_variant_attributes() + + # If there are a lot of variants on this template, it is much faster to + # build a query than using the existing o2m. + domain = [('product_tmpl_id', '=', self.id)] + for ptav in filtered_combination: + domain = expression.AND([[('attribute_value_ids.id', '=', ptav.product_attribute_value_id.id)], domain]) + res = self.env['product.product'].search(domain) + + # The domain above is checking for the `product.attribute.value`, but we + # need to make sure it's the same `product.template.attribute.value`. + # Also there should theorically be only 0 or 1 but an existing database + # might not be consistent so we need to make sure to take max 1. + return res.filtered( + lambda v: v.product_template_attribute_value_ids == filtered_combination + )[:1] + + @api.multi + def _get_first_possible_combination(self, parent_combination=None, necessary_values=None): + """ + Iterate the attributes and values in order and stop at the first + combination of values that is possible. + + When encountering an impossible combination, try to change the + value of latest attributes first. + + Ignore attributes that have no values. + + Note this method return the same result (empty recordset) if no + combination is possible at all which would be considered a negative + result, or if there are no attribute lines on the template in which + case the "empty combination" is actually a possible combination. + Therefore the result of this method when empty should be tested + with `_is_combination_possible` if it's important to know if the + resulting empty combination is actually possible or not. + + :param parent_combination: combination from which `self` is an + optional or accessory product. + :type parent_combination: recordset `product.template.attribute.value` + + :param necessary_values: values that must be in the returned combination + :type necessary_values: recordset of `product.template.attribute.value` + + :return: the first possible combination found, or empty if none possible + :rtype: recordset of `product.template.attribute.value` + """ + self.ensure_one() + + if not self.active: + return self.env['product.template.attribute.value'] + + attribute_lines = self._get_valid_product_template_attribute_lines() + + def iterate_attribute_lines(attribute_lines, combination): + """ + :param attribute_lines: recordset of product.template.attribute.line + that are still to iterate + :param combination: recordset of product.template.attribute.value + that have to be tested for possibility + + :return: the first possible combination found, or empty + :rtype: recordset of `product.template.attribute.value` + """ + if not attribute_lines: + if self._is_combination_possible(combination, parent_combination): + return combination + else: + return self.env['product.template.attribute.value'] + for cur in attribute_lines[0].product_template_value_ids: + res = iterate_attribute_lines(attribute_lines[1:], combination + cur) + if res and all(v in res for v in (necessary_values or [])): + return res + return self.env['product.template.attribute.value'] + + return iterate_attribute_lines(attribute_lines, self.env['product.template.attribute.value']) + + @api.multi + def _get_closest_possible_combination(self, combination): + """Get the first possible combination that is the closest to the given + combination. + + If the given combination is incomplete, try to complete it. + + If the given combination is invalid, try to remove values from it before + completing it. + + See `_get_first_possible_combination` note about empty result. + + :param combination: the values to include if they are possible + :type combination: recordset `product.template.attribute.value` + + :return: the first possible combination that is including as much + elements as possible from the given combination. + :rtype: recordset of product.template.attribute.value + """ + while True: + result = self._get_first_possible_combination(necessary_values=combination) + if result or not combination: + return result + combination = combination[:-1] @api.model def get_empty_list_help(self, help): diff --git a/addons/product/tests/common.py b/addons/product/tests/common.py index 50831e2a88c..8bfec6ff2d8 100644 --- a/addons/product/tests/common.py +++ b/addons/product/tests/common.py @@ -73,6 +73,7 @@ class TestProductCommon(common.SavepointCase): 'uom_po_id': cls.uom_unit.id, 'attribute_line_ids': [(0, 0, { 'attribute_id': cls.prod_att_1.id, + 'value_ids': [(6, 0, [cls.prod_attr1_v1.id, cls.prod_attr1_v2.id])] })] }) cls.product_7 = Product.create({ diff --git a/addons/product/tests/test_product_attribute_value_config.py b/addons/product/tests/test_product_attribute_value_config.py index eeb1275420a..19070a1c887 100644 --- a/addons/product/tests/test_product_attribute_value_config.py +++ b/addons/product/tests/test_product_attribute_value_config.py @@ -1,15 +1,17 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from odoo.tests import tagged from odoo.tests.common import TransactionCase -class TestProductAttributeValueConfig(TransactionCase): +class TestProductAttributeValueSetup(TransactionCase): def setUp(self): - super(TestProductAttributeValueConfig, self).setUp() + super(TestProductAttributeValueSetup, self).setUp() self.computer = self.env['product.template'].create({ - 'name': 'Super Computer' + 'name': 'Super Computer', + 'price': 2000, }) self._add_ssd_attribute() @@ -26,6 +28,150 @@ class TestProductAttributeValueConfig(TransactionCase): self.computer_case.create_variant_ids() + def _add_ssd_attribute(self): + self.ssd_attribute = self.env['product.attribute'].create({'name': 'Memory', 'sequence': 1}) + self.ssd_256 = self.env['product.attribute.value'].create({ + 'name': '256 GB', + 'attribute_id': self.ssd_attribute.id, + 'sequence': 1, + }) + self.ssd_512 = self.env['product.attribute.value'].create({ + 'name': '512 GB', + 'attribute_id': self.ssd_attribute.id, + 'sequence': 2, + }) + + self._add_ssd_attribute_line() + + def _add_ssd_attribute_line(self): + self.computer_ssd_attribute_lines = self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': self.computer.id, + 'attribute_id': self.ssd_attribute.id, + 'value_ids': [(6, 0, [self.ssd_256.id, self.ssd_512.id])], + }) + self.computer_ssd_attribute_lines.product_template_value_ids[0].price_extra = 200 + self.computer_ssd_attribute_lines.product_template_value_ids[1].price_extra = 400 + + def _add_ram_attribute(self): + self.ram_attribute = self.env['product.attribute'].create({'name': 'RAM', 'sequence': 2}) + self.ram_8 = self.env['product.attribute.value'].create({ + 'name': '8 GB', + 'attribute_id': self.ram_attribute.id, + 'sequence': 1, + }) + self.ram_16 = self.env['product.attribute.value'].create({ + 'name': '16 GB', + 'attribute_id': self.ram_attribute.id, + 'sequence': 2, + }) + self.ram_32 = self.env['product.attribute.value'].create({ + 'name': '32 GB', + 'attribute_id': self.ram_attribute.id, + 'sequence': 3, + }) + self.computer_ram_attribute_lines = self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': self.computer.id, + 'attribute_id': self.ram_attribute.id, + 'value_ids': [(6, 0, [self.ram_8.id, self.ram_16.id, self.ram_32.id])], + }) + self.computer_ram_attribute_lines.product_template_value_ids[0].price_extra = 20 + self.computer_ram_attribute_lines.product_template_value_ids[1].price_extra = 40 + self.computer_ram_attribute_lines.product_template_value_ids[2].price_extra = 80 + + def _add_hdd_attribute(self): + self.hdd_attribute = self.env['product.attribute'].create({'name': 'HDD', 'sequence': 3}) + self.hdd_1 = self.env['product.attribute.value'].create({ + 'name': '1 To', + 'attribute_id': self.hdd_attribute.id, + 'sequence': 1, + }) + self.hdd_2 = self.env['product.attribute.value'].create({ + 'name': '2 To', + 'attribute_id': self.hdd_attribute.id, + 'sequence': 2, + }) + self.hdd_4 = self.env['product.attribute.value'].create({ + 'name': '4 To', + 'attribute_id': self.hdd_attribute.id, + 'sequence': 3, + }) + + self._add_hdd_attribute_line() + + def _add_hdd_attribute_line(self): + self.computer_hdd_attribute_lines = self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': self.computer.id, + 'attribute_id': self.hdd_attribute.id, + 'value_ids': [(6, 0, [self.hdd_1.id, self.hdd_2.id, self.hdd_4.id])], + }) + self.computer_hdd_attribute_lines.product_template_value_ids[0].price_extra = 2 + self.computer_hdd_attribute_lines.product_template_value_ids[1].price_extra = 4 + self.computer_hdd_attribute_lines.product_template_value_ids[2].price_extra = 8 + + def _add_ram_exclude_for(self): + self._get_product_value_id(self.computer_ram_attribute_lines, self.ram_16).update({ + 'exclude_for': [(0, 0, { + 'product_tmpl_id': self.computer.id, + 'value_ids': [(6, 0, [self._get_product_value_id(self.computer_hdd_attribute_lines, self.hdd_1).id])] + })] + }) + + def _add_size_attribute(self): + self.size_attribute = self.env['product.attribute'].create({'name': 'Size', 'sequence': 4}) + self.size_m = self.env['product.attribute.value'].create({ + 'name': 'M', + 'attribute_id': self.size_attribute.id, + 'sequence': 1, + }) + self.size_l = self.env['product.attribute.value'].create({ + 'name': 'L', + 'attribute_id': self.size_attribute.id, + 'sequence': 2, + }) + self.size_xl = self.env['product.attribute.value'].create({ + 'name': 'XL', + 'attribute_id': self.size_attribute.id, + 'sequence': 3, + }) + self.computer_case_size_attribute_lines = self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': self.computer_case.id, + 'attribute_id': self.size_attribute.id, + 'value_ids': [(6, 0, [self.size_m.id, self.size_l.id, self.size_xl.id])], + }) + + def _get_product_value_id(self, product_template_attribute_lines, product_attribute_value): + return product_template_attribute_lines.product_template_value_ids.filtered( + lambda product_value_id: product_value_id.product_attribute_value_id == product_attribute_value)[0] + + def _get_product_template_attribute_value(self, product_attribute_value, model=False): + """ + 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 + """ + if not model: + model = self.computer + return model._get_valid_product_template_attribute_lines().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 _add_exclude(self, m1, m2, product_template=False): + m1.update({ + 'exclude_for': [(0, 0, { + 'product_tmpl_id': (product_template or self.computer).id, + 'value_ids': [(6, 0, [m2.id])] + })] + }) + + +@tagged('post_install', '-at_install') +class TestProductAttributeValueConfig(TestProductAttributeValueSetup): + def test_product_template_attribute_values_creation(self): self.assertEqual(len(self.computer_ssd_attribute_lines.product_template_value_ids), 2, 'Product attribute values (ssd) were not automatically created') @@ -36,6 +182,35 @@ class TestProductAttributeValueConfig(TransactionCase): self.assertEqual(len(self.computer_case_size_attribute_lines.product_template_value_ids), 3, 'Product attribute values (size) were not automatically created') + def test_get_variant_for_combination(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + + # completely defined variant + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + ok_variant = self.computer._get_variant_for_combination(combination) + self.assertEqual(ok_variant.product_template_attribute_value_ids, combination) + + # over defined variant + combination = computer_ssd_256 + computer_ram_8 + computer_ram_16 + computer_hdd_1 + variant = self.computer._get_variant_for_combination(combination) + self.assertEqual(len(variant), 0) + + # under defined variant + combination = computer_ssd_256 + computer_ram_8 + variant = self.computer._get_variant_for_combination(combination) + self.assertEqual(len(variant), 0) + + # also test _has_valid_attributes (case ok): + valid_value_ids = self.computer._get_valid_product_attribute_values()._without_no_variant_attributes() + valid_attribute_ids = self.computer._get_valid_product_attributes()._without_no_variant_attributes() + self.assertTrue(ok_variant._has_valid_attributes(valid_attribute_ids, valid_value_ids)) + + # also test _has_valid_attributes (case not ok): + self.assertFalse(ok_variant._has_valid_attributes(valid_attribute_ids, valid_value_ids - self.hdd_1)) + def test_product_filtered_exclude_for(self): """ Super Computer has 18 variants total (2 ssd * 3 ram * 3 hdd) @@ -45,10 +220,18 @@ class TestProductAttributeValueConfig(TransactionCase): => There has to be 16 variants left when filtered """ + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ssd_512 = self._get_product_template_attribute_value(self.ssd_512) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + + self.assertEqual(len(self.computer._get_possible_variants()), 18) self._add_ram_exclude_for() - self.assertEqual(len(self.computer.get_filtered_variants()), 16) - self.assertFalse(self._get_variant_for_attribute_values(self.computer, [self.ssd_256, self.ram_16, self.hdd_1])) - self.assertFalse(self._get_variant_for_attribute_values(self.computer, [self.ssd_512, self.ram_16, self.hdd_1])) + self.assertEqual(len(self.computer._get_possible_variants()), 16) + self.assertTrue(self.computer._get_variant_for_combination(computer_ssd_256 + computer_ram_8 + computer_hdd_1)._is_variant_possible()) + self.assertFalse(self.computer._get_variant_for_combination(computer_ssd_256 + computer_ram_16 + computer_hdd_1)._is_variant_possible()) + self.assertFalse(self.computer._get_variant_for_combination(computer_ssd_512 + computer_ram_16 + computer_hdd_1)._is_variant_possible()) def test_children_product_filtered_exclude_for(self): """ @@ -59,112 +242,154 @@ class TestProductAttributeValueConfig(TransactionCase): => There has to be 2 variants left when filtered """ + computer_hdd_4 = self._get_product_template_attribute_value(self.hdd_4) + computer_size_m = self._get_product_template_attribute_value(self.size_m, self.computer_case) + self._add_exclude(computer_hdd_4, computer_size_m, self.computer_case) + self.assertEqual(len(self.computer_case._get_possible_variants(computer_hdd_4)), 2) + self.assertFalse(self.computer_case._get_variant_for_combination(computer_size_m)._is_variant_possible(computer_hdd_4)) - self._add_hdd_excludes_computer_case() - hdd_4_variant = self.computer.product_variant_ids.filtered( - lambda variant: self.hdd_4 in variant.product_template_attribute_value_ids.mapped('product_attribute_value_id'))[0] - self.assertEqual(len(self.computer_case.get_filtered_variants(hdd_4_variant)), 2) - self.assertFalse(self._get_variant_for_attribute_values(self.computer_case, [self.size_m], hdd_4_variant)) + def test_is_combination_possible(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + self._add_exclude(computer_ram_16, computer_hdd_1) - def _add_ssd_attribute(self): - self.ssd_attribute = self.env['product.attribute'].create({'name': 'Memory'}) - self.ssd_256 = self.env['product.attribute.value'].create({ - 'name': '256 GB', - 'attribute_id': self.ssd_attribute.id + # CASE: basic + self.assertTrue(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + + # CASE: ram 16 excluding hdd1 + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_16 + computer_hdd_1)) + + # CASE: under defined combination + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_16)) + + # CASE: no combination, no variant, just return the only variant + mouse = self.env['product.template'].create({'name': 'Mouse'}) + self.assertTrue(mouse._is_combination_possible(self.env['product.template.attribute.value'])) + + # prep work for the last part of the test + color_attribute = self.env['product.attribute'].create({'name': 'Color'}) + color_red = self.env['product.attribute.value'].create({ + 'name': 'Red', + 'attribute_id': color_attribute.id, }) - self.ssd_512 = self.env['product.attribute.value'].create({ - 'name': '512 GB', - 'attribute_id': self.ssd_attribute.id + color_green = self.env['product.attribute.value'].create({ + 'name': 'Green', + 'attribute_id': color_attribute.id, }) - self.computer_ssd_attribute_lines = self.env['product.template.attribute.line'].create({ - 'product_tmpl_id': self.computer.id, - 'attribute_id': self.ssd_attribute.id, - 'value_ids': [(6, 0, [self.ssd_256.id, self.ssd_512.id])] + self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': mouse.id, + 'attribute_id': color_attribute.id, + 'value_ids': [(6, 0, [color_red.id, color_green.id])], }) - def _add_ram_attribute(self): - self.ram_attribute = self.env['product.attribute'].create({'name': 'RAM'}) - self.ram_8 = self.env['product.attribute.value'].create({ - 'name': '8 GB', - 'attribute_id': self.ram_attribute.id - }) - self.ram_16 = self.env['product.attribute.value'].create({ - 'name': '16 GB', - 'attribute_id': self.ram_attribute.id - }) - self.ram_32 = self.env['product.attribute.value'].create({ - 'name': '32 GB', - 'attribute_id': self.ram_attribute.id - }) - self.computer_ram_attribute_lines = self.env['product.template.attribute.line'].create({ - 'product_tmpl_id': self.computer.id, - 'attribute_id': self.ram_attribute.id, - 'value_ids': [(6, 0, [self.ram_8.id, self.ram_16.id, self.ram_32.id])] - }) + mouse.create_variant_ids() - def _add_hdd_attribute(self): - self.hdd_attribute = self.env['product.attribute'].create({'name': 'HDD'}) - self.hdd_1 = self.env['product.attribute.value'].create({ - 'name': '1 To', - 'attribute_id': self.hdd_attribute.id - }) - self.hdd_2 = self.env['product.attribute.value'].create({ - 'name': '2 To', - 'attribute_id': self.hdd_attribute.id - }) - self.hdd_4 = self.env['product.attribute.value'].create({ - 'name': '4 To', - 'attribute_id': self.hdd_attribute.id - }) - self.computer_hdd_attribute_lines = self.env['product.template.attribute.line'].create({ - 'product_tmpl_id': self.computer.id, - 'attribute_id': self.hdd_attribute.id, - 'value_ids': [(6, 0, [self.hdd_1.id, self.hdd_2.id, self.hdd_4.id])] - }) + mouse_color_red = self._get_product_template_attribute_value(color_red, mouse) + mouse_color_green = self._get_product_template_attribute_value(color_green, mouse) - def _add_ram_exclude_for(self): - self.ram_16_excludes_hdd_1 = self.env['product.template.attribute.exclusion'].create({ - 'product_tmpl_id': self.computer.id, - 'value_ids': [(6, 0, [self._get_product_value_id(self.computer_hdd_attribute_lines, self.hdd_1).id])] - }) - self._get_product_value_id(self.computer_ram_attribute_lines, self.ram_16).update({ - 'exclude_for': [(6, 0, [self.ram_16_excludes_hdd_1.id])] - }) + self._add_exclude(computer_ssd_256, mouse_color_green, mouse) - def _add_size_attribute(self): - self.size_attribute = self.env['product.attribute'].create({'name': 'Size'}) - self.size_m = self.env['product.attribute.value'].create({ - 'name': 'M', - 'attribute_id': self.size_attribute.id - }) - self.size_l = self.env['product.attribute.value'].create({ - 'name': 'L', - 'attribute_id': self.size_attribute.id - }) - self.size_xl = self.env['product.attribute.value'].create({ - 'name': 'XL', - 'attribute_id': self.size_attribute.id - }) - self.computer_case_size_attribute_lines = self.env['product.template.attribute.line'].create({ - 'product_tmpl_id': self.computer_case.id, - 'attribute_id': self.size_attribute.id, - 'value_ids': [(6, 0, [self.size_m.id, self.size_l.id, self.size_xl.id])] - }) + variant = self.computer._get_variant_for_combination(computer_ssd_256 + computer_ram_8 + computer_hdd_1) - def _add_hdd_excludes_computer_case(self): - self.hdd_4_excludes_computer_case_m = self.env['product.template.attribute.exclusion'].create({ - 'product_tmpl_id': self.computer_case.id, - 'value_ids': [(6, 0, [self._get_product_value_id(self.computer_case_size_attribute_lines, self.size_m).id])] - }) - self._get_product_value_id(self.computer_hdd_attribute_lines, self.hdd_4).update({ - 'exclude_for': [(6, 0, [self.hdd_4_excludes_computer_case_m.id])] - }) + # CASE: wrong attributes (mouse_color_red not on computer) + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_16 + mouse_color_red)) - def _get_product_value_id(self, product_template_attribute_lines, product_attribute_value): - return product_template_attribute_lines.product_template_value_ids.filtered( - lambda product_value_id: product_value_id.product_attribute_value_id == product_attribute_value)[0] + # CASE: parent ok + self.assertTrue(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1, mouse_color_red)) + self.assertTrue(mouse._is_combination_possible(mouse_color_red, computer_ssd_256 + computer_ram_8 + computer_hdd_1)) - def _get_variant_for_attribute_values(self, product, attribute_values, reference_product=None): - return product.get_filtered_variants(reference_product).filtered( - lambda variant: - all(attribute_value in variant.attribute_value_ids for attribute_value in attribute_values)) + # CASE: parent exclusion but good direction (parent is directional) + self.assertTrue(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1, mouse_color_green)) + + # CASE: parent exclusion and wrong direction (parent is directional) + self.assertFalse(mouse._is_combination_possible(mouse_color_green, computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + + # CASE: deleted combination + variant.unlink() + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + + def test_get_first_possible_combination(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ssd_512 = self._get_product_template_attribute_value(self.ssd_512) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_ram_32 = self._get_product_template_attribute_value(self.ram_32) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + computer_hdd_2 = self._get_product_template_attribute_value(self.hdd_2) + computer_hdd_4 = self._get_product_template_attribute_value(self.hdd_4) + self._add_exclude(computer_ram_16, computer_hdd_1) + + # ram_8 is allowed with the other attributes + self.assertEqual(self.computer._get_first_possible_combination(), computer_ssd_256 + computer_ram_8 + computer_hdd_1) + + # Below invalidate cache between every test to force reload the + # o2m attribute_line_ids to order it by the new sequence + + # Give priority to ram_16 but it is not allowed by hdd_1 so it should return hhd_2 instead + computer_ram_16.product_attribute_value_id.sequence = -1 + self.computer.invalidate_cache() # need o2m to be reordered + self.assertEqual(self.computer._get_first_possible_combination(), computer_ssd_256 + computer_ram_16 + computer_hdd_2) + + # Give priority to ram_32 and is allowed with the rest so it should return it + computer_ram_16.product_attribute_value_id.sequence = 2 + computer_ram_32.product_attribute_value_id.sequence = -1 + self.computer.invalidate_cache() # need o2m to be reordered + self.assertEqual(self.computer._get_first_possible_combination(), computer_ssd_256 + computer_ram_32 + computer_hdd_1) + + # Give priority to ram_16 but now it is not allowing any hdd so it should return ram_8 instead + computer_ram_32.product_attribute_value_id.sequence = 3 + computer_ram_16.product_attribute_value_id.sequence = -1 + self.computer.invalidate_cache() # need o2m to be reordered + self._add_exclude(computer_ram_16, computer_hdd_2) + self._add_exclude(computer_ram_16, computer_hdd_4) + self.assertEqual(self.computer._get_first_possible_combination(), computer_ssd_256 + computer_ram_8 + computer_hdd_1) + + # Only the last combination is possible + computer_ram_16.product_attribute_value_id.sequence = 2 + self.computer.invalidate_cache() # need o2m to be reordered + self._add_exclude(computer_ram_8, computer_hdd_1) + self._add_exclude(computer_ram_8, computer_hdd_2) + self._add_exclude(computer_ram_8, computer_hdd_4) + self._add_exclude(computer_ram_32, computer_hdd_1) + self._add_exclude(computer_ram_32, computer_hdd_2) + self._add_exclude(computer_ram_32, computer_ssd_256) + self.assertEqual(self.computer._get_first_possible_combination(), computer_ssd_512 + computer_ram_32 + computer_hdd_4) + + # No possible combination + self._add_exclude(computer_ram_32, computer_hdd_4) + self.assertEqual(self.computer._get_first_possible_combination(), self.env['product.template.attribute.value']) + + def test_get_closest_possible_combination(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + computer_hdd_2 = self._get_product_template_attribute_value(self.hdd_2) + computer_hdd_4 = self._get_product_template_attribute_value(self.hdd_4) + self._add_exclude(computer_ram_16, computer_hdd_1) + + # CASE nothing special + self.assertEqual(self.computer._get_closest_possible_combination(None), + computer_ssd_256 + computer_ram_8 + computer_hdd_1) + + # CASE contains computer_hdd_1 + self.assertEqual(self.computer._get_closest_possible_combination(computer_hdd_1), + computer_ssd_256 + computer_ram_8 + computer_hdd_1) + + # CASE contains computer_hdd_2 + self.assertEqual(self.computer._get_closest_possible_combination(computer_hdd_2), + computer_ssd_256 + computer_ram_8 + computer_hdd_2) + + # CASE contains computer_hdd_2, computer_ram_16 + self.assertEqual(self.computer._get_closest_possible_combination(computer_hdd_2 + computer_ram_16), + computer_ssd_256 + computer_ram_16 + computer_hdd_2) + + # CASE invalid combination (excluded): + self.assertEqual(self.computer._get_closest_possible_combination(computer_hdd_1 + computer_ram_16), + computer_ssd_256 + computer_ram_8 + computer_hdd_1) + + # CASE invalid combination (too much): + self.assertEqual(self.computer._get_closest_possible_combination(computer_ssd_256 + computer_ram_8 + computer_hdd_4 + computer_hdd_2), + computer_ssd_256 + computer_ram_8 + computer_hdd_4) diff --git a/addons/product/views/product_attribute_views.xml b/addons/product/views/product_attribute_views.xml index 503990fead8..84e82417237 100644 --- a/addons/product/views/product_attribute_views.xml +++ b/addons/product/views/product_attribute_views.xml @@ -27,7 +27,7 @@ diff --git a/addons/sale/models/analytic.py b/addons/sale/models/analytic.py index 9bac794a098..8fb938582a0 100644 --- a/addons/sale/models/analytic.py +++ b/addons/sale/models/analytic.py @@ -39,7 +39,7 @@ class AccountAnalyticLine(models.Model): self.ensure_one() if self.product_id.expense_policy == 'sales_price': return self.product_id.with_context( - partner=order.partner_id.id, + partner=order.partner_id, date_order=order.date_order, pricelist=order.pricelist_id.id, uom=self.product_uom_id.id diff --git a/addons/sale/models/product_product.py b/addons/sale/models/product_product.py index 027a371dc8a..3bf6b947e6b 100644 --- a/addons/sale/models/product_product.py +++ b/addons/sale/models/product_product.py @@ -53,6 +53,14 @@ class ProductProduct(models.Model): def _get_invoice_policy(self): return self.invoice_policy + @api.multi + def _get_combination_info_variant(self, add_qty=1, pricelist=False, parent_combination=False): + """Return the variant info based on its combination. + See `_get_combination_info` for more information. + """ + self.ensure_one() + return self.product_tmpl_id._get_combination_info(self.product_template_attribute_value_ids, self.id, add_qty, pricelist, parent_combination) + class ProductAttribute(models.Model): _inherit = "product.attribute" diff --git a/addons/sale/models/product_template.py b/addons/sale/models/product_template.py index cbcb9526287..8daa24e9f37 100644 --- a/addons/sale/models/product_template.py +++ b/addons/sale/models/product_template.py @@ -1,10 +1,14 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import json +import logging + from odoo import api, fields, models, _ from odoo.addons.base.models.res_partner import WARNING_MESSAGE, WARNING_HELP from odoo.tools.float_utils import float_round -import json + +_logger = logging.getLogger(__name__) class ProductTemplate(models.Model): @@ -55,21 +59,88 @@ class ProductTemplate(models.Model): } return action - def create_product_variant(self, product_template_attribute_value_ids): + @api.multi + def _create_product_variant(self, combination, log_warning=False): + """ Create if necessary and possible and return the product variant + matching the given combination for this template. + + It is possible to create only if the template has dynamic attributes + and the combination itself is possible. + + :param combination: the combination for which to get or create variant. + The combination must contain all necessary attributes, including + those of type no_variant. Indeed even though those attributes won't + be included in the variant if newly created, they are needed when + checking if the combination is possible. + :type combination: recordset of `product.template.attribute.value` + + :param log_warning: whether a warning should be logged on fail + :type log_warning: bool + + :return: the product variant matching the combination or none + :rtype: recordset of `product.product` + """ self.ensure_one() - attribute_value_ids = \ - self.env['product.template.attribute.value'] \ - .browse(json.loads(product_template_attribute_value_ids)) \ - .mapped('product_attribute_value_id') \ - .filtered(lambda attribute_value_id: attribute_value_id.attribute_id.create_variant != 'no_variant') + Product = self.env['product.product'] - product_variant = self.env['product.product'].create({ + product_variant = self._get_variant_for_combination(combination) + if product_variant: + return product_variant + + if not self.has_dynamic_attributes(): + if log_warning: + _logger.warning('The user #%s tried to create a variant for the non-dynamic product %s.' % (self.env.user.id, self.id)) + return Product + + if not self._is_combination_possible(combination): + if log_warning: + _logger.warning('The user #%s tried to create an invalid variant for the product %s.' % (self.env.user.id, self.id)) + return Product + + attribute_values = combination.mapped('product_attribute_value_id')._without_no_variant_attributes() + + return Product.sudo().create({ 'product_tmpl_id': self.id, - 'attribute_value_ids': [(6, 0, attribute_value_ids.ids)] + 'attribute_value_ids': [(6, 0, attribute_values.ids)] }) - return product_variant.id + @api.multi + def create_product_variant(self, product_template_attribute_value_ids): + """ Create if necessary and possible and return the id of the product + variant matching the given combination for this template. + + Note AWA: Known "exploit" issues with this method: + + - This method could be used by an unauthenticated user to generate a + lot of useless variants. Unfortunately, after discussing the + matter with ODO, there's no easy and user-friendly way to block + that behavior. + + We would have to use captcha/server actions to clean/... that + are all not user-friendly/overkill mechanisms. + + - This method could be used to try to guess what product variant ids + are created in the system and what product template ids are + configured as "dynamic", but that does not seem like a big deal. + + The error messages are identical on purpose to avoid giving too much + information to a potential attacker: + - returning 0 when failing + - returning the variant id whether it already existed or not + + :param product_template_attribute_value_ids: the combination for which + to get or create variant + :type product_template_attribute_value_ids: json encoded list of id + of `product.template.attribute.value` + + :return: id of the product variant matching the combination or 0 + :rtype: int + """ + combination = self.env['product.template.attribute.value'] \ + .browse(json.loads(product_template_attribute_value_ids)) + + return self._create_product_variant(combination, log_warning=True).id or 0 @api.onchange('type') def _onchange_type(self): @@ -93,3 +164,132 @@ class ProductTemplate(models.Model): 'template': '/sale/static/xls/product_pricelist_several.xls' }] return res + + @api.multi + def _get_combination_info(self, combination=False, product_id=False, add_qty=1, pricelist=False, parent_combination=False): + """ Return info about a given combination. + + Note: this method does not take into account whether the combination is + actually possible. + + :param combination: recordset of `product.template.attribute.value` + + :param product_id: id of a `product.product`. If no `combination` + is set, the method will try to load the variant `product_id` if + it exists instead of finding a variant based on the combination. + + If there is no combination, that means we definitely want a + variant and not something that will have no_variant set. + + :param add_qty: float with the quantity for which to get the info, + indeed some pricelist rules might depend on it. + + :param pricelist: `product.pricelist` the pricelist to use + (can be none, eg. from SO if no partner and no pricelist selected) + + :param parent_combination: if no combination and no product_id are + given, it will try to find the first possible combination, taking + into account parent_combination (if set) for the exclusion rules. + + :return: dict with product/combination info: + + - product_id: the variant id matching the combination (if it exists) + + - product_template_id: the current template id + + - display_name: the name of the combination + + - price: the computed price of the combination, take the catalog + price if no pricelist is given + + - list_price: the catalog price of the combination, but this is + not the "real" list_price, it has price_extra included (so + it's actually more closely related to `lst_price`), and it + is converted to the pricelist currency (if given) + + - has_discounted_price: True if the pricelist discount policy says + the price does not include the discount and there is actually a + discount applied (price < list_price), else False + """ + self.ensure_one() + + quantity = self.env.context.get('quantity', add_qty) + context = dict(self.env.context, quantity=quantity, pricelist=pricelist.id if pricelist else False) + product_template = self.with_context(context) + + combination = combination or product_template.env['product.template.attribute.value'] + + if not product_id and not combination: + combination = product_template._get_first_possible_combination(parent_combination) + + if product_id and not combination: + product = product_template.env['product.product'].browse(product_id) + else: + product = product_template._get_variant_for_combination(combination) + + if product: + # We need to add the price_extra for the attributes that are not + # in the variant, typically those of type no_variant, but it is + # possible that a no_variant attribute is still in a variant if + # the type of the attribute has been changed after creation. + no_variant_attributes_price_extra = [ + ptav.price_extra for ptav in combination.filtered( + lambda ptav: + ptav.price_extra and + ptav not in product.product_template_attribute_value_ids + ) + ] + if no_variant_attributes_price_extra: + product = product.with_context( + no_variant_attributes_price_extra=no_variant_attributes_price_extra + ) + list_price = product.price_compute('list_price')[product.id] + price = product.price if pricelist else list_price + else: + product_template = product_template.with_context(current_attributes_price_extra=[v.price_extra or 0.0 for v in combination]) + list_price = product_template.price_compute('list_price')[product_template.id] + price = product_template.price if pricelist else list_price + + display_name = product_template.name + + filtered_combination = combination._without_no_variant_attributes() + if filtered_combination: + display_name = '%s (%s)' % (display_name, ', '.join(filtered_combination.mapped('name'))) + + if pricelist and pricelist.currency_id != product_template.currency_id: + list_price = product_template.currency_id._convert( + list_price, pricelist.currency_id, product_template.company_id, + fields.Date.today() + ) + + price_without_discount = list_price if pricelist and pricelist.discount_policy == 'without_discount' else price + has_discounted_price = (pricelist or product_template).currency_id.compare_amounts(price_without_discount, price) == 1 + + return { + 'product_id': product.id, + 'product_template_id': product_template.id, + 'display_name': display_name, + 'price': price, + 'list_price': list_price, + 'has_discounted_price': has_discounted_price, + } + + @api.multi + def _is_add_to_cart_possible(self, parent_combination=None): + """ + It's possible to add to cart (potentially after configuration) if + there is at least one possible combination, or if there is no + `product.template.attribute.line` at all. + + :param parent_combination: the combination from which `self` is an + optional or accessory product. + :type parent_combination: recordset `product.template.attribute.value` + + :return: True if it's possible to add to cart, else False + :rtype: bool + """ + self.ensure_one() + if not self.active: + return False + combination = self._get_first_possible_combination(parent_combination) + return True if combination else self._is_combination_possible(combination, parent_combination) diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py index afad7904823..0c42e2bc529 100644 --- a/addons/sale/models/sale.py +++ b/addons/sale/models/sale.py @@ -1120,9 +1120,16 @@ class SaleOrderLine(models.Model): product_uom_qty = fields.Float(string='Ordered Quantity', digits=dp.get_precision('Product Unit of Measure'), required=True, default=1.0) product_uom = fields.Many2one('uom.uom', string='Unit of Measure') product_custom_attribute_value_ids = fields.One2many('product.attribute.custom.value', 'sale_order_line_id', string='User entered custom product attribute values') + # M2M holding the values of product.attribute with create_variant field set to 'no_variant' # It allows keeping track of the extra_price associated to those attribute values and add them to the SO line description + # Note: If the attributes are changed on the template, some or all records + # in `product_no_variant_attribute_value_ids` will be removed here, even + # from existing or locked sales order lines. Thus this field can only be + # relied on initially, but it cannot be used to recompute anything later + # because the result might be different then. product_no_variant_attribute_value_ids = fields.Many2many('product.template.attribute.value', string='Product attribute values that do not create variants') + # Non-stored related field to allow portal user to see the image of the product he has ordered product_image = fields.Binary('Product Image', related="product_id.image", store=False, readonly=False) @@ -1372,11 +1379,20 @@ class SaleOrderLine(models.Model): # TO DO: move me in master/saas-16 on sale.order # awa: don't know if it's still the case since we need the "product_no_variant_attribute_value_ids" field now # to be able to compute the full price - if self.product_no_variant_attribute_value_ids: - product = product.with_context(no_variant_attributes_price_extra=[ - no_variant_attribute_value.price_extra or 0 - for no_variant_attribute_value in self.product_no_variant_attribute_value_ids - ]) + + # it is possible that a no_variant attribute is still in a variant if + # the type of the attribute has been changed after creation. + no_variant_attributes_price_extra = [ + ptav.price_extra for ptav in self.product_no_variant_attribute_value_ids.filtered( + lambda ptav: + ptav.price_extra and + ptav not in product.product_template_attribute_value_ids + ) + ] + if no_variant_attributes_price_extra: + product = product.with_context( + no_variant_attributes_price_extra=no_variant_attributes_price_extra + ) if self.order_id.pricelist_id.discount_policy == 'with_discount': return product.with_context(pricelist=self.order_id.pricelist_id.id).price @@ -1397,6 +1413,16 @@ class SaleOrderLine(models.Model): if not self.product_id: return {'domain': {'product_uom': []}} + # remove the is_custom values that don't belong to this template + for pacv in self.product_custom_attribute_value_ids: + if pacv.attribute_value_id not in self.product_id.product_tmpl_id._get_valid_product_attribute_values(): + self.product_custom_attribute_value_ids -= pacv + + # remove the no_variant attributes that don't belong to this template + for ptav in self.product_no_variant_attribute_value_ids: + if ptav.product_attribute_value_id not in self.product_id.product_tmpl_id._get_valid_product_attribute_values(): + self.product_no_variant_attribute_value_ids -= ptav + vals = {} domain = {'product_uom': [('category_id', '=', self.product_id.uom_id.category_id.id)]} if not self.product_uom or (self.product_id.uom_id.id != self.product_uom.id): @@ -1405,7 +1431,7 @@ class SaleOrderLine(models.Model): product = self.product_id.with_context( lang=self.order_id.partner_id.lang, - partner=self.order_id.partner_id.id, + partner=self.order_id.partner_id, quantity=vals.get('product_uom_qty') or self.product_uom_qty, date=self.order_id.date_order, pricelist=self.order_id.pricelist_id.id, @@ -1429,20 +1455,6 @@ class SaleOrderLine(models.Model): name = self.get_sale_order_line_multiline_description_sale(product) - if self.product_custom_attribute_value_ids or self.product_no_variant_attribute_value_ids: - name += '\n' - - if self.product_custom_attribute_value_ids: - for product_custom_attribute_value in self.product_custom_attribute_value_ids: - if product_custom_attribute_value.custom_value and product_custom_attribute_value.custom_value.strip(): - name += '\n' + product_custom_attribute_value.attribute_value_id.name + ': ' + product_custom_attribute_value.custom_value.strip() - - if self.product_no_variant_attribute_value_ids: - for no_variant_attribute_value in self.product_no_variant_attribute_value_ids.filtered( - lambda product_attribute_value: not product_attribute_value.is_custom - ): - name += '\n' + no_variant_attribute_value.attribute_id.name + ': ' + no_variant_attribute_value.name - vals.update(name=name) self._compute_tax_id() @@ -1461,7 +1473,7 @@ class SaleOrderLine(models.Model): if self.order_id.pricelist_id and self.order_id.partner_id: product = self.product_id.with_context( lang=self.order_id.partner_id.lang, - partner=self.order_id.partner_id.id, + partner=self.order_id.partner_id, quantity=self.product_uom_qty, date=self.order_id.date_order, pricelist=self.order_id.pricelist_id.id, @@ -1557,7 +1569,7 @@ class SaleOrderLine(models.Model): self.discount = 0.0 product = self.product_id.with_context( lang=self.order_id.partner_id.lang, - partner=self.order_id.partner_id.id, + partner=self.order_id.partner_id, quantity=self.product_uom_qty, date=self.order_id.date_order, pricelist=self.order_id.pricelist_id.id, @@ -1591,4 +1603,38 @@ class SaleOrderLine(models.Model): BUT in event_sale we need to know specifically the sales order line as well as the product to generate the name: the product is not sufficient because we also need to know the event_id and the event_ticket_id (both which belong to the sale order line). """ - return product.get_product_multiline_description_sale() + return product.get_product_multiline_description_sale() + self._get_sale_order_line_multiline_description_variants() + + def _get_sale_order_line_multiline_description_variants(self): + """When using no_variant attributes or is_custom values, the product + itself is not sufficient to create the description: we need to add + information about those special attributes and values. + + See note about `product_no_variant_attribute_value_ids` above the field + definition: this method is not reliable to recompute the description at + a later time, it should only be used initially. + + :return: the description related to special variant attributes/values + :rtype: string + """ + if not self.product_custom_attribute_value_ids and not self.product_no_variant_attribute_value_ids: + return "" + + name = "\n" + + product_attribute_with_is_custom = self.product_custom_attribute_value_ids.mapped('attribute_value_id.attribute_id') + + # display the no_variant attributes, except those that are also + # displayed by a custom (avoid duplicate) + for no_variant_attribute_value in self.product_no_variant_attribute_value_ids.filtered( + lambda ptav: ptav.attribute_id not in product_attribute_with_is_custom + ): + name += "\n" + no_variant_attribute_value.attribute_id.name + ': ' + no_variant_attribute_value.name + + # display the is_custom values + for pacv in self.product_custom_attribute_value_ids: + name += "\n" + pacv.attribute_value_id.attribute_id.name + \ + ': ' + pacv.attribute_value_id.name + \ + ': ' + (pacv.custom_value or '').strip() + + return name diff --git a/addons/sale/static/src/js/product_configurator_controller.js b/addons/sale/static/src/js/product_configurator_controller.js index 824bcb923a7..60322d72435 100644 --- a/addons/sale/static/src/js/product_configurator_controller.js +++ b/addons/sale/static/src/js/product_configurator_controller.js @@ -48,21 +48,27 @@ var ProductConfiguratorFormController = FormController.extend({ * @override */ _onFieldChanged: function (event) { + this._super.apply(this, arguments); + var self = this; + var product_id = event.data.changes.product_template_id.id; + + // check to prevent traceback when emptying the field + if (!product_id) { + return; + } this.$el.parents('.modal').find('.o_sale_product_configurator_add').removeClass('disabled'); this._rpc({ route: '/product_configurator/configure', params: { - product_id: event.data.changes.product_template_id.id, + product_id: product_id, pricelist_id: this.renderer.pricelistId } }).then(function (configurator) { self.renderer.renderConfigurator(configurator); }); - - this._super.apply(this, arguments); }, //-------------------------------------------------------------------------- @@ -94,7 +100,7 @@ var ProductConfiguratorFormController = FormController.extend({ false ); - productReady.done(function (productId){ + productReady.done(function (productId) { $modal.find(productSelector.join(', ')).val(productId); var variantValues = self diff --git a/addons/sale/static/src/js/product_configurator_mixin.js b/addons/sale/static/src/js/product_configurator_mixin.js index aa1066a0c24..458dd5a580c 100644 --- a/addons/sale/static/src/js/product_configurator_mixin.js +++ b/addons/sale/static/src/js/product_configurator_mixin.js @@ -13,6 +13,7 @@ var ProductConfiguratorMixin = { 'click button.js_add_cart_json': 'onClickAddCartJSON', 'change [data-attribute_exclusions]': 'onChangeVariant' }, + isSelectedVariantAllowed: true, //-------------------------------------------------------------------------- // Public @@ -22,7 +23,6 @@ var ProductConfiguratorMixin = { * When a product is added or when the quantity is changed, * we need to refresh the total price row * TODO awa: add a container context to avoid global selectors ? - * */ computePriceTotal: function () { if ($('.js_price_total').length){ @@ -56,8 +56,10 @@ var ProductConfiguratorMixin = { $component = $(ev.currentTarget).closest('form'); } else if ($(ev.currentTarget).closest('.oe_optional_products_modal').length > 0){ $component = $(ev.currentTarget).closest('.oe_optional_products_modal'); - } else { + } else if ($(ev.currentTarget).closest('.o_product_configurator').length > 0) { $component = $(ev.currentTarget).closest('.o_product_configurator'); + } else { + $component = $(ev.currentTarget); } var qty = $component.find('input[name="add_qty"]').val(); @@ -68,10 +70,10 @@ var ProductConfiguratorMixin = { ajax.jsonRpc(this._getUri('/product_configurator/get_combination_info'), 'call', { product_template_id: parseInt($parent.find('.product_template_id').val()), - product_id: parseInt($parent.find('.product_id').val()), + product_id: this._getProductId($parent), combination: combination, add_qty: parseInt(qty), - pricelist_id: this.pricelistId + pricelist_id: this.pricelistId || false, }).then(function (combinationData) { self._onChangeCombination(ev, $parent, combinationData); }); @@ -291,23 +293,28 @@ var ProductConfiguratorMixin = { */ selectOrCreateProduct: function ($container, productId, productTemplateId, useAjax) { var self = this; + productId = parseInt(productId); + productTemplateId = parseInt(productTemplateId); var productReady = $.Deferred(); - if (productId && productId !== '0'){ + if (productId) { productReady.resolve(productId); } else { var params = { - model: 'product.template', - method: 'create_product_variant', - args: [ - productTemplateId, - JSON.stringify(self.getSelectedVariantValues($container)) - ] + product_template_id: productTemplateId, + product_template_attribute_value_ids: + JSON.stringify(self.getSelectedVariantValues($container)), }; + // Note about 12.0 compatibility: this route will not exist if + // updating the code but not restarting the server. (404) + // We don't handle that compatibility because the previous code was + // not working either: it was making an RPC that failed with any + // non-admin user anyway. To use this feature, restart the server. + var route = '/product_configurator/create_product_variant'; if (useAjax) { - productReady = ajax.jsonRpc('/web/dataset/call', 'call', params); + productReady = ajax.jsonRpc(route, 'call', params); } else { - productReady = this._rpc(params); + productReady = this._rpc({route: route, params: params}); } } @@ -335,6 +342,23 @@ var ProductConfiguratorMixin = { * @param {Array} combination the selected combination of product attribute values */ _checkExclusions: function ($parent, combination) { + + function areCombinationsEqual(c1, c2) { + return c1.length === c2.length && _.every(c1, function (ptav) { + return c2.indexOf(ptav) > -1; + }); + } + + function isCombinationInList(c1, list) { + return _.some(list, function (c2) { + return areCombinationsEqual(c1, c2); + }); + } + + function isPtavInCombination(ptav, combination) { + return combination.indexOf(ptav) > -1; + } + var self = this; var combinationData = $parent .find('ul[data-attribute_exclusions]') @@ -343,43 +367,62 @@ var ProductConfiguratorMixin = { $parent.find('option, input, label').removeClass('css_not_available'); var disable = false; + + // compatibility 12.0 + var filteredCombination = combination; + if (combinationData.no_variant_product_template_attribute_value_ids !== undefined) { + var no_variants = combinationData.no_variant_product_template_attribute_value_ids; + filteredCombination = _.filter(combination, function (ptav) { + return !isPtavInCombination(ptav, no_variants); + }); + } + + // exclusion rules: array of ptav + // for each of them, contains array with the other ptav they exclude if (combinationData.exclusions) { - _.each(combination, function (combinationValue){ - if (combinationData.exclusions && - combinationData.exclusions.hasOwnProperty(combinationValue)){ - // check that the selected combination is in the exclusions - _.each(combinationData.exclusions[combinationValue], function (exclusion) { - if (!disable && combination.indexOf(exclusion) > -1) { + // browse all the currently selected attributes + _.each(combination, function (current_ptav) { + if (combinationData.exclusions.hasOwnProperty(current_ptav)) { + // for each exclusion of the current attribute: + _.each(combinationData.exclusions[current_ptav], function (excluded_ptav) { + // disable if it excludes any other attribute already in the combination + if (isPtavInCombination(excluded_ptav, combination)) { disable = true; } - self._disableInput($parent, exclusion); + // disable the excluded input (even when not already selected) + // to give a visual feedback before click + self._disableInput($parent, excluded_ptav); }); } }); } - if (combinationData.parent_exclusions){ - _.each(combinationData.parent_exclusions, function (exclusion){ - if (!disable && combination.indexOf(exclusion) > -1) { - disable = true; - } - self._disableInput($parent, exclusion); - }); + // parent exclusions (tell which attributes are excluded from parent) + _.each(combinationData.parent_exclusions, function (ptav) { + if (isPtavInCombination(ptav, combination)) { + disable = true; + } + // disable the excluded input (even when not already selected) + // to give a visual feedback before click + self._disableInput($parent, ptav); + }); + + // archived variants + if (isCombinationInList(filteredCombination, combinationData.archived_combinations)) { + disable = true; } - if (combinationData.archived_combinations){ - _.each(combinationData.archived_combinations, function (archived_combination){ - if (disable) { - return; - } - - disable = _.every(archived_combination, function (attribute_value){ - return combination.indexOf(attribute_value) > -1; - }); - }); + // if not using dynamic attributes, exclude variants that are deleted + if (filteredCombination.length && // compatibility 12.0 list view of variants + combinationData.has_dynamic_attributes === false && + combinationData.existing_combinations !== undefined && + !isCombinationInList(filteredCombination, combinationData.existing_combinations) + ) { + disable = true; } + this.isSelectedVariantAllowed = !disable; $parent.toggleClass('css_not_available', disable); $parent.find("#add_to_cart").toggleClass('disabled', disable); $parent @@ -388,6 +431,15 @@ var ProductConfiguratorMixin = { .toggleClass('disabled', disable); }, + /** + * Extracted to a method to be extendable by other modules + * + * @param {$.Element} $parent + */ + _getProductId: function ($parent) { + return parseInt($parent.find('.product_id').val()); + }, + /** * Will disable the input/option that refers to the passed attributeValueId. * This is used for showing the user that some combinations are not available. @@ -418,7 +470,11 @@ var ProductConfiguratorMixin = { var $optional_price = $parent.find(".oe_optional:first .oe_currency_value"); $price.html(self._priceToStr(combination.price)); $default_price.html(self._priceToStr(combination.list_price)); - if (combination.list_price - combination.price >= 0.01) { + + // compatibility_check to remove in master + // needed for fix in 12.0 in the case of git pull and no server restart + var compatibility_check = combination.list_price - combination.price >= 0.01; + if (combination.has_discounted_price !== undefined ? combination.has_discounted_price : compatibility_check) { $default_price .closest('.oe_website_sale') .addClass("discount"); @@ -428,6 +484,9 @@ var ProductConfiguratorMixin = { .css('text-decoration', 'line-through'); $default_price.parent().removeClass('d-none'); } else { + $default_price + .closest('.oe_website_sale') + .removeClass("discount"); $optional_price.closest('.oe_optional').addClass('d-none'); $default_price.parent().addClass('d-none'); } @@ -438,11 +497,16 @@ var ProductConfiguratorMixin = { '.o_product_configurator' ]; - self._updateProductImage( - $parent.closest(rootComponentSelectors.join(', ')), - combination.product_id, - combination.product_template_id - ); + // update images only when changing product + if (combination.product_id !== this.last_product_id) { + this.last_product_id = combination.product_id; + self._updateProductImage( + $parent.closest(rootComponentSelectors.join(', ')), + combination.product_id, + combination.product_template_id, + combination.carousel + ); + } $parent .find('.product_id') @@ -492,47 +556,20 @@ var ProductConfiguratorMixin = { * @param {integer} productTemplateId */ _updateProductImage: function ($productContainer, productId, productTemplateId) { - var $img; var model = productId ? 'product.product' : 'product.template'; var modelId = productId || productTemplateId; - var imageSrc = '/web/image?model={0}&id={1}&field=image' + var imageSrc = '/web/image/{0}/{1}/image' .replace("{0}", model) .replace("{1}", modelId); - if ($productContainer.find('#o-carousel-product').length) { - $img = $productContainer.find('img.js_variant_img'); - $img.attr("src", imageSrc); - $img.parent().attr('data-oe-model', model).attr('data-oe-id', modelId) - .data('oe-model', model).data('oe-id', modelId); + var imagesSelectors = [ + 'span[data-oe-model^="product."][data-oe-type="image"] img:first', + 'img.product_detail_img', + 'span.variant_image img' + ]; - var $thumbnail = $productContainer.find('img.js_variant_img_small'); - if ($thumbnail.length !== 0) { // if only one, thumbnails are not displayed - $thumbnail.attr("src", "/web/image/{0}/{1}/image/90x90" - .replace('{0}', model) - .replace('{1}', modelId)); - $('.carousel').carousel(0); - } - } - else { - var imagesSelectors = [ - 'span[data-oe-model^="product."][data-oe-type="image"] img:first', - 'img.product_detail_img', - 'span.variant_image img' - ]; - - $img = $productContainer.find(imagesSelectors.join(', ')); - $img.attr('src', imageSrc); - $img.parent() - .attr('data-oe-model', model) - .attr('data-oe-id', modelId) - .data('oe-model', model) - .data('oe-id', modelId); - } - // reset zooming constructs - $img.filter('[data-zoom-image]').attr('data-zoom-image', $img.attr('src')); - if ($img.data('zoomOdoo') !== undefined) { - $img.data('zoomOdoo').isReady = false; - } + var $img = $productContainer.find(imagesSelectors.join(', ')); + $img.attr('src', imageSrc); }, /** @@ -553,11 +590,13 @@ var ProductConfiguratorMixin = { * Website behavior is slightly different from backend so we append * "_website" to URLs to lead to a different route * + * TODO this should be overriden in website_sale instead. + * * @private * @param {string} uri The uri to adapt */ _getUri: function (uri) { - if (this.isWebsite){ + if (this.isWebsite) { return uri + '_website'; } else { return uri; diff --git a/addons/sale/static/src/js/product_configurator_modal.js b/addons/sale/static/src/js/product_configurator_modal.js index e135d30e2d8..630774b8da5 100644 --- a/addons/sale/static/src/js/product_configurator_modal.js +++ b/addons/sale/static/src/js/product_configurator_modal.js @@ -72,10 +72,6 @@ var OptionalProductsModal = Dialog.extend(ServicesMixin, ProductConfiguratorMixi delete this.events['change [data-attribute_exclusions]']; delete this.events['click button.js_add_cart_json']; } - - this._opened.then(function () { - self.triggerVariantChange(self.$el); - }); }, /** * @override @@ -87,7 +83,8 @@ var OptionalProductsModal = Dialog.extend(ServicesMixin, ProductConfiguratorMixi var getModalContent = ajax.jsonRpc(uri, 'call', { product_id: self.rootProduct.product_id, variant_values: self.rootProduct.variant_values, - pricelist_id: self.pricelistId, + pricelist_id: self.pricelistId || false, + add_qty: self.rootProduct.quantity, kwargs: { context: _.extend({ 'quantity': self.rootProduct.quantity @@ -126,6 +123,7 @@ var OptionalProductsModal = Dialog.extend(ServicesMixin, ProductConfiguratorMixi self.$modal.attr('open', true); self.$modal.removeAttr("aria-hidden"); self.$modal.modal().appendTo(self.container); + self.$modal.focus(); self._opened.resolve(); } }); @@ -140,10 +138,18 @@ var OptionalProductsModal = Dialog.extend(ServicesMixin, ProductConfiguratorMixi * * @override */ - start: function (){ - this._super.apply(this, arguments); + start: function () { + var def = this._super.apply(this, arguments); + var self = this; this.$el.find('input[name="add_qty"]').val(this.rootProduct.quantity); + + return def.then(function () { + // This has to be triggered to compute the "out of stock" feature + self._opened.then(function () { + self.triggerVariantChange(self.$el); + }); + }); }, //-------------------------------------------------------------------------- @@ -359,7 +365,7 @@ var OptionalProductsModal = Dialog.extend(ServicesMixin, ProductConfiguratorMixi ajax.jsonRpc(self._getUri("/product_configurator/optional_product_items"), 'call', { 'product_id': productId, - 'pricelist_id': self.pricelistId + 'pricelist_id': self.pricelistId || false, }).then(function (addedItem) { var $addedItem = $(addedItem); $modal.find('tr:last').after($addedItem); @@ -410,7 +416,7 @@ var OptionalProductsModal = Dialog.extend(ServicesMixin, ProductConfiguratorMixi this._removeOptionOption($modal, productTemplateId); - $('tr:last').after($parent); + $modal.find('tr:last').after($parent); }, /** diff --git a/addons/sale/static/src/js/tour.js b/addons/sale/static/src/js/tour.js index 9992cbb3a42..fe4eac6fbbc 100644 --- a/addons/sale/static/src/js/tour.js +++ b/addons/sale/static/src/js/tour.js @@ -46,7 +46,7 @@ tour.register('sale_tour', { content: _t("Select a product, or create a new one on the fly."), position: "right", run: function (actions) { - actions.text("DESK0001", this.$anchor.find("input")); + actions.text("Chair", this.$anchor.find("input")); }, }, { trigger: ".ui-menu-item > a", diff --git a/addons/sale/static/src/scss/product_configurator.scss b/addons/sale/static/src/scss/product_configurator.scss index ca3b9f63af2..8cf022dad50 100644 --- a/addons/sale/static/src/scss/product_configurator.scss +++ b/addons/sale/static/src/scss/product_configurator.scss @@ -39,6 +39,13 @@ .css_not_available_msg { display: block; } + + .js_add, + .oe_price, + .oe_default_price, + .oe_optional { + display: none; + } } .css_quantity { @@ -113,12 +120,7 @@ label.css_attribute_color.css_not_available { } .o_total_row { - h5 { - padding-top: 0.4rem; - } - .js_price_total { - font-size: 1.5rem; - } + font-size: 1.2rem; } } @@ -140,10 +142,6 @@ label.css_attribute_color.css_not_available { background-image: none; } - .remove-optionnal-item, .add-optionnal-item { - padding-left: 5px; - } - .td-product_name { word-wrap: break-word; } diff --git a/addons/sale/tests/__init__.py b/addons/sale/tests/__init__.py index 08ee2e8d381..4e231c7ae06 100644 --- a/addons/sale/tests/__init__.py +++ b/addons/sale/tests/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from . import test_sale_product_attribute_value_config from . import test_sale_to_invoice from . import test_sale_order from . import test_sale_pricelist diff --git a/addons/sale/tests/test_sale_product_attribute_value_config.py b/addons/sale/tests/test_sale_product_attribute_value_config.py new file mode 100644 index 00000000000..386d3fe31d4 --- /dev/null +++ b/addons/sale/tests/test_sale_product_attribute_value_config.py @@ -0,0 +1,343 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.addons.product.tests.test_product_attribute_value_config import TestProductAttributeValueSetup +from odoo.tests import tagged + + +@tagged('post_install', '-at_install') +class TestSaleProductAttributeValueConfig(TestProductAttributeValueSetup): + def _setup_pricelist(self): + # easy currency rate, for a simple 2 ratio in the following tests + self.computer.currency_id.rate_ids[0].rate = 1 + currency_ratio = 2 + currency = self.env['res.currency'].browse(1 if self.computer.currency_id.id != 1 else 2) + currency.rate_ids[0].rate = currency_ratio + + discount = 10 + + pricelist = self.env['product.pricelist'].create({ + 'name': 'test pl', + 'currency_id': currency.id, + 'company_id': self.computer.company_id.id, + }) + + pricelist_item = self.env['product.pricelist.item'].create({ + 'min_quantity': 2, + 'compute_price': 'percentage', + 'percent_price': discount + }) + + pricelist.item_ids += pricelist_item + + return (pricelist, pricelist_item, currency_ratio, 1 - discount / 100) + + def test_01_is_combination_possible_archived(self): + """The goal is to test the possibility of archived combinations. + This test could not be put into product module because there was no + field which had product_id as required and without cascade on delete. + """ + def do_test(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + computer_hdd_2 = self._get_product_template_attribute_value(self.hdd_2) + + variant = self.computer._get_variant_for_combination(computer_ssd_256 + computer_ram_8 + computer_hdd_1) + variant2 = self.computer._get_variant_for_combination(computer_ssd_256 + computer_ram_8 + computer_hdd_2) + + # Create a dummy SO to prevent the variant from being deleted by + # create_variant_ids() because the variant is a related field that + # is required on the SO line + so = self.env['sale.order'].create({'partner_id': 1}) + self.env['sale.order.line'].create({ + 'order_id': so.id, + 'name': "test", + 'product_id': variant.id + }) + # additional variant to test correct ignoring when mismatch values + self.env['sale.order.line'].create({ + 'order_id': so.id, + 'name': "test", + 'product_id': variant2.id + }) + + variant2.active = False + # CASE: 1 not archived, 2 archived + self.assertTrue(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_2)) + # CASE: both archived combination (without no_variant) + variant.active = False + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_2)) + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + + # CASE: not archived (with no_variant) + self.computer_hdd_attribute_lines.unlink() + self.hdd_attribute.create_variant = 'no_variant' + self._add_hdd_attribute_line() + self.computer.create_variant_ids() + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + computer_hdd_2 = self._get_product_template_attribute_value(self.hdd_2) + + self.assertTrue(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + + # CASE: archived combination found (with no_variant) + variant = self.computer._get_variant_for_combination(computer_ssd_256 + computer_ram_8 + computer_hdd_1) + variant.active = False + self.assertFalse(self.computer._is_combination_possible(computer_ssd_256 + computer_ram_8 + computer_hdd_1)) + + # CASE: archived combination has different attributes (including no_variant) + self.computer_ssd_attribute_lines.unlink() + self.computer.create_variant_ids() + + variant4 = self.computer._get_variant_for_combination(computer_ram_8 + computer_hdd_1) + self.env['sale.order.line'].create({ + 'order_id': so.id, + 'name': "test", + 'product_id': variant4.id + }) + self.assertTrue(self.computer._is_combination_possible(computer_ram_8 + computer_hdd_1)) + + # CASE: archived combination has different attributes (without no_variant) + self.computer_hdd_attribute_lines.unlink() + self.hdd_attribute.create_variant = 'always' + self._add_hdd_attribute_line() + self.computer.create_variant_ids() + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + computer_hdd_2 = self._get_product_template_attribute_value(self.hdd_2) + + variant5 = self.computer._get_variant_for_combination(computer_ram_8 + computer_hdd_1) + self.env['sale.order.line'].create({ + 'order_id': so.id, + 'name': "test", + 'product_id': variant5.id + }) + + self.assertTrue(variant4 != variant5) + + self.assertTrue(self.computer._is_combination_possible(computer_ram_8 + computer_hdd_1)) + + computer_ssd_256_before = self._get_product_template_attribute_value(self.ssd_256) + + do_test(self) + + # CASE: add back the removed attribute and try everything again + # It will be the same attribute but the ptal and ptav will be different! + self.computer_ssd_attribute_lines = self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': self.computer.id, + 'attribute_id': self.ssd_attribute.id, + 'value_ids': [(6, 0, [self.ssd_256.id, self.ssd_512.id])], + }) + self.computer.create_variant_ids() + + computer_ssd_256_after = self._get_product_template_attribute_value(self.ssd_256) + self.assertTrue(computer_ssd_256_after != computer_ssd_256_before) + do_test(self) + + def test_02_get_combination_info(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + + # CASE: no pricelist, no currency, with existing combination, with price_extra on attributes + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + computer_variant = self.computer._get_variant_for_combination(combination) + + res = self.computer._get_combination_info(combination) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], computer_variant.id) + self.assertEqual(res['display_name'], "Super Computer (256 GB, 8 GB, 1 To)") + self.assertEqual(res['price'], 2222) + self.assertEqual(res['list_price'], 2222) + + # CASE: no combination, product given + res = self.computer._get_combination_info(self.env['product.template.attribute.value'], computer_variant.id) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], computer_variant.id) + # the variant has the same name as the template + self.assertEqual(res['display_name'], "Super Computer") + self.assertEqual(res['price'], 2222) + self.assertEqual(res['list_price'], 2222) + + # CASE: using pricelist, quantity rule + pricelist, pricelist_item, currency_ratio, discount_ratio = self._setup_pricelist() + + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], computer_variant.id) + self.assertEqual(res['display_name'], "Super Computer (256 GB, 8 GB, 1 To)") + self.assertEqual(res['price'], 2222 * currency_ratio * discount_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + + # CASE: no_variant combination, it's another variant now + + self.computer_ssd_attribute_lines.unlink() + self.ssd_attribute.create_variant = 'no_variant' + self._add_ssd_attribute_line() + self.computer.create_variant_ids() + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + + computer_variant_new = self.computer._get_variant_for_combination(combination) + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], computer_variant_new.id) + self.assertEqual(res['display_name'], "Super Computer (8 GB, 1 To)") + self.assertEqual(res['price'], 2222 * currency_ratio * discount_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + + # CASE: dynamic combination, but the variant already exists + self.computer_hdd_attribute_lines.unlink() + self.hdd_attribute.create_variant = 'dynamic' + self._add_hdd_attribute_line() + self.computer.create_variant_ids() + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], computer_variant_new.id) + self.assertEqual(res['display_name'], "Super Computer (8 GB, 1 To)") + self.assertEqual(res['price'], 2222 * currency_ratio * discount_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + + # CASE: dynamic combination, no variant existing + self._add_keyboard_attribute() + self.computer.create_variant_ids() + combination += self._get_product_template_attribute_value(self.keyboard_excluded) + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], False) + self.assertEqual(res['display_name'], "Super Computer (8 GB, 1 To, Excluded)") + self.assertEqual(res['price'], (2222 - 5) * currency_ratio * discount_ratio) + self.assertEqual(res['list_price'], (2222 - 5) * currency_ratio) + + # CASE: pricelist set value to 0, no variant + pricelist_item.percent_price = 100 + self.computer.invalidate_cache() # need o2m to be refetched + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['product_template_id'], self.computer.id) + self.assertEqual(res['product_id'], False) + self.assertEqual(res['display_name'], "Super Computer (8 GB, 1 To, Excluded)") + self.assertEqual(res['price'], 0) + self.assertEqual(res['list_price'], (2222 - 5) * currency_ratio) + + def test_03_get_combination_info_discount_policy(self): + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + + pricelist, pricelist_item, currency_ratio, discount_ratio = self._setup_pricelist() + + pricelist.discount_policy = 'with_discount' + + # CASE: no discount, setting with_discount + res = self.computer._get_combination_info(combination, add_qty=1, pricelist=pricelist) + self.assertEqual(res['price'], 2222 * currency_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + self.assertEqual(res['has_discounted_price'], False) + + # CASE: discount, setting with_discount + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['price'], 2222 * currency_ratio * discount_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + self.assertEqual(res['has_discounted_price'], False) + + # CASE: no discount, setting without_discount + pricelist.discount_policy = 'without_discount' + res = self.computer._get_combination_info(combination, add_qty=1, pricelist=pricelist) + self.assertEqual(res['price'], 2222 * currency_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + self.assertEqual(res['has_discounted_price'], False) + + # CASE: discount, setting without_discount + res = self.computer._get_combination_info(combination, add_qty=2, pricelist=pricelist) + self.assertEqual(res['price'], 2222 * currency_ratio * discount_ratio) + self.assertEqual(res['list_price'], 2222 * currency_ratio) + self.assertEqual(res['has_discounted_price'], True) + + def test_04_create_product_variant_non_dynamic(self): + """The goal of this test is to make sure the create_product_variant does + not create variant if the type is not dynamic. It can however return a + variant if it already exists.""" + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + self._add_exclude(computer_ram_16, computer_hdd_1) + + # CASE: variant is already created, it should return it + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + variant1 = self.computer._get_variant_for_combination(combination) + self.assertEqual(self.computer._create_product_variant(combination), variant1) + + # CASE: variant does not exist, but template is non-dynamic, so it + # should not create it + Product = self.env['product.product'] + variant1.unlink() + self.assertEqual(self.computer._create_product_variant(combination), Product) + + def test_05_create_product_variant_dynamic(self): + """The goal of this test is to make sure the create_product_variant does + work with dynamic. If the combination is possible, it should create it. + If it's not possible, it should not create it.""" + self.computer_hdd_attribute_lines.unlink() + self.computer.create_variant_ids() + self.hdd_attribute.create_variant = 'dynamic' + self._add_hdd_attribute_line() + self.computer.create_variant_ids() + self.computer.invalidate_cache() + + computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256) + computer_ram_8 = self._get_product_template_attribute_value(self.ram_8) + computer_ram_16 = self._get_product_template_attribute_value(self.ram_16) + computer_hdd_1 = self._get_product_template_attribute_value(self.hdd_1) + self._add_exclude(computer_ram_16, computer_hdd_1) + + # CASE: variant does not exist, but combination is not possible + # so it should not create it + impossible_combination = computer_ssd_256 + computer_ram_16 + computer_hdd_1 + Product = self.env['product.product'] + self.assertEqual(self.computer._create_product_variant(impossible_combination), Product) + + # CASE: the variant does not exist, and the combination is possible, so + # it should create it + combination = computer_ssd_256 + computer_ram_8 + computer_hdd_1 + variant = self.computer._create_product_variant(combination) + self.assertTrue(variant) + + # CASE: the variant already exists, so it should return it + self.assertEqual(variant, self.computer._create_product_variant(combination)) + + def _add_keyboard_attribute(self): + self.keyboard_attribute = self.env['product.attribute'].create({ + 'name': 'Keyboard', + 'sequence': 6, + 'create_variant': 'dynamic', + }) + self.keyboard_included = self.env['product.attribute.value'].create({ + 'name': 'Included', + 'attribute_id': self.keyboard_attribute.id, + 'sequence': 1, + }) + self.keyboard_excluded = self.env['product.attribute.value'].create({ + 'name': 'Excluded', + 'attribute_id': self.keyboard_attribute.id, + 'sequence': 2, + }) + self.computer_keyboard_attribute_lines = self.env['product.template.attribute.line'].create({ + 'product_tmpl_id': self.computer.id, + 'attribute_id': self.keyboard_attribute.id, + 'value_ids': [(6, 0, [self.keyboard_included.id, self.keyboard_excluded.id])], + }) + self.computer_keyboard_attribute_lines.product_template_value_ids[0].price_extra = 5 + self.computer_keyboard_attribute_lines.product_template_value_ids[1].price_extra = -5 diff --git a/addons/sale/views/sale_product_configurator_templates.xml b/addons/sale/views/sale_product_configurator_templates.xml index a08bbc35626..6711dd2db8b 100644 --- a/addons/sale/views/sale_product_configurator_templates.xml +++ b/addons/sale/views/sale_product_configurator_templates.xml @@ -6,31 +6,48 @@ + + +