[IMP/FIX] Preventing duplicates via checks and refactored create_variant to get_variant

Author: PCatinean

product_configurator/README.md

@@ -19,4 +19,4 @@ Usage
 
 This module is only the foundation for external configuration interfaces such as 'product_configurator_wizard' or 'website_product_configurator'.
 
-By itself this module does not configure custom products but offers the basis for generating, validating, updating configurable products using configuration interfaces.
+By itself this module does not configure custom products but offers the basis for generating, validating, updating configurable products using configuration interfaces.

product_configurator/models/product.py

@@ -327,21 +327,35 @@ def get_variant_vals(self, value_ids, custom_values=None, **kwargs):
 
     @api.multi
     def create_variant(self, value_ids, custom_values=None):
-        """ Creates a product.variant with the attributes passed via value_ids
-        and custom_values
+        """Wrapper method for backward compatibility"""
+        # TODO: Remove in newer versions
+        return self.create_get_variant(
+            value_ids=value_ids, custom_values=custom_values)
+
+    @api.multi
+    def create_get_variant(self, value_ids, custom_values=None):
+        """ Creates a new product variant with the attributes passed via value_ids
+        and custom_values or retrieves an existing one based on search result
 
             :param value_ids: list of product.attribute.values ids
             :param custom_values: dict {product.attribute.id: custom_value}
 
-            :returns: product.product recordset of products matching domain
+            :returns: new/existing product.product recordset
 
         """
         if custom_values is None:
             custom_values = {}
         valid = self.validate_configuration(value_ids, custom_values)
         if not valid:
             raise ValidationError(_('Invalid Configuration'))
-        # TODO: Add all custom values to order line instead of product
+
+        duplicates = self.search_variant(value_ids)
+
+        # Only return duplicates without custom values for now:
+        if duplicates.filtered(lambda p: not p.value_custom_ids):
+            return duplicates[0]
+
+        # TODO: Handle duplicates with custom values
         vals = self.get_variant_vals(value_ids, custom_values)
         variant = self.env['product.product'].create(vals)
 
@@ -383,7 +397,8 @@ def values_available(self, attr_val_ids, sel_val_ids):
         are available for selection given the configuration ids and the
         dependencies set on the product template
 
-        :param attr_val_ids: list of attribute values
+        :param attr_val_ids: list of attribute value ids to check for
+                             availability
         :param sel_val_ids: list of attribute value ids already selected
 
         :returns: list of available attribute values
@@ -500,6 +515,27 @@ def _get_conversions_dict(self):
         }
         return conversions
 
+    @api.multi
+    @api.constrains('attribute_value_ids')
+    def _check_duplicate_product(self):
+        if not self.config_ok:
+            return None
+
+        # All duplicates with and without custom values
+        duplicates = self.product_tmpl_id.search_variant(
+            self.attribute_value_ids.ids).filtered(lambda p: p.id != self.id)
+
+        # Prevent duplicates without custom values (only attribute values)
+        if duplicates.filtered(lambda p: not p.value_custom_ids):
+            raise ValidationError(
+                _("Configurable Products cannot have duplicates "
+                  "(identical attribute values)")
+            )
+
+        # TODO: For the future prevent duplicates with identical custom values
+        # or implement custom values on the order line level since they are
+        # specific to each order.
+
     @api.multi
     def _compute_product_price_extra(self):
         """Compute price of configurable products as sum

product_configurator/static/js/form_widgets.js

@@ -21,6 +21,6 @@ var FieldBooleanButton = core.form_widget_registry.map['boolean_button'].extend(
 });
 
 
-core.form_widget_registry.add('boolean_button', FieldBooleanButton)
+core.form_widget_registry.add('boolean_button', FieldBooleanButton);
 
-})
+});

product_configurator/tests/test_configuration_rules.py

@@ -101,4 +101,4 @@ def test_invalid_custom_value_configuration(self):
         self.assertFalse(validation, "Custom value accepted for fixed "
                          "attribute color")
 
-    # Test configuration with disallowed custom type value
+    # TODO: Test configuration with disallowed custom type value

product_configurator_wizard/tests/test_wizard.py

@@ -12,38 +12,112 @@ def setUp(self):
 
         self.so = self.env.ref('sale.sale_order_5')
 
-    def get_wizard_write_dict(self, wizard, attr_val_ext_ids):
+    def get_attr_values(self, attr_val_ext_ids=None):
+        if not attr_val_ext_ids:
+            attr_val_ext_ids = []
+        ext_id_prefix = 'product_configurator.product_attribute_value_%s'
+        attr_vals = self.env['product.attribute.value']
+
+        for ext_id in attr_val_ext_ids:
+            attr_vals += self.env.ref(ext_id_prefix % ext_id)
+
+        return attr_vals
+
+    def get_wizard_write_dict(self, wizard, attr_values):
         """Turn a series of attribute.value objects to a dictionary meant for
         writing values to the product.configurator wizard"""
 
         write_dict = {}
-        ext_id_prefix = 'product_configurator.product_attribute_value_%s'
 
-        for ext_id in attr_val_ext_ids:
-            val = self.env.ref(ext_id_prefix % ext_id)
-            write_dict.update({
-                wizard.field_prefix + str(val.attribute_id.id): val.id
-            })
+        multi_attr_ids = wizard.product_tmpl_id.attribute_line_ids.filtered(
+            lambda x: x.multi).mapped('attribute_id').ids
+
+        for val in attr_values:
+            field_name = wizard.field_prefix + str(val.attribute_id.id)
+            if val.attribute_id.id in multi_attr_ids:
+                write_dict.setdefault(field_name, [(6, 0, [])])
+                write_dict[field_name][0][2].append(val.id)
+                continue
+            write_dict.update({field_name: val.id})
+
         return write_dict
 
-    def test_wizard(self):
+    def wizard_write_proceed(self, wizard, attr_vals, value_ids=None):
+        """Writes config data to the wizard then proceeds to the next step"""
+        vals = self.get_wizard_write_dict(wizard, attr_vals)
+        wizard.write(vals)
+        wizard.action_next_step()
+        # Store the values since the wizard removes dynamic values from dict
+        if type(value_ids) == list:
+            value_ids += attr_vals.ids
+
+    def test_wizard_configuration(self):
         """Test product configurator wizard"""
 
         # Start a new configuration wizard
-        wizard = self.env['product.configurator'].create({
-            'product_tmpl_id': self.cfg_tmpl.id
+        wizard_obj = self.env['product.configurator'].with_context({
+            'active_model': 'sale.order',
+            'active_id': self.so.id
         })
 
+        wizard = wizard_obj.create({'product_tmpl_id': self.cfg_tmpl.id})
         wizard.action_next_step()
 
-        # Get write values the first configuration step
-        write_dict = self.get_wizard_write_dict(wizard, ['gasoline', '228i'])
+        value_ids = []
+
+        attr_vals = self.get_attr_values(['gasoline', '228i'])
+        self.wizard_write_proceed(wizard, attr_vals, value_ids)
+
+        attr_vals = self.get_attr_values(['silver', 'rims_387'])
+        self.wizard_write_proceed(wizard, attr_vals, value_ids)
 
-        # Store the values since the wizard removed dynamic values from dict
-        write_val_ids = write_dict.values()
+        attr_vals = self.get_attr_values(['model_sport_line'])
+        self.wizard_write_proceed(wizard, attr_vals, value_ids)
 
-        # Test wizard dynamic write
-        wizard.write(write_dict)
+        attr_vals = self.get_attr_values(['tapistry_black'])
+        self.wizard_write_proceed(wizard, attr_vals, value_ids)
 
-        self.assertTrue(wizard.value_ids.ids == write_val_ids,
+        attr_vals = self.get_attr_values(['steptronic', 'tow_hook', 'sunroof'])
+        vals = self.get_wizard_write_dict(wizard, attr_vals)
+        wizard.write(vals)
+        value_ids += attr_vals.ids
+
+        self.assertTrue(set(wizard.value_ids.ids) == set(value_ids),
                         "Wizard write did not update the config session")
+
+        wizard.action_next_step()
+
+        config_variants = self.env['product.product'].search([
+            ('config_ok', '=', True)
+        ])
+
+        self.assertTrue(len(config_variants) == 1,
+                        "Wizard did not create a configurable variant")
+
+    def test_reconfiguration(self):
+        """Test reconfiguration functionality of the wizard"""
+        self.test_wizard_configuration()
+
+        order_line = self.so.order_line.filtered(
+            lambda l: l.product_id.config_ok
+        )
+
+        reconfig_action = order_line.reconfigure_product()
+
+        wizard = self.env['product.configurator'].browse(
+            reconfig_action.get('res_id')
+        )
+
+        attr_vals = self.get_attr_values(['diesel', '220d'])
+        self.wizard_write_proceed(wizard, attr_vals)
+
+        # Cycle through steps until wizard ends
+        while wizard.action_next_step():
+            pass
+
+        config_variants = self.env['product.product'].search([
+            ('config_ok', '=', True)
+        ])
+
+        self.assertTrue(len(config_variants) == 2,
+                        "Wizard reconfiguration did not create a new variant")

product_configurator_wizard/wizard/product_configurator.py

@@ -804,22 +804,6 @@ def action_config_done(self):
                 l.value or l.attachment_ids for l in self.custom_value_ids
         }
 
-        if self.product_id:
-            product_tmpl = self.product_id.product_tmpl_id
-            remove_cv_links = map(
-                lambda cv: (2, cv), self.product_id.value_custom_ids.ids)
-            new_cv_links = product_tmpl.encode_custom_values(custom_vals)
-            self.product_id.write({
-                'attribute_value_ids': [(6, 0, self.value_ids.ids)],
-                'value_custom_ids':  remove_cv_links + new_cv_links,
-            })
-            if self.order_line_id:
-                order_line_vals = self._extra_line_values(
-                    self.order_line_id.order_id, self.product_id, new=False)
-                self.order_line_id.write(order_line_vals)
-            self.unlink()
-            return
-        #
         # This try except is too generic.
         # The create_variant routine could effectively fail for
         # a large number of reasons, including bad programming.
@@ -828,7 +812,7 @@ def action_config_done(self):
         # error legitimately raised in a nested routine
         # is passed through.
         try:
-            variant = self.product_tmpl_id.create_variant(
+            variant = self.product_tmpl_id.create_get_variant(
                 self.value_ids.ids, custom_vals)
         except ValidationError:
             raise
@@ -841,11 +825,14 @@ def action_config_done(self):
         so = self.env['sale.order'].browse(self.env.context.get('active_id'))
 
         line_vals = {'product_id': variant.id}
-        line_vals.update(self._extra_line_values(so, variant, new=True))
+        line_vals.update(self._extra_line_values(
+            self.order_line_id.order_id or so, variant, new=True)
+        )
 
-        so.write({
-            'order_line': [(0, 0, line_vals)]
-        })
+        if self.order_line_id:
+            self.order_line_id.write(line_vals)
+        else:
+            so.write({'order_line': [(0, 0, line_vals)]})
 
         self.unlink()
         return

website_product_configurator/controllers/main.py

@@ -568,28 +568,12 @@ def image_update(self, product_tmpl, cfg_vals,
         return self.get_config_image(product_tmpl, value_ids, size)
 
     def configure_product(self, product_tmpl, value_ids, custom_vals=None):
-        """Used for searching a variant with the values passed in cfg_vals
-           and returning a redirect to it. In case a product is not found with
-           the given valid configuration a new variant is generated with the
-           specific values and then returned
-
-           :param product_tmpl: product.template object being configured
-           :param cfg_vals: dict representing the client-side configuration
-
-           :returns: product.product object found or created
-           """
-        # TODO: Implement a search and create method that can be extended
-        # easily
+        """Method kept for backward compatiblity"""
+        # TODO: Remove in next version
         if custom_vals is None:
             custom_vals = {}
 
-        product = product_tmpl.search_variant(value_ids, custom_vals)
-        if product:
-            if len(product) > 1:
-                return False
-            else:
-                return product
-        return product_tmpl.sudo().create_variant(value_ids, custom_vals)
+        return product_tmpl.sudo().create_get_variant(value_ids, custom_vals)
 
     def get_attr_classes(self, attr_line, attr_value=False, custom=False):
         """Computes classes for attribute elements in frontend for the purpose
@@ -713,11 +697,13 @@ def cfg_session(self, cfg_session, **post):
         except:
             return request.redirect('/configurator')
         if post:
-            product = self.configure_product(
-                cfg_session.product_tmpl_id, cfg_session.value_ids.ids, {
-                    x.attribute_id.id: x.value or x.attachment_ids for x in
-                    cfg_session.custom_value_ids
-                })
+            custom_vals = {
+                x.attribute_id.id: x.value or x.attachment_ids for x in
+                cfg_session.custom_value_ids
+            }
+            product = cfg_session.product_tmpl_id.sudo().create_get_variant(
+                cfg_session.value_ids.ids, custom_vals
+            )
             cfg_session.sudo().unlink()
             return self.cart_update(product, post)