-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[ADD][16.0] Sale Margin Update #3374
base: 16.0
Are you sure you want to change the base?
Conversation
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.
Hi @ljimenezsidoo great to see your contribution. You are proposing a new addon so I would prefer make a deeper description of PR's digging on the specific requirement is trying to fulfill, comparing maybe with standard behavior or whether it is possible with a functional approach to address the real issue. If You clarify those points I think will be more helpful to understand what is this about. Maybe U can tag me then. Thank U.
c5e288e
to
d844045
Compare
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.
LGTM
FIX requested changes ADD test
d844045
to
cf1e13b
Compare
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.
LGTM
This PR has the |
@ljimenezsidoo can U review mine in exchange #3380 ? |
@@ -0,0 +1,38 @@ | |||
############################################################################### |
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.
You can adopt short licensing headers.
@@ -0,0 +1,5 @@ | |||
############################################################################### |
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 is not needed
@@ -0,0 +1,19 @@ | |||
############################################################################### |
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 is not OCA guidelines. You can put here short license headers.
See this example: https://github.com/OCA/maintainer-tools/blob/master/template/module/models/model_name.py#L1
"res_model": "recalculate.price.margin", | ||
"view_mode": "form", | ||
"target": "new", | ||
"context": {"default_line_id": self.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.
You should reuse current context.
|
||
|
||
class TestRecalculatePriceMargin(TransactionCase): | ||
def setUp(self): |
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.
Use classmethod instead, for performance reasons.
from odoo.tests.common import TransactionCase | ||
|
||
|
||
class TestRecalculatePriceMargin(TransactionCase): |
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.
You can use this instead: https://github.com/odoo/odoo/blob/16.0/odoo/addons/base/tests/common.py#L19
for performances reasons.
required=True, | ||
) | ||
line_id = fields.Many2one( | ||
"sale.order.line", |
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.
"sale.order.line", | |
"sale.order.line", | |
ondelete="cascade" |
"sale.order.line", | ||
) | ||
order_id = fields.Many2one( | ||
"sale.order", |
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.
"sale.order", | |
"sale.order", | |
ondelete="cascade" |
This module adds a wizard that enables users to recalculate the unit price of products on a sale order line based on a specified margin percentage. It allows recalculating the sale price dynamically to ensure the desired profit margin is achieved.
Margin-based Price Adjustment: Users can specify a margin percentage to calculate and set the product's sale price according to that margin.
Negative and High Margin Constraints: The module includes validation to prevent negative margins and margins equal to or greater than 100%.