[IMP] account: allow deleting moves that were posted before it they are the last element of the sequence chain

We don't check anymore that a move has been posted before in order to allow its deletion. Instead, we look at its sequence: if it's set and is not at the end of the sequence chain it belongs to, we know removing the move will create a gap, whatever the move state, so we forbid it.

If the user really wants to remove the move anyway, it is possible, by manually emptying the name of the move to delete. This way, he won't have any sequence anymore, and will be deletable. Users should however, rather rely on move cancellations and reversals, just for the sake of a cleaner accounting.

addons/account/models/account_move.py

@@ -1900,9 +1900,13 @@ class AccountMove(models.Model):
         return res
 
     @api.ondelete(at_uninstall=False)
-    def _unlink_except_posted_before(self):
-        if not self._context.get('force_delete') and any(move.posted_before for move in self):
-            raise UserError(_("You cannot delete an entry which has been posted once."))
+    def _unlink_except_parts_of_chain(self):
+        """ Moves with a sequence number can only be deleted if they are the last element of a chain of sequence.
+        If they are not, deleting them would create a gap. If the user really wants to do this, he still can
+        explicitly empty the 'name' field of the move; but we discourage that practice.
+        """
+        if not self._context.get('force_delete') and any(move.name != '/' and not move._is_last_from_seq_chain() for move in self):
+            raise UserError(_("You cannot delete this entry, as it has already consumed a sequence number and is not the last one in the chain. Probably you should revert it instead."))
 
     def unlink(self):
         self.line_ids.unlink()

addons/account/models/sequence_mixin.py

@@ -132,7 +132,7 @@ class SequenceMixin(models.AbstractModel):
         self.ensure_one()
         return "00000000"
 
-    def _get_last_sequence(self, relaxed=False):
+    def _get_last_sequence(self, relaxed=False, with_prefix=None):
         """Retrieve the previous sequence.
 
         This is done by taking the number with the greatest alphabetical value within
@@ -149,6 +149,7 @@ class SequenceMixin(models.AbstractModel):
         :param relaxed: this should be set to True when a previous request didn't find
             something without. This allows to find a pattern from a previous period, and
             try to adapt it for the new period.
+        :param with_prefix: The sequence prefix to restrict the search on, if any.
 
         :return: the string of the previous sequence or None if there wasn't any.
         """
@@ -159,6 +160,9 @@ class SequenceMixin(models.AbstractModel):
         if self.id or self.id.origin:
             where_string += " AND id != %(id)s "
             param['id'] = self.id or self.id.origin
+        if with_prefix:
+            where_string += " AND sequence_prefix = %(with_prefix)s "
+            param['with_prefix'] = with_prefix
 
         query = """
             UPDATE {table} SET write_date = write_date WHERE id = (
@@ -239,3 +243,14 @@ class SequenceMixin(models.AbstractModel):
 
         self[self._sequence_field] = format.format(**format_values)
         self._compute_split_sequence()
+
+    def _is_last_from_seq_chain(self):
+        """Tells whether or not this element is the last one of the sequence chain.
+        :return: True if it is the last element of the chain.
+        """
+        last_sequence = self._get_last_sequence(with_prefix=self.sequence_prefix)
+        if not last_sequence:
+            return True
+        seq_format, seq_format_values = self._get_sequence_format_param(last_sequence)
+        seq_format_values['seq'] += 1
+        return seq_format.format(**seq_format_values) == self.name

addons/account/tests/test_account_move_entry.py

@@ -654,12 +654,59 @@ class TestAccountMove(AccountTestInvoicingCommon):
 
             self.assertEqual(moves.mapped('name'), ['CT/2016/01/0001', 'CT/2016/01/0002', '/'])
             moves.button_draft()
-            moves.posted_before = False
-            moves.unlink()
+            moves.with_context(force_delete=True).unlink()
             journal.unlink()
             account.unlink()
             env0.cr.commit()
 
+    def test_sequence_deletion(self):
+        """ The last element of a sequence chain should always be deletable
+        (as long as in draft state, of course). Trying to delete another part
+        of the chain shouldn't work.
+        """
+        def create_test_move(journal, date, name=None, post=True):
+            move = self.test_move.copy({
+                'journal_id': journal.id,
+                'date': date,
+            })
+
+            if name:
+                move.name = name
+
+            if post:
+                move.action_post()
+
+            return move
+
+        journal = self.env['account.journal'].create({
+            'name': 'Test sequences - deletion',
+            'code': 'SEQDEL',
+            'type': 'general',
+        })
+
+        move_1_1 = create_test_move(journal, '2021-01-01', name='TOTO/2021/01/0001')
+        move_1_2 = create_test_move(journal, '2021-01-02')
+        move_1_3 = create_test_move(journal, '2021-01-03')
+        move_2_1 = create_test_move(journal, '2021-02-01')
+        move_draft = create_test_move(journal, '2021-02-02', post=False)
+        move_2_2 = create_test_move(journal, '2021-02-03')
+        move_3_1 = create_test_move(journal, '2021-02-10', name='TURLUTUTU/21/02/001')
+
+        # A draft move without any name can always be deleted.
+        move_draft.unlink()
+
+        # The moves that are not at the end of their sequence chain cannot be deleted
+        for move in (move_1_1, move_1_2, move_2_1):
+            with self.assertRaises(UserError):
+                move.button_draft()
+                move.unlink()
+
+        # The last element of each sequence chain should allow deletion.
+        # Everything should be deletable if we follow this order (a bit randomized on purpose)
+        for move in (move_1_3, move_1_2, move_3_1, move_2_2, move_2_1, move_1_1):
+            move.button_draft()
+            move.unlink()
+
     def test_add_followers_on_post(self):
         # Add some existing partners, some from another company
         company = self.env['res.company'].create({'name': 'Oopo'})

addons/account/tests/test_tour.py

@@ -10,6 +10,5 @@ class TestUi(odoo.tests.HttpCase):
         # This tour doesn't work with demo data on runbot
         all_moves = self.env['account.move'].search([('move_type', '!=', 'entry')])
         all_moves.button_draft()
-        all_moves.posted_before = False
-        all_moves.unlink()
+        all_moves.with_context(force_delete=True).unlink()
         self.start_tour("/web", 'account_tour', login="admin")