fix: duplicate leave expiry creation (#21506)

* fix: validate existing ledger entries to avoid duplicates

* patch: remove duplicate ledger entries created

* fix: consider only submitted ledger entries

* fix: delete duplicate leaves from the ledger

* fix: check if duplicate ledger entry exists

* chore: formatting changes

Co-authored-by: Nabin Hait <nabinhait@gmail.com>

erpnext/hr/doctype/leave_application/leave_application.py

@@ -541,7 +541,7 @@ 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):
+def get_leaves_for_period(employee, leave_type, from_date, to_date, do_not_skip_expired_leaves=False):
 	leave_entries = get_leave_entries(employee, leave_type, from_date, to_date)
 	leave_days = 0
 
@@ -551,8 +551,8 @@ def get_leaves_for_period(employee, leave_type, from_date, to_date):
 		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' \
+		elif inclusive_period and leave_entry.transaction_type == 'Leave Allocation' and leave_entry.is_expired \
-			and leave_entry.is_expired and not skip_expiry_leaves(leave_entry, to_date):
+			and (do_not_skip_expired_leaves or not skip_expiry_leaves(leave_entry, to_date)):
 			leave_days += leave_entry.leaves
 
 		elif leave_entry.transaction_type == 'Leave Application':

erpnext/hr/doctype/leave_ledger_entry/leave_ledger_entry.py

@@ -88,32 +88,40 @@ def get_previous_expiry_ledger_entry(ledger):
 	}, fieldname=['name'])
 
 def process_expired_allocation():
-	''' Check if a carry forwarded allocation has expired and create a expiry ledger entry '''
+	''' Check if a carry forwarded allocation has expired and create a expiry ledger entry
+		Case 1: carry forwarded expiry period is set for the leave type,
+			create a separate leave expiry entry against each entry of carry forwarded and non carry forwarded leaves
+		Case 2: leave type has no specific expiry period for carry forwarded leaves
+			and there is no carry forwarded leave allocation, create a single expiry against the remaining leaves.
+	'''
 
 	# fetch leave type records that has carry forwarded leaves expiry
 	leave_type_records = frappe.db.get_values("Leave Type", filters={
 			'expire_carry_forwarded_leaves_after_days': (">", 0)
 		}, fieldname=['name'])
 
-	leave_type = [record[0] for record in leave_type_records]
+	leave_type = [record[0] for record in leave_type_records] or ['']
 
-	expired_allocation = frappe.db.sql_list("""SELECT name
+	# fetch non expired leave ledger entry of transaction_type allocation
-		FROM `tabLeave Ledger Entry`
+	expire_allocation = frappe.db.sql("""
-		WHERE
+		SELECT
-			`transaction_type`='Leave Allocation'
+			leaves, to_date, employee, leave_type,
-			AND `is_expired`=1""")
+			is_carry_forward, transaction_name as name, transaction_type
-
+		FROM `tabLeave Ledger Entry` l
-	expire_allocation = frappe.get_all("Leave Ledger Entry",
+		WHERE (NOT EXISTS
-		fields=['leaves', 'to_date', 'employee', 'leave_type', 'is_carry_forward', 'transaction_name as name', 'transaction_type'],
+			(SELECT name
-		filters={
+				FROM `tabLeave Ledger Entry`
-			'to_date': ("<", today()),
+				WHERE
-			'transaction_type': 'Leave Allocation',
+					transaction_name = l.transaction_name
-			'transaction_name': ('not in', expired_allocation)
+					AND transaction_type = 'Leave Allocation'
-		},
+					AND name<>l.name
-		or_filters={
+					AND docstatus = 1
-			'is_carry_forward': 0,
+					AND (
-			'leave_type': ('in', leave_type)
+						is_carry_forward=l.is_carry_forward
-		})
+						OR (is_carry_forward = 0 AND leave_type not in %s)
+			)))
+			AND transaction_type = 'Leave Allocation'
+			AND to_date < %s""", (leave_type, today()), as_dict=1)
 
 	if expire_allocation:
 		create_expiry_ledger_entry(expire_allocation)
@@ -133,6 +141,7 @@ def get_remaining_leaves(allocation):
 			'employee': allocation.employee,
 			'leave_type': allocation.leave_type,
 			'to_date': ('<=', allocation.to_date),
+			'docstatus': 1
 		}, fieldname=['SUM(leaves)'])
 
 @frappe.whitelist()
@@ -159,7 +168,8 @@ def expire_allocation(allocation, expiry_date=None):
 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)
+	leaves_taken = get_leaves_for_period(allocation.employee, allocation.leave_type,
+		allocation.from_date, allocation.to_date, do_not_skip_expired_leaves=True)
 	leaves = flt(allocation.leaves) + flt(leaves_taken)
 
 	# allow expired leaves entry to be created

erpnext/patches.txt

@@ -662,5 +662,6 @@ erpnext.patches.v12_0.set_updated_purpose_in_pick_list
 erpnext.patches.v12_0.repost_stock_ledger_entries_for_target_warehouse
 erpnext.patches.v12_0.update_end_date_and_status_in_email_campaign
 erpnext.patches.v13_0.move_tax_slabs_from_payroll_period_to_income_tax_slab #123
+erpnext.patches.v12_0.remove_duplicate_leave_ledger_entries
 execute:frappe.delete_doc_if_exists("Page", "appointment-analytic")
 erpnext.patches.v12_0.unset_customer_supplier_based_on_type_of_item_price

erpnext/patches/v12_0/remove_duplicate_leave_ledger_entries.py

@@ -0,0 +1,44 @@
+# Copyright (c) 2018, Frappe and Contributors
+# License: GNU General Public License v3. See license.txt
+
+from __future__ import unicode_literals
+import frappe
+
+def execute():
+	"""Delete duplicate leave ledger entries of type allocation created."""
+	if not frappe.db.a_row_exists("Leave Ledger Entry"):
+		return
+
+	duplicate_records_list = get_duplicate_records()
+	delete_duplicate_ledger_entries(duplicate_records_list)
+
+def get_duplicate_records():
+	"""Fetch all but one duplicate records from the list of expired leave allocation."""
+	return frappe.db.sql_list("""
+		WITH duplicate_records AS
+		(SELECT
+			name, transaction_name, is_carry_forward,
+			ROW_NUMBER() over(partition by transaction_name order by creation)as row
+		FROM `tabLeave Ledger Entry` l
+		WHERE (EXISTS
+			(SELECT name
+				FROM `tabLeave Ledger Entry`
+				WHERE
+					transaction_name = l.transaction_name
+					AND transaction_type = 'Leave Allocation'
+					AND name <> l.name
+					AND employee = l.employee
+					AND docstatus = 1
+					AND leave_type = l.leave_type
+					AND is_carry_forward=l.is_carry_forward
+					AND to_date = l.to_date
+					AND from_date = l.from_date
+					AND is_expired = 1
+		)))
+		SELECT name FROM duplicate_records WHERE row > 1
+	""")
+
+def delete_duplicate_ledger_entries(duplicate_records_list):
+	"""Delete duplicate leave ledger entries."""
+	if duplicate_records_list:
+		frappe.db.sql(''' DELETE FROM `tabLeave Ledger Entry` WHERE name in {0}'''.format(tuple(duplicate_records_list))) #nosec