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

Jump to bottom

[14.0][IMP]Account_invoice_triple_discount: For in invoice_line_ids and not in line_ids #1442

Closed
wants to merge 9 commits into from
Closed

[14.0][IMP]Account_invoice_triple_discount: For in invoice_line_ids and not in line_ids #1442

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
18e8df2
[IMP]For in invoice line and not in line_ids
matteonext Apr 17, 2023
e90c53f
[IMP]Add tests related to avoid updates in confilct with l10n-italy m…
matteonext Jun 7, 2023
d25a872
[IMP]Pre commit stuff
matteonext Jun 7, 2023
69c4fdf
[IMP]Fix invoice balance
matteonext Jul 27, 2023
c6e7391
[FIX] Fix _recompute_tax_lines to avoid issues with negative tax line…
stenext Aug 3, 2023
7fc2600
[FIX] Fix taxes assigment in tests
stenext Aug 3, 2023
9a26232
[FIX] Removed unuseful comment in tests
stenext Aug 4, 2023
10df6db
[FIX] Fix test refund_negative_taxes to justify improvement in _recom…
stenext Aug 4, 2023
43cf80b
[FIX] Changed _recompute_tax_lines doc according to method changes
stenext Sep 5, 2023
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
12 changes: 5 additions & 7 deletions account_invoice_triple_discount/models/account_move.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,17 @@ def _recompute_tax_lines(self, **kwargs):
simulate a multiple discount by changing the unit price. Values are
odooNextev marked this conversation as resolved.
Show resolved Hide resolved
restored after the original process is done
"""
digits = self.line_ids._fields["discount"]._digits
self.line_ids._fields["discount"]._digits = (16, 16)
old_values_by_line_id = {}
digits = self.line_ids._fields["price_unit"]._digits
self.line_ids._fields["price_unit"]._digits = (16, 16)
odooNextev marked this conversation as resolved.
Show resolved Hide resolved
for line in self.line_ids:
aggregated_discount = line._compute_aggregated_discount(line.discount)
old_values_by_line_id[line.id] = {
"price_unit": line.price_unit,
"discount": line.discount,
}
price_unit = line.price_unit * (1 - aggregated_discount / 100)
line.update({"price_unit": price_unit, "discount": 0})
self.line_ids._fields["price_unit"]._digits = digits
line.update({"discount": aggregated_discount})
res = super(AccountMove, self)._recompute_tax_lines(**kwargs)
self.line_ids._fields["discount"]._digits = digits
for line in self.line_ids:
if line.id not in old_values_by_line_id:
continue
Expand All @@ -39,6 +37,6 @@ def _has_discount(self):
return any(
[
line._compute_aggregated_discount(line.discount) > 0
for line in self.invoice_line_ids
for line in self.line_ids
Comment on lines -42 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

This check was on line_ids, why is it changed? Does not seem to affect the added test result

]
)
31 changes: 31 additions & 0 deletions account_invoice_triple_discount/tests/test_invoice_triple_discount.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,22 @@ def setUpClass(cls):
)
cls.sale_journal = cls.Journal.search([("type", "=", "sale")], limit=1)

def _create_refund(self):
refund_form = Form(
self.env["account.move"].with_context(default_move_type="in_refund")
)
refund_form.name = "Test Refund for Triple Discount"

with refund_form.invoice_line_ids.new() as refund_line:
refund_line.quantity = 10
refund_line.name = "Negative amounts"
refund_line.price_unit = -2.00
refund_line.tax_ids.clear()
refund_line.tax_ids.add(self.tax)

refund = refund_form.save()
return refund

def create_simple_invoice(self, amount):
invoice_form = Form(
self.AccountMove.with_context(
Expand Down Expand Up @@ -164,3 +180,18 @@ def test_04_discounts_decimals_tax(self):
invoice_form.save()

self.assertEqual(invoice.amount_tax, 177.61)

def test_05_refund_negative_taxes(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this locally and this test is passing even without the changes to account_move.
When adding a fix, the included test should pass with the fix and break without it.

Copy link
Author

Choose a reason for hiding this comment

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

@SirTakobi now I fixed this test and without our fix in _recompute_tax_lines it fails

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

"""
Tests refund negative taxes
"""
refund = self._create_refund()
for line in refund.invoice_line_ids:
line.with_context(check_move_validity=False).update(
{"price_unit": -line.price_unit}
)
refund.with_context(check_move_validity=False)._recompute_dynamic_lines(
recompute_all_taxes=True,
)
self.assertEqual(refund.move_type, "in_refund")
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this locally and here the refund is no more balanced: calling refund._check_balanced() raises:

odoo.sql_db: bad query: UPDATE "account_move_line" SET "amount_currency"='0.00',"amount_residual"='23.00',"amount_residual_currency"='0.00',"balance"='23.00',"credit"='0.00',"debit"='23.00',"price_subtotal"='0.00',"price_total"='0.00',"price_unit"='0.00',"reconciled"=false,"tax_audit"='',"write_uid"=1,"write_date"=(now() at time zone 'UTC') WHERE id IN (125)
ERROR: new row for relation "account_move_line" violates check constraint "account_move_line_check_amount_currency_balance_sign"
DETAIL:  Failing row contains (125, 44, Test Refund for Triple Discount, 2023-08-07, null, draft, 2, 1, 2, 14, 50049, 10, , 1.00, 0.00, 0.00, 23.00, 0.00, 23.00, 0.00, 0.00, 0.00, f, f, 2023-08-07, 2, null, null, null, null, null, null, null, null, null, 0.00, t, null, , 23.00, 0.00, null, null, null, null, f, t, 1, 2023-08-07 08:48:13.137036, 1, 2023-08-07 08:48:13.137036, 0.00, 0.00).

so I have the same concerns of 2 months ago: #1442 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

I tried this locally and here the refund is no more balanced: calling refund._check_balanced() raises:

odoo.sql_db: bad query: UPDATE "account_move_line" SET "amount_currency"='0.00',"amount_residual"='23.00',"amount_residual_currency"='0.00',"balance"='23.00',"credit"='0.00',"debit"='23.00',"price_subtotal"='0.00',"price_total"='0.00',"price_unit"='0.00',"reconciled"=false,"tax_audit"='',"write_uid"=1,"write_date"=(now() at time zone 'UTC') WHERE id IN (125)
ERROR: new row for relation "account_move_line" violates check constraint "account_move_line_check_amount_currency_balance_sign"
DETAIL:  Failing row contains (125, 44, Test Refund for Triple Discount, 2023-08-07, null, draft, 2, 1, 2, 14, 50049, 10, , 1.00, 0.00, 0.00, 23.00, 0.00, 23.00, 0.00, 0.00, 0.00, f, f, 2023-08-07, 2, null, null, null, null, null, null, null, null, null, 0.00, t, null, , 23.00, 0.00, null, null, null, null, f, t, 1, 2023-08-07 08:48:13.137036, 1, 2023-08-07 08:48:13.137036, 0.00, 0.00).

so I have the same concerns of 2 months ago: #1442 (comment)

@SirAionTech I don't understand what's wrong in lines recomputation at line 193...
If you duplicate lines 193-195 or manually launch double _recompute_dynamic_lines() from debug console, lines will be balanced and it will pass the test...
I don't know why we need 2 recomputations to balance taxes lines and I guess there are no issues in this method cause it's from Odoo standard accounting

self.assertEqual(round(refund.amount_total, 2), 23.0)
Loading