From efa60a1538905cd34a1a47939ea0ff8339f214ba Mon Sep 17 00:00:00 2001 From: Laurent Smet Date: Tue, 21 Jan 2020 09:02:08 +0000 Subject: [PATCH] [FIX] account: Fix rounding issue on price_unit with price included tax Suppose a price_unit of 2300 with a 5.5% price-included tax applied on it. The computed balance will be computed as follow: 2300.0 / 1.055 = 2180.0948 ~ 2180.09. Since accounting / business fields are synchronized, the inverse computation will try to recompute the price_unit based on the balance: 2180.09 * 1.055 = 2299.99495 ~ 2299.99. --issue: 2168073 --- addons/account/models/account_move.py | 51 +++- .../tests/test_account_move_in_invoice.py | 2 +- .../tests/test_account_move_in_refund.py | 2 +- .../tests/test_account_move_out_invoice.py | 267 +++++++++++++++++- .../tests/test_account_move_out_refund.py | 2 +- 5 files changed, 306 insertions(+), 18 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index fb418b40808..e728827d457 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -2753,32 +2753,34 @@ class AccountMoveLine(models.Model): 'credit': price_subtotal < 0.0 and -price_subtotal or 0.0, } - def _get_fields_onchange_balance(self, quantity=None, discount=None, balance=None, move_type=None, currency=None, taxes=None): + def _get_fields_onchange_balance(self, quantity=None, discount=None, balance=None, move_type=None, currency=None, taxes=None, price_subtotal=None): self.ensure_one() return self._get_fields_onchange_balance_model( quantity=quantity or self.quantity, discount=discount or self.discount, balance=balance or self.balance, move_type=move_type or self.move_id.type, - currency=currency or self.currency_id, + currency=currency or self.currency_id or self.move_id.currency_id, taxes=taxes or self.tax_ids, + price_subtotal=price_subtotal or self.price_subtotal, ) @api.model - def _get_fields_onchange_balance_model(self, quantity, discount, balance, move_type, currency, taxes): + def _get_fields_onchange_balance_model(self, quantity, discount, balance, move_type, currency, taxes, price_subtotal): ''' This method is used to recompute the values of 'quantity', 'discount', 'price_unit' due to a change made in some accounting fields such as 'balance'. This method is a bit complex as we need to handle some special cases. For example, setting a positive balance with a 100% discount. - :param quantity: The current quantity. - :param discount: The current discount. - :param balance: The new balance. - :param move_type: The type of the move. - :param currency: The currency. - :param taxes: The applied taxes. - :return: A dictionary containing 'quantity', 'discount', 'price_unit'. + :param quantity: The current quantity. + :param discount: The current discount. + :param balance: The new balance. + :param move_type: The type of the move. + :param currency: The currency. + :param taxes: The applied taxes. + :param price_subtotal: The price_subtotal. + :return: A dictionary containing 'quantity', 'discount', 'price_unit'. ''' if move_type in self.move_id.get_outbound_types(): sign = 1 @@ -2788,6 +2790,14 @@ class AccountMoveLine(models.Model): sign = 1 balance *= sign + # Avoid rounding issue when dealing with price included taxes. For example, when the price_unit is 2300.0 and + # a 5.5% price included tax is applied on it, a balance of 2300.0 / 1.055 = 2180.094 ~ 2180.09 is computed. + # However, when triggering the inverse, 2180.09 + (2180.09 * 0.055) = 2180.09 + 119.90 = 2299.99 is computed. + # To avoid that, set the price_subtotal at the balance if the difference between them looks like a rounding + # issue. + if currency.is_zero(balance - price_subtotal): + return {} + taxes = taxes.flatten_taxes_hierarchy() if taxes and any(tax.price_include for tax in taxes): # Inverse taxes. E.g: @@ -3155,7 +3165,7 @@ class AccountMoveLine(models.Model): vals.setdefault('company_currency_id', move.company_id.currency_id.id) # important to bypass the ORM limitation where monetary fields are not rounded; more info in the commit message if move.is_invoice(include_receipts=True): - currency = self.env['res.currency'].browse(vals.get('currency_id')) + currency = move.currency_id partner = self.env['res.partner'].browse(vals.get('partner_id')) taxes = self.resolve_2many_commands('tax_ids', vals.get('tax_ids', []), fields=['id']) tax_ids = set(tax['id'] for tax in taxes) @@ -3170,13 +3180,24 @@ class AccountMoveLine(models.Model): balance = vals.get('amount_currency', 0.0) else: balance = vals.get('debit', 0.0) - vals.get('credit', 0.0) + price_subtotal = self._get_price_total_and_subtotal_model( + vals.get('price_unit', 0.0), + vals.get('quantity', 0.0), + vals.get('discount', 0.0), + currency, + self.env['product.product'].browse(vals.get('product_id')), + partner, + taxes, + move.type, + ).get('price_subtotal', 0.0) vals.update(self._get_fields_onchange_balance_model( vals.get('quantity', 0.0), vals.get('discount', 0.0), balance, move.type, currency, - taxes + taxes, + price_subtotal )) vals.update(self._get_price_total_and_subtotal_model( vals.get('price_unit', 0.0), @@ -3356,9 +3377,11 @@ class AccountMoveLine(models.Model): # in onchange and in create/write. So, if something changed in accounting [resp. business] fields, # business [resp. accounting] fields are recomputed. if any(field in vals for field in ACCOUNTING_FIELDS): - price_subtotal = line.currency_id and line.amount_currency or line.debit - line.credit + balance = line.currency_id and line.amount_currency or line.debit - line.credit + price_subtotal = line._get_price_total_and_subtotal().get('price_subtotal', 0.0) to_write = line._get_fields_onchange_balance( - balance=price_subtotal, + balance=balance, + price_subtotal=price_subtotal, ) to_write.update(line._get_price_total_and_subtotal( price_unit=to_write.get('price_unit', line.price_unit), diff --git a/addons/account/tests/test_account_move_in_invoice.py b/addons/account/tests/test_account_move_in_invoice.py index 2a4539ffc0a..1a0f8d27a5f 100644 --- a/addons/account/tests/test_account_move_in_invoice.py +++ b/addons/account/tests/test_account_move_in_invoice.py @@ -1062,7 +1062,7 @@ class TestAccountMoveInInvoiceOnchanges(AccountTestInvoicingCommon): { **self.product_line_vals_1, 'quantity': 0.1, - 'price_unit': 0.1, + 'price_unit': 0.05, 'price_subtotal': 0.01, 'price_total': 0.01, 'debit': 0.01, diff --git a/addons/account/tests/test_account_move_in_refund.py b/addons/account/tests/test_account_move_in_refund.py index 22751e93fdd..8bb1c5b8335 100644 --- a/addons/account/tests/test_account_move_in_refund.py +++ b/addons/account/tests/test_account_move_in_refund.py @@ -774,7 +774,7 @@ class TestAccountMoveInRefundOnchanges(AccountTestInvoicingCommon): { **self.product_line_vals_1, 'quantity': 0.1, - 'price_unit': 0.1, + 'price_unit': 0.05, 'price_subtotal': 0.01, 'price_total': 0.01, 'credit': 0.01, diff --git a/addons/account/tests/test_account_move_out_invoice.py b/addons/account/tests/test_account_move_out_invoice.py index bd3a717185e..34277fa494c 100644 --- a/addons/account/tests/test_account_move_out_invoice.py +++ b/addons/account/tests/test_account_move_out_invoice.py @@ -810,6 +810,271 @@ class TestAccountMoveOutInvoiceOnchanges(AccountTestInvoicingCommon): 'amount_total': 1730.0, }) + def test_out_invoice_line_onchange_taxes_2_price_unit_tax_included(self): + ''' Seek for rounding issue in the price unit. Suppose a price_unit of 2300 with a 5.5% price-included tax + applied on it. + + The computed balance will be computed as follow: 2300.0 / 1.055 = 2180.0948 ~ 2180.09. + Since accounting / business fields are synchronized, the inverse computation will try to recompute the + price_unit based on the balance: 2180.09 * 1.055 = 2299.99495 ~ 2299.99. + + This test ensures the price_unit is not overridden in such case. + ''' + tax_price_include = self.env['account.tax'].create({ + 'name': 'Tax 5.5% price included', + 'amount': 5.5, + 'amount_type': 'percent', + 'price_include': True, + }) + + # == Single-currency == + + # price_unit=2300 with 15% tax (excluded) + 5.5% tax (included). + move_form = Form(self.invoice) + move_form.invoice_line_ids.remove(1) + with move_form.invoice_line_ids.edit(0) as line_form: + line_form.price_unit = 2300 + line_form.tax_ids.add(tax_price_include) + move_form.save() + + self.assertInvoiceValues(self.invoice, [ + { + **self.product_line_vals_1, + 'price_unit': 2300.0, + 'price_subtotal': 2180.09, + 'price_total': 2627.01, + 'tax_ids': (self.product_a.taxes_id + tax_price_include).ids, + 'credit': 2180.09, + }, + { + **self.tax_line_vals_1, + 'price_unit': 327.01, + 'price_subtotal': 327.01, + 'price_total': 327.01, + 'credit': 327.01, + }, + { + 'name': tax_price_include.name, + 'product_id': False, + 'account_id': self.product_line_vals_1['account_id'], + 'partner_id': self.partner_a.id, + 'product_uom_id': False, + 'quantity': 1.0, + 'discount': 0.0, + 'price_unit': 119.91, + 'price_subtotal': 119.91, + 'price_total': 119.91, + 'tax_ids': [], + 'tax_line_id': tax_price_include.id, + 'currency_id': False, + 'amount_currency': 0.0, + 'debit': 0.0, + 'credit': 119.91, + 'date_maturity': False, + 'tax_exigible': True, + }, + { + **self.term_line_vals_1, + 'price_unit': -2627.01, + 'price_subtotal': -2627.01, + 'price_total': -2627.01, + 'debit': 2627.01, + }, + ], { + **self.move_vals, + 'amount_untaxed': 2180.09, + 'amount_tax': 446.92, + 'amount_total': 2627.01, + }) + + move_form = Form(self.invoice) + with move_form.invoice_line_ids.edit(0) as line_form: + line_form.price_unit = -2300 + move_form.save() + + self.assertInvoiceValues(self.invoice, [ + { + **self.product_line_vals_1, + 'price_unit': -2300.0, + 'price_subtotal': -2180.09, + 'price_total': -2627.01, + 'tax_ids': (self.product_a.taxes_id + tax_price_include).ids, + 'debit': 2180.09, + 'credit': 0.0, + }, + { + **self.tax_line_vals_1, + 'price_unit': -327.01, + 'price_subtotal': -327.01, + 'price_total': -327.01, + 'debit': 327.01, + 'credit': 0.0, + }, + { + 'name': tax_price_include.name, + 'product_id': False, + 'account_id': self.product_line_vals_1['account_id'], + 'partner_id': self.partner_a.id, + 'product_uom_id': False, + 'quantity': 1.0, + 'discount': 0.0, + 'price_unit': -119.91, + 'price_subtotal': -119.91, + 'price_total': -119.91, + 'tax_ids': [], + 'tax_line_id': tax_price_include.id, + 'currency_id': False, + 'amount_currency': 0.0, + 'debit': 119.91, + 'credit': 0.0, + 'date_maturity': False, + 'tax_exigible': True, + }, + { + **self.term_line_vals_1, + 'price_unit': 2627.01, + 'price_subtotal': 2627.01, + 'price_total': 2627.01, + 'debit': 0.0, + 'credit': 2627.01, + }, + ], { + **self.move_vals, + 'amount_untaxed': -2180.09, + 'amount_tax': -446.92, + 'amount_total': -2627.01, + }) + + # == Multi-currencies == + + move_form = Form(self.invoice) + move_form.currency_id = self.currency_data['currency'] + with move_form.invoice_line_ids.edit(0) as line_form: + line_form.price_unit = 2300 + move_form.save() + + self.assertInvoiceValues(self.invoice, [ + { + **self.product_line_vals_1, + 'price_unit': 2300.0, + 'price_subtotal': 2180.095, + 'price_total': 2627.014, + 'tax_ids': (self.product_a.taxes_id + tax_price_include).ids, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': -2180.095, + 'credit': 1090.05, + }, + { + **self.tax_line_vals_1, + 'price_unit': 327.014, + 'price_subtotal': 327.014, + 'price_total': 327.014, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': -327.014, + 'credit': 163.51, + }, + { + 'name': tax_price_include.name, + 'product_id': False, + 'account_id': self.product_line_vals_1['account_id'], + 'partner_id': self.partner_a.id, + 'product_uom_id': False, + 'quantity': 1.0, + 'discount': 0.0, + 'price_unit': 119.905, + 'price_subtotal': 119.905, + 'price_total': 119.905, + 'tax_ids': [], + 'tax_line_id': tax_price_include.id, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': -119.905, + 'debit': 0.0, + 'credit': 59.95, + 'date_maturity': False, + 'tax_exigible': True, + }, + { + **self.term_line_vals_1, + 'price_unit': -2627.014, + 'price_subtotal': -2627.014, + 'price_total': -2627.014, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': 2627.014, + 'debit': 1313.51, + }, + ], { + **self.move_vals, + 'currency_id': self.currency_data['currency'].id, + 'amount_untaxed': 2180.095, + 'amount_tax': 446.919, + 'amount_total': 2627.014, + }) + + move_form = Form(self.invoice) + with move_form.invoice_line_ids.edit(0) as line_form: + line_form.price_unit = -2300 + move_form.save() + + self.assertInvoiceValues(self.invoice, [ + { + **self.product_line_vals_1, + 'price_unit': -2300.0, + 'price_subtotal': -2180.095, + 'price_total': -2627.014, + 'tax_ids': (self.product_a.taxes_id + tax_price_include).ids, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': 2180.095, + 'debit': 1090.05, + 'credit': 0.0, + }, + { + **self.tax_line_vals_1, + 'price_unit': -327.014, + 'price_subtotal': -327.014, + 'price_total': -327.014, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': 327.014, + 'debit': 163.51, + 'credit': 0.0, + }, + { + 'name': tax_price_include.name, + 'product_id': False, + 'account_id': self.product_line_vals_1['account_id'], + 'partner_id': self.partner_a.id, + 'product_uom_id': False, + 'quantity': 1.0, + 'discount': 0.0, + 'price_unit': -119.905, + 'price_subtotal': -119.905, + 'price_total': -119.905, + 'tax_ids': [], + 'tax_line_id': tax_price_include.id, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': 119.905, + 'debit': 59.95, + 'credit': 0.0, + 'date_maturity': False, + 'tax_exigible': True, + }, + { + **self.term_line_vals_1, + 'price_unit': 2627.014, + 'price_subtotal': 2627.014, + 'price_total': 2627.014, + 'currency_id': self.currency_data['currency'].id, + 'amount_currency': -2627.014, + 'debit': 0.0, + 'credit': 1313.51, + }, + ], { + **self.move_vals, + 'currency_id': self.currency_data['currency'].id, + 'amount_untaxed': -2180.095, + 'amount_tax': -446.919, + 'amount_total': -2627.014, + }) + def test_out_invoice_line_onchange_analytic(self): self.env.user.groups_id += self.env.ref('analytic.group_analytic_accounting') self.env.user.groups_id += self.env.ref('analytic.group_analytic_tags') @@ -1206,7 +1471,7 @@ class TestAccountMoveOutInvoiceOnchanges(AccountTestInvoicingCommon): { **self.product_line_vals_1, 'quantity': 0.1, - 'price_unit': 0.1, + 'price_unit': 0.05, 'price_subtotal': 0.01, 'price_total': 0.01, 'credit': 0.01, diff --git a/addons/account/tests/test_account_move_out_refund.py b/addons/account/tests/test_account_move_out_refund.py index fcee11fa0a2..05892ef4184 100644 --- a/addons/account/tests/test_account_move_out_refund.py +++ b/addons/account/tests/test_account_move_out_refund.py @@ -774,7 +774,7 @@ class TestAccountMoveOutRefundOnchanges(AccountTestInvoicingCommon): { **self.product_line_vals_1, 'quantity': 0.1, - 'price_unit': 0.1, + 'price_unit': 0.05, 'price_subtotal': 0.01, 'price_total': 0.01, 'debit': 0.01,