From 668e9a4f0f8ae15e32b0777d3a474edaf051ba77 Mon Sep 17 00:00:00 2001
From: Laurent Stukkens <laurent.stukkens@acsone.eu>
Date: Sat, 4 Nov 2023 13:10:48 +0100
Subject: [PATCH] [IMP] account_reversal: add reversal_id field

This commit adds the `reversal_id` field in order to improve
the UX of the move reversal functionality. The field has been
added to the form view close to the `reversed_entry_id` field
already present in the view.

This commit also introduces a constraint preventing the user
to flag a move as to be reversed when a non cancelled reversal
move has already been created. This allows simplifying the domain
used on the `to_be_reversed` filter.

Finally, it also convert the deprecated `SavepointCase` test
into a `TransactionCase` one.

Fixes: #1761
---
 account_reversal/models/account_move.py       | 38 ++++++++++++++++++-
 .../tests/test_account_reversal.py            | 23 ++++++++++-
 account_reversal/views/account_move_view.xml  |  6 ++-
 3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/account_reversal/models/account_move.py b/account_reversal/models/account_move.py
index 598de82a31f..5cbdf5ea04f 100644
--- a/account_reversal/models/account_move.py
+++ b/account_reversal/models/account_move.py
@@ -4,7 +4,12 @@
 # Copyright 2016 Antonio Espinosa <antonio.espinosa@tecnativa.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
-from odoo import fields, models
+from odoo import _, api, fields, models
+from odoo.exceptions import ValidationError
+
+
+class MoveAlreadyReversedValidationError(ValidationError):
+    pass
 
 
 class AccountMove(models.Model):
@@ -12,9 +17,38 @@ class AccountMove(models.Model):
 
     to_be_reversed = fields.Boolean(
         copy=False,
-        help="Check this box if your entry has to be reversed at the end " "of period.",
+        help="Check this box if your entry has to be reversed at the end of period.",
+    )
+    reversal_id = fields.Many2one(
+        "account.move",
+        compute="_compute_reversal_id",
+        string="Reversal Entry",
+        readonly=True,
     )
 
+    @api.depends("reversal_move_id")
+    def _compute_reversal_id(self):
+        for move in self:
+            move.reversal_id = move._get_reversal_id()
+
+    @api.constrains("to_be_reversed", "reversal_move_id")
+    def _check_to_be_reversed(self):
+        for move in self:
+            if move.to_be_reversed and move._get_reversal_id():
+                raise MoveAlreadyReversedValidationError(
+                    _(
+                        "The move has already been reversed, "
+                        "so you are not allowed to mark it as to be reversed."
+                    )
+                )
+
+    def _get_reversal_id(self):
+        # Although theoretically a o2o, reversal_move_id is technically a o2m,
+        # which does not prevent having more than one record. That is why we are using
+        # a slicing in order to get the first record or an empty recordset.
+        self.ensure_one()
+        return self.reversal_move_id.filtered(lambda m: m.state != "cancel")[:1]
+
     def _mark_as_reversed(self):
         self.filtered("to_be_reversed").write({"to_be_reversed": False})
 
diff --git a/account_reversal/tests/test_account_reversal.py b/account_reversal/tests/test_account_reversal.py
index 4ce9d868037..658e511aeb7 100644
--- a/account_reversal/tests/test_account_reversal.py
+++ b/account_reversal/tests/test_account_reversal.py
@@ -5,10 +5,14 @@
 import random
 
 from odoo.tests import Form
-from odoo.tests.common import SavepointCase
+from odoo.tests.common import TransactionCase
 
+from odoo.addons.account_reversal.models.account_move import (
+    MoveAlreadyReversedValidationError,
+)
 
-class TestAccountReversal(SavepointCase):
+
+class TestAccountReversal(TransactionCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
@@ -164,3 +168,18 @@ def test_reverse_huge_move(self):
 
         self.assertEqual(len(reversal_move.line_ids), 200)
         self.assertEqual(reversal_move.state, "posted")
+
+    def test_already_reversed_constraint(self):
+        account_move = self._create_move()
+        account_move.action_post()
+        account_move.to_be_reversed = True
+        reversed_account_move = account_move._reverse_moves()
+        self.assertEqual(account_move.reversal_id, reversed_account_move)
+        self.assertFalse(account_move.to_be_reversed)
+        with self.assertRaises(MoveAlreadyReversedValidationError), self.cr.savepoint():
+            account_move.to_be_reversed = True
+        # Cancelled reverse moves are not taken into account in reversal_id and the constraint
+        # on to_be_reversed.
+        reversed_account_move.button_cancel()
+        self.assertFalse(account_move.reversal_id)
+        account_move.to_be_reversed = True
diff --git a/account_reversal/views/account_move_view.xml b/account_reversal/views/account_move_view.xml
index 5c6e688b152..107fe6982a8 100644
--- a/account_reversal/views/account_move_view.xml
+++ b/account_reversal/views/account_move_view.xml
@@ -25,6 +25,10 @@
                 position="after"
             >
                 <field name="to_be_reversed" />
+                <field
+                    name="reversal_id"
+                    attrs="{'invisible': [('move_type', '!=', 'entry')]}"
+                />
             </xpath>
         </field>
     </record>
@@ -38,7 +42,7 @@
                 <filter
                     name="to_be_reversed"
                     string="To Be Reversed"
-                    domain="[('to_be_reversed', '=', True), ('reversed_entry_id', '=', False)]"
+                    domain="[('to_be_reversed', '=', True)]"
                     help="Journal Entries to be Reversed"
                 />
             </filter>