From 5f976e0ef4d62267aee977b960fb215bcb89f122 Mon Sep 17 00:00:00 2001
From: Dossogne Bertrand <bedo@odoo.com>
Date: Fri, 1 Sep 2023 17:08:27 +0200
Subject: [PATCH] [FIX] hr_holidays: fix accrual allocation lastcall

Before this commit, creating a new allocation based on accruals
and setting a start date prior to the creation date would compute
the amount of days that should already have been accrued during that time.
However, the lastcall field is not updated and running the accrual cron
leads to a wrong duration.

How to recreate the issue:
- create an accrual plan
- create an allocation set on that accrual
- set a start date in the past for that allocation
- the duration should be automatically set to a value above 0
- save and validate
- run the accrual cron
- the allocation now has its duration doubled

After this commit, the lastcall should be set to the allocation creation
date. This way, running the cron will correctly keep the value of the amount
of days.

closes odoo/odoo#134251

Signed-off-by: Sofie Gvaladze (sgv) <sgv@odoo.com>
---
 .../hr_holidays/models/hr_leave_allocation.py |   5 +-
 .../tests/test_accrual_allocations.py         | 377 ++++++++++--------
 2 files changed, 212 insertions(+), 170 deletions(-)

diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py
index 765d5f4ff37..9e5a6216fc1 100644
--- a/addons/hr_holidays/models/hr_leave_allocation.py
+++ b/addons/hr_holidays/models/hr_leave_allocation.py
@@ -586,9 +586,8 @@ class HolidaysAllocation(models.Model):
             employee_id = values.get('employee_id', False)
             if not values.get('department_id'):
                 values.update({'department_id': self.env['hr.employee'].browse(employee_id).department_id.id})
-            # default `lastcall` to `nextcall`
-            if 'date_from' in values and 'lastcall' not in values:
-                values['lastcall'] = values['date_from']
+            if 'lastcall' not in values:
+                values['lastcall'] = fields.Date.today()
         holidays = super(HolidaysAllocation, self.with_context(mail_create_nosubscribe=True)).create(vals_list)
         for holiday in holidays:
             partners_to_subscribe = set()
diff --git a/addons/hr_holidays/tests/test_accrual_allocations.py b/addons/hr_holidays/tests/test_accrual_allocations.py
index 2f9c284e006..8ee9899c2f2 100644
--- a/addons/hr_holidays/tests/test_accrual_allocations.py
+++ b/addons/hr_holidays/tests/test_accrual_allocations.py
@@ -769,17 +769,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'action_with_unused_accruals': 'all',
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual Allocation - Test',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': datetime.date(2020, 8, 16),
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time('2020-8-16'):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual Allocation - Test',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': datetime.date(2020, 8, 16),
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
         with freeze_time('2022-1-10'):
             allocation._update_accrual()
         self.assertAlmostEqual(allocation.number_of_days, 30.82, 2, "Invalid number of days")
@@ -817,17 +818,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'first_day': 31,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual Allocation - Test',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': datetime.date(2022, 1, 31),
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time('2022-1-31'):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual Allocation - Test',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': datetime.date(2022, 1, 31),
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
         with freeze_time('2022-7-20'):
             allocation._update_accrual()
         # The first level gives 3 days
@@ -863,17 +865,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 }),
             ],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual Allocation - Test',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': datetime.date(2021, 1, 1),
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time('2021-1-1'):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual Allocation - Test',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': datetime.date(2021, 1, 1),
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
         with freeze_time('2022-4-4'):
             allocation._update_accrual()
         self.assertEqual(allocation.number_of_days, 3, "Invalid number of days")
@@ -895,17 +898,19 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 })
             ],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual Allocation - Test',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': datetime.date(2019, 1, 1),
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time('2019-1-1'):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual Allocation - Test',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': datetime.date(2019, 1, 1),
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
+
         with freeze_time('2022-4-1'):
             allocation._update_accrual()
         self.assertAlmostEqual(allocation.number_of_days, 3, 2, "Invalid number of days")
@@ -923,15 +928,16 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'maximum_leave': 5,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2021-09-03',
-        })
+        with freeze_time("2021-9-3"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2021-09-03',
+            })
 
         with freeze_time("2021-10-3"):
             allocation.action_confirm()
@@ -952,15 +958,16 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'cap_accrued_time': False,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2021-09-03',
-        })
+        with freeze_time("2021-9-3"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2021-09-03',
+            })
 
         with freeze_time("2021-10-3"):
             allocation.action_confirm()
@@ -983,17 +990,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'maximum_leave': 5,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2022-01-01',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time("2022-1-1"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2022-01-01',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
 
         with freeze_time("2022-3-2"):
             allocation._update_accrual()
@@ -1013,7 +1021,7 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
             allocation._update_accrual()
         self.assertEqual(allocation.number_of_days, 10, "Should accrue 5 additional days")
 
-    def test_aaaccrual_leaves_taken_maximum_hours(self):
+    def test_accrual_leaves_taken_maximum_hours(self):
         accrual_plan = self.env['hr.leave.accrual.plan'].with_context(tracking_disable=True).create({
             'name': 'Accrual Plan For Test',
             'level_ids': [(0, 0, {
@@ -1027,17 +1035,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'maximum_leave': 10,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type_hour.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2022-01-01',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time(datetime.date(2022, 1, 1)):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type_hour.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2022-01-01',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
 
         with freeze_time(datetime.date(2022, 4, 1)):
             allocation._update_accrual()
@@ -1072,37 +1081,37 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'maximum_leave': 5,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-24',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
-
         with freeze_time("2023-4-24"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-24',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
+
             allocation._update_accrual()
 
         self.assertEqual(allocation.number_of_days, 0, "Should accrue 0 days, because the period is not done yet.")
 
         accrual_plan.accrued_gain_time = 'start'
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-24',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
-
         with freeze_time("2023-4-24"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-24',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
+
             allocation._update_accrual()
 
         self.assertEqual(allocation.number_of_days, 1, "Should accrue 1 day, at the start of the period.")
@@ -1122,19 +1131,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'maximum_leave': 15,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-13',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
-
         with freeze_time("2023-4-13"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-13',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
             allocation._update_accrual()
 
         self.assertAlmostEqual(allocation.number_of_days, 1.5, 2)
@@ -1170,19 +1178,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 })
             ],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-26',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
-
         with freeze_time("2023-4-26"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-26',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
             allocation._update_accrual()
         self.assertEqual(allocation.number_of_days, 1, "Should accrue 1 day, at the start of the period.")
 
@@ -1211,19 +1218,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'action_with_unused_accruals': 'lost',
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-26',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
-
         with freeze_time("2023-4-26"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-26',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
             allocation._update_accrual()
         self.assertEqual(allocation.number_of_days, 0, "Should accrue 0 days, days are added on 27th.")
 
@@ -1262,17 +1268,18 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 'postpone_max_days': 69,
             })],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 0,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-20',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time("2023-04-20"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 0,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-20',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
 
         with freeze_time("2024-04-20"):
             allocation._update_accrual()
@@ -1325,18 +1332,54 @@ class TestAccrualAllocations(TestHrHolidaysCommon):
                 }),
             ],
         })
-        allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
-            'name': 'Accrual allocation for employee',
-            'accrual_plan_id': accrual_plan.id,
-            'employee_id': self.employee_emp.id,
-            'holiday_status_id': self.leave_type.id,
-            'number_of_days': 9,
-            'allocation_type': 'accrual',
-            'date_from': '2023-04-4',
-        })
-        allocation.action_confirm()
-        allocation.action_validate()
+        with freeze_time("2023-04-04"):
+            allocation = self.env['hr.leave.allocation'].with_user(self.user_hrmanager_id).with_context(tracking_disable=True).create({
+                'name': 'Accrual allocation for employee',
+                'accrual_plan_id': accrual_plan.id,
+                'employee_id': self.employee_emp.id,
+                'holiday_status_id': self.leave_type.id,
+                'number_of_days': 9,
+                'allocation_type': 'accrual',
+                'date_from': '2023-04-4',
+            })
+            allocation.action_confirm()
+            allocation.action_validate()
 
         with freeze_time("2026-08-01"):
             allocation._update_accrual()
         self.assertAlmostEqual(allocation.number_of_days, 1.315, 3)
+
+    def test_accrual_creation_on_anterior_date(self):
+        accrual_plan = self.env['hr.leave.accrual.plan'].with_context(tracking_disable=True).create({
+            'name': 'Weekly accrual',
+            'carryover_date': 'allocation',
+            'level_ids': [(0, 0, {
+                'added_value_type': 'day',
+                'start_count': 0,
+                'start_type': 'day',
+                'added_value': 1,
+                'frequency': 'weekly',
+                'cap_accrued_time': False,
+                'action_with_unused_accruals': 'lost',
+            })],
+        })
+        with freeze_time('2023-09-01'):
+            accrual_allocation = self.env['hr.leave.allocation'].new({
+                'name': 'Employee allocation',
+                'holiday_status_id': self.leave_type.id,
+                'date_from': '2023-01-01',
+                'employee_id': self.employee_emp.id,
+                'allocation_type': 'accrual',
+                'accrual_plan_id': accrual_plan.id,
+            })
+            # As the duration is set to a onchange, we need to force that onchange to run
+            accrual_allocation._onchange_date_from()
+            accrual_allocation.action_validate()
+            # The amount of days should be computed as if it was accrued since
+            # the start date of the allocation.
+            self.assertAlmostEqual(accrual_allocation.number_of_days, 34.0, places=0)
+            self.assertFalse(accrual_allocation.lastcall == accrual_allocation.date_from)
+            accrual_allocation._update_accrual()
+            # The amount being already computed, the amount should stay the same after the cron
+            # running on the same day.
+            self.assertAlmostEqual(accrual_allocation.number_of_days, 34.0, places=0)