Skip to content

Commit

Permalink
[FIX] sale,website_sale: Display extra prices with taxes included
Browse files Browse the repository at this point in the history
Steps to reproduce the issue:

  - Install eCommerce module
  - Go to Settings
  - Ensure `Product Prices` is set to 'Tax included'
  - Create a new Product as storable
  - Set `Sales Price` to $1.0 and `Customer Taxes` to 10.00 %
  - Under `Variants` tab add an Attribute with 2 values
  - Set "Price Extra" to $2.0 to one of the variants
  - Go to the Shop and select the product

Issue:

  The extra price badge does not include the taxes
  ($2.0 instead of $2.2), however the final price does.

Cause:

  The price_extra field from ptav (used in the badge) does not include
  the taxes.
  However, when calculating the final price, we do first the sum of all
  prices (including the extra prices) and then apply the taxes.

Solution:

  In the template, for each 'variants', we call `_get_combination_info`
  to get the variant.price_extra with taxes included/excluded
  depending the `Product Prices` setting.

opw-2669871

closes odoo#81860

X-original-commit: 83c927f
Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>
Signed-off-by: Nasreddin Boulif (bon) <bon@odoo.com>
  • Loading branch information
nboulif committed Dec 27, 2021
1 parent 962919e commit 6f04935
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 28 deletions.
10 changes: 9 additions & 1 deletion addons/sale/models/product_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,11 @@ def _get_combination_info(self, combination=False, product_id=False, add_qty=1,
price = product.price if pricelist else list_price
display_image = bool(product.image_1920)
display_name = product.display_name
price_extra = (product.price_extra or 0.0 ) + (sum(no_variant_attributes_price_extra) or 0.0)
else:
product_template = product_template.with_context(current_attributes_price_extra=[v.price_extra or 0.0 for v in combination])
current_attributes_price_extra = [v.price_extra or 0.0 for v in combination]
product_template = product_template.with_context(current_attributes_price_extra=current_attributes_price_extra)
price_extra = sum(current_attributes_price_extra)
list_price = product_template.price_compute('list_price')[product_template.id]
price = product_template.price if pricelist else list_price
display_image = bool(product_template.image_1920)
Expand All @@ -251,6 +254,10 @@ def _get_combination_info(self, combination=False, product_id=False, add_qty=1,
list_price, pricelist.currency_id, product_template._get_current_company(pricelist=pricelist),
fields.Date.today()
)
price_extra = product_template.currency_id._convert(
price_extra, pricelist.currency_id, product_template._get_current_company(pricelist=pricelist),
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
Expand All @@ -262,6 +269,7 @@ def _get_combination_info(self, combination=False, product_id=False, add_qty=1,
'display_image': display_image,
'price': price,
'list_price': list_price,
'price_extra': price_extra,
'has_discounted_price': has_discounted_price,
}

Expand Down
11 changes: 11 additions & 0 deletions addons/sale/tests/test_sale_product_attribute_value_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ def test_02_get_combination_info(self):
self.assertEqual(res['display_name'], "Super Computer (256 GB, 8 GB, 1 To)")
self.assertEqual(res['price'], 2222)
self.assertEqual(res['list_price'], 2222)
self.assertEqual(res['price_extra'], 222)

# CASE: no combination, product given
res = self.computer._get_combination_info(self.env['product.template.attribute.value'], computer_variant.id)
Expand All @@ -219,6 +220,7 @@ def test_02_get_combination_info(self):
self.assertEqual(res['display_name'], "Super Computer (256 GB, 8 GB, 1 To)")
self.assertEqual(res['price'], 2222)
self.assertEqual(res['list_price'], 2222)
self.assertEqual(res['price_extra'], 222)

# CASE: using pricelist, quantity rule
pricelist, pricelist_item, currency_ratio, discount_ratio = self._setup_pricelist()
Expand All @@ -229,6 +231,7 @@ def test_02_get_combination_info(self):
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)
self.assertEqual(res['price_extra'], 222 * currency_ratio)

# CASE: no_variant combination, it's another variant now

Expand All @@ -249,6 +252,7 @@ def test_02_get_combination_info(self):
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)
self.assertEqual(res['price_extra'], 222 * currency_ratio)

# CASE: dynamic combination, but the variant already exists
self.computer_hdd_attribute_lines.write({'active': False})
Expand All @@ -268,6 +272,7 @@ def test_02_get_combination_info(self):
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)
self.assertEqual(res['price_extra'], 222 * currency_ratio)

# CASE: dynamic combination, no variant existing
# Test invalidate_cache on product.template _create_variant_ids
Expand All @@ -279,6 +284,7 @@ def test_02_get_combination_info(self):
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)
self.assertEqual(res['price_extra'], (222 - 5) * currency_ratio)

# CASE: pricelist set value to 0, no variant
# Test invalidate_cache on product.pricelist write
Expand All @@ -289,6 +295,7 @@ def test_02_get_combination_info(self):
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)
self.assertEqual(res['price_extra'], (222 - 5) * currency_ratio)

def test_03_get_combination_info_discount_policy(self):
computer_ssd_256 = self._get_product_template_attribute_value(self.ssd_256)
Expand All @@ -304,25 +311,29 @@ def test_03_get_combination_info_discount_policy(self):
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['price_extra'], 222 * 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['price_extra'], 222 * 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['price_extra'], 222 * 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['price_extra'], 222 * currency_ratio)
self.assertEqual(res['has_discounted_price'], True)

def test_04_create_product_variant_non_dynamic(self):
Expand Down
41 changes: 18 additions & 23 deletions addons/sale/views/variant_templates.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,7 @@
t-att-data-is_single="single"
t-att-data-is_single_and_custom="single_and_custom">
<span t-field="ptav.name"/>
<span t-if="ptav.price_extra" class="badge badge-pill badge-secondary">
<!--
price_extra is displayed as catalog price instead of
price after pricelist because it is impossible to
compute. Indeed, the pricelist rule might depend on the
selected variant, so the price_extra will be different
depending on the selected combination. The price of an
attribute is therefore variable and it's not very
accurate to display it.
-->
<t t-esc="ptav.price_extra > 0 and '+' or '-'"/>
<span t-esc="abs(ptav.price_extra)" class="variant_price_extra" style="white-space: nowrap;"
t-options='{
"widget": "monetary",
"from_currency": product.currency_id,
"display_currency": (pricelist or product).currency_id
}'/>
</span>
<t t-call="sale.badge_extra_price"/>
</option>
</t>
</select>
Expand Down Expand Up @@ -133,13 +116,25 @@
</ul>
</template>
<template id="badge_extra_price" name="Badge Extra Price">
<span class="badge badge-pill badge-light border" t-if="ptav.price_extra">
<!-- see note above about price_extra -->
<span class="sign_badge_price_extra" t-esc="ptav.price_extra > 0 and '+' or '-'"/>
<span t-esc="abs(ptav.price_extra)" class="variant_price_extra text-muted font-italic" style="white-space: nowrap;"
<t t-set="combination_info_variant" t-value="product._get_combination_info(ptav, pricelist=pricelist)"/>
<span class="badge badge-pill badge-light border" t-if="combination_info_variant['price_extra']">
<!--
price_extra is displayed as catalog price instead of
price after pricelist because it is impossible to
compute. Indeed, the pricelist rule might depend on the
selected variant, so the price_extra will be different
depending on the selected combination. The price of an
attribute is therefore variable and it's not very
accurate to display it.
To cover some generic cases, the price_extra also
covers the price-included taxes in e-commerce flows.
(See the override of `_get_combination_info`)
-->
<span class="sign_badge_price_extra" t-esc="combination_info_variant['price_extra'] > 0 and '+' or '-'"/>
<span t-esc="abs(combination_info_variant['price_extra'])" class="variant_price_extra text-muted font-italic" style="white-space: nowrap;"
t-options='{
"widget": "monetary",
"from_currency": product.currency_id,
"display_currency": (pricelist or product).currency_id
}'/>
</span>
Expand Down
3 changes: 3 additions & 0 deletions addons/website_sale/models/product_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,16 @@ def _get_combination_info(self, combination=False, product_id=False, add_qty=1,
list_price = taxes.compute_all(combination_info['list_price'], pricelist.currency_id, quantity_1, product, partner)[tax_display]
else:
list_price = price
combination_info['price_extra'] = self.env['account.tax']._fix_tax_included_price_company(combination_info['price_extra'], product_taxes, taxes, company_id)
price_extra = taxes.compute_all(combination_info['price_extra'], pricelist.currency_id, quantity_1, product, partner)[tax_display]
has_discounted_price = pricelist.currency_id.compare_amounts(list_price, price) == 1

combination_info.update(
base_unit_name=product.base_unit_name,
base_unit_price=product.base_unit_price,
price=price,
list_price=list_price,
price_extra=price_extra,
has_discounted_price=has_discounted_price,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def test_get_combination_info(self):
combination_info = self.computer._get_combination_info()
self.assertEqual(combination_info['price'], 2222 * discount_rate * currency_ratio)
self.assertEqual(combination_info['list_price'], 2222 * discount_rate * currency_ratio)
self.assertEqual(combination_info['price_extra'], 222 * currency_ratio)
self.assertEqual(combination_info['has_discounted_price'], False)

# CASE: B2C setting
Expand All @@ -53,6 +54,7 @@ def test_get_combination_info(self):
combination_info = self.computer._get_combination_info()
self.assertEqual(combination_info['price'], 2222 * discount_rate * currency_ratio * tax_ratio)
self.assertEqual(combination_info['list_price'], 2222 * discount_rate * currency_ratio * tax_ratio)
self.assertEqual(combination_info['price_extra'], round(222 * currency_ratio * tax_ratio, 2))
self.assertEqual(combination_info['has_discounted_price'], False)

# CASE: pricelist 'without_discount'
Expand All @@ -63,6 +65,7 @@ def test_get_combination_info(self):
combination_info = self.computer._get_combination_info()
self.assertEqual(pricelist.currency_id.compare_amounts(combination_info['price'], 2222 * discount_rate * currency_ratio * tax_ratio), 0)
self.assertEqual(pricelist.currency_id.compare_amounts(combination_info['list_price'], 2222 * currency_ratio * tax_ratio), 0)
self.assertEqual(pricelist.currency_id.compare_amounts(combination_info['price_extra'], 222 * currency_ratio * tax_ratio), 0)
self.assertEqual(combination_info['has_discounted_price'], True)

def test_get_combination_info_with_fpos(self):
Expand All @@ -76,6 +79,14 @@ def test_get_combination_info_with_fpos(self):
'price': 2000,
}).with_context(website_id=current_website.id)

computer_ssd_attribute_lines = self.env['product.template.attribute.line'].create({
'product_tmpl_id': test_product.id,
'attribute_id': self.ssd_attribute.id,
'value_ids': [(6, 0, [self.ssd_256.id])],
})
computer_ssd_attribute_lines.product_template_value_ids[0].price_extra = 200
combination = computer_ssd_attribute_lines.product_template_value_ids[0]

# Add fixed price for pricelist
pricelist.item_ids = self.env['product.pricelist.item'].create({
'applied_on': "1_product",
Expand Down Expand Up @@ -107,30 +118,34 @@ def test_get_combination_info_with_fpos(self):
'tax_dest_id': tax0.id,
})

combination_info = test_product._get_combination_info()
combination_info = test_product._get_combination_info(combination)
self.assertEqual(combination_info['price'], 575, "500$ + 15% tax")
self.assertEqual(combination_info['list_price'], 575, "500$ + 15% tax (2)")
self.assertEqual(combination_info['price_extra'], 230, "200$ + 15% tax")

# Now with fiscal position, taxes should be mapped
self.env.user.partner_id.country_id = self.env.ref('base.be').id
combination_info = test_product._get_combination_info()
combination_info = test_product._get_combination_info(combination)
self.assertEqual(combination_info['price'], 500, "500% + 0% tax (mapped from fp 15% -> 0% for BE)")
self.assertEqual(combination_info['list_price'], 500, "500% + 0% tax (mapped from fp 15% -> 0% for BE) (2)")
self.assertEqual(combination_info['price_extra'], 200, "200% + 0% tax (mapped from fp 15% -> 0% for BE)")

# Try same flow with tax included
tax15.write({'price_include': True})

# Reset / Safety check
self.env.user.partner_id.country_id = None
combination_info = test_product._get_combination_info()
combination_info = test_product._get_combination_info(combination)
self.assertEqual(combination_info['price'], 500, "434.78$ + 15% tax")
self.assertEqual(combination_info['list_price'], 500, "434.78$ + 15% tax (2)")
self.assertEqual(combination_info['price_extra'], 200, "173.91$ + 15% tax")

# Now with fiscal position, taxes should be mapped
self.env.user.partner_id.country_id = self.env.ref('base.be').id
combination_info = test_product._get_combination_info()
combination_info = test_product._get_combination_info(combination)
self.assertEqual(round(combination_info['price'], 2), 434.78, "434.78$ + 0% tax (mapped from fp 15% -> 0% for BE)")
self.assertEqual(round(combination_info['list_price'], 2), 434.78, "434.78$ + 0% tax (mapped from fp 15% -> 0% for BE)")
self.assertEqual(combination_info['price_extra'], 173.91, "173.91$ + 0% tax (mapped from fp 15% -> 0% for BE)")

@tagged('post_install', '-at_install')
class TestWebsiteSaleProductPricelist(TestSaleProductAttributeValueCommon):
Expand Down

0 comments on commit 6f04935

Please sign in to comment.