From 26b40e7cfd6649a08e430ce5ce9000f61cd09d89 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 4 Feb 2022 00:01:05 +0530 Subject: [PATCH] refactor: Leaves Taken calculation - fix issue where Leaves Taken also adds leaves expiring on boundary date as leaves taken, causing ambiguity - remove unnecessary `skip_expiry_leaves` function - `get_allocation_expiry` considering cancelled entries too --- .../doctype/leave_application/leave_application.py | 14 +++++--------- .../leave_ledger_entry/leave_ledger_entry.py | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py index 70250f5bcf..0cda5e267c 100755 --- a/erpnext/hr/doctype/leave_application/leave_application.py +++ b/erpnext/hr/doctype/leave_application/leave_application.py @@ -480,7 +480,8 @@ def get_allocation_expiry(employee, leave_type, to_date, from_date): 'leave_type': leave_type, 'is_carry_forward': 1, 'transaction_type': 'Leave Allocation', - 'to_date': ['between', (from_date, to_date)] + 'to_date': ['between', (from_date, to_date)], + 'docstatus': 1 },fields=['to_date']) return expiry[0]['to_date'] if expiry else None @@ -636,18 +637,18 @@ def get_remaining_leaves(allocation, leaves_taken, date, expiry): return _get_remaining_leaves(total_leaves, allocation.to_date) -def get_leaves_for_period(employee, leave_type, from_date, to_date, do_not_skip_expired_leaves=False): +def get_leaves_for_period(employee, leave_type, from_date, to_date, skip_expired_leaves=True): leave_entries = get_leave_entries(employee, leave_type, from_date, to_date) leave_days = 0 for leave_entry in leave_entries: inclusive_period = leave_entry.from_date >= getdate(from_date) and leave_entry.to_date <= getdate(to_date) - if inclusive_period and leave_entry.transaction_type == 'Leave Encashment': + if inclusive_period and leave_entry.transaction_type == 'Leave Encashment': leave_days += leave_entry.leaves elif inclusive_period and leave_entry.transaction_type == 'Leave Allocation' and leave_entry.is_expired \ - and (do_not_skip_expired_leaves or not skip_expiry_leaves(leave_entry, to_date)): + and not skip_expired_leaves: leave_days += leave_entry.leaves elif leave_entry.transaction_type == 'Leave Application': @@ -669,11 +670,6 @@ def get_leaves_for_period(employee, leave_type, from_date, to_date, do_not_skip_ return leave_days -def skip_expiry_leaves(leave_entry, date): - ''' Checks whether the expired leaves coincide with the to_date of leave balance check. - This allows backdated leave entry creation for non carry forwarded allocation ''' - end_date = frappe.db.get_value("Leave Allocation", {'name': leave_entry.transaction_name}, ['to_date']) - return True if end_date == date and not leave_entry.is_carry_forward else False def get_leave_entries(employee, leave_type, from_date, to_date): ''' Returns leave entries between from_date and to_date. ''' diff --git a/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py b/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py index 5c5299ea7e..a5923e0021 100644 --- a/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py +++ b/erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py @@ -171,7 +171,7 @@ def expire_carried_forward_allocation(allocation): ''' Expires remaining leaves in the on carried forward allocation ''' from erpnext.hr.doctype.leave_application.leave_application import get_leaves_for_period leaves_taken = get_leaves_for_period(allocation.employee, allocation.leave_type, - allocation.from_date, allocation.to_date, do_not_skip_expired_leaves=True) + allocation.from_date, allocation.to_date, skip_expired_leaves=False) leaves = flt(allocation.leaves) + flt(leaves_taken) # allow expired leaves entry to be created