From 7ea521a23b857ae92895ba5bc1f7bbd3aefef209 Mon Sep 17 00:00:00 2001
From: "Dylan Kiss (dyki)" <dyki@odoo.com>
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#121832
X-original-commit: 9bf54f099a609e54ae165dd481452aeceb495f87
Signed-off-by: William André (wan) <wan@odoo.com>
---
addons/account/models/account_move.py | 25 ++++++----
addons/account/tests/test_sequence_mixin.py | 55 +++++++++++++++++++++
2 files changed, 70 insertions(+), 10 deletions(-)
diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py
index 2f71bd22964c..4ea04af02c9e 100644
--- a/addons/account/models/account_move.py
+++ b/addons/account/models/account_move.py
@@ -1224,23 +1224,28 @@ class AccountMove(models.Model):
)
)
self = self.sorted(lambda m: (m.date, m.ref or '', m.id))
- highest_name = self[0]._get_last_sequence(lock=False) if self else False
# Group the moves by journal and month
for move in self:
- if not highest_name and move == self[0] and not move.posted_before and move.date:
- # 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)
- pass
- elif (move.name and move.name != '/') or move.state != 'posted':
+ move_has_name = move.name and move.name != '/'
+ if move_has_name or move.state != 'posted':
try:
if not move.posted_before:
+ # The move was never posted, so the name can potentially be changed.
move._constrains_date_sequence()
- # Has already a name or is not posted, we don't add to a batch
- continue
+ # Either the move was posted before, or the name already matches the date.
+ # We can skip recalculating the name when either
+ # - the move was posted before and already has a name, or
+ # - the move has no name, but is in a period with other moves (so name should be `/`)
+ if move_has_name and move.posted_before or not move_has_name and move._get_last_sequence(lock=False):
+ continue
except ValidationError:
- # Has never been posted and the name doesn't match the date: recompute it
- pass
+ # The move was never posted and the current name doesn't match the date. We should calculate the
+ # name later on, unless ...
+ if move._get_last_sequence(lock=False):
+ # ... we are in a period already containing moves: reset the name to `/` (draft)
+ move.name = '/'
+ continue
group = grouped[journal_key(move)][date_key(move)]
if not group['records']:
# Compute all the values needed to sequence this whole group
diff --git a/addons/account/tests/test_sequence_mixin.py b/addons/account/tests/test_sequence_mixin.py
index eb2d62b2252d..6345c4931a3f 100644
--- a/addons/account/tests/test_sequence_mixin.py
+++ b/addons/account/tests/test_sequence_mixin.py
@@ -65,6 +65,61 @@ class TestSequenceMixin(TestSequenceMixinCommon):
self.test_move.action_post()
self.assertEqual(self.test_move.name, 'MyMISC/2020/0000001')
+
+ 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')
+
+
def test_journal_sequence(self):
self.assertEqual(self.test_move.name, 'MISC/2016/01/0001')
self.test_move.action_post()
--
GitLab