odoo
/
odoo
2
0
Fork 0
Code Pull Requests Releases Activity

[FIX] product: pricelist: multiple corrections in UoM-related conversions 

Pricelist computations need to consider 2 different Units
of Measure:
 - The default product UoM (product.uom_id), used as reference
   for the various quantities and amounts specified in each
   pricelist rules.
 - The `context UoM` is the UoM in which the result is requested,
   that is the list price UoM.

For example the 'price_min_margin' amount is meant for the unit
price of 1 x default UoM. When the context UoM is not the default
product UoM, it can be any UoM of the same UoM Category, and the
various quantities and amounts specified on the rule need to
be adapted accordingly:
  - min_quantity (expressed in terms of the default UoM)
  - price_surcharge (specified for 1 x default UoM)
  - price_min_margin (specified for 1 x default UoM)
  - price_max_margin (specified for 1 x default UoM)

The UoM corrections were not done consistently and resulted in
wrong prices when computing the price using a non-default UoM.
The cases were a conversion was needed or not were not properly
identified within the _price_rule_get_multi().

After this commit, the various code branches in _price_rule_get_multi
always ensures that:
 - price requested for: `qty` of `qty_uom_id`
 - `qty_in_product_uom` is the requested `qty` converted to default UoM
 - current (intermediary) price: `price` for  `price_uom_id`

Therefore `price` and `price_uom_id` are always in sync, and `price_uom_id`
can always be compared with `qty_uom_id' in order to know whether
a conversion is still needed.

This patch also corrects and extends the regression tests
introduced at revision 79ebe10.
This commit is contained in:
Olivier Dony 2014-12-09 15:28:37 +01:00
parent f9ad381c17
commit da12f2b809
3 changed files with 93 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
addons/product/pricelist.py
View File

@ -247,19 +247,27 @@ class product_pricelist(osv.osv):
results = {}
for product, qty, partner in products_by_qty_by_partner:
uom_price_already_computed = False
results[product.id] = 0.0
price = False
rule_id = False
price = False
# Final unit price is computed according to `qty` in the `qty_uom_id` UoM.
# An intermediary unit price may be computed according to a different UoM, in
# which case the price_uom_id contains that UoM.
# The final price will be converted to match `qty_uom_id`.
qty_uom_id = context.get('uom') or product.uom_id.id
price_uom_id = product.uom_id.id
qty_in_product_uom = qty
if qty_uom_id != product.uom_id.id:
try:
qty_in_product_uom = product_uom_obj._compute_qty(
cr, uid, context['uom'], qty, product.uom_id.id or product.uos_id.id)
except except_orm:
# Ignored - incompatible UoM in context, use default product UoM
pass
for rule in items:
if 'uom' in context and product.uom_id and context['uom'] != product.uom_id.id:
try:
qty_in_product_uom = product_uom_obj._compute_qty(cr, uid, context['uom'], qty, product.uom_id.id, dict(context.items() + [('raise-exception', False)]))
except except_orm:
qty_in_product_uom = qty
else:
qty_in_product_uom = qty
if rule.min_quantity and qty_in_product_uom<rule.min_quantity:
if rule.min_quantity and qty_in_product_uom < rule.min_quantity:
continue
if is_product_template:
if rule.product_tmpl_id and product.id != rule.product_tmpl_id.id:
@ -287,7 +295,7 @@ class product_pricelist(osv.osv):
rule.base_pricelist_id, [(product,
qty, False)], context=context)[product.id]
ptype_src = rule.base_pricelist_id.currency_id.id
uom_price_already_computed = True
price_uom_id = qty_uom_id
price = currency_obj.compute(cr, uid,
ptype_src, pricelist.currency_id.id,
price_tmp, round=False,
@ -302,12 +310,10 @@ class product_pricelist(osv.osv):
seller = product.seller_ids[0]
if seller:
qty_in_seller_uom = qty
from_uom = context.get('uom') or product.uom_id.id
seller_uom = seller.product_uom and seller.product_uom.id or False
if seller_uom and from_uom and from_uom != seller_uom:
qty_in_seller_uom = product_uom_obj._compute_qty(cr, uid, from_uom, qty, to_uom_id=seller_uom)
else:
uom_price_already_computed = True
seller_uom = seller.product_uom.id
if qty_uom_id != seller_uom:
qty_in_seller_uom = product_uom_obj._compute_qty(cr, uid, qty_uom_id, qty, to_uom_id=seller_uom)
price_uom_id = seller_uom
for line in seller.pricelist_ids:
if line.min_quantity <= qty_in_seller_uom:
price = line.price
@ -317,34 +323,40 @@ class product_pricelist(osv.osv):
price_types[rule.base] = price_type_obj.browse(cr, uid, int(rule.base))
price_type = price_types[rule.base]
uom_price_already_computed = True
price = currency_obj.compute(cr, uid,
# price_get returns the price in the context UoM, i.e. qty_uom_id
price_uom_id = qty_uom_id
price = currency_obj.compute(
cr, uid,
price_type.currency_id.id, pricelist.currency_id.id,
product_obj._price_get(cr, uid, [product],
price_type.field, context=context)[product.id], round=False, context=context)
product_obj._price_get(cr, uid, [product], price_type.field, context=context)[product.id],
round=False, context=context)
if price is not False:
price_limit = price
price = price * (1.0+(rule.price_discount or 0.0))
if rule.price_round:
price = tools.float_round(price, precision_rounding=rule.price_round)
if context.get('uom'):
# compute price_surcharge based on reference uom
factor = product_uom_obj.browse(cr, uid, context.get('uom'), context=context).factor
else:
factor = 1.0
price += (rule.price_surcharge or 0.0) / factor
convert_to_price_uom = (lambda price: product_uom_obj._compute_price(
cr, uid, product.uom_id.id,
price, price_uom_id))
if rule.price_surcharge:
price_surcharge = convert_to_price_uom(rule.price_surcharge)
price += price_surcharge
if rule.price_min_margin:
price = max(price, price_limit+rule.price_min_margin)
price_min_margin = convert_to_price_uom(rule.price_min_margin)
price = max(price, price_limit + price_min_margin)
if rule.price_max_margin:
price = min(price, price_limit+rule.price_max_margin)
price_max_margin = convert_to_price_uom(rule.price_max_margin)
price = min(price, price_limit + price_max_margin)
rule_id = rule.id
break
if price:
if 'uom' in context and not uom_price_already_computed:
uom = product.uos_id or product.uom_id
price = product_uom_obj._compute_price(cr, uid, uom.id, price, context['uom'])
# Final price conversion to target UoM
price = product_uom_obj._compute_price(cr, uid, price_uom_id, price, qty_uom_id)
results[product.id] = (price, rule_id)
return results

3
addons/product/product.py
View File

@ -189,7 +189,8 @@ class product_uom(osv.osv):
return amount
def _compute_price(self, cr, uid, from_uom_id, price, to_uom_id=False):
if not from_uom_id or not price or not to_uom_id:
if (not from_uom_id or not price or not to_uom_id
or (to_uom_id == from_uom_id)):
return price
from_unit, to_unit = self.browse(cr, uid, [from_uom_id, to_uom_id])
if from_unit.category_id.id != to_unit.category_id.id:

47
addons/product/tests/test_pricelist.py
View File

@ -15,6 +15,8 @@ class TestPricelist(TransactionCase):
self.datacard_id = self.ir_model_data.get_object_reference(cr, uid, 'product', 'product_product_46')[1]
self.unit_id = self.ir_model_data.get_object_reference(cr, uid, 'product', 'product_uom_unit')[1]
self.dozen_id = self.ir_model_data.get_object_reference(cr, uid, 'product', 'product_uom_dozen')[1]
self.tonne_id = self.ir_model_data.xmlid_to_res_id(cr, uid, 'product.product_uom_ton')
self.kg_id = self.ir_model_data.xmlid_to_res_id(cr, uid, 'product.product_uom_kgm')
self.public_pricelist_id = self.ir_model_data.get_object_reference(cr, uid, 'product', 'list0')[1]
self.sale_pricelist_id = self.product_pricelist.create(cr, uid, {
@ -64,7 +66,50 @@ class TestPricelist(TransactionCase):
usb_adapter_unit = self.product_product.browse(cr, uid, self.usb_adapter_id, context=unit_context)
usb_adapter_dozen = self.product_product.browse(cr, uid, self.usb_adapter_id, context=dozen_context)
self.assertAlmostEqual(usb_adapter_unit.price*12, usb_adapter_dozen.price)
datacard_unit = self.product_product.browse(cr, uid, self.datacard_id, context=unit_context)
datacard_dozen = self.product_product.browse(cr, uid, self.datacard_id, context=dozen_context)
# price_surcharge applies to product default UoM, here "Units", so surcharge will be multiplied
self.assertAlmostEqual(datacard_unit.price*12, datacard_dozen.price)
def test_20_pricelist_uom(self):
# Verify that the pricelist rules are correctly using the product's default UoM
# as reference, and return a result according to the target UoM (as specific in the context)
cr, uid = self.cr, self.uid
kg, tonne = self.kg_id, self.tonne_id
tonne_price = 100
# make sure 'tonne' resolves down to 1 'kg'.
self.uom.write(cr, uid, tonne, {'rounding': 0.001})
# setup product stored in 'tonnes', with a discounted pricelist for qty > 3 tonnes
spam_id = self.product_product.copy(cr, uid, self.usb_adapter_id,
{ 'name': '1 tonne of spam',
'uom_id': self.tonne_id,
'uos_id': self.tonne_id,
'uom_po_id': self.tonne_id,
'list_price': tonne_price,
})
pricelist_version_id = self.ir_model_data.xmlid_to_res_id(cr, uid, 'product.ver0')
self.registry('product.pricelist.item').create(cr, uid,
{ 'price_version_id': pricelist_version_id,
'sequence': 10,
'name': '3+ tonnes: -10 EUR discount/t',
'base': 1, # based on public price
'min_quantity': 3, # min = 3 tonnes
'price_surcharge': -10, # -10 EUR / tonne
'product_id': spam_id,
})
pricelist_id = self.public_pricelist_id
def test_unit_price(qty, uom, expected_unit_price):
unit_price = self.registry('product.pricelist').price_get(cr, uid, [pricelist_id],
spam_id, qty,
context={'uom': uom})[pricelist_id]
self.assertAlmostEqual(unit_price, expected_unit_price, msg='Computed unit price is wrong')
# Test prices - they are *per unit*, the quantity is only here to match the pricelist rules!
test_unit_price(2, kg, tonne_price / 1000.0)
test_unit_price(2000, kg, tonne_price / 1000.0)
test_unit_price(3500, kg, (tonne_price - 10) / 1000.0)
test_unit_price(2, tonne, tonne_price)
test_unit_price(3, tonne, tonne_price - 10)