From de69aab5febb5824e5b99e561b547e53bff644d9 Mon Sep 17 00:00:00 2001 From: "Dylan Kiss (dyki)" Date: Tue, 12 Dec 2023 08:50:23 +0000 Subject: [PATCH] [FIX] point_of_sale: invoice refunds after session closing How to reproduce: - Open a PoS session - Create an order, pay it and validate - In the same session, find the order in `Orders` (filter on Paid) and make a full refund - Close the session - Open a new session and open the "refund" order in Orders - Click the "Invoice" button to invoice this order Observed behavior: - We get a UserError saying the reversal entry is unbalanced, where the credit side equals 0.00 and the credit note is not generated. Expected behavior: - We are able to generate the credit note correctly. When computing the reversal entry, it seems we inverted the sign of the quantity field of each order line when reversing a refund. However, that did not make sense because the actual amounts are already reversed for a refund. task-3611296 closes odoo/odoo#147934 X-original-commit: a6b10b65e3a04ce96519edc846801304cc47f40b Signed-off-by: Laurent Smet (las) Signed-off-by: Dylan Kiss (dyki) --- addons/point_of_sale/models/pos_order.py | 8 +- addons/point_of_sale/tests/common.py | 8 +- .../tests/test_pos_basic_config.py | 139 ++++++++++++++++++ 3 files changed, 151 insertions(+), 4 deletions(-) diff --git a/addons/point_of_sale/models/pos_order.py b/addons/point_of_sale/models/pos_order.py index 9683b0d4ea8..37dd9b43acc 100644 --- a/addons/point_of_sale/models/pos_order.py +++ b/addons/point_of_sale/models/pos_order.py @@ -700,7 +700,7 @@ class PosOrder(models.Model): rate = self.currency_id._get_conversion_rate(self.currency_id, company_currency, self.company_id, self.date_order) # Concert each order line to a dictionary containing business values. Also, prepare for taxes computation. - base_line_vals_list = self._prepare_tax_base_line_values(sign=sign) + base_line_vals_list = self._prepare_tax_base_line_values(sign=-1) tax_results = self.env['account.tax']._compute_taxes(base_line_vals_list) total_balance = 0.0 @@ -843,7 +843,11 @@ class PosOrder(models.Model): move_lines.append(aml_values) # Make a move with all the lines. - reversal_entry = self.env['account.move'].with_context(default_journal_id=self.config_id.journal_id.id).create({ + reversal_entry = self.env['account.move'].with_context( + default_journal_id=self.config_id.journal_id.id, + skip_invoice_sync=True, + skip_invoice_line_sync=True, + ).create({ 'journal_id': self.config_id.journal_id.id, 'date': fields.Date.context_today(self), 'ref': _('Reversal of POS closing entry %s for order %s from session %s', self.session_move_id.name, self.name, self.session_id.name), diff --git a/addons/point_of_sale/tests/common.py b/addons/point_of_sale/tests/common.py index 9a5b39580a4..d84f99f58d1 100644 --- a/addons/point_of_sale/tests/common.py +++ b/addons/point_of_sale/tests/common.py @@ -378,13 +378,13 @@ class TestPoSCommon(ValuationReconciliationTestCommon): cls.tax_tag_refund_base = create_tag('Refund Base tag') cls.tax_tag_refund_tax = create_tag('Refund Tax tag') - def create_tax(percentage, price_include=False): + def create_tax(percentage, price_include=False, include_base_amount=False): return cls.env['account.tax'].create({ 'name': f'Tax {percentage}%', 'amount': percentage, 'price_include': price_include, 'amount_type': 'percent', - 'include_base_amount': False, + 'include_base_amount': include_base_amount, 'invoice_repartition_line_ids': [ (0, 0, { 'repartition_type': 'base', @@ -442,6 +442,8 @@ class TestPoSCommon(ValuationReconciliationTestCommon): tax_fixed006 = create_tax_fixed(0.06, price_include=True) tax_fixed012 = create_tax_fixed(0.12, price_include=True) tax7 = create_tax(7, price_include=False) + tax8 = create_tax(8, include_base_amount=True) + tax9 = create_tax(9) tax10 = create_tax(10, price_include=True) tax21 = create_tax(21, price_include=True) @@ -455,6 +457,8 @@ class TestPoSCommon(ValuationReconciliationTestCommon): return { 'tax7': tax7, + 'tax8': tax8, + 'tax9': tax9, 'tax10': tax10, 'tax21': tax21, 'tax_fixed006': tax_fixed006, diff --git a/addons/point_of_sale/tests/test_pos_basic_config.py b/addons/point_of_sale/tests/test_pos_basic_config.py index 6759cdafcf6..50f8284506a 100644 --- a/addons/point_of_sale/tests/test_pos_basic_config.py +++ b/addons/point_of_sale/tests/test_pos_basic_config.py @@ -26,6 +26,7 @@ class TestPoSBasicConfig(TestPoSCommon): self.product3 = self.create_product('Product 3', self.categ_basic, 30.0, 15) self.product4 = self.create_product('Product_4', self.categ_basic, 9.96, 4.98) self.product99 = self.create_product('Product_99', self.categ_basic, 99, 50) + self.product_multi_tax = self.create_product('Multi-tax product', self.categ_basic, 100, 100, (self.taxes['tax8'] | self.taxes['tax9']).ids) self.adjust_inventory([self.product1, self.product2, self.product3], [100, 50, 50]) def test_orders_no_invoiced(self): @@ -893,6 +894,93 @@ class TestPoSBasicConfig(TestPoSCommon): # calling load_pos_data should not raise an error self.pos_session.load_pos_data() + def test_invoice_past_refund(self): + """ Test invoicing a past refund + + Orders + ====== + +------------------+----------+-----------+----------+-----+-------+ + | order | payments | invoiced? | product | qty | total | + +------------------+----------+-----------+----------+-----+-------+ + | order 1 | cash | no | product3 | 1 | 30 | + +------------------+----------+-----------+----------+-----+-------+ + | order 2 (return) | cash | no | product3 | -1 | -30 | + +------------------+----------+-----------+----------+-----+-------+ + + Expected Result + =============== + +---------------------+---------+ + | account | balance | + +---------------------+---------+ + | sale (sales) | -30 | + | sale (refund) | 30 | + +---------------------+---------+ + | Total balance | 0.0 | + +---------------------+---------+ + """ + def _before_closing_cb(): + # Return the order + order_to_return = self.pos_session.order_ids.filtered(lambda order: '12345-123-1234' in order.pos_reference) + order_to_return.refund() + refund_order = self.pos_session.order_ids.filtered(lambda order: order.state == 'draft') + + # Check if there's an amount to pay + self.assertAlmostEqual(refund_order.amount_total - refund_order.amount_paid, -30) + + # Pay the refund + context_make_payment = {"active_ids": [refund_order.id], "active_id": refund_order.id} + make_payment = self.env['pos.make.payment'].with_context(context_make_payment).create({ + 'payment_method_id': self.cash_pm1.id, + 'amount': -30, + }) + make_payment.check() + + self._run_test({ + 'payment_methods': self.cash_pm1 | self.bank_pm1, + 'orders': [ + {'pos_order_lines_ui_args': [(self.product3, 1)], 'payments': [(self.cash_pm1, 30)], 'uid': '12345-123-1234'}, + ], + 'before_closing_cb': _before_closing_cb, + 'journal_entries_before_closing': {}, + 'journal_entries_after_closing': { + 'session_journal_entry': { + 'line_ids': [ + {'account_id': self.sales_account.id, 'partner_id': False, 'debit': 0, 'credit': 30, 'reconciled': False}, + {'account_id': self.sales_account.id, 'partner_id': False, 'debit': 30, 'credit': 0, 'reconciled': False}, + ], + }, + 'cash_statement': [], + 'bank_payments': [], + }, + }) + + closed_session = self.pos_session + self.assertTrue(closed_session.state == 'closed', 'Session should be closed.') + + return_to_invoice = closed_session.order_ids[1] + test_customer = self.env['res.partner'].create({'name': 'Test Customer'}) + new_session_date = return_to_invoice.date_order + relativedelta(days=2) + + with freeze_time(new_session_date): + # Create a new session after 2 days + self.open_new_session(0) + # Invoice the uninvoiced refund + return_to_invoice.write({'partner_id': test_customer.id}) + return_to_invoice.action_pos_order_invoice() + # Check the credit note + self.assertTrue(return_to_invoice.account_move, 'Invoice should be created.') + self.assertEqual(return_to_invoice.account_move.move_type, 'out_refund', 'Invoice should be a credit note.') + self.assertEqual(return_to_invoice.account_move.invoice_date, new_session_date, 'Invoice date should be the same as the session it is created in.') + self.assertRecordValues(return_to_invoice.account_move, [{ + 'amount_untaxed': 30, + 'amount_tax': 0, + 'amount_total': 30, + }]) + self.assertRecordValues(return_to_invoice.account_move.line_ids, [ + {'account_id': self.sales_account.id, 'balance': 30}, + {'account_id': self.receivable_account.id, 'balance': -30}, + ]) + def test_invoice_past_order(self): # create 1 uninvoiced order then close the session self._run_test({ @@ -942,3 +1030,54 @@ class TestPoSBasicConfig(TestPoSCommon): # is the real payment date and not to the invoice_date payment = invoice.line_ids.full_reconcile_id.reconciled_line_ids.move_id - invoice self.assertEqual(payment.date, order_to_invoice.date_order.date()) + + def test_invoice_past_order_affecting_taxes(self): + """ Test whether two taxes affecting each other don't trigger a recomputation on invoice generation + """ + # Create 1 uninvoiced order then close the session + self._run_test({ + 'payment_methods': self.cash_pm1 | self.bank_pm1, + 'orders': [ + {'pos_order_lines_ui_args': [(self.product_multi_tax, 1)], 'payments': [(self.bank_pm1, 117.72)], 'customer': False, 'is_invoiced': False, 'uid': '00100-010-0001'}, + ], + 'journal_entries_before_closing': {}, + 'journal_entries_after_closing': { + 'session_journal_entry': { + 'line_ids': [ + {'account_id': self.tax_received_account.id, 'partner_id': False, 'debit': 0, 'credit': 8, 'reconciled': False}, + {'account_id': self.tax_received_account.id, 'partner_id': False, 'debit': 0, 'credit': 9.72, 'reconciled': False}, + {'account_id': self.sales_account.id, 'partner_id': False, 'debit': 0, 'credit': 100, 'reconciled': False}, + {'account_id': self.bank_pm1.receivable_account_id.id, 'partner_id': False, 'debit': 117.72, 'credit': 0, 'reconciled': True}, + ], + }, + 'cash_statement': [], + 'bank_payments': [ + ((117.72, ), { + 'line_ids': [ + {'account_id': self.bank_pm1.outstanding_account_id.id, 'partner_id': False, 'debit': 117.72, 'credit': 0, 'reconciled': False}, + {'account_id': self.bank_pm1.receivable_account_id.id, 'partner_id': False, 'debit': 0, 'credit': 117.72, 'reconciled': True}, + ] + }) + ], + }, + }) + + closed_session = self.pos_session + self.assertTrue(closed_session.state == 'closed', 'Session should be closed.') + + order_to_invoice = closed_session.order_ids[0] + test_customer = self.env['res.partner'].create({'name': 'Test Customer'}) + + # Create a new session + self.open_new_session(0) + # Invoice the uninvoiced order + order_to_invoice.write({'partner_id': test_customer.id}) + order_to_invoice.action_pos_order_invoice() + # Check the invoice for the lines + self.assertTrue(order_to_invoice.account_move, 'Invoice should be created.') + self.assertRecordValues(order_to_invoice.account_move.line_ids, [ + {'account_id': self.sales_account.id, 'balance': -100, 'reconciled': False}, + {'account_id': self.tax_received_account.id, 'balance': -8, 'reconciled': False}, + {'account_id': self.tax_received_account.id, 'balance': -9.72, 'reconciled': False}, + {'account_id': self.receivable_account.id, 'balance': 117.72, 'reconciled': True}, + ])