Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] pricelist: pricelist revamp #159746

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
1 change: 0 additions & 1 deletion addons/delivery/tests/test_delivery_cost.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def test_01_delivery_cost_from_pricelist(self):
'applied_on': '0_product_variant',
'product_id': self.normal_delivery.product_id.id,
})],
'discount_policy': 'without_discount',
})

# Create sales order with Normal Delivery Charges
Expand Down
8 changes: 6 additions & 2 deletions addons/event_booth_sale/models/sale_order_line.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,12 @@ def _get_display_price(self):
if self.event_booth_pending_ids and self.event_id:
company = self.event_id.company_id or self.env.company
pricelist = self.order_id.pricelist_id
if pricelist.discount_policy == "with_discount":
event_booths = self.event_booth_pending_ids.with_context(**self._get_pricelist_price_context())
price_context = self._get_pricelist_price_context()
if pricelist._get_product_rule_policy(
self.product_id,
quantity=price_context['quantity']
) != 'percentage':
event_booths = self.event_booth_pending_ids.with_context(**price_context)
total_price = sum(booth.booth_category_id.price_reduce for booth in event_booths)
else:
total_price = sum(booth.price for booth in self.event_booth_pending_ids)
Expand Down
1 change: 0 additions & 1 deletion addons/event_booth_sale/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def setUpClass(cls):
})
cls.test_pricelist_with_discount_included = cls.env['product.pricelist'].sudo().create({
'name': 'Test Pricelist',
'discount_policy': 'with_discount',
'item_ids': [
Command.create({
'compute_price': 'percentage',
Expand Down
8 changes: 6 additions & 2 deletions addons/event_sale/models/sale_order_line.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,12 @@ def _get_display_price(self):
event_ticket = self.event_ticket_id
company = event_ticket.company_id or self.env.company
pricelist = self.order_id.pricelist_id
if pricelist.discount_policy == "with_discount":
price = event_ticket.with_context(**self._get_pricelist_price_context()).price_reduce
price_context = self._get_pricelist_price_context()
if pricelist._get_product_rule_policy(
event_ticket.product_id,
quantity=price_context['quantity']
) != 'percentage':
price = event_ticket.with_context(**price_context).price_reduce
Comment on lines +103 to +108
Copy link
Contributor

Choose a reason for hiding this comment

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

First of all, there is a computed field pricelist_item_id that you can use to know the rule that will be used to compute the SOL prices, no need to call the same (costly) method multiple times (and no need to rely on that pricelist context, it should only be used for the price_reduce computation).
Also, we'll have to take a look together, but we might have to update the price computation of event tickets & booths, to make it so that it works better with the update pricelist logic

else:
price = event_ticket.price
return self._convert_to_sol_currency(price, company.currency_id)
Expand Down
2 changes: 0 additions & 2 deletions addons/event_sale/tests/test_event_sale.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,6 @@ def test_ticket_price_with_pricelist_and_tax(self):
'product_tmpl_id': event_product.id,
})

pricelist.discount_policy = 'without_discount'

so = self.env['sale.order'].create({
'partner_id': self.env.user.partner_id.id,
'pricelist_id': pricelist.id,
Expand Down
2 changes: 1 addition & 1 deletion addons/l10n_ar_website_sale/demo/website_demo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
</function>
<function model="product.pricelist" name="write">
<value model="product.pricelist" eval="obj().search([('currency_id', '=', ref('base.ARS')), ('company_id', '=', ref('base.company_ri'))]).id"/>
<value model="website" eval="{'sequence': 1, 'website_id': ref('l10n_ar_website_sale.default_website_ri')}"/>
<value model="website" eval="{'sequence': 1, 'website_ids': [(4, ref('l10n_ar_website_sale.default_website_ri'))]}"/>
</function>
<function model="product.product" name="write">
<value model="product.product" eval="obj().search([('taxes_id', '=', False)]).ids"/>
Expand Down
2 changes: 1 addition & 1 deletion addons/l10n_ec_website_sale/demo/website_demo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
('currency_id', '=', ref('base.USD')),
('company_id', '=', ref('base.demo_company_ec'))
]).id"/>
<value model="website" eval="{'sequence': 1, 'website_id': ref('l10n_ec_website_sale.default_website_ec')}"/>
<value model="website" eval="{'sequence': 1, 'website_ids': [(4, ref('l10n_ec_website_sale.default_website_ec'))]}"/>
</function>

</odoo>
19 changes: 18 additions & 1 deletion addons/point_of_sale/models/pos_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def _prepare_invoice_lines(self):
line = line_values['record']
invoice_lines_values = self._get_invoice_lines_values(line_values, line)
invoice_lines.append((0, None, invoice_lines_values))
if line.order_id.pricelist_id.discount_policy == 'without_discount' and float_compare(line.price_unit, line.product_id.lst_price, precision_rounding=self.currency_id.rounding) < 0:
if line.discount_policy == 'without_discount' and float_compare(line.price_unit, line.product_id.lst_price, precision_rounding=self.currency_id.rounding) < 0:
invoice_lines.append((0, None, {
'name': _('Price discount from %(original_price)s to %(discounted_price)s',
original_price=float_repr(line.product_id.lst_price, self.currency_id.decimal_places),
Expand Down Expand Up @@ -1192,6 +1192,14 @@ class PosOrderLine(models.Model):
combo_line_ids = fields.One2many('pos.order.line', 'combo_parent_id', string='Combo Lines') # FIXME rename to child_line_ids

combo_line_id = fields.Many2one('pos.combo.line', string='Combo Line')
discount_policy = fields.Selection(selection=[
('with_discount', "Discount included in the price"),
('without_discount', "Show public price & discount to the customer"),
],
default='with_discount',
compute='_compute_discount_policy',
store=True,
)

@api.model
def _load_pos_data_domain(self, data):
Expand All @@ -1213,6 +1221,15 @@ def _compute_refund_qty(self):
for orderline in self:
orderline.refunded_qty = -sum(orderline.mapped('refund_orderline_ids.qty'))

@api.depends('order_id.pricelist_id', 'product_id', 'qty')
def _compute_discount_policy(self):
for line in self:
line.discount_policy = 'without_discount' if line.order_id.pricelist_id._get_product_rule_policy(
line.product_id,
quantity=line.qty,
uom=line.product_uom_id,
) == 'percentage' else 'with_discount'

def _prepare_refund_data(self, refund_order, PosOrderLineLot):
"""
This prepares data for refund order line. Inheritance may inject more data here
Expand Down
2 changes: 1 addition & 1 deletion addons/point_of_sale/models/product.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ def _load_pos_data_domain(self, data):

@api.model
def _load_pos_data_fields(self, config_id):
return ['id', 'name', 'display_name', 'discount_policy', 'item_ids']
return ['id', 'name', 'display_name', 'item_ids']


class ProductPricelistItem(models.Model):
Expand Down
12 changes: 9 additions & 3 deletions addons/point_of_sale/static/src/app/models/pos_order_line.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,15 @@ export class PosOrderline extends Base {
}

display_discount_policy() {
return this.order_id.pricelist_id
? this.order_id.pricelist_id.discount_policy
: "with_discount";
if (
this.order_id.pricelist_id &&
this.order_id.pricelist_id.item_ids
.map((rule) => rule.compute_price)
.includes("percentage")
) {
return "without_discount";
}
return "with_discount";
}

get_lst_price() {
Expand Down
11 changes: 4 additions & 7 deletions addons/point_of_sale/tests/test_frontend.py
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,6 @@ def test_fiscal_position_no_tax(self):

pricelist = self.env['product.pricelist'].create({
'name': 'Test Pricelist',
'discount_policy': 'without_discount',
})

self.main_pos_config.write({
Expand All @@ -865,28 +864,26 @@ def test_06_pos_discount_display_with_multiple_pricelist(self):

base_pricelist = self.env['product.pricelist'].create({
'name': 'base_pricelist',
'discount_policy': 'without_discount',
})

self.env['product.pricelist.item'].create({
'pricelist_id': base_pricelist.id,
'product_tmpl_id': test_product.product_tmpl_id.id,
'compute_price': 'fixed',
'compute_price': 'percentage',
'applied_on': '1_product',
'fixed_price': 7,
'percent_price': 30,
})

special_pricelist = self.env['product.pricelist'].create({
'name': 'special_pricelist',
'discount_policy': 'without_discount',
})
self.env['product.pricelist.item'].create({
'pricelist_id': special_pricelist.id,
'base': 'pricelist',
'base_pricelist_id': base_pricelist.id,
'compute_price': 'formula',
'compute_price': 'percentage',
'applied_on': '3_global',
'price_discount': 10,
'percent_price': 10,
})

self.main_pos_config.write({
Expand Down
4 changes: 0 additions & 4 deletions addons/point_of_sale/views/res_config_settings_views.xml
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,6 @@
<setting id="multiple_prices_setting" string="Flexible Pricelists" help="Set multiple prices per product, automated discounts, etc.">
<field name="pos_use_pricelist" readonly="pos_has_active_session"/>
<div class="content-group" invisible="not pos_use_pricelist">
<div class="mt16">
<field name="group_sale_pricelist" invisible="1"/>
<field name="product_pricelist_setting" widget="radio" class="o_light_label"/>
</div>
<div class="row mt16">
<label string="Available" for="pos_available_pricelist_ids" class="col-lg-3 o_light_label"/>
<field name="pos_available_pricelist_ids" widget="many2many_tags" domain="['|',('company_id', '=', company_id),('company_id', '=', False)]" readonly="pos_has_active_session"/>
Expand Down
1 change: 0 additions & 1 deletion addons/pos_loyalty/tests/test_frontend.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,6 @@ def test_coupon_change_pricelist(self):

pricelist = self.env["product.pricelist"].create({
"name": "Test multi-currency",
"discount_policy": "without_discount",
"currency_id": self.env.ref("base.USD").id,
"item_ids": [
(0, 0, {
Expand Down
33 changes: 22 additions & 11 deletions addons/product/models/product_pricelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Pricelist(models.Model):
_inherit = ['mail.thread', 'mail.activity.mixin']
_description = "Pricelist"
_rec_names_search = ['name', 'currency_id'] # TODO check if should be removed
_order = "sequence asc, id asc"
_order = "sequence, id, name"

def _default_currency_id(self):
return self.env.company.currency_id.id
Expand Down Expand Up @@ -45,16 +45,6 @@ def _default_currency_id(self):
tracking=10,
)

discount_policy = fields.Selection(
selection=[
('with_discount', "Discount included in the price"),
('without_discount', "Show public price & discount to the customer"),
],
default='with_discount',
required=True,
tracking=15,
)

item_ids = fields.One2many(
comodel_name='product.pricelist.item',
inverse_name='pricelist_id',
Expand Down Expand Up @@ -160,6 +150,27 @@ def _get_product_rule(self, product, *args, **kwargs):
self and self.ensure_one() # self is at most one record
return self._compute_price_rule(product, *args, compute_price=False, **kwargs)[product.id][1]

def _get_product_rule_policy(self, product, *args, **kwargs):
"""Compute the pricelist price & rule for the specified product, qty & uom.
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong docstring 👀
Also not sure we need this util as it means you will compute the applicable rule multiple times, whereas you should strive to only do so once for performance reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well if there's already a pricelist_item_id there wont be a need to recompute it again indeed

Copy link
Contributor

Choose a reason for hiding this comment

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

Only on sales order line for now, we'll have to find another solution for configurator & ecommerce logic.


Note: self and self.ensure_one()

:param product: product record (product.product/product.template)
:param float quantity: quantity of products requested (in given uom)
:param currency: record of currency (res.currency) (optional)
:param uom: unit of measure (uom.uom record) (optional)
If not specified, prices returned are expressed in product uoms
:param date: date to use for price computation and currency conversions (optional)
:type date: date or datetime

:returns: applied pricelist rule id
:rtype: int or False
"""

self and self.ensure_one()
rule_id = self._get_product_rule(product, *args, **kwargs)
return self.env['product.pricelist.item'].browse(rule_id).compute_price

def _compute_price_rule(
self, products, quantity, currency=None, uom=None, date=False, compute_price=True,
**kwargs
Expand Down