Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions account_invoice_triple_discount/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from . import line_triple_discount
from . import account_invoice
52 changes: 10 additions & 42 deletions account_invoice_triple_discount/models/account_invoice.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,60 +2,28 @@
# Copyright 2017 Tecnativa - Pedro M. Baeza
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo import api, fields, models
from odoo.addons import decimal_precision as dp
from odoo import api, models


class AccountInvoice(models.Model):
_inherit = "account.invoice"

def get_taxes_values(self):
vals = {}
for line in self.invoice_line_ids:
vals[line] = {
'price_unit': line.price_unit,
'discount': line.discount,
}
price_unit = line.price_unit * (1 - (line.discount or 0.0) / 100.0)
price_unit *= (1 - (line.discount2 or 0.0) / 100.0)
price_unit *= (1 - (line.discount3 or 0.0) / 100.0)
line.update({
'price_unit': price_unit,
'discount': 0.0,
})
tax_grouped = super(AccountInvoice, self).get_taxes_values()
for line in self.invoice_line_ids:
line.update(vals[line])
lines = self.invoice_line_ids
prev_values = lines.triple_discount_preprocess()
tax_grouped = super().get_taxes_values()
lines.triple_discount_postprocess(prev_values)
return tax_grouped


class AccountInvoiceLine(models.Model):
_inherit = "account.invoice.line"

discount2 = fields.Float(
'Discount 2 (%)',
digits=dp.get_precision('Discount'),
)
discount3 = fields.Float(
'Discount 3 (%)',
digits=dp.get_precision('Discount'),
)
_name = "account.invoice.line"
_inherit = ["line.triple_discount.mixin", "account.invoice.line"]

@api.multi
@api.depends('discount2', 'discount3')
@api.depends('discount2', 'discount3', 'discounting_type')
def _compute_price(self):
for line in self:
prev_price_unit = line.price_unit
prev_discount = line.discount
price_unit = line.price_unit * (1 - (line.discount or 0.0) / 100.0)
price_unit *= (1 - (line.discount2 or 0.0) / 100.0)
price_unit *= (1 - (line.discount3 or 0.0) / 100.0)
line.update({
'price_unit': price_unit,
'discount': 0.0,
})
prev_values = line.triple_discount_preprocess()
super(AccountInvoiceLine, line)._compute_price()
line.update({
'price_unit': prev_price_unit,
'discount': prev_discount,
})
line.triple_discount_postprocess(prev_values)
100 changes: 100 additions & 0 deletions account_invoice_triple_discount/models/line_triple_discount.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# Copyright 2017 Tecnativa - David Vidal
# Copyright 2017 Tecnativa - Pedro M. Baeza
# Copyright 2021 Simone Rubino - Agile Business Group
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

from odoo import api, fields, models
from odoo.addons import decimal_precision as dp


class LineTripleDiscount (models.AbstractModel):
_name = "line.triple_discount.mixin"
_description = "Add triple discount fields and logic"

discount = fields.Float()
discount2 = fields.Float(
'Discount 2 (%)',
digits=dp.get_precision('Discount'),
)
discount3 = fields.Float(
'Discount 3 (%)',
digits=dp.get_precision('Discount'),
)
discounting_type = fields.Selection(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand where this field can be changed ? Should not be a setting on the related res.partner ? I mean some suppliers are using additive method and other multiplicative method.

Don't you think ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This can be changed directly on the account invoice line's form, but it can be interesting to inherit that from the partner.
I'm currently investigating why Travis is still 🔴 (that's why this is still a Draft), after that I can try to add this new behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

While implementing this feature I noticed that it is not as trivial as it seems because default value for invoice line cannot rely on the line's fields (default methods are api.model), so I'd need to save it in the context etc..
I think that deserves its own PR, and here let's focus on the described issue, is that ok for you?

BTW Now that Travis is 🟢, I'll mark this PR as ready for reviews

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

well, I was more thinking on storing the discounting_type only on the res.partner or at least on the order/invoice method.
it will be a lighter implementation.
Do you think there are real cases when there are some line with additive method, and other with multiplicative ?

I mean, setting this value 50 times, if you have 50 lines will be a mess, in a UX point of view.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That is why I was talking about a default value for new lines: each of the 50 lines would have the default value inherited from the partner; and switching partner in the invoice would trigger the edit of all the invoice's lines.

I don't know about real cases of lines having different kinds of discounts but this is how sale orders + triple_discount currently work and it would be nice to have it in invoices too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added the logic explained above, can you update your review?

@legalsylvain legalsylvain Jul 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know about real cases of lines having different kinds of discounts but this is how sale orders + triple_discount currently work and it would be nice to have it in invoices too.

  1. Well if it's not a use case, it should be changed I think. Don't you ?

  2. the problem with the current implementation is that it will create inconsistencies. I mean, it's possible to have "multiplicative" and invoice level and "additive" on line level.

  3. you are adding new field on invoice / invoice.line. don't you think you should add a pre-migration script to populate correctly the database. I fear that an update of the current module on a production database will be slow. Don't you think ?

string="Discounting type",
selection=[
('additive', 'Additive'),
('multiplicative', 'Multiplicative'),
],
default="multiplicative",
required=True,
help="Specifies whether discounts should be additive "
"or multiplicative.\nAdditive discounts are summed first and "
"then applied.\nMultiplicative discounts are applied sequentially.\n"
"Multiplicative discounts are default",
)

_sql_constraints = [
('discount2_limit', 'CHECK (discount2 <= 100.0)',
'Discount 2 must be lower than 100%.'),
('discount3_limit', 'CHECK (discount3 <= 100.0)',
'Discount 3 must be lower than 100%.'),
]

def _get_final_discount(self):
self.ensure_one()
if self.discounting_type == "additive":
return self._additive_discount()
elif self.discounting_type == "multiplicative":
return self._multiplicative_discount()

def _additive_discount(self):
self.ensure_one()
discount = sum(
[getattr(self, x) or 0.0 for x in self._discount_fields()]
)
if discount <= 0:
return 0
elif discount >= 100:
return 100
return discount

def _multiplicative_discount(self):
self.ensure_one()
discounts = [1 - (self[x] or 0.0) / 100
for x in self._discount_fields()]
final_discount = 1
for discount in discounts:
final_discount *= discount
return 100 - final_discount * 100

def _discount_fields(self):
return ['discount', 'discount2', 'discount3']

@api.multi
def triple_discount_preprocess(self):
"""Save the values of the discounts in a dictionary,
to be restored in postprocess.
Resetting discount2 and discount3 to 0.0 avoids issues if
this method is called multiple times.
Updating the cache provides consistency through recomputations."""
prev_values = dict()
for line in self:
prev_values[line] = dict(
discount=line.discount,
discount2=line.discount2,
discount3=line.discount3,
)
line.update({
'discount': line._get_final_discount(),
'discount2': 0.0,
'discount3': 0.0
})
return prev_values

@api.model
def triple_discount_postprocess(self, prev_values):
"""Restore the discounts of the lines in the dictionary prev_values.
Updating the cache provides consistency through recomputations."""
for line, prev_vals_dict in list(prev_values.items()):
line.update(prev_vals_dict)
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,23 @@ def test_02_discounts_multiple_lines(self):
self.invoice_line1.discount = 50.0
self.invoice._onchange_invoice_line_ids()
self.assertEqual(self.invoice.amount_total, 365.0)

def test_03_discounts(self):
""" Tests discounts in edge case """
invoice = self.env['account.invoice'].create({
'name': "Test Customer Invoice",
'journal_id': self.env['account.journal'].search(
[('type', '=', 'sale')])[0].id,
'partner_id': self.partner.id,
'account_id': self.account.id,
'invoice_line_ids': [(0, 0, {
'name': 'Line 1',
'price_unit': 25.0,
'account_id': self.account.id,
'quantity': 65,
'discount': 50,
'discount2': 13,
'discount3': 0,
})],
})
self.assertEqual(invoice.invoice_line_ids.price_subtotal, 706.88)
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
position="after">
<field name="discount2" groups="base.group_no_one"/>
<field name="discount3" groups="base.group_no_one"/>
<field name="discounting_type" groups="base.group_no_one"/>
</xpath>
</field>
</record>
Expand All @@ -24,6 +25,7 @@
position="after">
<field name="discount2" groups="base.group_no_one"/>
<field name="discount3" groups="base.group_no_one"/>
<field name="discounting_type" groups="base.group_no_one"/>
</xpath>
</field>
</record>
Expand Down