diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index fff0967f25a..08bc93760a3 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -856,6 +856,7 @@ def get_leave_allocation_records(employee, date, leave_type=None): Min(Ledger.from_date).as_("from_date"), Max(Ledger.to_date).as_("to_date"), Ledger.leave_type, + Ledger.employee, ) .where( (Ledger.from_date <= date) @@ -895,6 +896,7 @@ def get_leave_allocation_records(employee, date, leave_type=None): "unused_leaves": d.cf_leaves, "new_leaves_allocated": d.new_leaves, "leave_type": d.leave_type, + "employee": d.employee, } ), ) @@ -933,26 +935,51 @@ def get_remaining_leaves( return remaining_leaves - leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt( - leaves_taken - ) - - # balance for carry forwarded leaves if cf_expiry and allocation.unused_leaves: + # allocation contains both carry forwarded and new leaves + new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(allocation, cf_expiry) + if getdate(date) > getdate(cf_expiry): - # carry forwarded leave expiry date passed + # carry forwarded leaves have expired cf_leaves = remaining_cf_leaves = 0 else: - cf_leaves = flt(allocation.unused_leaves) + flt(leaves_taken) + cf_leaves = flt(allocation.unused_leaves) + flt(cf_leaves_taken) remaining_cf_leaves = _get_remaining_leaves(cf_leaves, cf_expiry) - leave_balance = flt(allocation.new_leaves_allocated) + flt(cf_leaves) - leave_balance_for_consumption = flt(allocation.new_leaves_allocated) + flt(remaining_cf_leaves) + # new leaves allocated - new leaves taken + cf leave balance + # Note: `new_leaves_taken` is added here because its already a -ve number in the ledger + leave_balance = (flt(allocation.new_leaves_allocated) + flt(new_leaves_taken)) + flt(cf_leaves) + leave_balance_for_consumption = ( + flt(allocation.new_leaves_allocated) + flt(new_leaves_taken) + ) + flt(remaining_cf_leaves) + else: + # allocation only contains newly allocated leaves + leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt( + leaves_taken + ) remaining_leaves = _get_remaining_leaves(leave_balance_for_consumption, allocation.to_date) 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 45e9a87428e..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, @@ -96,6 +97,9 @@ class TestLeaveApplication(unittest.TestCase): from_date = get_year_start(getdate()) to_date = get_year_ending(getdate()) self.holiday_list = make_holiday_list(from_date=from_date, to_date=to_date) + list_without_weekly_offs = make_holiday_list( + "Holiday List w/o Weekly Offs", from_date=from_date, to_date=to_date, add_weekly_offs=False + ) if not frappe.db.exists("Leave Type", "_Test Leave Type"): frappe.get_doc( @@ -990,8 +994,12 @@ class TestLeaveApplication(unittest.TestCase): } self.assertEqual(leave_allocation, expected) - @set_holiday_list("Salary Slip Test Holiday List", "_Test Company") + @set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") def test_leave_details_with_expired_cf_leaves(self): + """Tests leave details: + Case 1: All leaves available before cf leave expiry + Case 2: Remaining Leaves after cf leave expiry + """ employee = get_employee() leave_type = create_leave_type( leave_type_name="_Test_CF_leave_expiry", @@ -1004,11 +1012,11 @@ class TestLeaveApplication(unittest.TestCase): "Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date" ) - # all leaves available before cf leave expiry + # case 1: all leaves available before cf leave expiry leave_details = get_leave_details(employee.name, add_days(cf_expiry, -1)) self.assertEqual(leave_details["leave_allocation"][leave_type.name]["remaining_leaves"], 30.0) - # cf leaves expired + # case 2: cf leaves expired leave_details = get_leave_details(employee.name, add_days(cf_expiry, 1)) expected_data = { "total_leaves": 30.0, @@ -1017,6 +1025,119 @@ class TestLeaveApplication(unittest.TestCase): "leaves_pending_approval": 0.0, "remaining_leaves": 15.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_across_cf_expiry(self): + """Tests leave details with leave application across cf expiry, such that: + cf leaves are partially expired and partially consumed + """ + 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 + application = make_leave_application( + employee.name, + cf_expiry, + add_days(cf_expiry, 3), + leave_type.name, + ) + + leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4)) + expected_data = { + "total_leaves": 30.0, + "expired_leaves": 14.0, + "leaves_taken": 4.0, + "leaves_pending_approval": 0.0, + "remaining_leaves": 12.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_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: + cf leaves are completely expired and only newly allocated leaves are consumed + """ + 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 after cf expiry + application = make_leave_application( + employee.name, + add_days(cf_expiry, 1), + add_days(cf_expiry, 4), + leave_type.name, + ) + + leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4)) + expected_data = { + "total_leaves": 30.0, + "expired_leaves": 15.0, + "leaves_taken": 4.0, + "leaves_pending_approval": 0.0, + "remaining_leaves": 11.0, + } + self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data) @set_holiday_list("Salary Slip Test Holiday List", "_Test Company") @@ -1043,6 +1164,7 @@ class TestLeaveApplication(unittest.TestCase): "unused_leaves": 15.0, "new_leaves_allocated": 15.0, "leave_type": leave_type.name, + "employee": employee.name, } self.assertEqual(details.get(leave_type.name), expected_data) diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index 32d0c7ed08f..993e62bc691 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1587,9 +1587,8 @@ def setup_test(): frappe.db.set_value("HR Settings", None, "leave_approval_notification_template", None) -def make_holiday_list(list_name=None, from_date=None, to_date=None): - if not (from_date and to_date): - fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) +def make_holiday_list(list_name=None, from_date=None, to_date=None, add_weekly_offs=True): + fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) name = list_name or "Salary Slip Test Holiday List" frappe.delete_doc_if_exists("Holiday List", name, force=True) @@ -1600,10 +1599,13 @@ def make_holiday_list(list_name=None, from_date=None, to_date=None): "holiday_list_name": name, "from_date": from_date or fiscal_year[1], "to_date": to_date or fiscal_year[2], - "weekly_off": "Sunday", } ).insert() - holiday_list.get_weekly_off_dates() + + if add_weekly_offs: + holiday_list.weekly_off = "Sunday" + holiday_list.get_weekly_off_dates() + holiday_list.save() holiday_list = holiday_list.name