fix: honour 'include holidays' setting while marking attendance for leave application (#29425)

erpnext/hr/doctype/leave_application/leave_application.py

@@ -22,6 +22,7 @@ from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
 from erpnext.hr.doctype.leave_block_list.leave_block_list import get_applicable_block_dates
 from erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry import create_leave_ledger_entry
 from erpnext.hr.utils import (
+	get_holiday_dates_for_employee,
 	get_leave_period,
 	set_employee_name,
 	share_doc_with_approver,
@@ -159,20 +160,44 @@ class LeaveApplication(Document):
 				.format(formatdate(future_allocation[0].from_date), future_allocation[0].name))
 
 	def update_attendance(self):
-		if self.status == "Approved":
+		if self.status != "Approved":
+			return
+
+		holiday_dates = []
+		if not frappe.db.get_value("Leave Type", self.leave_type, "include_holiday"):
+			holiday_dates = get_holiday_dates_for_employee(self.employee, self.from_date, self.to_date)
+
 		for dt in daterange(getdate(self.from_date), getdate(self.to_date)):
 			date = dt.strftime("%Y-%m-%d")
-				status = "Half Day" if self.half_day_date and getdate(date) == getdate(self.half_day_date) else "On Leave"
+			attendance_name = frappe.db.exists("Attendance", dict(employee = self.employee,
-				attendance_name = frappe.db.exists('Attendance', dict(employee = self.employee,
 				attendance_date = date, docstatus = ('!=', 2)))
 
+			# don't mark attendance for holidays
+			# if leave type does not include holidays within leaves as leaves
+			if date in holiday_dates:
+				if attendance_name:
+					# cancel and delete existing attendance for holidays
+					attendance = frappe.get_doc("Attendance", attendance_name)
+					attendance.flags.ignore_permissions = True
+					if attendance.docstatus == 1:
+						attendance.cancel()
+					frappe.delete_doc("Attendance", attendance_name, force=1)
+				continue
+
+			self.create_or_update_attendance(attendance_name, date)
+
+	def create_or_update_attendance(self, attendance_name, date):
+		status = "Half Day" if self.half_day_date and getdate(date) == getdate(self.half_day_date) else "On Leave"
+
 		if attendance_name:
 			# update existing attendance, change absent to on leave
 			doc = frappe.get_doc('Attendance', attendance_name)
 			if doc.status != status:
-						doc.db_set('status', status)
+				doc.db_set({
-						doc.db_set('leave_type', self.leave_type)
+					'status': status,
-						doc.db_set('leave_application', self.name)
+					'leave_type': self.leave_type,
+					'leave_application': self.name
+				})
 		else:
 			# make new attendance and submit it
 			doc = frappe.new_doc("Attendance")

erpnext/hr/doctype/leave_application/test_leave_application.py

@@ -5,7 +5,16 @@ import unittest
 
 import frappe
 from frappe.permissions import clear_user_permissions_for_doctype
-from frappe.utils import add_days, add_months, getdate, nowdate
+from frappe.utils import (
+	add_days,
+	add_months,
+	get_first_day,
+	get_last_day,
+	get_year_ending,
+	get_year_start,
+	getdate,
+	nowdate,
+)
 
 from erpnext.hr.doctype.employee.test_employee import make_employee
 from erpnext.hr.doctype.leave_allocation.test_leave_allocation import create_leave_allocation
@@ -19,6 +28,10 @@ from erpnext.hr.doctype.leave_policy_assignment.leave_policy_assignment import (
 	create_assignment_for_multiple_employees,
 )
 from erpnext.hr.doctype.leave_type.test_leave_type import create_leave_type
+from erpnext.payroll.doctype.salary_slip.test_salary_slip import (
+	make_holiday_list,
+	make_leave_application,
+)
 
 test_dependencies = ["Leave Allocation", "Leave Block List", "Employee"]
 
@@ -61,13 +74,15 @@ class TestLeaveApplication(unittest.TestCase):
 		for dt in ["Leave Application", "Leave Allocation", "Salary Slip", "Leave Ledger Entry"]:
 			frappe.db.sql("DELETE FROM `tab%s`" % dt) #nosec
 
+		frappe.set_user("Administrator")
+
 	@classmethod
 	def setUpClass(cls):
 		set_leave_approver()
 		frappe.db.sql("delete from tabAttendance where employee='_T-Employee-00001'")
 
 	def tearDown(self):
-		frappe.set_user("Administrator")
+		frappe.db.rollback()
 
 	def _clear_roles(self):
 		frappe.db.sql("""delete from `tabHas Role` where parent in
@@ -106,6 +121,72 @@ class TestLeaveApplication(unittest.TestCase):
 		for d in ('2018-01-01', '2018-01-02', '2018-01-03'):
 			self.assertTrue(getdate(d) in dates)
 
+	def test_attendance_for_include_holidays(self):
+		# Case 1: leave type with 'Include holidays within leaves as leaves' enabled
+		frappe.delete_doc_if_exists("Leave Type", "Test Include Holidays", force=1)
+		leave_type = frappe.get_doc(dict(
+			leave_type_name="Test Include Holidays",
+			doctype="Leave Type",
+			include_holiday=True
+		)).insert()
+
+		date = getdate()
+		make_allocation_record(leave_type=leave_type.name, from_date=get_year_start(date), to_date=get_year_ending(date))
+
+		holiday_list = make_holiday_list()
+		frappe.db.set_value("Company", "_Test Company", "default_holiday_list", holiday_list)
+		first_sunday = get_first_sunday(holiday_list)
+
+		leave_application = make_leave_application("_T-Employee-00001", first_sunday, add_days(first_sunday, 3), leave_type.name)
+		leave_application.reload()
+		self.assertEqual(leave_application.total_leave_days, 4)
+		self.assertEqual(frappe.db.count('Attendance', {'leave_application': leave_application.name}), 4)
+
+		leave_application.cancel()
+
+	def test_attendance_update_for_exclude_holidays(self):
+		# Case 2: leave type with 'Include holidays within leaves as leaves' disabled
+		frappe.delete_doc_if_exists("Leave Type", "Test Do Not Include Holidays", force=1)
+		leave_type = frappe.get_doc(dict(
+			leave_type_name="Test Do Not Include Holidays",
+			doctype="Leave Type",
+			include_holiday=False
+		)).insert()
+
+		date = getdate()
+		make_allocation_record(leave_type=leave_type.name, from_date=get_year_start(date), to_date=get_year_ending(date))
+
+		holiday_list = make_holiday_list()
+		frappe.db.set_value("Company", "_Test Company", "default_holiday_list", holiday_list)
+		first_sunday = get_first_sunday(holiday_list)
+
+		# already marked attendance on a holiday should be deleted in this case
+		config = {
+			"doctype": "Attendance",
+			"employee": "_T-Employee-00001",
+			"status": "Present"
+		}
+		attendance_on_holiday = frappe.get_doc(config)
+		attendance_on_holiday.attendance_date = first_sunday
+		attendance_on_holiday.save()
+
+		# already marked attendance on a non-holiday should be updated
+		attendance = frappe.get_doc(config)
+		attendance.attendance_date = add_days(first_sunday, 3)
+		attendance.save()
+
+		leave_application = make_leave_application("_T-Employee-00001", first_sunday, add_days(first_sunday, 3), leave_type.name)
+		leave_application.reload()
+		# holiday should be excluded while marking attendance
+		self.assertEqual(leave_application.total_leave_days, 3)
+		self.assertEqual(frappe.db.count("Attendance", {"leave_application": leave_application.name}), 3)
+
+		# attendance on holiday deleted
+		self.assertFalse(frappe.db.exists("Attendance", attendance_on_holiday.name))
+
+		# attendance on non-holiday updated
+		self.assertEqual(frappe.db.get_value("Attendance", attendance.name, "status"), "On Leave")
+
 	def test_block_list(self):
 		self._clear_roles()
 
@@ -241,7 +322,13 @@ class TestLeaveApplication(unittest.TestCase):
 		leave_period = get_leave_period()
 		today = nowdate()
 		holiday_list = 'Test Holiday List for Optional Holiday'
-		optional_leave_date = add_days(today, 7)
+		employee = get_employee()
+
+		default_holiday_list = make_holiday_list()
+		frappe.db.set_value("Company", "_Test Company", "default_holiday_list", default_holiday_list)
+		first_sunday = get_first_sunday(default_holiday_list)
+
+		optional_leave_date = add_days(first_sunday, 1)
 
 		if not frappe.db.exists('Holiday List', holiday_list):
 			frappe.get_doc(dict(
@@ -253,7 +340,6 @@ class TestLeaveApplication(unittest.TestCase):
 					dict(holiday_date = optional_leave_date, description = 'Test')
 				]
 			)).insert()
-		employee = get_employee()
 
 		frappe.db.set_value('Leave Period', leave_period.name, 'optional_holiday_list', holiday_list)
 		leave_type = 'Test Optional Type'
@@ -266,7 +352,7 @@ class TestLeaveApplication(unittest.TestCase):
 
 		allocate_leaves(employee, leave_period, leave_type, 10)
 
-		date = add_days(today, 6)
+		date = add_days(first_sunday, 2)
 
 		leave_application = frappe.get_doc(dict(
 			doctype = 'Leave Application',
@@ -637,13 +723,13 @@ def create_carry_forwarded_allocation(employee, leave_type):
 			carry_forward=1)
 		leave_allocation.submit()
 
-def make_allocation_record(employee=None, leave_type=None):
+def make_allocation_record(employee=None, leave_type=None, from_date=None, to_date=None):
 	allocation = frappe.get_doc({
 		"doctype": "Leave Allocation",
 		"employee": employee or "_T-Employee-00001",
 		"leave_type": leave_type or "_Test Leave Type",
-		"from_date": "2013-01-01",
+		"from_date": from_date or "2013-01-01",
-		"to_date": "2019-12-31",
+		"to_date": to_date or "2019-12-31",
 		"new_leaves_allocated": 30
 	})
 
@@ -692,3 +778,16 @@ def allocate_leaves(employee, leave_period, leave_type, new_leaves_allocated, el
 	}).insert()
 
 	allocate_leave.submit()
+
+
+def get_first_sunday(holiday_list):
+	month_start_date = get_first_day(nowdate())
+	month_end_date = get_last_day(nowdate())
+	first_sunday = frappe.db.sql("""
+		select holiday_date from `tabHoliday`
+		where parent = %s
+			and holiday_date between %s and %s
+		order by holiday_date
+	""", (holiday_list, month_start_date, month_end_date))[0][0]
+
+	return first_sunday

erpnext/payroll/doctype/salary_slip/test_salary_slip.py

@@ -994,6 +994,8 @@ def make_leave_application(employee, from_date, to_date, leave_type, company=Non
 	))
 	leave_application.submit()
 
+	return leave_application
+
 def setup_test():
 	make_earning_salary_component(setup=True, company_list=["_Test Company"])
 	make_deduction_salary_component(setup=True, company_list=["_Test Company"])