fix(compensatory leave request): Create leave ledger entry on submit/cancel (#19476)

* fix: allow creation of leave ledger entry

* feat(compensatory-leave-request): create leave ledger entry on submit

* style: add descriptive comments

* test: allow creation of leave period based on company

* fix(leave-allocation): check name of leave allocation for determining overlap

* fix: validate new leaves allocated before updating leave allocation

* test: compensatory leave request creation

* fix: skip leave entries for non expired leave allocation

* test: creation of leave ledger entry on creation of compensatory leave request

* fix: minor changes

* fix: fetch leave approver defined in employee in leave application

* fix: attendance creation and leave balance calculation

* test: create leave period for the compensatory off

* style: add descriptive method name

* test: updation of leave allocation on submit

* fix: remove db_set from compensatory off

erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py

@@ -5,9 +5,10 @@
 from __future__ import unicode_literals
 import frappe
 from frappe import _
-from frappe.utils import date_diff, add_days, getdate
+from frappe.utils import date_diff, add_days, getdate, cint
 from frappe.model.document import Document
-from erpnext.hr.utils import validate_dates, validate_overlap, get_leave_period, get_holidays_for_employee
+from erpnext.hr.utils import validate_dates, validate_overlap, get_leave_period, \
+	get_holidays_for_employee, create_additional_leave_ledger_entry
 
 class CompensatoryLeaveRequest(Document):
 
@@ -25,16 +26,14 @@ class CompensatoryLeaveRequest(Document):
 			frappe.throw(_("Leave Type is madatory"))
 
 	def validate_attendance(self):
-		query = """select attendance_date, status
+		attendance = frappe.get_all('Attendance',
-			from `tabAttendance` where
+			filters={
-			attendance_date between %(work_from_date)s and %(work_end_date)s
+				'attendance_date': ['between', (self.work_from_date, self.work_end_date)],
-			and docstatus=1 and status = 'Present' and employee=%(employee)s"""
+				'status': 'Present',
+				'docstatus': 1,
+				'employee': self.employee
+			}, fields=['attendance_date', 'status'])
 
-		attendance = frappe.db.sql(query, {
-			"work_from_date": self.work_from_date,
-			"work_end_date": self.work_end_date,
-			"employee": self.employee
-		}, as_dict=True)
 		if len(attendance) < date_diff(self.work_end_date, self.work_from_date) + 1:
 			frappe.throw(_("You are not present all day(s) between compensatory leave request days"))
 
@@ -50,13 +49,19 @@ class CompensatoryLeaveRequest(Document):
 			date_difference -= 0.5
 		leave_period = get_leave_period(self.work_from_date, self.work_end_date, company)
 		if leave_period:
-			leave_allocation = self.exists_allocation_for_period(leave_period)
+			leave_allocation = self.get_existing_allocation_for_period(leave_period)
 			if leave_allocation:
 				leave_allocation.new_leaves_allocated += date_difference
-				leave_allocation.submit()
+				leave_allocation.validate()
+				leave_allocation.db_set("new_leaves_allocated", leave_allocation.total_leaves_allocated)
+				leave_allocation.db_set("total_leaves_allocated", leave_allocation.total_leaves_allocated)
+
+				# generate additional ledger entry for the new compensatory leaves off
+				create_additional_leave_ledger_entry(leave_allocation, date_difference, add_days(self.work_end_date, 1))
+
 			else:
 				leave_allocation = self.create_leave_allocation(leave_period, date_difference)
-			self.db_set("leave_allocation", leave_allocation.name)
+			self.leave_allocation=leave_allocation.name
 		else:
 			frappe.throw(_("There is no leave period in between {0} and {1}").format(self.work_from_date, self.work_end_date))
 
@@ -68,11 +73,16 @@ class CompensatoryLeaveRequest(Document):
 			leave_allocation = frappe.get_doc("Leave Allocation", self.leave_allocation)
 			if leave_allocation:
 				leave_allocation.new_leaves_allocated -= date_difference
-				if leave_allocation.total_leaves_allocated - date_difference <= 0:
+				if leave_allocation.new_leaves_allocated - date_difference <= 0:
-					leave_allocation.total_leaves_allocated = 0
+					leave_allocation.new_leaves_allocated = 0
-				leave_allocation.submit()
+				leave_allocation.validate()
+				leave_allocation.db_set("new_leaves_allocated", leave_allocation.total_leaves_allocated)
+				leave_allocation.db_set("total_leaves_allocated", leave_allocation.total_leaves_allocated)
 
-	def exists_allocation_for_period(self, leave_period):
+				# create reverse entry on cancelation
+				create_additional_leave_ledger_entry(leave_allocation, date_difference * -1, add_days(self.work_end_date, 1))
+
+	def get_existing_allocation_for_period(self, leave_period):
 		leave_allocation = frappe.db.sql("""
 			select name
 			from `tabLeave Allocation`
@@ -95,17 +105,18 @@ class CompensatoryLeaveRequest(Document):
 
 	def create_leave_allocation(self, leave_period, date_difference):
 		is_carry_forward = frappe.db.get_value("Leave Type", self.leave_type, "is_carry_forward")
-		allocation = frappe.new_doc("Leave Allocation")
+		allocation = frappe.get_doc(dict(
-		allocation.employee = self.employee
+			doctype="Leave Allocation",
-		allocation.employee_name = self.employee_name
+			employee=self.employee,
-		allocation.leave_type = self.leave_type
+			employee_name=self.employee_name,
-		allocation.from_date = add_days(self.work_end_date, 1)
+			leave_type=self.leave_type,
-		allocation.to_date = leave_period[0].to_date
+			from_date=add_days(self.work_end_date, 1),
-		allocation.new_leaves_allocated = date_difference
+			to_date=leave_period[0].to_date,
-		allocation.total_leaves_allocated = date_difference
+			carry_forward=cint(is_carry_forward),
-		allocation.description = self.reason
+			new_leaves_allocated=date_difference,
-		if is_carry_forward == 1:
+			total_leaves_allocated=date_difference,
-			allocation.carry_forward = True
+			description=self.reason
-		allocation.save(ignore_permissions = True)
+		))
+		allocation.insert(ignore_permissions=True)
 		allocation.submit()
-		return allocation
+		return allocation

erpnext/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py

@@ -5,37 +5,128 @@ from __future__ import unicode_literals
 
 import frappe
 import unittest
+from frappe.utils import today, add_months, add_days
+from erpnext.hr.doctype.attendance_request.test_attendance_request import get_employee
+from erpnext.hr.doctype.leave_period.test_leave_period import create_leave_period
+from erpnext.hr.doctype.leave_application.leave_application import get_leave_balance_on
 
-# class TestCompensatoryLeaveRequest(unittest.TestCase):
+class TestCompensatoryLeaveRequest(unittest.TestCase):
-# 	def get_compensatory_leave_request(self):
+	def setUp(self):
-# 		return frappe.get_doc('Compensatory Leave Request', dict(
+		frappe.db.sql(''' delete from `tabCompensatory Leave Request`''')
-# 			employee = employee,
+		frappe.db.sql(''' delete from `tabLeave Ledger Entry`''')
-# 			work_from_date = today,
+		frappe.db.sql(''' delete from `tabLeave Allocation`''')
-# 			work_to_date = today,
+		frappe.db.sql(''' delete from `tabAttendance` where attendance_date in {0} '''.format((today(), add_days(today(), -1)))) #nosec
-# 			reason = 'test'
+		create_leave_period(add_months(today(), -3), add_months(today(), 3), "_Test Company")
-# 		)).insert()
+		create_holiday_list()
-#
+
-# 	def test_creation_of_leave_allocation(self):
+		employee = get_employee()
-# 		employee = get_employee()
+		employee.holiday_list = "_Test Compensatory Leave"
-# 		today = get_today()
+		employee.save()
-#
+
-# 		compensatory_leave_request = self.get_compensatory_leave_request(today)
+	def test_leave_balance_on_submit(self):
-#
+		''' check creation of leave allocation on submission of compensatory leave request '''
-# 		before = get_leave_balance(employee, compensatory_leave_request.leave_type)
+		employee = get_employee()
-#
+		mark_attendance(employee)
-# 		compensatory_leave_request.submit()
+		compensatory_leave_request = get_compensatory_leave_request(employee.name)
-#
+
-# 		self.assertEqual(get_leave_balance(employee, compensatory_leave_request.leave_type), before + 1)
+		before = get_leave_balance_on(employee.name, compensatory_leave_request.leave_type, today())
-#
+		compensatory_leave_request.submit()
-# 	def test_max_compensatory_leave(self):
+
-# 		employee = get_employee()
+		self.assertEqual(get_leave_balance_on(employee.name, compensatory_leave_request.leave_type, add_days(today(), 1)), before + 1)
-# 		today = get_today()
+
-#
+	def test_leave_allocation_update_on_submit(self):
-# 		compensatory_leave_request = self.get_compensatory_leave_request()
+		employee = get_employee()
-#
+		mark_attendance(employee, date=add_days(today(), -1))
-# 		frappe.db.set_value('Leave Type', compensatory_leave_request.leave_type, 'max_leaves_allowed', 0)
+		compensatory_leave_request = get_compensatory_leave_request(employee.name, leave_date=add_days(today(), -1))
-#
+		compensatory_leave_request.submit()
-# 		self.assertRaises(MaxLeavesLimitCrossed, compensatory_leave_request.submit)
+
-#
+		# leave allocation creation on submit
-# 		frappe.db.set_value('Leave Type', compensatory_leave_request.leave_type, 'max_leaves_allowed', 10)
+		leaves_allocated = frappe.db.get_value('Leave Allocation', {
-#
+				'name': compensatory_leave_request.leave_allocation
+			}, ['total_leaves_allocated'])
+		self.assertEqual(leaves_allocated, 1)
+
+		mark_attendance(employee)
+		compensatory_leave_request = get_compensatory_leave_request(employee.name)
+		compensatory_leave_request.submit()
+
+		# leave allocation updates on submission of second compensatory leave request
+		leaves_allocated = frappe.db.get_value('Leave Allocation', {
+				'name': compensatory_leave_request.leave_allocation
+			}, ['total_leaves_allocated'])
+		self.assertEqual(leaves_allocated, 2)
+
+	def test_creation_of_leave_ledger_entry_on_submit(self):
+		''' check creation of leave ledger entry on submission of leave request '''
+		employee = get_employee()
+		mark_attendance(employee)
+		compensatory_leave_request = get_compensatory_leave_request(employee.name)
+		compensatory_leave_request.submit()
+
+		filters = dict(transaction_name=compensatory_leave_request.leave_allocation)
+		leave_ledger_entry = frappe.get_all('Leave Ledger Entry', fields='*', filters=filters)
+
+		self.assertEquals(len(leave_ledger_entry), 1)
+		self.assertEquals(leave_ledger_entry[0].employee, compensatory_leave_request.employee)
+		self.assertEquals(leave_ledger_entry[0].leave_type, compensatory_leave_request.leave_type)
+		self.assertEquals(leave_ledger_entry[0].leaves, 1)
+
+		# check reverse leave ledger entry on cancellation
+		compensatory_leave_request.cancel()
+		leave_ledger_entry = frappe.get_all('Leave Ledger Entry', fields='*', filters=filters, order_by = 'creation desc')
+
+		self.assertEquals(len(leave_ledger_entry), 2)
+		self.assertEquals(leave_ledger_entry[0].employee, compensatory_leave_request.employee)
+		self.assertEquals(leave_ledger_entry[0].leave_type, compensatory_leave_request.leave_type)
+		self.assertEquals(leave_ledger_entry[0].leaves, -1)
+
+def get_compensatory_leave_request(employee, leave_date=today()):
+	prev_comp_leave_req = frappe.db.get_value('Compensatory Leave Request',
+		dict(leave_type='Compensatory Off',
+			work_from_date=leave_date,
+			work_end_date=leave_date,
+			employee=employee), 'name')
+	if prev_comp_leave_req:
+		return frappe.get_doc('Compensatory Leave Request', prev_comp_leave_req)
+
+	return frappe.get_doc(dict(
+			doctype='Compensatory Leave Request',
+			employee=employee,
+			leave_type='Compensatory Off',
+			work_from_date=leave_date,
+			work_end_date=leave_date,
+			reason='test'
+		)).insert()
+
+def mark_attendance(employee, date=today(), status='Present'):
+	if not frappe.db.exists(dict(doctype='Attendance', employee=employee.name, attendance_date=date, status='Present')):
+		attendance = frappe.get_doc({
+				"doctype": "Attendance",
+				"employee": employee.name,
+				"attendance_date": date,
+				"status": status
+		})
+		attendance.save()
+		attendance.submit()
+
+def create_holiday_list():
+	if frappe.db.exists("Holiday List", "_Test Compensatory Leave"):
+		return
+
+	holiday_list = frappe.get_doc({
+		"doctype": "Holiday List",
+		"from_date": add_months(today(), -3),
+		"to_date": add_months(today(), 3),
+		"holidays": [
+			{
+				"description": "Test Holiday",
+				"holiday_date": today()
+			},
+			{
+				"description": "Test Holiday 1",
+				"holiday_date": add_days(today(), -1)
+			}
+		],
+		"holiday_list_name": "_Test Compensatory Leave"
+	})
+	holiday_list.save()

erpnext/hr/doctype/leave_allocation/leave_allocation.py

@@ -69,10 +69,14 @@ class LeaveAllocation(Document):
 
 	def validate_allocation_overlap(self):
 		leave_allocation = frappe.db.sql("""
-			select name from `tabLeave Allocation`
+			SELECT
-			where employee=%s and leave_type=%s and docstatus=1
+				name
-			and to_date >= %s and from_date <= %s""",
+			FROM `tabLeave Allocation`
-			(self.employee, self.leave_type, self.from_date, self.to_date))
+			WHERE
+				employee=%s AND leave_type=%s
+				AND name <> %s AND docstatus=1
+				AND to_date >= %s AND from_date <= %s""",
+			(self.employee, self.leave_type, self.name, self.from_date, self.to_date))
 
 		if leave_allocation:
 			frappe.msgprint(_("{0} already allocated for Employee {1} for period {2} to {3}")

erpnext/hr/doctype/leave_application/leave_application.js

@@ -170,7 +170,7 @@ frappe.ui.form.on("Leave Application", {
 				frm.set_value('to_date', '');
 				return;
 			}
-				// server call is done to include holidays in leave days calculations
+			// server call is done to include holidays in leave days calculations
 			return frappe.call({
 				method: 'erpnext.hr.doctype.leave_application.leave_application.get_number_of_leave_days',
 				args: {
@@ -193,7 +193,7 @@ frappe.ui.form.on("Leave Application", {
 
 	set_leave_approver: function(frm) {
 		if(frm.doc.employee) {
-				// server call is done to include holidays in leave days calculations
+			// server call is done to include holidays in leave days calculations
 			return frappe.call({
 				method: 'erpnext.hr.doctype.leave_application.leave_application.get_leave_approver',
 				args: {

erpnext/hr/doctype/leave_application/leave_application.py

@@ -549,10 +549,10 @@ def get_leaves_for_period(employee, leave_type, from_date, to_date):
 			leave_days += leave_entry.leaves
 
 		elif inclusive_period and leave_entry.transaction_type == 'Leave Allocation' \
-			and not skip_expiry_leaves(leave_entry, to_date):
+			and leave_entry.is_expired and not skip_expiry_leaves(leave_entry, to_date):
 			leave_days += leave_entry.leaves
 
-		else:
+		elif leave_entry.transaction_type == 'Leave Application':
 			if leave_entry.from_date < getdate(from_date):
 				leave_entry.from_date = from_date
 			if leave_entry.to_date > getdate(to_date):
@@ -579,14 +579,15 @@ def skip_expiry_leaves(leave_entry, date):
 def get_leave_entries(employee, leave_type, from_date, to_date):
 	''' Returns leave entries between from_date and to_date '''
 	return frappe.db.sql("""
-		select employee, leave_type, from_date, to_date, leaves, transaction_type, is_carry_forward, transaction_name
+		SELECT
-		from `tabLeave Ledger Entry`
+			employee, leave_type, from_date, to_date, leaves, transaction_name, transaction_type,
-		where employee=%(employee)s and leave_type=%(leave_type)s
+			is_carry_forward, is_expired
-			and docstatus=1
+		FROM `tabLeave Ledger Entry`
-			and leaves<0
+		WHERE employee=%(employee)s AND leave_type=%(leave_type)s
-			and (from_date between %(from_date)s and %(to_date)s
+			AND docstatus=1 AND leaves<0
-				or to_date between %(from_date)s and %(to_date)s
+			AND (from_date between %(from_date)s AND %(to_date)s
-				or (from_date < %(from_date)s and to_date > %(to_date)s))
+				OR to_date between %(from_date)s AND %(to_date)s
+				OR (from_date < %(from_date)s AND to_date > %(to_date)s))
 	""", {
 		"from_date": from_date,
 		"to_date": to_date,
@@ -773,4 +774,4 @@ def get_leave_approver(employee):
 		leave_approver = frappe.db.get_value('Department Approver', {'parent': department,
 			'parentfield': 'leave_approvers', 'idx': 1}, 'approver')
 
-	return leave_approver
+	return leave_approver

erpnext/hr/doctype/leave_application/test_leave_application.py

@@ -301,7 +301,7 @@ class TestLeaveApplication(unittest.TestCase):
 			to_date = add_days(date, 2),
 			company = "_Test Company",
 			docstatus = 1,
-            status = "Approved"
+			status = "Approved"
 		))
 		leave_application.submit()
 
@@ -314,7 +314,7 @@ class TestLeaveApplication(unittest.TestCase):
 			to_date = add_days(date, 8),
 			company = "_Test Company",
 			docstatus = 1,
-            status = "Approved"
+			status = "Approved"
 		))
 		self.assertRaises(frappe.ValidationError, leave_application.insert)
 

erpnext/hr/doctype/leave_period/test_leave_period.py

@@ -43,10 +43,18 @@ class TestLeavePeriod(unittest.TestCase):
 		leave_period.grant_leave_allocation(employee=employee_doc_name)
 		self.assertEqual(get_leave_balance_on(employee_doc_name, leave_type, today()), 20)
 
-def create_leave_period(from_date, to_date):
+def create_leave_period(from_date, to_date, company=None):
+	leave_period = frappe.db.get_value('Leave Period',
+		dict(company=company or erpnext.get_default_company(),
+			from_date=from_date,
+			to_date=to_date,
+			is_active=1), 'name')
+	if leave_period:
+		return frappe.get_doc("Leave Period", leave_period)
+
 	leave_period = frappe.get_doc({
 		"doctype": "Leave Period",
-		"company": erpnext.get_default_company(),
+		"company": company or erpnext.get_default_company(),
 		"from_date": from_date,
 		"to_date": to_date,
 		"is_active": 1

erpnext/hr/utils.py

@@ -321,11 +321,11 @@ def allocate_earned_leaves():
 				if new_allocation == allocation.total_leaves_allocated:
 					continue
 				allocation.db_set("total_leaves_allocated", new_allocation, update_modified=False)
-				create_earned_leave_ledger_entry(allocation, earned_leaves, today)
+				create_additional_leave_ledger_entry(allocation, earned_leaves, today)
 
-def create_earned_leave_ledger_entry(allocation, earned_leaves, date):
+def create_additional_leave_ledger_entry(allocation, leaves, date):
-	''' Create leave ledger entry based on the earned leave frequency '''
+	''' Create leave ledger entry for leave types '''
-	allocation.new_leaves_allocated = earned_leaves
+	allocation.new_leaves_allocated = leaves
 	allocation.from_date = date
 	allocation.unused_leaves = 0
 	allocation.create_leave_ledger_entry()
@@ -389,6 +389,7 @@ def get_sal_slip_total_benefit_given(employee, payroll_period, component=False):
 
 def get_holidays_for_employee(employee, start_date, end_date):
 	holiday_list = get_holiday_list_for_employee(employee)
+
 	holidays = frappe.db.sql_list('''select holiday_date from `tabHoliday`
 		where
 			parent=%(holiday_list)s