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.

Sign up for GitHub

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

[16.0] [ADD] sale_commission_margin: Decuct commission from margin #591

Open
wants to merge 2 commits into
base: 16.0
Choose a base branch
from
Open

[16.0] [ADD] sale_commission_margin: Decuct commission from margin #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1a971c
[ADD] sale_commission_margin: Deduct commission from margin
luisDIXMIT Jan 24, 2025
20753b2
[IMP] commission: Make fields precomputed
luisDIXMIT Jan 27, 2025
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
4 changes: 4 additions & 0 deletions commission/models/commission_mixin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ class CommissionMixin(models.AbstractModel):
readonly=False,
store=True,
copy=True,
precompute=True,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these changes are correct.

Copy link
Member

Choose a reason for hiding this comment

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

This changes are necessary in order to avoid warnings. The problem comes from the fact that margin fields are pre-computed, then it needs pre-computed fields.

However, after checking Odoo documentation, we can use pre-compute to fill this fields, as they are generated in batch (they are lines). This is why we thought it was ok.

https://www.odoo.com/documentation/16.0/developer/reference/backend/orm.html?highlight=precompute

If you have a reason that we cannot see (it might be possible), we would be happy if you can enlighten us.

Copy link
Member

Choose a reason for hiding this comment

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

Then change this attribute in the module sale_commission_margin, not here. I prefer to not change it due to things like Precomputing a field can be counterproductive if the records of the given model are not created in batch

Copy link
Member

Choose a reason for hiding this comment

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

I understand your concerns, but the following sentences states:

Following the remark above, precomputed fields can be interesting on the lines of a one2many, which are usually created in batch by the ORM itself, provided that they are created by writing on the record that contains them

Usually, invoice_lines and sale order lines are generated with [(0, 0, {}), ...]. That means that they shouldn't be affected by the issue that you stated.

I prefer to do it directly on the base model in order to avoid future errors or incompatibilities (if we add fields that are not pre-computed, warnings will be raised if this module is installed). However, if there are not other options, we can do it as you suggest.

Also, it is recommended to change the priority of the view on sale.order in order to show the commission and then the margin. Should we do that directly on this new module or can we do it on sale_commission?

Copy link
Member

@pedrobaeza pedrobaeza Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Uhm, I'm still a bit reluctant. We don't know how can it impact. If precompute mode was so good, then it would be enabled by default. I still think that should be in the new module.

About the view priority switch, it can also be done on the new module. That way, you identify the whys of that changes.

Copy link
Member

Choose a reason for hiding this comment

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

For that reason I am suggesting to use a new version. If we are going to block this kind of changes do we need to block all refactorings (even on migrations) too?

Also, by checking the Odoo documentation we showed that the solution makes sense. I don't understand why there are so many complains if we have provided answers with the issue and we have proposed a solution that minimizes effects on the current versions. Keeping it in the module might have other undesired effects as failing integrations with other commission modules and warnings. This is the reason why we are asking for the integration on the main module (we can accept to do it on a non released module).

Just to end, it is not "my" module. It is an OCA module that is fixing a behavior that is not working as expected when we install commission and a core community odoo module. It was documented 7 years ago and no one has addressed it since today

Copy link
Member

Choose a reason for hiding this comment

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

This sentence in the ORM documentation is the key: Precomputing a field can be counterproductive if the records of the given model are not created in batch. Activating precompute is not needed unless you install sale_margin and this glue module. That's why I say that flag should go into the glue module, not on commission or sale_commission one.

I don't see either why it's a problem to put it in the glue module and why you want it to put on the base module if it's not going to have any benefit, but on contrary, it can be counterproductive as stated in the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

And the next sentence states that it makes sense in O2M fields like sale order lines, purchase order lines or account move lines:

Following the remark above, precomputed fields can be interesting on the lines of a one2many, which are usually created in batch by the ORM itself, provided that they are created by writing on the record that contains them.

It is not counterproductive IMO. Right now, commission and sale_margin do not work properly between them. We are just fixing this behavior.

The other option is to write down that we need to exclude sale_margin when using commission, as it makes no sense

Copy link
Member

Choose a reason for hiding this comment

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

But you are needing the same this glue module for deducting the amount, so even if you put precompute=True in the base module, you will still need the glue module. That's why I think it should go all together, the change to precompute=True, the view change, and the business logic.

Copy link
Member

Choose a reason for hiding this comment

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

And then we are leaving a big hole for possible future problems.

I could understand your position when we are struggling with my own modules or OCA modules, but we are talking about an integration with a core community module of Odoo.

This are small changes that could be done on the commission module and we both know it.

)
product_id = fields.Many2one(comodel_name="product.product", string="Product")
commission_free = fields.Boolean(
string="Comm. free",
compute="_compute_commission_free",
store=True,
readonly=True,
precompute=True,
)
commission_status = fields.Char(
compute="_compute_commission_status",
Expand Down Expand Up @@ -126,11 +128,13 @@ class CommissionLineMixin(models.AbstractModel):
store=True,
readonly=False,
copy=True,
precompute=True,
)
amount = fields.Monetary(
string="Commission Amount",
compute="_compute_amount",
store=True,
precompute=True,
)
# Fields to be overriden with proper source (via related or computed field)
currency_id = fields.Many2one(comodel_name="res.currency")
Expand Down
96 changes: 96 additions & 0 deletions sale_commission_margin/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
======================
Sale Commission Margin
======================

..
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!! This file is generated by oca-gen-addon-readme !!
!! changes will be overwritten. !!
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
!! source digest: sha256:4868da548ee8d901030583d2f63cdf09d01f8f9f6a607bce11ab7432430d3099
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

.. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png
:target: https://odoo-community.org/page/development-status
:alt: Beta
.. |badge2| image:: https://img.shields.io/badge/licence-AGPL--3-blue.png
:target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
:alt: License: AGPL-3
.. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fcommission-lightgray.png?logo=github
:target: https://github.com/OCA/commission/tree/16.0/sale_commission_margin
:alt: OCA/commission
.. |badge4| image:: https://img.shields.io/badge/weblate-Translate%20me-F47D42.png
:target: https://translation.odoo-community.org/projects/commission-16-0/commission-16-0-sale_commission_margin
:alt: Translate me on Weblate
.. |badge5| image:: https://img.shields.io/badge/runboat-Try%20me-875A7B.png
:target: https://runboat.odoo-community.org/builds?repo=OCA/commission&target_branch=16.0
:alt: Try me on Runboat

|badge1| |badge2| |badge3| |badge4| |badge5|

This module extends the functionality of sale_margin and sale_commission
to deduct commissions from margin

**Table of contents**

.. contents::
:local:

Usage
=====

- Go to Sales -> Configuration -> Settings

- Scroll to pricing settings and check 'Margins' box.

- Go to Sales -> Orders -> Quotations

- Create new quotation and set:

1. Customer
2. One order line

- In the order line created press edit agents button and set an agent.

- The agent commission amount should be deducted from margin.

Bug Tracker
===========

Bugs are tracked on `GitHub Issues <https://github.com/OCA/commission/issues>`_.
In case of trouble, please check there if your issue has already been reported.
If you spotted it first, help us to smash it by providing a detailed and welcomed
`feedback <https://github.com/OCA/commission/issues/new?body=module:%20sale_commission_margin%0Aversion:%2016.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.

Do not contact contributors directly about support or help with technical issues.

Credits
=======

Authors
-------

* Dixmit

Contributors
------------

-Luis Rodríguez [email protected]
(`www.dixmit.com <http://www.dixmit.com>`__)

Maintainers
-----------

This module is maintained by the OCA.

.. image:: https://odoo-community.org/logo.png
:alt: Odoo Community Association
:target: https://odoo-community.org

OCA, or the Odoo Community Association, is a nonprofit organization whose
mission is to support the collaborative development of Odoo features and
promote its widespread use.

This module is part of the `OCA/commission <https://github.com/OCA/commission/tree/16.0/sale_commission_margin>`_ project on GitHub.

You are welcome to contribute. To learn how please visit https://odoo-community.org/page/Contribute.
1 change: 1 addition & 0 deletions sale_commission_margin/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import models
17 changes: 17 additions & 0 deletions sale_commission_margin/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2025 Dixmit
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).

{
"name": "Sale Commission Margin",
"summary": """This addons allows commissions to be deducted from the margin.""",
"version": "16.0.1.0.0",
"license": "AGPL-3",
"author": "Dixmit,Odoo Community Association (OCA)",
"website": "https://github.com/OCA/commission",
"depends": [
"sale_margin",
"sale_commission",
],
"data": [],
"demo": [],
}
1 change: 1 addition & 0 deletions sale_commission_margin/models/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from . import sale_order_line
23 changes: 23 additions & 0 deletions sale_commission_margin/models/sale_order_line.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright 2025 Dixmit
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).

from odoo import api, models


class SaleOrderLine(models.Model):

_inherit = "sale.order.line"

@api.depends(
"price_subtotal", "product_uom_qty", "purchase_price", "agent_ids.amount"
)
def _compute_margin(self):
for line in self:
line.margin = (
line.price_subtotal
- (line.purchase_price * line.product_uom_qty)
- sum(line.mapped("agent_ids.amount"))
)
line.margin_percent = (
line.price_subtotal and line.margin / line.price_subtotal
)
1 change: 1 addition & 0 deletions sale_commission_margin/readme/CONTRIBUTORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Luis Rodríguez <[email protected]> (www.dixmit.com)
1 change: 1 addition & 0 deletions sale_commission_margin/readme/DESCRIPTION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This module extends the functionality of sale_margin and sale_commission to deduct commissions from margin
9 changes: 9 additions & 0 deletions sale_commission_margin/readme/USAGE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
- Go to Sales -> Configuration -> Settings
- Scroll to pricing settings and check 'Margins' box.
- Go to Sales -> Orders -> Quotations
- Create new quotation and set:
1. Customer
2. One order line

- In the order line created press edit agents button and set an agent.
- The agent commission amount should be deducted from margin.
Binary file added sale_commission_margin/static/description/icon.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading