From bc12269ef46940473dc07bdb4ddf5f84557ef919 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Thu, 9 Mar 2023 15:10:28 +0530 Subject: [PATCH] fix: adjust excess cf leaves in new leaves taken if number exceeds cf leaves allocation --- .../leave_application/leave_application.py | 25 ++++++++--- .../test_leave_application.py | 41 +++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index de88b3d807c..08bc93760a3 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -937,12 +937,7 @@ def get_remaining_leaves( if cf_expiry and allocation.unused_leaves: # allocation contains both carry forwarded and new leaves - cf_leaves_taken = get_leaves_for_period( - allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry - ) - new_leaves_taken = get_leaves_for_period( - allocation.employee, allocation.leave_type, add_days(cf_expiry, 1), allocation.to_date - ) + new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(allocation, cf_expiry) if getdate(date) > getdate(cf_expiry): # carry forwarded leaves have expired @@ -967,6 +962,24 @@ def get_remaining_leaves( return frappe._dict(leave_balance=leave_balance, leave_balance_for_consumption=remaining_leaves) +def get_new_and_cf_leaves_taken(allocation: Dict, cf_expiry: str) -> Tuple[float, float]: + """returns new leaves taken and carry forwarded leaves taken within an allocation period based on cf leave expiry""" + cf_leaves_taken = get_leaves_for_period( + allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry + ) + new_leaves_taken = get_leaves_for_period( + allocation.employee, allocation.leave_type, add_days(cf_expiry, 1), allocation.to_date + ) + + # using abs because leaves taken is a -ve number in the ledger + if abs(cf_leaves_taken) > allocation.unused_leaves: + # adjust the excess leaves in new_leaves_taken + new_leaves_taken += -(abs(cf_leaves_taken) - allocation.unused_leaves) + cf_leaves_taken = -allocation.unused_leaves + + return new_leaves_taken, cf_leaves_taken + + def get_leaves_for_period( employee: str, leave_type: str, from_date: str, to_date: str, skip_expired_leaves: bool = True ) -> float: diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py index f41fd93e564..231b14f6163 100644 --- a/erpnext/hr/doctype/leave_application/test_leave_application.py +++ b/erpnext/hr/doctype/leave_application/test_leave_application.py @@ -28,6 +28,7 @@ from erpnext.hr.doctype.leave_application.leave_application import ( get_leave_allocation_records, get_leave_balance_on, get_leave_details, + get_new_and_cf_leaves_taken, ) from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import ( create_assignment_for_multiple_employees, @@ -1063,6 +1064,46 @@ class TestLeaveApplication(unittest.TestCase): self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") + def test_leave_details_with_application_across_cf_expiry_2(self): + """Tests the same case as above but with leave days greater than cf leaves allocated""" + employee = get_employee() + leave_type = create_leave_type( + leave_type_name="_Test_CF_leave_expiry", + is_carry_forward=1, + expire_carry_forwarded_leaves_after_days=90, + ).insert() + + leave_alloc = create_carry_forwarded_allocation(employee, leave_type) + cf_expiry = frappe.db.get_value( + "Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date" + ) + + # leave application across cf expiry, 20 days leave + application = make_leave_application( + employee.name, + add_days(cf_expiry, -16), + add_days(cf_expiry, 3), + leave_type.name, + ) + + # 15 cf leaves and 5 new leaves should be consumed + # after adjustment of the actual days breakup (17 and 3) because only 15 cf leaves have been allocated + new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(leave_alloc, cf_expiry) + self.assertEqual(new_leaves_taken, -5.0) + self.assertEqual(cf_leaves_taken, -15.0) + + leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4)) + expected_data = { + "total_leaves": 30.0, + "expired_leaves": 0, + "leaves_taken": 20.0, + "leaves_pending_approval": 0.0, + "remaining_leaves": 10.0, + } + + self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") def test_leave_details_with_application_after_cf_expiry(self): """Tests leave details with leave application after cf expiry, such that: