Skip to content
This repository has been archived by the owner on Apr 28, 2022. It is now read-only.

[FIX] Wizard Attributes #104

Open
wants to merge 6 commits into
base: 10.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions product_configurator/models/product.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,9 @@ def validate_configuration(self, value_ids, custom_vals=None, final=True):
if custom_vals is None:
custom_vals = {}

# Check if all the values passed are not restricted
avail_val_ids = self.values_available(value_ids, value_ids)

for line in self.attribute_line_ids:
# Validate custom values
attr = line.attribute_id
Expand All @@ -466,11 +469,12 @@ def validate_configuration(self, value_ids, custom_vals=None, final=True):
common_vals = set(value_ids) & set(line.value_ids.ids)
custom_val = custom_vals.get(attr.id)
if line.required and not common_vals and not custom_val:
# TODO: Verify custom value type to be correct
return False
# If there are possible values, then enforce
# the "required" status.
if set(line.value_ids.ids) & set(avail_val_ids):
# TODO: Verify custom value type to be correct
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree that there might be a case where a user might want "required if options available" rather than flat required.

On one hand you can have this flexibility on the other hand you are not guaranteed to have a required value and if you mess up your configuration restrictions the configuration will silently pass.

Maybe there's a way we can differentiate between two cases or also provide an error message when a line is required and has certain configurations which do not provide any options for the required line.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PCatinean My initial feeling was similar, and I was going to make a 2 way flag. But I fell back to this - if there are no possible selections based on other domains, then it just seemed logical that it can't be invalid - the definition must be...

If you want it changed to two types of required, I think we should do it as another PR after this one is merged.


# Check if all all the values passed are not restricted
avail_val_ids = self.values_available(value_ids, value_ids)
if set(value_ids) - set(avail_val_ids):
return False

Expand Down
1 change: 1 addition & 0 deletions product_configurator_wizard/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-

from . import test_wizard
from . import test_wizard_attrs
69 changes: 54 additions & 15 deletions product_configurator_wizard/tests/test_wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ def get_attr_values(self, attr_val_ext_ids=None):

return attr_vals

def get_wizard_write_dict(self, wizard, attr_values):
def get_wizard_write_dict(self, wizard, attr_values, remove_values=None):
"""Turn a series of attribute.value objects to a dictionary meant for
writing values to the product.configurator wizard"""

write_dict = {}
if remove_values is None:
remove_values = []

multi_attr_ids = wizard.product_tmpl_id.attribute_line_ids.filtered(
lambda x: x.multi).mapped('attribute_id').ids
Expand All @@ -40,6 +42,10 @@ def get_wizard_write_dict(self, wizard, attr_values):
continue
write_dict.update({field_name: val.id})

for val in remove_values:
field_name = wizard.field_prefix + str(val.attribute_id.id)
write_dict.update({field_name: False})

return write_dict

def wizard_write_proceed(self, wizard, attr_vals, value_ids=None):
Expand All @@ -54,6 +60,10 @@ def wizard_write_proceed(self, wizard, attr_vals, value_ids=None):
def test_wizard_configuration(self):
"""Test product configurator wizard"""

config_variants = self.env['product.product'].search([
('config_ok', '=', True)
])

# Start a new configuration wizard
wizard_obj = self.env['product.configurator'].with_context({
'active_model': 'sale.order',
Expand Down Expand Up @@ -87,37 +97,66 @@ def test_wizard_configuration(self):

wizard.action_next_step()

config_variants = self.env['product.product'].search([
new_config_variants = self.env['product.product'].search([
('config_ok', '=', True)
])

self.assertTrue(len(config_variants) == 1,
self.assertTrue(len(new_config_variants - config_variants) == 1,
"Wizard did not create a configurable variant")

def do_reconfigure(self, order_line, attr_vals):
reconfig_action = order_line.reconfigure_product()

wizard = self.env['product.configurator'].browse(
reconfig_action.get('res_id')
)

self.wizard_write_proceed(wizard, attr_vals)

# if wizard only had one step, it would already have completed
if wizard.exists():
# Cycle through steps until wizard ends
while wizard.action_next_step():
pass

def test_reconfiguration(self):
"""Test reconfiguration functionality of the wizard"""
config_variants = self.env['product.product'].search([
('config_ok', '=', True)
])

self.test_wizard_configuration()

existing_lines = self.so.order_line

order_line = self.so.order_line.filtered(
lambda l: l.product_id.config_ok
)

reconfig_action = order_line.reconfigure_product()
self.do_reconfigure(order_line,
self.get_attr_values(['diesel', '220d'])
)

wizard = self.env['product.configurator'].browse(
reconfig_action.get('res_id')
)
new_config_variants = self.env['product.product'].search([
('config_ok', '=', True)
])

attr_vals = self.get_attr_values(['diesel', '220d'])
self.wizard_write_proceed(wizard, attr_vals)
self.assertTrue(len(new_config_variants - config_variants) == 2,
"Wizard reconfiguration did not create a new variant")

# Cycle through steps until wizard ends
while wizard.action_next_step():
pass
created_line = self.so.order_line - existing_lines
self.assertTrue(len(created_line) == 0,
"Wizard created an order line on reconfiguration")

config_variants = self.env['product.product'].search([
# test that running through again with the same values does not
# create another variant
self.do_reconfigure(order_line,
self.get_attr_values(['diesel', '220d'])
)

new_config_variants = self.env['product.product'].search([
('config_ok', '=', True)
])

self.assertTrue(len(config_variants) == 2,
"Wizard reconfiguration did not create a new variant")
self.assertTrue(len(new_config_variants - config_variants) == 2,
"Wizard reconfiguration created a redundant variant")
233 changes: 233 additions & 0 deletions product_configurator_wizard/tests/test_wizard_attrs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
# -*- coding: utf-8 -*-

from odoo.addons.product_configurator_wizard.tests.test_wizard \
import ConfigurationRules


class ConfigurationAttributes(ConfigurationRules):

def setUp(self):
"""
Product with 3 sizes:
Small or Medium allow Blue or Red, colour is required
Large does not allow colour selection
"""
super(ConfigurationAttributes, self).setUp()

self.attr_size = self.env['product.attribute'].create(
{'name': 'Size'})
self.attr_val_small = self.env['product.attribute.value'].create(
{'attribute_id': self.attr_size.id,
'name': 'Small',
}
)
self.attr_val_med = self.env['product.attribute.value'].create(
{'attribute_id': self.attr_size.id,
'name': 'Medium',
}
)
self.attr_val_large = self.env['product.attribute.value'].create(
{'attribute_id': self.attr_size.id,
'name': 'Large (Green)',
}
)
domain_small_med = self.env['product.config.domain'].create(
{'name': 'Small/Med',
'domain_line_ids': [
(0, 0, {'attribute_id': self.attr_size.id,
'condition': 'in',
'operator': 'and',
'value_ids':
[(6, 0,
[self.attr_val_small.id, self.attr_val_med.id]
)]
}),
],
}
)
self.attr_colour = self.env['product.attribute'].create(
{'name': 'Colour'})
self.attr_val_blue = self.env['product.attribute.value'].create(
{'attribute_id': self.attr_colour.id,
'name': 'Blue',
}
)
self.attr_val_red = self.env['product.attribute.value'].create(
{'attribute_id': self.attr_colour.id,
'name': 'Red',
}
)
self.product_temp = self.env['product.template'].create(
{'name': 'Config Product',
'config_ok': True,
'type': 'product',
'categ_id': self.env['ir.model.data'].xmlid_to_res_id(
'product.product_category_5'
),
'attribute_line_ids': [
(0, 0, {'attribute_id': self.attr_size.id,
'value_ids': [
(6, 0, self.attr_size.value_ids.ids),
],
'required': True,
}),
(0, 0, {'attribute_id': self.attr_colour.id,
'value_ids': [
(6, 0, self.attr_colour.value_ids.ids),
],
'required': False,
})
],
}
)
self.template_colour_line = \
self.product_temp.attribute_line_ids.filtered(
lambda a: a.attribute_id == self.attr_colour)
self.env['product.config.line'].create({
'product_tmpl_id': self.product_temp.id,
'attribute_line_id': self.template_colour_line.id,
'value_ids': [(6, 0, self.attr_colour.value_ids.ids)],
'domain_id': domain_small_med.id,
})

def test_configurations_option_or_not_reqd(self):
# Start a new configuration wizard
wizard_obj = self.env['product.configurator'].with_context({
'active_model': 'sale.order',
'active_id': self.so.id
# 'default_order_id': self.so.id
})

wizard = wizard_obj.create({'product_tmpl_id': self.product_temp.id})
wizard.action_next_step()

dynamic_fields = {}
for attribute_line in self.product_temp.attribute_line_ids:
field_name = '%s%s' % (
wizard.field_prefix,
attribute_line.attribute_id.id
)
dynamic_fields[field_name] = [] if attribute_line.multi else False
field_name_colour = '%s%s' % (
wizard.field_prefix,
self.attr_colour.id
)
ro_name_colour = '%s%s' % (
wizard.ro_field_prefix,
self.attr_colour.id
)
reqd_name_colour = '%s%s' % (
wizard.reqd_field_prefix,
self.attr_colour.id
)

# Define small without colour specified
self.wizard_write_proceed(wizard, [self.attr_val_small])
new_variant = self.product_temp.product_variant_ids
self.assertTrue(len(new_variant) == 1 and
set(new_variant.attribute_value_ids.ids) ==
set([self.attr_val_small.id]),
"Wizard did not accurately create a variant with "
"optional value undefined")
config_variants = self.product_temp.product_variant_ids

order_line = self.so.order_line.filtered(
lambda l: l.product_id.config_ok
)

# Redefine to medium without colour
self.do_reconfigure(order_line, [self.attr_val_med])
new_variant = self.product_temp.product_variant_ids - config_variants
self.assertTrue(len(new_variant) == 1 and
set(new_variant.attribute_value_ids.ids) ==
set([self.attr_val_med.id]),
"Wizard did not accurately reconfigure a variant with "
"optional value undefined")
config_variants = self.product_temp.product_variant_ids

# Redefine to medium blue
self.do_reconfigure(order_line, [self.attr_val_blue])
new_variant = self.product_temp.product_variant_ids - config_variants
self.assertTrue(len(new_variant) == 1 and
set(new_variant.attribute_value_ids.ids) ==
set([self.attr_val_med.id, self.attr_val_blue.id]),
"Wizard did not accurately reconfigure a variant with "
"to add an optional value")
config_variants = self.product_temp.product_variant_ids

# Redefine to large - should remove colour, as this is invalid
reconfig_action = order_line.reconfigure_product()
wizard = self.env['product.configurator'].browse(
reconfig_action.get('res_id')
)
attr_large_dict = self.get_wizard_write_dict(wizard,
[self.attr_val_large])
attr_blue_dict = self.get_wizard_write_dict(wizard,
[self.attr_val_blue])
oc_vals = dynamic_fields.copy()
oc_vals.update({'id': wizard.id})
oc_vals.update(dict(attr_blue_dict, **attr_large_dict))
oc_result = wizard.onchange(
oc_vals,
attr_large_dict.keys()[0],
{}
)
self.assertTrue(field_name_colour in oc_result['value'] and
not oc_result['value'][field_name_colour],
"Colour should have been cleared by wizard"
)
self.assertTrue(ro_name_colour in oc_result['value'] and
oc_result['value'][ro_name_colour],
"Colour should have become readonly"
)
self.assertFalse(oc_result['value'].get(reqd_name_colour, False),
"Colour should not have become required"
)

vals = self.get_wizard_write_dict(wizard, [self.attr_val_large],
remove_values=[self.attr_val_blue])
wizard.write(vals)
wizard.action_next_step()
if wizard.exists():
while wizard.action_next_step():
pass
new_variant = self.product_temp.product_variant_ids - config_variants
self.assertTrue(len(new_variant) == 1 and
set(new_variant.attribute_value_ids.ids) ==
set([self.attr_val_large.id]),
"Wizard did not accurately reconfigure a variant "
"to remove invalid attribute")

# Now test reqd attribute changes.
last_variant = new_variant
self.template_colour_line.write({'required': True})

# Redefine to medium - should make colour required
reconfig_action = order_line.reconfigure_product()
wizard = self.env['product.configurator'].browse(
reconfig_action.get('res_id')
)
attr_med_dict = self.get_wizard_write_dict(wizard,
[self.attr_val_med])
oc_vals = dynamic_fields.copy()
oc_vals.update({'id': wizard.id})
oc_vals.update(attr_med_dict)
oc_result = wizard.onchange(
oc_vals,
attr_med_dict.keys()[0],
{}
)
self.assertTrue(oc_result['value'].get(reqd_name_colour),
"Colour should have become required"
)
# Redefine to large - should make colour not required, despite master
# file
vals = self.get_wizard_write_dict(wizard, [self.attr_val_large])
wizard.write(vals)
wizard.action_next_step()
if wizard.exists():
while wizard.action_next_step():
pass
self.assertTrue(order_line.product_id == last_variant,
"Wizard did not end up with the same "
"variant")
Loading