-
Notifications
You must be signed in to change notification settings - Fork 9
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
[4804][IMP] sale_order_pricelist_commitment_date #228
base: 12.0
Are you sure you want to change the base?
[4804][IMP] sale_order_pricelist_commitment_date #228
Conversation
… secondary_uom_price calculation
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.
IMO, it would be better to just update sale_order_pricelist_commitment_date
instead of having changes on the same method in two separate modules.
- Add
sale_order_secondary_unit_price
as a dependency ofsale_order_pricelist_commitment_date
- Adjust
onchange_commitment_date()
accordingly - No changes on
sale_order_secondary_unit_price
0452024
to
83a0dce
Compare
"depends": [ | ||
"sale", | ||
"product_secondary_unit", | ||
], |
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.
Avoid making unnecessary changes.
date=self.date_order, | ||
pricelist=self.pricelist_id.id, | ||
uom=line.product_uom.id, | ||
commitment_date=self.commitment_date, |
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.
date=self.date_order, | |
pricelist=self.pricelist_id.id, | |
uom=line.product_uom.id, | |
commitment_date=self.commitment_date, | |
date=self.commitment_date, | |
pricelist=self.pricelist_id.id, | |
uom=line.product_uom.id, | |
fiscal_position=self.env.context.get("fiscal_position"), |
How about making it like this, and removing the adjustment on _compute_price_rule()
?
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.
Sorry, removing the adjustments on _compute_price_rule()
was a mistake -- it would break the price recalculation behavior when product, quantity, or UoM is changed.
date=self.commitment_date, | ||
pricelist=self.pricelist_id.id, | ||
uom=line.product_uom.id, | ||
fiscal_position=self.env.context.get("fiscal_position"), |
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.
date=self.commitment_date, | |
pricelist=self.pricelist_id.id, | |
uom=line.product_uom.id, | |
fiscal_position=self.env.context.get("fiscal_position"), | |
date=self.date_order, | |
pricelist=self.pricelist_id.id, | |
uom=line.product_uom.id, | |
fiscal_position=self.env.context.get("fiscal_position"), | |
commitment_date=self.commitment_date, |
# Copyright 2020 Quartile Limited | ||
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). | ||
|
||
from odoo import api, fields, models | ||
|
||
|
||
class Pricelist(models.Model): | ||
_inherit = "product.pricelist" | ||
|
||
@api.multi | ||
def _compute_price_rule(self, products_qty_partner, date=False, uom_id=False): | ||
date = ( | ||
self._context.get("commitment_date") | ||
or self._context.get("date") | ||
or fields.Date.today() | ||
) | ||
date = fields.Date.to_date(date) | ||
return super(Pricelist, self)._compute_price_rule( | ||
products_qty_partner=products_qty_partner, date=date, uom_id=uom_id | ||
) |
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.
Please revive this.
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.
Code review.
@kanda999 @nobuQuartile Please review when you are available. |
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.
Functional test
LGTM
4804