-
-
Notifications
You must be signed in to change notification settings - Fork 693
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
[12.0][FIX] account_invoice_triple_discount: subtotal computation #876
Closed
SimoRubi
wants to merge
5
commits into
OCA:12.0
from
SimoRubi:12.0-fix-account_invoice_triple_discount-subtotal_computation
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
baa149f
[FIX] account_invoice_triple_discount: Added mixin for triple discoun…
SimoRubi 36f964c
Fix cache management.
SimoRubi d3a4e19
Add default discount type for partner
SimoRubi 03d42d4
Add default discount type for invoice lines, from partner
SimoRubi 83c27b1
[FIX] account_invoice_triple_discount: Computed discount might exceed…
SimoRubi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
], | ||
'data': [ | ||
'views/account_invoice_view.xml', | ||
'views/res_partner_views.xml', | ||
], | ||
'installable': True, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
|
||
from . import line_triple_discount | ||
from . import account_invoice | ||
from . import res_partner |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
111 changes: 111 additions & 0 deletions
111
account_invoice_triple_discount/models/line_triple_discount.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
# 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( | ||
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() | ||
|
||
# The newly computed discount might have | ||
# more digits than allowed from field's precision, | ||
# so let's increase it just for saving it correctly in cache | ||
discount_field = self._fields['discount'] | ||
discount_original_digits = discount_field._digits | ||
discount_field._digits = (16, 10) | ||
|
||
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 | ||
}) | ||
|
||
# Restore discount field's precision | ||
discount_field._digits = discount_original_digits | ||
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) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Copyright 2021 Simone Rubino - Agile Business Group | ||
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). | ||
|
||
from odoo import fields, models | ||
|
||
|
||
class ResPartner (models.Model): | ||
_inherit = 'res.partner' | ||
|
||
discounting_type = fields.Selection( | ||
string="Discounting type", | ||
selection=[ | ||
('additive', 'Additive'), | ||
('multiplicative', 'Multiplicative'), | ||
], | ||
default="multiplicative", | ||
required=True, | ||
help=""" | ||
Specifies whether discounts should be additive or multiplicative. | ||
Additive discounts are summed first and then applied. | ||
Multiplicative discounts (default) are applied sequentially. | ||
This type of discount will be the default for this partner's invoices. | ||
""", | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
account_invoice_triple_discount/views/res_partner_views.xml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<!-- | ||
Copyright 2021 Simone Rubino - Agile Business Group | ||
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). | ||
--> | ||
|
||
<odoo> | ||
<record id="view_partner_property_form" model="ir.ui.view"> | ||
<field name="name">Add triple discount fields</field> | ||
<field name="model">res.partner</field> | ||
<field name="inherit_id" ref="account.view_partner_property_form"/> | ||
<field name="arch" type="xml"> | ||
<page name="accounting" position="inside"> | ||
<group name="triple_discount"> | ||
<field name="discounting_type"/> | ||
</group> | ||
</page> | ||
</field> | ||
</record> | ||
</odoo> |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 usingadditive
method and othermultiplicative
method.Don't you think ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 withmultiplicative
?I mean, setting this value 50 times, if you have 50 lines will be a mess, in a UX point of view.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well if it's not a use case, it should be changed I think. Don't you ?
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.
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 ?