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")