From 4f114b62e248efad1a5e9feca4ff25d5b578d7d5 Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Fri, 13 Feb 2026 16:07:36 +0530 Subject: [PATCH 1/9] fix(leave-application): prevent half-day selection on holidays Closes #3331 --- .../leave_application/leave_application.js | 25 ++++++- .../leave_application/leave_application.py | 18 ++++- .../test_leave_application.py | 73 +++++++++++++++---- 3 files changed, 99 insertions(+), 17 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index 4a0bb09f34..195921eb8c 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -168,6 +168,7 @@ frappe.ui.form.on("Leave Application", { }, half_day_date(frm) { + frm.trigger("validate_half_day_date"); frm.trigger("calculate_total_days"); }, @@ -229,7 +230,29 @@ frappe.ui.form.on("Leave Application", { }); } }, - + validate_half_day_date: function (frm) { + if (frm.doc.half_day_date) { + return frappe.call({ + method: "hrms.hr.doctype.leave_application.leave_application.get_half_day_validation", + args: { + employee: frm.doc.employee, + from_date: frm.doc.from_date, + to_date: frm.doc.to_date, + half_day_date: frm.doc.half_day_date, + }, + callback: function (r) { + if (r && r.message) { + frm.trigger("calculate_total_days"); + } else { + frm.set_value("half_day_date", ""); + frappe.msgprint( + __("Half Day Date must be between From Date and To Date and should not be a holiday."), + ); + } + } + }); + } + }, calculate_total_days: function (frm) { if (frm.doc.from_date && frm.doc.to_date && frm.doc.employee && frm.doc.leave_type) { // server call is done to include holidays in leave days calculations diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index 3560660ead..ba7911844e 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -22,6 +22,7 @@ from erpnext.buying.doctype.supplier_scorecard.supplier_scorecard import daterange from erpnext.setup.doctype.employee.employee import get_holiday_list_for_employee +from erpnext.setup.doctype.employee.employee import is_holiday import hrms from hrms.api import get_current_employee_info @@ -207,6 +208,9 @@ def validate_dates(self): ): frappe.throw(_("Half Day Date should be between From Date and To Date")) + if self.half_day and self.half_day_date and is_holiday(employee=self.employee, date=self.half_day_date): + frappe.throw(_("Half Day Date cannot be a holiday")) + if not is_lwp(self.leave_type): self.validate_dates_across_allocation() self.validate_back_dated_application() @@ -905,6 +909,16 @@ def get_allocation_expiry_for_cf_leaves( return expiry[0][0] if expiry else "" +@frappe.whitelist() +def get_half_day_validation(employee, from_date, to_date, half_day_date): + if is_holiday(employee=employee, date=half_day_date): + return False + + if not (getdate(from_date) <= getdate(half_day_date) <= getdate(to_date)): + return False + + return True + @frappe.whitelist() def get_number_of_leave_days( employee: str, @@ -919,9 +933,11 @@ def get_number_of_leave_days( (Based on the include_holiday setting in Leave Type)""" number_of_days = 0 if cint(half_day) == 1: + half_day_is_holiday = is_holiday(employee=employee, date=half_day_date) + if getdate(from_date) == getdate(to_date): number_of_days = 0.5 - elif half_day_date and getdate(from_date) <= getdate(half_day_date) <= getdate(to_date): + elif half_day_date and getdate(from_date) <= getdate(half_day_date) <= getdate(to_date) and not half_day_is_holiday: number_of_days = date_diff(to_date, from_date) + 0.5 else: number_of_days = date_diff(to_date, from_date) + 1 diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index 95830dbef2..6437b269f1 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -14,6 +14,7 @@ nowdate, ) +from erpnext.setup.doctype.employee.employee import is_holiday from erpnext.setup.doctype.employee.test_employee import make_employee from hrms.hr.doctype.attendance.attendance import mark_attendance @@ -152,6 +153,11 @@ def get_application(self, doc): application.to_date = "2013-01-05" return application + def get_non_holiday_date(self, employee, date): + while is_holiday(employee=employee, date=date): + date = add_days(date, -1) + return date + @assign_holiday_list("Salary Slip Test Holiday List", "_Test Company") def test_validate_application_across_allocations(self): # Test validation for application dates when negative balance is disabled @@ -905,15 +911,19 @@ def test_ledger_entry_creation_on_intermediate_allocation_expiry(self): create_carry_forwarded_allocation(employee, leave_type) + half_day_date = self.get_non_holiday_date(employee.name, add_days(nowdate(), -3)) + from_date = add_days(half_day_date, 0) + to_date = add_days(half_day_date, 10) + leave_application = frappe.get_doc( dict( doctype="Leave Application", employee=employee.name, leave_type=leave_type.name, - from_date=add_days(nowdate(), -3), - to_date=add_days(nowdate(), 7), + from_date=from_date, + to_date=to_date, half_day=1, - half_day_date=add_days(nowdate(), -3), + half_day_date=half_day_date, description="_Test Reason", company="_Test Company", docstatus=1, @@ -1102,6 +1112,36 @@ def test_get_leave_details_for_dashboard(self): self.assertEqual(leave_allocation["leaves_pending_approval"], 1) self.assertEqual(leave_allocation["remaining_leaves"], 26) + @assign_holiday_list("Salary Slip Test Holiday List", "_Test Company") + def test_half_day_date_cannot_be_holiday(self): + employee = get_employee() + date = getdate() + make_allocation_record( + employee=employee.name, + leave_type="_Test Leave Type", + from_date=get_year_start(date), + to_date=get_year_ending(date), + ) + + holiday_date = add_days(get_year_start(date), 1) + add_date_to_holiday_list(holiday_date, self.holiday_list) + + leave_application = frappe.get_doc( + dict( + doctype="Leave Application", + employee=employee.name, + leave_type="_Test Leave Type", + from_date=holiday_date, + to_date=holiday_date, + half_day=1, + half_day_date=holiday_date, + company="_Test Company", + status="Approved", + ) + ) + + self.assertRaises(frappe.ValidationError, leave_application.insert) + @assign_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") def test_leave_details_with_expired_cf_leaves(self): """Tests leave details: @@ -1317,14 +1357,15 @@ def test_modifying_attendance_when_half_day_exists_from_checkins(self): attendance_name = mark_attendance( employee=employee.name, attendance_date=nowdate(), status="Half Day", half_day_status="Absent" ) + half_day_date = self.get_non_holiday_date(employee.name, nowdate()) leave_application = make_leave_application( employee.name, - nowdate(), - nowdate(), + half_day_date, + half_day_date, leave_type.name, submit=True, half_day=1, - half_day_date=nowdate(), + half_day_date=half_day_date, ) attendance = frappe.get_value( "Attendance", @@ -1349,14 +1390,15 @@ def test_modifying_attendance_from_absent_to_half_day(self): # when existing attendance is absent attendance_name = mark_attendance(employee=employee.name, attendance_date=nowdate(), status="Absent") + half_day_date = self.get_non_holiday_date(employee.name, nowdate()) leave_application = make_leave_application( employee.name, - add_days(nowdate(), -3), - add_days(nowdate(), 3), + add_days(half_day_date, -3), + add_days(half_day_date, 3), leave_type.name, submit=True, half_day=1, - half_day_date=nowdate(), + half_day_date=half_day_date, ) attendance = frappe.get_value( "Attendance", @@ -1379,14 +1421,15 @@ def test_half_day_status_for_two_half_leaves(self): ) create_carry_forwarded_allocation(employee, leave_type) # attendance from one half leave + half_day_date = self.get_non_holiday_date(employee.name, nowdate()) first_leave_application = make_leave_application( employee.name, - nowdate(), - nowdate(), + half_day_date, + half_day_date, leave_type.name, submit=True, half_day=1, - half_day_date=nowdate(), + half_day_date=half_day_date, ) half_day_status_after_first_application = frappe.get_value( "Attendance", @@ -1397,12 +1440,12 @@ def test_half_day_status_for_two_half_leaves(self): self.assertEqual(half_day_status_after_first_application, "Present") second_leave_application = make_leave_application( employee.name, - nowdate(), - nowdate(), + half_day_date, + half_day_date, leave_type.name, submit=True, half_day=1, - half_day_date=nowdate(), + half_day_date=half_day_date, ) half_day_status_after_second_application = frappe.get_value( "Attendance", From a21f0f6793f46d047d2d6b964c4cdf3bd507806d Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Mon, 16 Feb 2026 13:41:18 +0530 Subject: [PATCH 2/9] style(lint): linter --- .../leave_application/leave_application.js | 8 +++++--- .../leave_application/leave_application.py | 16 ++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index 195921eb8c..6fa84d909a 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -246,10 +246,12 @@ frappe.ui.form.on("Leave Application", { } else { frm.set_value("half_day_date", ""); frappe.msgprint( - __("Half Day Date must be between From Date and To Date and should not be a holiday."), - ); + __( + "Half Day Date must be between From Date and To Date and should not be a holiday.", + ), + ); } - } + }, }); } }, diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index ba7911844e..4291c7f36c 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -21,8 +21,7 @@ ) from erpnext.buying.doctype.supplier_scorecard.supplier_scorecard import daterange -from erpnext.setup.doctype.employee.employee import get_holiday_list_for_employee -from erpnext.setup.doctype.employee.employee import is_holiday +from erpnext.setup.doctype.employee.employee import get_holiday_list_for_employee, is_holiday import hrms from hrms.api import get_current_employee_info @@ -208,7 +207,11 @@ def validate_dates(self): ): frappe.throw(_("Half Day Date should be between From Date and To Date")) - if self.half_day and self.half_day_date and is_holiday(employee=self.employee, date=self.half_day_date): + if ( + self.half_day + and self.half_day_date + and is_holiday(employee=self.employee, date=self.half_day_date) + ): frappe.throw(_("Half Day Date cannot be a holiday")) if not is_lwp(self.leave_type): @@ -919,6 +922,7 @@ def get_half_day_validation(employee, from_date, to_date, half_day_date): return True + @frappe.whitelist() def get_number_of_leave_days( employee: str, @@ -937,7 +941,11 @@ def get_number_of_leave_days( if getdate(from_date) == getdate(to_date): number_of_days = 0.5 - elif half_day_date and getdate(from_date) <= getdate(half_day_date) <= getdate(to_date) and not half_day_is_holiday: + elif ( + half_day_date + and getdate(from_date) <= getdate(half_day_date) <= getdate(to_date) + and not half_day_is_holiday + ): number_of_days = date_diff(to_date, from_date) + 0.5 else: number_of_days = date_diff(to_date, from_date) + 1 From ba3c126caaa0f4071070444efa29b1b1f2b1daf5 Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Wed, 4 Mar 2026 16:42:32 +0530 Subject: [PATCH 3/9] refactor(leave-application): improve half-day calculation. --- .../leave_application/leave_application.py | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index 4291c7f36c..2d0196a37a 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -935,22 +935,16 @@ def get_number_of_leave_days( ) -> float: """Returns number of leave days between 2 dates after considering half day and holidays (Based on the include_holiday setting in Leave Type)""" - number_of_days = 0 - if cint(half_day) == 1: - half_day_is_holiday = is_holiday(employee=employee, date=half_day_date) + number_of_days = date_diff(to_date, from_date) + 1 - if getdate(from_date) == getdate(to_date): - number_of_days = 0.5 - elif ( + if cint(half_day) == 1: + is_valid_half_day = ( half_day_date and getdate(from_date) <= getdate(half_day_date) <= getdate(to_date) - and not half_day_is_holiday - ): - number_of_days = date_diff(to_date, from_date) + 0.5 - else: - number_of_days = date_diff(to_date, from_date) + 1 - else: - number_of_days = date_diff(to_date, from_date) + 1 + and not is_holiday(employee=employee, date=half_day_date) + ) + if is_valid_half_day: + number_of_days -= 0.5 if not frappe.db.get_value("Leave Type", leave_type, "include_holiday"): number_of_days = flt(number_of_days) - flt( From 0ad5cd48f74de5c6762df6719cc799f02769fa31 Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Wed, 4 Mar 2026 17:59:13 +0530 Subject: [PATCH 4/9] fix(leave-application): update half-day validation, adjust test assertions and refactor get_half_day_validation function --- .../leave_application/leave_application.js | 31 +++++++------------ .../leave_application/leave_application.py | 21 ++++++------- .../test_leave_application.py | 5 +-- 3 files changed, 24 insertions(+), 33 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index 6fa84d909a..20bc16c828 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -232,26 +232,17 @@ frappe.ui.form.on("Leave Application", { }, validate_half_day_date: function (frm) { if (frm.doc.half_day_date) { - return frappe.call({ - method: "hrms.hr.doctype.leave_application.leave_application.get_half_day_validation", - args: { - employee: frm.doc.employee, - from_date: frm.doc.from_date, - to_date: frm.doc.to_date, - half_day_date: frm.doc.half_day_date, - }, - callback: function (r) { - if (r && r.message) { - frm.trigger("calculate_total_days"); - } else { - frm.set_value("half_day_date", ""); - frappe.msgprint( - __( - "Half Day Date must be between From Date and To Date and should not be a holiday.", - ), - ); - } - }, + return frm.call("get_half_day_validation", {}, function (r) { + if (r && r.message) { + frm.trigger("calculate_total_days"); + } else { + frm.set_value("half_day_date", ""); + frappe.msgprint( + __( + "Half Day Date must be between From Date and To Date and should not be a holiday.", + ), + ); + } }); } }, diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index 2d0196a37a..fee0cc3a8f 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -889,6 +889,16 @@ def onload(self): frappe.db.get_single_value("HR Settings", "prevent_self_leave_approval"), ) + @frappe.whitelist() + def get_half_day_validation(self) -> bool: + if is_holiday(employee=self.employee, date=self.half_day_date): + return False + + if not (getdate(self.from_date) <= getdate(self.half_day_date) <= getdate(self.to_date)): + return False + + return True + def get_allocation_expiry_for_cf_leaves( employee: str, leave_type: str, to_date: datetime.date, from_date: datetime.date @@ -912,17 +922,6 @@ def get_allocation_expiry_for_cf_leaves( return expiry[0][0] if expiry else "" -@frappe.whitelist() -def get_half_day_validation(employee, from_date, to_date, half_day_date): - if is_holiday(employee=employee, date=half_day_date): - return False - - if not (getdate(from_date) <= getdate(half_day_date) <= getdate(to_date)): - return False - - return True - - @frappe.whitelist() def get_number_of_leave_days( employee: str, diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index 6437b269f1..ab94737eb3 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -900,6 +900,7 @@ def test_creation_of_leave_ledger_entry_on_submit(self): leave_application.cancel() self.assertFalse(frappe.db.exists("Leave Ledger Entry", {"transaction_name": leave_application.name})) + @assign_holiday_list("Holiday List w/o Weekly Offs", "_Test Company") def test_ledger_entry_creation_on_intermediate_allocation_expiry(self): employee = get_employee() leave_type = create_leave_type( @@ -911,7 +912,7 @@ def test_ledger_entry_creation_on_intermediate_allocation_expiry(self): create_carry_forwarded_allocation(employee, leave_type) - half_day_date = self.get_non_holiday_date(employee.name, add_days(nowdate(), -3)) + half_day_date = add_days(nowdate(), -3) from_date = add_days(half_day_date, 0) to_date = add_days(half_day_date, 10) @@ -940,7 +941,7 @@ def test_ledger_entry_creation_on_intermediate_allocation_expiry(self): self.assertEqual(leave_ledger_entry[0].employee, leave_application.employee) self.assertEqual(leave_ledger_entry[0].leave_type, leave_application.leave_type) self.assertEqual(leave_ledger_entry[0].leaves, -8.5) - self.assertEqual(leave_ledger_entry[1].leaves, -2) + self.assertEqual(leave_ledger_entry[1].leaves, -2.0) def test_leave_application_creation_after_expiry(self): # test leave balance for carry forwarded allocation From 72beff91cc8856f9bfe386bb62ecff6d986f6bc2 Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Wed, 4 Mar 2026 18:22:17 +0530 Subject: [PATCH 5/9] refactor(test-leave-application): simplify get_doc args --- .../test_leave_application.py | 358 ++++++++---------- 1 file changed, 157 insertions(+), 201 deletions(-) diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index ab94737eb3..483b4ba5b6 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -132,7 +132,7 @@ def setUp(self): if not frappe.db.exists("Leave Type", "_Test Leave Type"): frappe.get_doc( - dict(leave_type_name="_Test Leave Type", doctype="Leave Type", include_holiday=True) + leave_type_name="_Test Leave Type", doctype="Leave Type", include_holiday=True ).insert() def tearDown(self): @@ -163,7 +163,7 @@ def test_validate_application_across_allocations(self): # Test validation for application dates when negative balance is disabled frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) leave_type = frappe.get_doc( - dict(leave_type_name="Test Leave Validation", doctype="Leave Type", allow_negative=False) + leave_type_name="Test Leave Validation", doctype="Leave Type", allow_negative=False ).insert() employee = get_employee() @@ -171,16 +171,14 @@ def test_validate_application_across_allocations(self): first_sunday = get_first_sunday(self.holiday_list, for_date=get_year_start(date)) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date=add_days(first_sunday, 1), - to_date=add_days(first_sunday, 4), - company="_Test Company", - status="Approved", - leave_approver="test@example.com", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date=add_days(first_sunday, 1), + to_date=add_days(first_sunday, 4), + company="_Test Company", + status="Approved", + leave_approver="test@example.com", ) # Application period cannot be outside leave allocation period self.assertRaises(frappe.ValidationError, leave_application.insert) @@ -190,16 +188,14 @@ def test_validate_application_across_allocations(self): ) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date=add_days(first_sunday, -10), - to_date=add_days(first_sunday, 1), - company="_Test Company", - status="Approved", - leave_approver="test@example.com", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date=add_days(first_sunday, -10), + to_date=add_days(first_sunday, 1), + company="_Test Company", + status="Approved", + leave_approver="test@example.com", ) # Application period cannot be across two allocation records @@ -210,7 +206,7 @@ def test_insufficient_leave_balance_validation(self): # CASE 1: Validation when allow negative is disabled frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) leave_type = frappe.get_doc( - dict(leave_type_name="Test Leave Validation", doctype="Leave Type", allow_negative=False) + leave_type_name="Test Leave Validation", doctype="Leave Type", allow_negative=False ).insert() employee = get_employee() @@ -225,16 +221,14 @@ def test_insufficient_leave_balance_validation(self): leaves=2, ) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date=add_days(first_sunday, 1), - to_date=add_days(first_sunday, 3), - company="_Test Company", - status="Approved", - leave_approver="test@example.com", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date=add_days(first_sunday, 1), + to_date=add_days(first_sunday, 3), + company="_Test Company", + status="Approved", + leave_approver="test@example.com", ) self.assertRaises(InsufficientLeaveBalanceError, leave_application.insert) @@ -250,12 +244,10 @@ def test_separate_leave_ledger_entry_for_boundary_applications(self): # creates separate leave ledger entries frappe.delete_doc_if_exists("Leave Type", "Test Leave Validation", force=1) leave_type = frappe.get_doc( - dict( - leave_type_name="Test Leave Validation", - doctype="Leave Type", - allow_negative=True, - include_holiday=True, - ) + leave_type_name="Test Leave Validation", + doctype="Leave Type", + allow_negative=True, + include_holiday=True, ).insert() employee = get_employee() @@ -367,7 +359,7 @@ 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) + leave_type_name="Test Include Holidays", doctype="Leave Type", include_holiday=True ).insert() date = getdate() @@ -584,36 +576,30 @@ def test_optional_leave(self): if not frappe.db.exists("Holiday List", holiday_list): frappe.get_doc( - dict( - doctype="Holiday List", - holiday_list_name=holiday_list, - from_date=add_months(today, -6), - to_date=add_months(today, 6), - holidays=[dict(holiday_date=optional_leave_date, description="Test")], - ) + doctype="Holiday List", + holiday_list_name=holiday_list, + from_date=add_months(today, -6), + to_date=add_months(today, 6), + holidays=[dict(holiday_date=optional_leave_date, description="Test")], ).insert() frappe.db.set_value("Leave Period", leave_period.name, "optional_holiday_list", holiday_list) leave_type = "Test Optional Type" if not frappe.db.exists("Leave Type", leave_type): - frappe.get_doc( - dict(leave_type_name=leave_type, doctype="Leave Type", is_optional_leave=1) - ).insert() + frappe.get_doc(leave_type_name=leave_type, doctype="Leave Type", is_optional_leave=1).insert() allocate_leaves(employee, leave_period, leave_type, 10) date = add_days(first_sunday, 2) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - company="_Test Company", - description="_Test Reason", - leave_type=leave_type, - from_date=date, - to_date=date, - ) + doctype="Leave Application", + employee=employee.name, + company="_Test Company", + description="_Test Reason", + leave_type=leave_type, + from_date=date, + to_date=date, ) # can only apply on optional holidays @@ -632,7 +618,7 @@ def test_leaves_allowed(self): leave_period = get_leave_period() frappe.delete_doc_if_exists("Leave Type", "Test Leave Type", force=1) leave_type = frappe.get_doc( - dict(leave_type_name="Test Leave Type", doctype="Leave Type", max_leaves_allowed=5) + leave_type_name="Test Leave Type", doctype="Leave Type", max_leaves_allowed=5 ).insert() date = add_days(nowdate(), -7) @@ -640,32 +626,28 @@ def test_leaves_allowed(self): allocate_leaves(employee, leave_period, leave_type.name, 5) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - description="_Test Reason", - from_date=date, - to_date=add_days(date, 2), - company="_Test Company", - docstatus=1, - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + description="_Test Reason", + from_date=date, + to_date=add_days(date, 2), + company="_Test Company", + docstatus=1, + status="Approved", ) leave_application.submit() leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - description="_Test Reason", - from_date=add_days(date, 4), - to_date=add_days(date, 8), - company="_Test Company", - docstatus=1, - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + description="_Test Reason", + from_date=add_days(date, 4), + to_date=add_days(date, 8), + company="_Test Company", + docstatus=1, + status="Approved", ) self.assertRaises(frappe.ValidationError, leave_application.insert) @@ -674,47 +656,41 @@ def test_applicable_after(self): leave_period = get_leave_period() frappe.delete_doc_if_exists("Leave Type", "Test Leave Type", force=1) leave_type = frappe.get_doc( - dict(leave_type_name="Test Leave Type", doctype="Leave Type", applicable_after=15) + leave_type_name="Test Leave Type", doctype="Leave Type", applicable_after=15 ).insert() date = add_days(nowdate(), -7) frappe.db.set_value("Employee", employee.name, "date_of_joining", date) allocate_leaves(employee, leave_period, leave_type.name, 10) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - description="_Test Reason", - from_date=date, - to_date=add_days(date, 4), - company="_Test Company", - docstatus=1, - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + description="_Test Reason", + from_date=date, + to_date=add_days(date, 4), + company="_Test Company", + docstatus=1, + status="Approved", ) self.assertRaises(frappe.ValidationError, leave_application.insert) frappe.delete_doc_if_exists("Leave Type", "Test Leave Type 1", force=1) - leave_type_1 = frappe.get_doc( - dict(leave_type_name="Test Leave Type 1", doctype="Leave Type") - ).insert() + leave_type_1 = frappe.get_doc(leave_type_name="Test Leave Type 1", doctype="Leave Type").insert() allocate_leaves(employee, leave_period, leave_type_1.name, 10) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type_1.name, - description="_Test Reason", - from_date=date, - to_date=add_days(date, 4), - company="_Test Company", - docstatus=1, - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type_1.name, + description="_Test Reason", + from_date=date, + to_date=add_days(date, 4), + company="_Test Company", + docstatus=1, + status="Approved", ) self.assertTrue(leave_application.insert()) @@ -725,12 +701,10 @@ def test_max_continuous_leaves(self): leave_period = get_leave_period() frappe.delete_doc_if_exists("Leave Type", "Test Leave Type", force=1) leave_type = frappe.get_doc( - dict( - leave_type_name="Test Leave Type", - doctype="Leave Type", - max_leaves_allowed=15, - max_continuous_days_allowed=3, - ) + leave_type_name="Test Leave Type", + doctype="Leave Type", + max_leaves_allowed=15, + max_continuous_days_allowed=3, ).insert() date = add_days(nowdate(), -7) @@ -738,17 +712,15 @@ def test_max_continuous_leaves(self): allocate_leaves(employee, leave_period, leave_type.name, 10) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - description="_Test Reason", - from_date=date, - to_date=add_days(date, 4), - company="_Test Company", - docstatus=1, - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + description="_Test Reason", + from_date=date, + to_date=add_days(date, 4), + company="_Test Company", + docstatus=1, + status="Approved", ) self.assertRaises(frappe.ValidationError, leave_application.insert) @@ -757,11 +729,9 @@ def test_max_continuous_leaves(self): def test_max_consecutive_leaves_across_leave_applications(self): employee = get_employee() leave_type = frappe.get_doc( - dict( - leave_type_name="Test Consecutive Leave Type", - doctype="Leave Type", - max_continuous_days_allowed=10, - ) + leave_type_name="Test Consecutive Leave Type", + doctype="Leave Type", + max_continuous_days_allowed=10, ).insert() make_allocation_record( employee=employee.name, leave_type=leave_type.name, from_date="2013-01-01", to_date="2013-12-31" @@ -769,43 +739,37 @@ def test_max_consecutive_leaves_across_leave_applications(self): # before frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date="2013-01-30", - to_date="2013-02-03", - company="_Test Company", - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date="2013-01-30", + to_date="2013-02-03", + company="_Test Company", + status="Approved", ).insert() # after frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date="2013-02-06", - to_date="2013-02-10", - company="_Test Company", - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date="2013-02-06", + to_date="2013-02-10", + company="_Test Company", + status="Approved", ).insert() # current from_date = getdate("2013-02-04") to_date = getdate("2013-02-05") leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date=from_date, - to_date=to_date, - company="_Test Company", - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date=from_date, + to_date=to_date, + company="_Test Company", + status="Approved", ) # 11 consecutive leaves @@ -833,32 +797,28 @@ def test_current_leave_on_submit(self): leave_type = "Sick Leave" if not frappe.db.exists("Leave Type", leave_type): - frappe.get_doc(dict(leave_type_name=leave_type, doctype="Leave Type")).insert() + frappe.get_doc(leave_type_name=leave_type, doctype="Leave Type").insert() allocation = frappe.get_doc( - dict( - doctype="Leave Allocation", - employee=employee.name, - leave_type=leave_type, - from_date="2018-10-01", - to_date="2018-10-10", - new_leaves_allocated=1, - ) + doctype="Leave Allocation", + employee=employee.name, + leave_type=leave_type, + from_date="2018-10-01", + to_date="2018-10-10", + new_leaves_allocated=1, ) allocation.insert(ignore_permissions=True) allocation.submit() leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type, - description="_Test Reason", - from_date="2018-10-02", - to_date="2018-10-02", - company="_Test Company", - status="Approved", - leave_approver="test@example.com", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type, + description="_Test Reason", + from_date="2018-10-02", + to_date="2018-10-02", + company="_Test Company", + status="Approved", + leave_approver="test@example.com", ) self.assertTrue(leave_application.insert()) leave_application.submit() @@ -917,19 +877,17 @@ def test_ledger_entry_creation_on_intermediate_allocation_expiry(self): to_date = add_days(half_day_date, 10) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type=leave_type.name, - from_date=from_date, - to_date=to_date, - half_day=1, - half_day_date=half_day_date, - description="_Test Reason", - company="_Test Company", - docstatus=1, - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type=leave_type.name, + from_date=from_date, + to_date=to_date, + half_day=1, + half_day_date=half_day_date, + description="_Test Reason", + company="_Test Company", + docstatus=1, + status="Approved", ) leave_application.submit() @@ -1128,17 +1086,15 @@ def test_half_day_date_cannot_be_holiday(self): add_date_to_holiday_list(holiday_date, self.holiday_list) leave_application = frappe.get_doc( - dict( - doctype="Leave Application", - employee=employee.name, - leave_type="_Test Leave Type", - from_date=holiday_date, - to_date=holiday_date, - half_day=1, - half_day_date=holiday_date, - company="_Test Company", - status="Approved", - ) + doctype="Leave Application", + employee=employee.name, + leave_type="_Test Leave Type", + from_date=holiday_date, + to_date=holiday_date, + half_day=1, + half_day_date=holiday_date, + company="_Test Company", + status="Approved", ) self.assertRaises(frappe.ValidationError, leave_application.insert) From 72c935602dc528a88c3af91e8bc6332aa16d4460 Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Tue, 31 Mar 2026 15:08:29 +0530 Subject: [PATCH 6/9] fix(leave-application): improve half-day date validation logic and refactor related functions --- .../leave_application/leave_application.js | 24 ++++++++---------- .../leave_application/leave_application.py | 25 +++---------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/hrms/hr/doctype/leave_application/leave_application.js b/hrms/hr/doctype/leave_application/leave_application.js index 20bc16c828..4c164d2397 100755 --- a/hrms/hr/doctype/leave_application/leave_application.js +++ b/hrms/hr/doctype/leave_application/leave_application.js @@ -231,20 +231,18 @@ frappe.ui.form.on("Leave Application", { } }, validate_half_day_date: function (frm) { - if (frm.doc.half_day_date) { - return frm.call("get_half_day_validation", {}, function (r) { - if (r && r.message) { - frm.trigger("calculate_total_days"); - } else { - frm.set_value("half_day_date", ""); - frappe.msgprint( - __( - "Half Day Date must be between From Date and To Date and should not be a holiday.", - ), - ); - } - }); + if (!frm.doc.half_day_date) { + return; } + + return frm + .call("validate_half_day_date") + .then(() => { + frm.trigger("calculate_total_days"); + }) + .catch(() => { + frm.set_value("half_day_date", ""); + }); }, calculate_total_days: function (frm) { if (frm.doc.from_date && frm.doc.to_date && frm.doc.employee && frm.doc.leave_type) { diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index fee0cc3a8f..5753af44e6 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -197,22 +197,7 @@ def validate_dates(self): if self.from_date and self.to_date and (getdate(self.to_date) < getdate(self.from_date)): frappe.throw(_("To date cannot be before from date")) - if ( - self.half_day - and self.half_day_date - and ( - getdate(self.half_day_date) < getdate(self.from_date) - or getdate(self.half_day_date) > getdate(self.to_date) - ) - ): - frappe.throw(_("Half Day Date should be between From Date and To Date")) - - if ( - self.half_day - and self.half_day_date - and is_holiday(employee=self.employee, date=self.half_day_date) - ): - frappe.throw(_("Half Day Date cannot be a holiday")) + self.validate_half_day_date() if not is_lwp(self.leave_type): self.validate_dates_across_allocation() @@ -890,14 +875,12 @@ def onload(self): ) @frappe.whitelist() - def get_half_day_validation(self) -> bool: + def validate_half_day_date(self) -> bool: if is_holiday(employee=self.employee, date=self.half_day_date): - return False + frappe.throw(_("Half Day Date cannot be a holiday")) if not (getdate(self.from_date) <= getdate(self.half_day_date) <= getdate(self.to_date)): - return False - - return True + frappe.throw(_("Half Day Date should be between From Date and To Date")) def get_allocation_expiry_for_cf_leaves( From 47e93907a5c6743000a5cfc4a2ada38abde39dd3 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 17 Apr 2026 11:43:06 +0530 Subject: [PATCH 7/9] chore: resolve conflicts --- .../doctype/leave_application/test_leave_application.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/hrms/hr/doctype/leave_application/test_leave_application.py b/hrms/hr/doctype/leave_application/test_leave_application.py index cb3bb5e803..9da65b1b5e 100644 --- a/hrms/hr/doctype/leave_application/test_leave_application.py +++ b/hrms/hr/doctype/leave_application/test_leave_application.py @@ -65,14 +65,6 @@ def setUp(self): "Holiday List w/o Weekly Offs", from_date=from_date, to_date=to_date, add_weekly_offs=False ) - if not frappe.db.exists("Leave Type", "_Test Leave Type"): - frappe.get_doc( - leave_type_name="_Test Leave Type", doctype="Leave Type", include_holiday=True - ).insert() - - def tearDown(self): - frappe.set_user("Administrator") - def _clear_roles(self): frappe.db.sql( """delete from `tabHas Role` where parent in From ce5ca472db50fbc3dc12ab9e29df5571cbfd1b63 Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Fri, 17 Apr 2026 20:09:24 +0530 Subject: [PATCH 8/9] fix: add validation to prevent half day leave on holidays --- hrms/hr/doctype/leave_application/leave_application.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hrms/hr/doctype/leave_application/leave_application.py b/hrms/hr/doctype/leave_application/leave_application.py index 955ac36504..0ea5c7dd0f 100755 --- a/hrms/hr/doctype/leave_application/leave_application.py +++ b/hrms/hr/doctype/leave_application/leave_application.py @@ -909,6 +909,9 @@ def onload(self): @frappe.whitelist() def validate_half_day_date(self) -> bool: + if not self.half_day: + return + if is_holiday(employee=self.employee, date=self.half_day_date): frappe.throw(_("Half Day Date cannot be a holiday")) From ec41c3410fb2e549a201b398dd1e846970af6fcd Mon Sep 17 00:00:00 2001 From: Krishna Shirsath Date: Mon, 20 Apr 2026 12:16:02 +0530 Subject: [PATCH 9/9] fix(employee attendance tool): update half day leave handling to account for holidays --- .../test_employee_attendance_tool.py | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/hrms/hr/doctype/employee_attendance_tool/test_employee_attendance_tool.py b/hrms/hr/doctype/employee_attendance_tool/test_employee_attendance_tool.py index 817f40d44a..981b87282e 100644 --- a/hrms/hr/doctype/employee_attendance_tool/test_employee_attendance_tool.py +++ b/hrms/hr/doctype/employee_attendance_tool/test_employee_attendance_tool.py @@ -4,6 +4,7 @@ import frappe from frappe.utils import add_days, getdate +from erpnext.setup.doctype.employee.employee import is_holiday from erpnext.setup.doctype.employee.test_employee import make_employee from hrms.hr.doctype.attendance.attendance import mark_attendance @@ -78,13 +79,16 @@ def test_get_employees_for_half_day_attendance(self): # only half day attendance created from leave type should be fetched to update in the tool employee = frappe.get_doc("Employee", self.employee1) leave_type = create_leave_type(leave_type="_Test Employee Attendance Tool", include_holidays=0) + date = getdate() + while is_holiday(employee=employee.name, date=date): + date = add_days(date, -1) frappe.get_doc( { "doctype": "Leave Allocation", "employee": employee.name, "employee_name": employee.employee_name, "leave_type": leave_type.name, - "from_date": add_days(getdate(), -2), + "from_date": add_days(date, -2), "new_leaves_allocated": 15, "carry_forward": 0, "to_date": add_days(getdate(), 30), @@ -92,16 +96,14 @@ def test_get_employees_for_half_day_attendance(self): ).submit() make_leave_application( employee=employee.name, - from_date=getdate(), - to_date=getdate(), + from_date=date, + to_date=date, leave_type=leave_type.name, half_day=1, - half_day_date=getdate(), - ) - mark_attendance( - self.employee2, attendance_date=getdate(), status="Half Day", half_day_status="Absent" + half_day_date=date, ) - total_employees = get_employees(getdate(), company="_Test Company") + mark_attendance(self.employee2, attendance_date=date, status="Half Day", half_day_status="Absent") + total_employees = get_employees(date, company="_Test Company") half_marked_employees = total_employees.get("half_day_marked") self.assertEqual(len(half_marked_employees), 1) self.assertEqual(half_marked_employees[0].get("employee_name"), employee.employee_name) @@ -114,8 +116,12 @@ def test_update_half_day_attendance(self): employee2 = frappe.get_doc("Employee", self.employee2) leave_type = create_leave_type(leave_type="_Test Employee Attendance Tool", include_holidays=0) date = add_days(getdate(), -1) - create_leave_allocation(employee2, leave_type) - create_leave_allocation(employee4, leave_type) + while is_holiday(employee=employee2.name, date=date) or is_holiday( + employee=employee4.name, date=date + ): + date = add_days(date, -1) + create_leave_allocation(employee2, leave_type, date) + create_leave_allocation(employee4, leave_type, date) make_leave_application( employee=employee2.name, from_date=date, @@ -229,14 +235,15 @@ def test_get_unmarked_attendance_with_shift(self): self.assertNotIn(self.employee3.name, filtered) -def create_leave_allocation(employee, leave_type): +def create_leave_allocation(employee, leave_type, date=None): + from_date = add_days(date or getdate(), -2) frappe.get_doc( { "doctype": "Leave Allocation", "employee": employee.name, "employee_name": employee.employee_name, "leave_type": leave_type.name, - "from_date": add_days(getdate(), -2), + "from_date": from_date, "new_leaves_allocated": 15, "carry_forward": 0, "to_date": add_days(getdate(), 30),