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
diff --git a/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py b/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py
index bc4a1b4..7a9727f 100644
--- a/erpnext/hr/doctype/compensatory_leave_request/compensatory_leave_request.py
+++ b/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 @@
frappe.throw(_("Leave Type is madatory"))
def validate_attendance(self):
- query = """select attendance_date, status
- from `tabAttendance` where
- attendance_date between %(work_from_date)s and %(work_end_date)s
- and docstatus=1 and status = 'Present' and employee=%(employee)s"""
+ attendance = frappe.get_all('Attendance',
+ filters={
+ 'attendance_date': ['between', (self.work_from_date, self.work_end_date)],
+ '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 @@
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 @@
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:
- leave_allocation.total_leaves_allocated = 0
- leave_allocation.submit()
+ if leave_allocation.new_leaves_allocated - date_difference <= 0:
+ leave_allocation.new_leaves_allocated = 0
+ 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 @@
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.employee = self.employee
- allocation.employee_name = self.employee_name
- allocation.leave_type = self.leave_type
- allocation.from_date = add_days(self.work_end_date, 1)
- allocation.to_date = leave_period[0].to_date
- allocation.new_leaves_allocated = date_difference
- allocation.total_leaves_allocated = date_difference
- allocation.description = self.reason
- if is_carry_forward == 1:
- allocation.carry_forward = True
- allocation.save(ignore_permissions = True)
+ allocation = frappe.get_doc(dict(
+ doctype="Leave Allocation",
+ employee=self.employee,
+ employee_name=self.employee_name,
+ leave_type=self.leave_type,
+ from_date=add_days(self.work_end_date, 1),
+ to_date=leave_period[0].to_date,
+ carry_forward=cint(is_carry_forward),
+ new_leaves_allocated=date_difference,
+ total_leaves_allocated=date_difference,
+ description=self.reason
+ ))
+ allocation.insert(ignore_permissions=True)
allocation.submit()
- return allocation
+ return allocation
\ No newline at end of file
diff --git a/erpnext/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py b/erpnext/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py
index f2ca1f4..1615ab3 100644
--- a/erpnext/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py
+++ b/erpnext/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py
@@ -5,37 +5,128 @@
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):
-# def get_compensatory_leave_request(self):
-# return frappe.get_doc('Compensatory Leave Request', dict(
-# employee = employee,
-# work_from_date = today,
-# work_to_date = today,
-# reason = 'test'
-# )).insert()
-#
-# def test_creation_of_leave_allocation(self):
-# employee = get_employee()
-# today = get_today()
-#
-# compensatory_leave_request = self.get_compensatory_leave_request(today)
-#
-# before = get_leave_balance(employee, compensatory_leave_request.leave_type)
-#
-# compensatory_leave_request.submit()
-#
-# self.assertEqual(get_leave_balance(employee, compensatory_leave_request.leave_type), before + 1)
-#
-# def test_max_compensatory_leave(self):
-# employee = get_employee()
-# today = get_today()
-#
-# compensatory_leave_request = self.get_compensatory_leave_request()
-#
-# frappe.db.set_value('Leave Type', compensatory_leave_request.leave_type, 'max_leaves_allowed', 0)
-#
-# self.assertRaises(MaxLeavesLimitCrossed, compensatory_leave_request.submit)
-#
-# frappe.db.set_value('Leave Type', compensatory_leave_request.leave_type, 'max_leaves_allowed', 10)
-#
+class TestCompensatoryLeaveRequest(unittest.TestCase):
+ def setUp(self):
+ frappe.db.sql(''' delete from `tabCompensatory Leave Request`''')
+ frappe.db.sql(''' delete from `tabLeave Ledger Entry`''')
+ frappe.db.sql(''' delete from `tabLeave Allocation`''')
+ frappe.db.sql(''' delete from `tabAttendance` where attendance_date in {0} '''.format((today(), add_days(today(), -1)))) #nosec
+ create_leave_period(add_months(today(), -3), add_months(today(), 3), "_Test Company")
+ create_holiday_list()
+
+ employee = get_employee()
+ employee.holiday_list = "_Test Compensatory Leave"
+ employee.save()
+
+ def test_leave_balance_on_submit(self):
+ ''' check creation of leave allocation on submission of compensatory leave request '''
+ employee = get_employee()
+ mark_attendance(employee)
+ compensatory_leave_request = get_compensatory_leave_request(employee.name)
+
+ before = get_leave_balance_on(employee.name, compensatory_leave_request.leave_type, today())
+ compensatory_leave_request.submit()
+
+ self.assertEqual(get_leave_balance_on(employee.name, compensatory_leave_request.leave_type, add_days(today(), 1)), before + 1)
+
+ def test_leave_allocation_update_on_submit(self):
+ employee = get_employee()
+ mark_attendance(employee, date=add_days(today(), -1))
+ compensatory_leave_request = get_compensatory_leave_request(employee.name, leave_date=add_days(today(), -1))
+ compensatory_leave_request.submit()
+
+ # leave allocation creation on submit
+ 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()
\ No newline at end of file
diff --git a/erpnext/hr/doctype/leave_allocation/leave_allocation.py b/erpnext/hr/doctype/leave_allocation/leave_allocation.py
index 874ae7a..d13bb45 100755
--- a/erpnext/hr/doctype/leave_allocation/leave_allocation.py
+++ b/erpnext/hr/doctype/leave_allocation/leave_allocation.py
@@ -69,10 +69,14 @@
def validate_allocation_overlap(self):
leave_allocation = frappe.db.sql("""
- select name from `tabLeave Allocation`
- where employee=%s and leave_type=%s and docstatus=1
- and to_date >= %s and from_date <= %s""",
- (self.employee, self.leave_type, self.from_date, self.to_date))
+ SELECT
+ name
+ FROM `tabLeave Allocation`
+ 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}")
diff --git a/erpnext/hr/doctype/leave_application/leave_application.js b/erpnext/hr/doctype/leave_application/leave_application.js
index db3819e..e32d570 100755
--- a/erpnext/hr/doctype/leave_application/leave_application.js
+++ b/erpnext/hr/doctype/leave_application/leave_application.js
@@ -170,7 +170,7 @@
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 @@
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: {
diff --git a/erpnext/hr/doctype/leave_application/leave_application.py b/erpnext/hr/doctype/leave_application/leave_application.py
index 0e66305..65fcbf7 100755
--- a/erpnext/hr/doctype/leave_application/leave_application.py
+++ b/erpnext/hr/doctype/leave_application/leave_application.py
@@ -549,10 +549,10 @@
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 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
- from `tabLeave Ledger Entry`
- where employee=%(employee)s and leave_type=%(leave_type)s
- and docstatus=1
- and leaves<0
- and (from_date between %(from_date)s and %(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))
+ SELECT
+ employee, leave_type, from_date, to_date, leaves, transaction_name, transaction_type,
+ is_carry_forward, is_expired
+ FROM `tabLeave Ledger Entry`
+ WHERE employee=%(employee)s AND leave_type=%(leave_type)s
+ AND docstatus=1 AND leaves<0
+ AND (from_date between %(from_date)s AND %(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 @@
leave_approver = frappe.db.get_value('Department Approver', {'parent': department,
'parentfield': 'leave_approvers', 'idx': 1}, 'approver')
- return leave_approver
+ return leave_approver
\ No newline at end of file
diff --git a/erpnext/hr/doctype/leave_application/test_leave_application.py b/erpnext/hr/doctype/leave_application/test_leave_application.py
index 38ae808..b9c0210 100644
--- a/erpnext/hr/doctype/leave_application/test_leave_application.py
+++ b/erpnext/hr/doctype/leave_application/test_leave_application.py
@@ -301,7 +301,7 @@
to_date = add_days(date, 2),
company = "_Test Company",
docstatus = 1,
- status = "Approved"
+ status = "Approved"
))
leave_application.submit()
@@ -314,7 +314,7 @@
to_date = add_days(date, 8),
company = "_Test Company",
docstatus = 1,
- status = "Approved"
+ status = "Approved"
))
self.assertRaises(frappe.ValidationError, leave_application.insert)
diff --git a/erpnext/hr/doctype/leave_period/test_leave_period.py b/erpnext/hr/doctype/leave_period/test_leave_period.py
index 850a08d..1762cf9 100644
--- a/erpnext/hr/doctype/leave_period/test_leave_period.py
+++ b/erpnext/hr/doctype/leave_period/test_leave_period.py
@@ -43,10 +43,18 @@
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
diff --git a/erpnext/hr/utils.py b/erpnext/hr/utils.py
index 1464a77..c3e8d27 100644
--- a/erpnext/hr/utils.py
+++ b/erpnext/hr/utils.py
@@ -321,11 +321,11 @@
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):
- ''' Create leave ledger entry based on the earned leave frequency '''
- allocation.new_leaves_allocated = earned_leaves
+def create_additional_leave_ledger_entry(allocation, leaves, date):
+ ''' Create leave ledger entry for leave types '''
+ allocation.new_leaves_allocated = leaves
allocation.from_date = date
allocation.unused_leaves = 0
allocation.create_leave_ledger_entry()
@@ -389,6 +389,7 @@
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