From 72b580243aeda1ce48f5d9b9dd4e5f981f878e4e Mon Sep 17 00:00:00 2001 From: "Dylan Kiss (dyki)" Date: Tue, 16 May 2023 14:59:13 +0000 Subject: [PATCH] [FIX] account: reset draft move name in nonempty period MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, when we create a draft move in an empty period, a sequence number (name) gets generated and set on the move. This is fine. When subsequently we change the date of that move to a period that already has entries in it, the sequence number (name) for our draft move is recalculated according to the new period. When we post a new move in this same period afterwards, and then delete our previous draft move, we are left with a gap in the sequence. Example: We already have a move on 2023-01-01 with name `2023/01/0001`. We add two new moves `A` and `B` as follows. | Step | Move | Action | Date | Name | | ---- | ---- | ----------- | ---------- | -------------- | | 1 | `A` | Add | 2023-02-01 | `2023/02/0001` | | 2 | `A` | Change date | 2023-01-10 | `2023/01/0002` | | 3 | `B` | Add | 2023-01-15 | `/` | | 4 | `B` | Post | 2023-01-15 | `2023/01/0003` | | 5 | `A` | Delete | | | A gap is now created, since we have `2023/01/0001` and `2023/01/0003`, but `2023/01/0002` was deleted (possible since it was in draft). To solve this issue, we now make sure that when a draft entry is moved to a period that already has entries in it, we reset the name to `/`, to not consume a sequence number and prevent possible gaps in the sequence later on. task-3326834 closes odoo/odoo#123329 X-original-commit: 31f94b31b820799459ad70593abb74a2f5415b47 Signed-off-by: William André (wan) Signed-off-by: Dylan Kiss (dyki) --- addons/account/models/account_move.py | 30 ++++---- addons/account/models/account_payment.py | 3 + addons/account/tests/test_sequence_mixin.py | 78 ++++++++++++++++++--- 3 files changed, 88 insertions(+), 23 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index f2c3320ed0b..bbdcade9efb 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -731,24 +731,28 @@ class AccountMove(models.Model): self = self.sorted(lambda m: (m.date, m.ref or '', m.id)) for move in self: - name_not_set = not move.name or move.name == '/' - if not move.highest_name and not move.posted_before and move.date and name_not_set: - # In the form view, we need to compute a default sequence so that the user can edit - # it. We only check the first move as an approximation (enough for new in form view) - move._set_next_sequence() - elif move.quick_edit_mode and not move.posted_before: - # We always suggest the next sequence as the default name of the new move - if name_not_set or not move._sequence_matches_date(): - move._set_next_sequence() - elif not move.posted_before and not move._sequence_matches_date(): - # The date changed before posting on first move of period - move._set_next_sequence() - elif (name_not_set and move.state == 'posted'): + move_has_name = move.name and move.name != '/' + if move_has_name or move.state != 'posted': + if not move.posted_before and not move._sequence_matches_date(): + if move._get_last_sequence(): + # The name does not match the date and the move is not the first in the period: + # Reset to draft + move.name = '/' + continue + else: + if move_has_name and move.posted_before or not move_has_name and move._get_last_sequence(): + # The move either + # - has a name and was posted before, or + # - doesn't have a name, but is not the first in the period + # so we don't recompute the name + continue + if move.date and (not move_has_name or not move._sequence_matches_date()): move._set_next_sequence() self.filtered(lambda m: not m.name).name = '/' self._inverse_name() + @api.depends('journal_id', 'date') def _compute_highest_name(self): for record in self: diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index c8d1c8a47ce..dbb78fc8967 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -720,7 +720,10 @@ class AccountPayment(models.Model): to_write['line_ids'] = [(0, 0, line_vals) for line_vals in pay._prepare_move_line_default_vals(write_off_line_vals=write_off_line_vals)] pay.move_id.write(to_write) + self.env.add_to_compute(self.env['account.move']._fields['name'], pay.move_id) + # We need to reset the cached name, since it was recomputed on the delegate account.move model + payments.invalidate_recordset(fnames=['name']) return payments def write(self, vals): diff --git a/addons/account/tests/test_sequence_mixin.py b/addons/account/tests/test_sequence_mixin.py index 7f91130191c..3eede3718e4 100644 --- a/addons/account/tests/test_sequence_mixin.py +++ b/addons/account/tests/test_sequence_mixin.py @@ -86,15 +86,14 @@ class TestSequenceMixin(TestSequenceMixinCommon): }), ] }) - bill = bill.copy({'date': '2016-01-01'}) - bill.copy({'date': '2016-01-01'}) + bill = bill.copy({'date': '2016-02-01'}) - self.assertEqual(bill.name, 'BILL/2016/01/0002') + self.assertEqual(bill.name, 'BILL/2016/02/0001') with Form(bill) as bill_form: - bill_form.date = '2016-01-02' - self.assertEqual(bill_form.name, 'BILL/2016/01/0002') bill_form.date = '2016-02-02' self.assertEqual(bill_form.name, 'BILL/2016/02/0001') + bill_form.date = '2016-03-01' + self.assertEqual(bill_form.name, 'BILL/2016/03/0001') bill_form.date = '2017-01-01' self.assertEqual(bill_form.name, 'BILL/2017/01/0001') @@ -109,18 +108,77 @@ class TestSequenceMixin(TestSequenceMixinCommon): }), ] }) - invoice = invoice.copy({'date': '2016-01-01'}) - invoice.copy({'date': '2016-01-01'}) - self.assertEqual(invoice.name, 'INV/2016/00002') + self.assertEqual(invoice.name, 'INV/2016/00001') with Form(invoice) as invoice_form: invoice_form.date = '2016-01-02' - self.assertEqual(invoice_form.name, 'INV/2016/00002') + self.assertEqual(invoice_form.name, 'INV/2016/00001') invoice_form.date = '2016-02-02' - self.assertEqual(invoice_form.name, 'INV/2016/00002') + self.assertEqual(invoice_form.name, 'INV/2016/00001') invoice_form.date = '2017-01-01' self.assertEqual(invoice_form.name, 'INV/2017/00001') + + def test_sequence_draft_change_date(self): + # When a draft entry is added to an empty period, it should get a name. + # When a draft entry with a name is moved to a period already having entries, its name should be reset to '/'. + + new_move = self.test_move.copy({'date': '2016-02-01'}) + new_multiple_move_1 = self.test_move.copy({'date': '2016-03-01'}) + new_multiple_move_2 = self.test_move.copy({'date': '2016-04-01'}) + new_moves = new_multiple_move_1 + new_multiple_move_2 + + # Empty period, so a name should be set + self.assertEqual(new_move.name, 'MISC/2016/02/0001') + self.assertEqual(new_multiple_move_1.name, 'MISC/2016/03/0001') + self.assertEqual(new_multiple_move_2.name, 'MISC/2016/04/0001') + + # Move to an existing period with another move in it + new_move.date = fields.Date.to_date('2016-01-10') + new_moves.date = fields.Date.to_date('2016-01-15') + + # Not an empty period, so names should be reset to '/' (draft) + self.assertEqual(new_move.name, '/') + self.assertEqual(new_multiple_move_1.name, '/') + self.assertEqual(new_multiple_move_2.name, '/') + + # Move back to a period with no moves in it + new_move.date = fields.Date.to_date('2016-02-01') + new_moves.date = fields.Date.to_date('2016-03-01') + + # All moves in the previously empty periods should be given a name instead of `/` + self.assertEqual(new_move.name, 'MISC/2016/02/0001') + self.assertEqual(new_multiple_move_1.name, 'MISC/2016/03/0001') + # Since this is the second one in the same period, it should remain `/` + self.assertEqual(new_multiple_move_2.name, '/') + + # Move both moves back to different periods, both with already moves in it. + new_multiple_move_1.date = fields.Date.to_date('2016-01-10') + new_multiple_move_2.date = fields.Date.to_date('2016-02-10') + + # Moves are not in empty periods, so names should be set to '/' (draft) + self.assertEqual(new_multiple_move_1.name, '/') + self.assertEqual(new_multiple_move_2.name, '/') + + # Change the journal of the last two moves (empty) + journal = self.env['account.journal'].create({ + 'name': 'awesome journal', + 'type': 'general', + 'code': 'AJ', + }) + new_moves.journal_id = journal + + # Both moves should be assigned a name, since no moves are in the journal and they are in different periods. + self.assertEqual(new_multiple_move_1.name, 'AJ/2016/01/0001') + self.assertEqual(new_multiple_move_2.name, 'AJ/2016/02/0001') + + # When the date is removed in the form view, the name should not recompute + with Form(new_multiple_move_1) as move_form: + move_form.date = False + self.assertEqual(new_multiple_move_1.name, 'AJ/2016/01/0001') + move_form.date = fields.Date.to_date('2016-01-10') + + def test_journal_sequence(self): self.assertEqual(self.test_move.name, 'MISC/2016/01/0001') self.test_move.action_post()