[FIX] account,stock_account: Fix bad bug fixes

https://github.com/odoo/odoo/commit/5309c252f6d0bc4994265a4c39259b70e7507fb1

This fix isn't working when using the 'modify' refund_method.
Also, you can't change the behavior of the whole invoices when there is only one move dealing with the auto_post feature.

https://github.com/odoo/odoo/commit/c9cd83e538a0be9d626d7587c465924146327cb2

This fix is only fixing the reversal wizard but must be done everytime a journal entry is duplicated.

closes odoo/odoo#56904

X-original-commit: 3057ee6cdd63c0adb5037041492427268b5e7549
Signed-off-by: oco-odoo <oco-odoo@users.noreply.github.com>
Signed-off-by: Laurent Smet <smetl@users.noreply.github.com>

addons/account/models/account_move.py

@@ -2231,7 +2231,7 @@ class AccountMove(models.Model):
                 'move_type': reverse_type_map[move.move_type],
                 'reversed_entry_id': move.id,
             })
-            move_vals_list.append(move._reverse_move_vals(default_values, cancel=cancel))
+            move_vals_list.append(move.with_context(move_reverse_cancel=cancel)._reverse_move_vals(default_values, cancel=cancel))
 
         reverse_moves = self.env['account.move'].create(move_vals_list)
         for move, reverse_move in zip(self, reverse_moves.with_context(check_move_validity=False)):
@@ -4477,6 +4477,7 @@ class AccountMoveLine(models.Model):
 
     def copy_data(self, default=None):
         res = super(AccountMoveLine, self).copy_data(default=default)
+
         for line, values in zip(self, res):
             # Don't copy the name of a payment term line.
             if line.move_id.is_invoice() and line.account_id.user_type_id.type in ('receivable', 'payable'):

addons/account/tests/test_account_move_out_invoice.py

@@ -6,6 +6,7 @@ from odoo import fields
 from odoo.exceptions import UserError
 
 from unittest.mock import patch
+from datetime import timedelta
 
 
 @tagged('post_install', '-at_install')
@@ -1840,6 +1841,22 @@ class TestAccountMoveOutInvoiceOnchanges(AccountTestInvoicingCommon):
             'payment_state': 'paid',
         })
 
+    def test_out_invoice_create_refund_auto_post(self):
+        self.invoice.action_post()
+
+        move_reversal = self.env['account.move.reversal'].with_context(active_model="account.move", active_ids=self.invoice.ids).create({
+            'date': fields.Date.today() + timedelta(days=7),
+            'reason': 'no reason',
+            'refund_method': 'modify',
+        })
+        move_reversal.reverse_moves()
+        refund = self.env['account.move'].search([('move_type', '=', 'out_refund'), ('company_id', '=', self.invoice.company_id.id)])
+
+        self.assertRecordValues(refund, [{
+            'state': 'draft',
+            'auto_post': True,
+        }])
+
     def test_out_invoice_create_1(self):
         # Test creating an account_move with the least information.
         move = self.env['account.move'].create({

addons/account/tests/test_reconciliation.py

@@ -631,75 +631,6 @@ class TestReconciliationExec(TestAccountReconciliationCommon):
 
         self.assertTrue(receivable_line.matched_debit_ids)
 
-    def test_reconciliation_revert_future(self):
-        company = self.company
-        company.tax_cash_basis_journal_id = self.cash_basis_journal
-        tax_cash_basis10percent = self.tax_cash_basis.copy({'amount': 10})
-        self.tax_waiting_account.reconcile = True
-        tax_waiting_account10 = self.tax_waiting_account.copy({
-            'name': 'TAX WAIT 10',
-            'code': 'TWAIT1',
-        })
-
-        AccountMoveLine = self.env['account.move.line'].with_context(check_move_validity=False)
-
-        # Purchase
-        purchase_move = self.env['account.move'].create({
-            'name': 'invoice',
-            'journal_id': self.purchase_journal.id,
-        })
-
-        purchase_payable_line0 = AccountMoveLine.create({
-            'account_id': self.account_rsa.id,
-            'credit': 175,
-            'move_id': purchase_move.id,
-        })
-
-        AccountMoveLine.create({
-            'name': 'expenseTaxed 10%',
-            'account_id': self.expense_account.id,
-            'debit': 50,
-            'move_id': purchase_move.id,
-            'tax_ids': [(4, tax_cash_basis10percent.id, False)],
-        })
-        tax_line0 = AccountMoveLine.create({
-            'name': 'TaxLine0',
-            'account_id': tax_waiting_account10.id,
-            'debit': 5,
-            'move_id': purchase_move.id,
-            'tax_repartition_line_id': tax_cash_basis10percent.invoice_repartition_line_ids.filtered(lambda x: x.repartition_type == 'tax').id,
-            'tax_base_amount': 50,
-        })
-        AccountMoveLine.create({
-            'name': 'expenseTaxed 20%',
-            'account_id': self.expense_account.id,
-            'debit': 100,
-            'move_id': purchase_move.id,
-            'tax_ids': [(4, self.tax_cash_basis.id, False)],
-        })
-        tax_line1 = AccountMoveLine.create({
-            'name': 'TaxLine1',
-            'account_id': self.tax_waiting_account.id,
-            'debit': 20,
-            'move_id': purchase_move.id,
-            'tax_repartition_line_id': self.tax_cash_basis.invoice_repartition_line_ids.filtered(lambda x: x.repartition_type == 'tax').id,
-            'tax_base_amount': 100,
-        })
-        purchase_move.action_post()
-
-        reverse_date = fields.Date.context_today(purchase_move) + timedelta(days=+7)
-        revertWidget = self.env['account.move.reversal'].create({
-            'move_ids': [(6, 0, [purchase_move.id])],
-            'date': reverse_date,
-            'reason': ['cancel'],
-        })
-        action = revertWidget.reverse_moves();
-        reverse_move = self.env['account.move'].search([('reversed_entry_id', '=', purchase_move.id)])
-        self.assertEqual(len(reverse_move), 1)
-        self.assertEqual(reverse_move.state, 'draft')
-        self.assertEqual(reverse_move.auto_post, True)
-        self.assertEqual(reverse_move.date, reverse_date)
-
     def test_reconciliation_with_currency(self):
         #reconciliation on an account having a foreign currency being
         #the same as the company one
@@ -829,7 +760,6 @@ class TestReconciliationExec(TestAccountReconciliationCommon):
                         |   202.59 (225.00)  INV 2  > Done in foreign
                         |   372.10 (413.25)  PAYMENT > Done in domestic (the 413.25 is virtual, non stored)
                         |    17.78  (19.75)  WriteOff > Done in domestic (the 19.75 is virtual, non stored)
-
         Reconciliation should be full
         Invoices should be marked as paid
         """
@@ -898,7 +828,6 @@ class TestReconciliationExec(TestAccountReconciliationCommon):
                         |   202.59 (225.00)  INV 2  > Done in foreign
                         |   372.10 (413.25)  PAYMENT > Done in domestic (the 413.25 is virtual, non stored)
                         |    83.31  (92.52)  WriteOff > Done in domestic (the 92.52 is virtual, non stored)
-
         Reconciliation should be full
         Invoices should be marked as paid
         """
@@ -976,7 +905,6 @@ class TestReconciliationExec(TestAccountReconciliationCommon):
                         |   250.00 (250.00)  INV 2  > Done in foreign
                         |   314.07 (314.07)  PAYMENT > Done in domestic (foreign non stored)
                         |    35.93  (60.93)  WriteOff > Done in domestic (foreign non stored). WriteOff is included in payment
-
         Reconciliation should be full, without exchange difference
         Invoices should be marked as paid
         """
@@ -1173,7 +1101,6 @@ class TestReconciliationExec(TestAccountReconciliationCommon):
                           |   5384.43 (5979.95)  PAYMENT > Done in domestic (foreign non stored)
                           |      0.05    (0.00)  WriteOff > Done in domestic (foreign non stored). WriteOff is included in payment,
                                                                 so, the amount in currency is irrelevant
-
         Reconciliation should be full, without exchange difference
         Invoices should be marked as paid
         """
@@ -1278,4 +1205,4 @@ class TestReconciliationExec(TestAccountReconciliationCommon):
         exchange_rcv = inv1_receivable.full_reconcile_id.exchange_move_id.line_ids.filtered(lambda l: l.account_id.internal_type == 'receivable')
         self.assertEqual(exchange_rcv.amount_currency, 0.01)
 
-        self.assertTrue(inv1.payment_state in ('in_payment', 'paid'), "Invoice should be paid")
+        self.assertTrue(inv1.payment_state in ('in_payment', 'paid'), "Invoice should be paid")

addons/account/wizard/account_move_reversal.py

@@ -71,9 +71,6 @@ class AccountMoveReversal(models.TransientModel):
             'auto_post': True if reverse_date > fields.Date.context_today(self) else False,
         }
 
-    def _reverse_moves_post_hook(self, moves):
-        return
-
     def reverse_moves(self):
         self.ensure_one()
         moves = self.move_ids
@@ -83,28 +80,31 @@ class AccountMoveReversal(models.TransientModel):
         for move in moves:
             default_values_list.append(self._prepare_default_reversal(move))
 
-        # Handle reverse method.
-        if self.refund_method == 'cancel':
-            if any(vals.get('auto_post', False) for vals in default_values_list):
-                new_moves = moves._reverse_moves(default_values_list)
-            else:
-                new_moves = moves._reverse_moves(default_values_list, cancel=True)
-        elif self.refund_method == 'modify':
-            moves._reverse_moves(default_values_list, cancel=True)
-            moves_vals_list = []
-            for move in moves.with_context(include_business_fields=True):
-                moves_vals_list.append(move.copy_data({
-                    'date': self.date if self.date_mode == 'custom' else move.date,
-                })[0])
-            new_moves = self.env['account.move'].create(moves_vals_list)
-        elif self.refund_method == 'refund':
-            new_moves = moves._reverse_moves(default_values_list)
-        else:
-            return
+        batches = [
+            [self.env['account.move'], [], True],   # Moves to be cancelled by the reverses.
+            [self.env['account.move'], [], False],  # Others.
+        ]
+        for move, default_vals in zip(moves, default_values_list):
+            is_auto_post = bool(default_vals.get('auto_post'))
+            is_cancel_needed = not is_auto_post and self.refund_method in ('cancel', 'modify')
+            batch_index = 0 if is_cancel_needed else 1
+            batches[batch_index][0] |= move
+            batches[batch_index][1].append(default_vals)
 
-        self.new_move_ids = new_moves
-        if new_moves:
-            self._reverse_moves_post_hook(new_moves)
+        # Handle reverse method.
+        moves_to_redirect = self.env['account.move']
+        for moves, default_values_list, is_cancel_needed in batches:
+            new_moves = moves._reverse_moves(default_values_list, cancel=is_cancel_needed)
+
+            if self.refund_method == 'modify':
+                moves_vals_list = []
+                for move in moves.with_context(include_business_fields=True):
+                    moves_vals_list.append(move.copy_data({'date': self.date if self.date_mode == 'custom' else move.date})[0])
+                new_moves = self.env['account.move'].create(moves_vals_list)
+
+            moves_to_redirect |= new_moves
+
+        self.new_move_ids = moves_to_redirect
 
         # Create action.
         action = {
@@ -112,14 +112,14 @@ class AccountMoveReversal(models.TransientModel):
             'type': 'ir.actions.act_window',
             'res_model': 'account.move',
         }
-        if len(new_moves) == 1:
+        if len(moves_to_redirect) == 1:
             action.update({
                 'view_mode': 'form',
-                'res_id': new_moves.id,
+                'res_id': moves_to_redirect.id,
             })
         else:
             action.update({
                 'view_mode': 'tree,form',
-                'domain': [('id', 'in', new_moves.ids)],
+                'domain': [('id', 'in', moves_to_redirect.ids)],
             })
         return action

addons/stock_account/models/account_move.py

@@ -25,6 +25,19 @@ class AccountMove(models.Model):
             move_vals['line_ids'] = [vals for vals in move_vals['line_ids'] if not vals[2]['is_anglo_saxon_line']]
         return move_vals
 
+    def copy_data(self, default=None):
+        # OVERRIDE
+        # Don't keep anglo-saxon lines when copying a journal entry.
+        res = super().copy_data(default=default)
+
+        if not self._context.get('move_reverse_cancel'):
+            for copy_vals in res:
+                if 'line_ids' in copy_vals:
+                    copy_vals['line_ids'] = [line_vals for line_vals in copy_vals['line_ids']
+                                             if line_vals[0] != 0 or not line_vals[2]['is_anglo_saxon_line']]
+
+        return res
+
     def _post(self, soft=True):
         # OVERRIDE
 

addons/stock_account/wizard/__init__.py

@@ -1,7 +1,6 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from . import account_move_reversal
 from . import stock_quantity_history
 from . import stock_picking_return
 from . import stock_valuation_layer_revaluation

addons/stock_account/wizard/account_move_reversal.py

@@ -1,11 +0,0 @@
-# -*- coding: utf-8 -*-
-from odoo import models
-
-
-class AccountMoveReversal(models.TransientModel):
-    _inherit = "account.move.reversal"
-
-    def _reverse_moves_post_hook(self, moves):
-        if self.refund_method == "modify":
-            moves.mapped("line_ids").filtered(lambda line: line.is_anglo_saxon_line).unlink()
-        return super()._reverse_moves_post_hook(moves)