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.

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

[FIX] account,sale: fix res.partner credit_to_invoice computation #166087

Conversation

svfu-odoo
Copy link
Contributor

@svfu-odoo svfu-odoo commented May 18, 2024

Currently there are 2 issues with the function used to compute credit_to_invoice on model 'res.partner' (_compute_credit_to_invoice): 1) On 17.0 a traceback has been reported.
There were instances in which the amount_to_invoice was None and not 0
(to be looked at in a separate fix).
In such a case the None value is passed to the float function and causes a traceback.
2) From a performance perspective it is unnecessary to aggregate all the amount_to_invoice values
into an array and then postprocess them in python.
Effectively the only thing we do in the postprocess is sum all the values but ignore all non-postive values.
Thus we can just ignore sales orders with amount_to_invoice <= 0 and let the SQL / the database handle the summing.
This way we avoid the overhead from passing around the array(s) (size proportional to the number of sales orders)
and just pass around a single value.

This commit introduces the changes mentioned in (2). These changes also solve (1):
The additional condition in the domain leads to sales orders where amount_to_invoice is None being ignored. (SQL / the DB handles (1) for us now.)

related PR introducing the changed lines of code: #162770
a comment about the issue in 17.0: b5d02cc#r141990142

Currently there are 2 issues with the function used to compute `credit_to_invoice`
on model 'res.partner' (`_compute_credit_to_invoice`):
1) On 17.0 a traceback has been reported.
   There were instances in which the `amount_to_invoice` was `None` and not `0`
   (to be looked at in a separate fix).
   In such a case the `None` value is passed to the `float` function and causes a traceback.
2) From a performance perspective it is unnecessary to aggregate all the `amount_to_invoice` values
   into an array and then postprocess them in python.
   Effectively the only thing we do in the postprocess is sum all the values but ignore all non-postive values.
   Thus we can just ignore sales orders with `amount_to_invoice <= 0` and let the SQL / the database handle the summing.
   This way we avoid the overhead from passing around the array(s) (size proportional to the number of sales orders)
   and just pass around a single value.

This commit introduces the changes mentioned in (2).
These changes also solve (1):
The additional condition in the domain leads to sales orders where `amount_to_invoice` is `None` being ignored.
(SQL / the DB handles (1) for us now.)
@robodoo
Copy link
Contributor

robodoo commented May 18, 2024

@C3POdoo C3POdoo requested a review from a team May 18, 2024 06:53
@C3POdoo C3POdoo added the RD research & development, internal work label May 18, 2024
@svfu-odoo svfu-odoo requested a review from smetl May 18, 2024 08:19
svfu-odoo referenced this pull request May 18, 2024
Currently the amount of the partner credit warning
on invoices is not computed correctly in some cases.
(1) There are Sales Order credits
    and we create an invoice independent of any of SO
(2) We create an invoice from a Sales Order with
    an amount greater than the Sales Order.

For (1) the problem is that we substract the current
amount of the invoice from the sales order credits
(since we need to do this in the sales order to invoice flow)
But in case the invoice does not come from a sales order
this is wrong.

For (2) the problem is that the `amount_to_invoice` of a
Sales Order may be negative. This can happen if we invoice
more than the sales order amount.
E.g. we can end up with one invoiced order with -100 and
one uninvoiced order of 100. In the sum this would leave us
with "nothing to invoice" (100 + -100 = 0).

Reproduce (1):
All monetary values here are in company currency.
- Ensure that the partner credit is 0:
  i.e. remove all Sales Orders and Invoices.
- Activate 'Sales Credit Limit' in the settings
  and set the 'Default Credit Limit' to 100
- Create a new customer; here 'Test'
- Create a Sales Order for customer of 200
  and deliver it
- The total partner credit is now 200
- Create an Invoice (not from the Sales Order)
  for customer 'Test' (without any lines at first).
- There should be a warning that the total amount
  due is 200 (since it exceeds the limit of 100).
- Add a line of 200 to the Invoice
  (any value ≤ 200 will do).
- The total amount due in the warning is still 200
  but it should be 400 (200 + 200)
- Change the line to 800 (any value > 200 will do)
- The total amount due in the warning is now 800
  but it should be 1000 (200 + 800)

Reproduce (2):
All monetary values here are in company currency.
- Ensure that the partner credit is 0:
  i.e. remove all Sales Orders and Invoices.
- Activate 'Sales Credit Limit' in the settings
  and set the 'Default Credit Limit' to 100
- Create a new customer; here 'Test'
- Create 2 Sales Orders for customer of 200 each
  and deliver it
- The total partner credit is now 400
- Create an Invoice from exactly one of the Sales Orders
  (full amount or downpayment does not matter here)
- There should be a warning that the total amount
  due is 400 (since it exceeds the limit of 100).
- Modify the invoice (any line from the sales order)
  s.t. it has a total of 400
- Confirm the invoice
- The amount_to_invoice of the Sales Order is now -200.
  The sum of the amount_to_invoice of the Sales Orders is 0.
- Create a new invoice without any lines.
- There is a warning that the total amount
  due is 400 (= 200 + -200 + 400 ; SO + SO + invoice).
  But it should be 600 since the "overinvoiced" SO
  should just be counted with amount_to_invoice 0.

closes #164921

closes #164971

X-original-commit: ddc17e6
Signed-off-by: Laurent Smet (las) <las@odoo.com>
Signed-off-by: Sven Führ (svfu) <svfu@odoo.com>
Copy link
Contributor

@smetl smetl left a comment

Choose a reason for hiding this comment

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

robodoo pushed a commit that referenced this pull request May 21, 2024
Currently there are 2 issues with the function used to compute `credit_to_invoice`
on model 'res.partner' (`_compute_credit_to_invoice`):
1) On 17.0 a traceback has been reported.
   There were instances in which the `amount_to_invoice` was `None` and not `0`
   (to be looked at in a separate fix).
   In such a case the `None` value is passed to the `float` function and causes a traceback.
2) From a performance perspective it is unnecessary to aggregate all the `amount_to_invoice` values
   into an array and then postprocess them in python.
   Effectively the only thing we do in the postprocess is sum all the values but ignore all non-postive values.
   Thus we can just ignore sales orders with `amount_to_invoice <= 0` and let the SQL / the database handle the summing.
   This way we avoid the overhead from passing around the array(s) (size proportional to the number of sales orders)
   and just pass around a single value.

This commit introduces the changes mentioned in (2).
These changes also solve (1):
The additional condition in the domain leads to sales orders where `amount_to_invoice` is `None` being ignored.
(SQL / the DB handles (1) for us now.)

closes #166087

Signed-off-by: Laurent Smet (las) <las@odoo.com>
@robodoo robodoo closed this May 21, 2024
@svfu-odoo
Copy link
Contributor Author

Added additional changes in 17.0+ #166236 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants