From 7d541b9956992cdda48089587d83c8ef3cf5a641 Mon Sep 17 00:00:00 2001 From: oco-odoo Date: Thu, 25 Mar 2021 13:24:41 +0000 Subject: [PATCH] [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 | 10 ++-- addons/account/models/sequence_mixin.py | 17 ++++++- .../account/tests/test_account_move_entry.py | 51 ++++++++++++++++++- addons/account/tests/test_tour.py | 3 +- 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 4ad7fb48508..8fff0895aec 100644 --- a/addons/account/models/account_move.py +++ b/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() diff --git a/addons/account/models/sequence_mixin.py b/addons/account/models/sequence_mixin.py index f2a1b09f0da..09977376e43 100644 --- a/addons/account/models/sequence_mixin.py +++ b/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 diff --git a/addons/account/tests/test_account_move_entry.py b/addons/account/tests/test_account_move_entry.py index e5d7697ad07..450636444aa 100644 --- a/addons/account/tests/test_account_move_entry.py +++ b/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'}) diff --git a/addons/account/tests/test_tour.py b/addons/account/tests/test_tour.py index 8dc70195f36..e399cbf8744 100644 --- a/addons/account/tests/test_tour.py +++ b/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")