[16.0][IMP]commission, account_commission: settlements grouped by payment date #643
Conversation
|
Hi @pedrobaeza, |
matteonext
force-pushed
the
16.0-imp-commission
branch
4 times, most recently
from
b7da3bb to
f801d17
Compare
|
Hi @pedrobaeza, @etobella sorry for bother! |
matteonext
force-pushed
the
16.0-imp-commission
branch
2 times, most recently
from
49a3dbf to
3650074
Compare
matteonext
force-pushed
the
16.0-imp-commission
branch
2 times, most recently
from
6de78ca to
58aa857
Compare
matteonext
force-pushed
the
16.0-imp-commission
branch
3 times, most recently
from
19a7e9a to
56d6ba3
Compare
matteonext
force-pushed
the
16.0-imp-commission
branch
from
56d6ba3 to
91a9d45
Compare
|
Hi @pedrobaeza, @etobella sorry for bother! |
etobella
left a comment
etobella
left a comment
There was a problem hiding this comment.
From my side, the code looks ok,
However, I don't know if it makes sense to split in an extra module and add the necessary hooks in account_comission 🤔
@pedrobaeza WDYT?
pedrobaeza
left a comment
pedrobaeza
left a comment
There was a problem hiding this comment.
For not forcing a new module being an small configurable addition, and having a test, let's keep it here, but please remove the empty lines inside methods, following the same style as the rest, and check the comments inline.
Another thing to think is to put a generic field "Group by", including this option and the standard one, and leaving the door opened for other groupings.
| order="invoice_date", | ||
|
|
||
| lines = self.env["account.invoice.line.agent"].search( | ||
| self._get_account_settle_domain(agent, date_to_agent) |
There was a problem hiding this comment.
Why are you removing the order here?
| if dates: | ||
| payment_date_by_inv[inv.id] = max(dates) | ||
|
|
||
| # Filtra solo le righe con fatture che hanno pagamenti entro date_payment_to |
| if line.agent_id.commission_id.settled_dates_based_on != "payment": | ||
| return super().get_period_date(line) | ||
| return self.get_latest_payment_date(line.invoice_id) |
There was a problem hiding this comment.
Better readability doing the contrary:
| if line.agent_id.commission_id.settled_dates_based_on != "payment": | |
| return super().get_period_date(line) | |
| return self.get_latest_payment_date(line.invoice_id) | |
| if line.agent_id.commission_id.settled_dates_based_on == "payment": | |
| return self.get_latest_payment_date(line.invoice_id) | |
| return super().get_period_date(line) |
| for ( | ||
| _partial, | ||
| _amount, | ||
| counterpart_line, | ||
| ) in invoice_partials: |
There was a problem hiding this comment.
Avoid excessive lines:
| for ( | |
| _partial, | |
| _amount, | |
| counterpart_line, | |
| ) in invoice_partials: | |
| for (_p, _a, counterpart_line) in invoice_partials: |
| be settled as well, resulting in a 0 net commission between both operations. | ||
|
|
||
| #. For payment-based commissions, you can choose how settlements are grouped. | ||
| By default, they’re grouped by 'Invoice Date', but you can also group them |
| comodel_name="account.move", | ||
| compute="_compute_invoice_id", | ||
| ) | ||
| commission_grouped_by = fields.Selection( |
There was a problem hiding this comment.
I would prefer to remove this.
5 participants
This pr allows to generate commissions based on the payment date.
For example, if you have three invoices from three different months but with a payment date in the same month, the current behavior generates three separate period, one for each month. With this change, however, all commissions are assigned to a single period.
It covers a similar scope but is managed in a different way #581