diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 335726863d5..1368149c9f7 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -2269,7 +2269,7 @@ class AccountMove(models.Model): 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): + if not self._context.get('force_delete') and not self.filtered(lambda move: move.name != '/')._is_end_of_seq_chain(): 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): diff --git a/addons/account/models/sequence_mixin.py b/addons/account/models/sequence_mixin.py index 231448b3668..48f98a2870f 100644 --- a/addons/account/models/sequence_mixin.py +++ b/addons/account/models/sequence_mixin.py @@ -3,8 +3,10 @@ from odoo import api, fields, models, _ from odoo.exceptions import ValidationError from odoo.tools.misc import format_date +from odoo.tools import frozendict import re +from collections import defaultdict from psycopg2 import sql @@ -245,6 +247,7 @@ class SequenceMixin(models.AbstractModel): 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) @@ -253,3 +256,29 @@ class SequenceMixin(models.AbstractModel): 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 + + def _is_end_of_seq_chain(self): + """Tells whether or not these elements are the last ones of the sequence chain. + + :return: True if self are the last elements of the chain. + """ + batched = defaultdict(lambda: {'last_rec': self.browse(), 'seq_list': []}) + for record in self: + seq_format, format_values = self._get_sequence_format_param(record[record._sequence_field]) + seq = format_values.pop('seq') + batch = batched[(seq_format, frozendict(format_values))] + batch['seq_list'].append(seq) + if batch['last_rec'].sequence_number < record.sequence_number: + batch['last_rec'] = record + + for values in batched.values(): + # The sequences we are deleting are not sequential + seq_list = values['seq_list'] + if max(seq_list) - min(seq_list) != len(seq_list) - 1: + return False + + # last_rec must have the highest number in the database + record = values['last_rec'] + if not record._is_last_from_seq_chain(): + return False + return True diff --git a/addons/account/tests/test_sequence_mixin.py b/addons/account/tests/test_sequence_mixin.py index bd0566f252b..a52c7981bbe 100644 --- a/addons/account/tests/test_sequence_mixin.py +++ b/addons/account/tests/test_sequence_mixin.py @@ -12,9 +12,7 @@ import json import psycopg2 -@tagged('post_install', '-at_install') -class TestSequenceMixin(AccountTestInvoicingCommon): - +class TestSequenceMixinCommon(AccountTestInvoicingCommon): @classmethod def setUpClass(cls, chart_template_ref=None): super().setUpClass(chart_template_ref=chart_template_ref) @@ -41,6 +39,9 @@ class TestSequenceMixin(AccountTestInvoicingCommon): move.action_post() return move + +@tagged('post_install', '-at_install') +class TestSequenceMixin(TestSequenceMixinCommon): def test_sequence_change_date(self): """Change the sequence when we change the date iff it has never been posted.""" # Check setup @@ -394,36 +395,63 @@ class TestSequenceMixin(AccountTestInvoicingCommon): account.unlink() env0.cr.commit() - def test_sequence_deletion(self): - """The last element of a sequence chain should always be deletable if in draft state. - Trying to delete another part of the chain shouldn't work. - """ - journal = self.env['account.journal'].create({ +@tagged('post_install', '-at_install') +class TestSequenceMixinDeletion(TestSequenceMixinCommon): + @classmethod + def setUpClass(cls, chart_template_ref=None): + super().setUpClass(chart_template_ref=chart_template_ref) + journal = cls.env['account.journal'].create({ 'name': 'Test sequences - deletion', 'code': 'SEQDEL', 'type': 'general', }) - move_1_1 = self.create_move('2021-01-01', journal, name='TOTO/2021/01/0001', post=True) - move_1_2 = self.create_move('2021-01-02', journal, post=True) - move_1_3 = self.create_move('2021-01-03', journal, post=True) - move_2_1 = self.create_move('2021-02-01', journal, post=True) - move_draft = self.create_move('2021-02-02', journal, post=False) - move_2_2 = self.create_move('2021-02-03', journal, post=True) - move_3_1 = self.create_move('2021-02-10', journal, name='TURLUTUTU/21/02/001', post=True) + cls.move_1_1 = cls.create_move('2021-01-01', journal, name='TOTO/2021/01/0001', post=True) + cls.move_1_2 = cls.create_move('2021-01-02', journal, post=True) + cls.move_1_3 = cls.create_move('2021-01-03', journal, post=True) + cls.move_2_1 = cls.create_move('2021-02-01', journal, post=True) + cls.move_draft = cls.create_move('2021-02-02', journal, post=False) + cls.move_2_2 = cls.create_move('2021-02-03', journal, post=True) + cls.move_3_1 = cls.create_move('2021-02-10', journal, name='TURLUTUTU/21/02/001', post=True) + + def test_sequence_deletion_1(self): + """The last element of a sequence chain should always be deletable if in draft state. + + Trying to delete another part of the chain shouldn't work. + """ # A draft move without any name can always be deleted. - move_draft.unlink() + self.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): + for move in (self.move_1_1, self.move_1_2, self.move_2_1): + move.button_draft() 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): + for move in (self.move_1_3, self.move_1_2, self.move_3_1, self.move_2_2, self.move_2_1, self.move_1_1): move.button_draft() move.unlink() + + def test_sequence_deletion_2(self): + """Can delete in batch.""" + all_moves = (self.move_1_3 + self.move_1_2 + self.move_3_1 + self.move_2_2 + self.move_2_1 + self.move_1_1) + all_moves.button_draft() + all_moves.unlink() + + def test_sequence_deletion_3(self): + """Cannot delete non sequential batches.""" + all_moves = (self.move_1_3 + self.move_3_1 + self.move_2_2 + self.move_2_1 + self.move_1_1) + all_moves.button_draft() + with self.assertRaises(UserError): + all_moves.unlink() + + def test_sequence_deletion_4(self): + """Cannot delete batches not containing the last entry.""" + all_moves = (self.move_1_2 + self.move_3_1 + self.move_2_2 + self.move_2_1 + self.move_1_1) + all_moves.button_draft() + with self.assertRaises(UserError): + all_moves.unlink()