fix:half day on holiday issue number 3331

[Hemil-Sangani]

Issue
Employees are currently able to apply half-day leave on holidays (like Sundays or public holidays), even when the selected leave type does not allow leave on holidays.
This leads to incorrect leave entries and confusion during payroll and attendance reconciliation.

Before Fix

• The system allowed users to select a holiday and apply for half-day leave.
• No validation existed to check whether a half-day leave date falls on a holiday.
• This resulted in invalid half-day leave records being submitted and approved.

After Fix

• A validation now checks the employee’s assigned holiday list.
• If the selected date is a holiday and Leave Type has 'Include holidays within leaves as leaves" disabled, the system prevents submission and shows this message:
  "Half-day leave cannot be applied on a holiday. Please choose a working day."
• Full-day leave behaviour remains unchanged.

Impact / Result
This update ensures that half-day leave is applied only on valid working days and prevents inaccurate leave records.

Summary by CodeRabbit

• Bug Fixes
  • Half-day leave applications are now validated against holiday calendars: half-day requests that fall on holidays are rejected when the selected leave type excludes holidays, preventing invalid leave bookings and ensuring leave counts remain accurate. Users will receive a clear validation message when a half-day falls on a holiday.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The leave application's validate_dates method now enforces holiday checking for half-day leaves. If a half-day leave includes half_day_date and the linked Leave Type has include_holiday set to false, the code fetches the employee's Holiday List and verifies whether the half_day_date is a holiday. If the date is a holiday, validation fails with an error. This change ties half-day validation to the Leave Type's include_holiday setting.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: fixing the issue where half-day leave could be applied on holidays, directly referencing issue #3331.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1af6314 and ffe5ed6.

📒 Files selected for processing (1)

• hrms/hr/doctype/leave_application/leave_application.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: asmitahase
Repo: frappe/hrms PR: 3500
File: hrms/hr/utils.py:540-541
Timestamp: 2025-08-25T11:35:14.372Z
Learning: The check_effective_date function in hrms/hr/utils.py has a logic flaw where it only compares expected_date.day == today.day for all frequencies. This means quarterly leaves allocate on the matching day of every month instead of only on quarter-end dates, defeating the purpose of frequency-based allocations. The fix should use full date comparison for First/Last Day allocations and proper period-based logic for Date of Joining allocations.

Learnt from: asmitahase
Repo: frappe/hrms PR: 3335
File: hrms/hr/report/employee_leave_balance/employee_leave_balance.py:198-213
Timestamp: 2025-08-14T10:46:28.404Z
Learning: The original employee_leave_balance report used sophisticated record-by-record processing logic to handle partial date overlaps correctly. When an allocation spans beyond the reporting period, it calculated expired leaves by taking the full allocation minus leaves already used within that specific period, and only counted allocations that START within the filter period as new allocations. The current refactored implementation using database aggregations loses this nuanced business logic and can produce incorrect results for partially overlapping allocations.

📚 Learning: 2025-08-25T11:35:14.372Z

Learnt from: asmitahase
Repo: frappe/hrms PR: 3500
File: hrms/hr/utils.py:540-541
Timestamp: 2025-08-25T11:35:14.372Z
Learning: The check_effective_date function in hrms/hr/utils.py has a logic flaw where it only compares expected_date.day == today.day for all frequencies. This means quarterly leaves allocate on the matching day of every month instead of only on quarter-end dates, defeating the purpose of frequency-based allocations. The fix should use full date comparison for First/Last Day allocations and proper period-based logic for Date of Joining allocations.

Applied to files:

• hrms/hr/doctype/leave_application/leave_application.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Summary

[ruchamahabal]

There is already a validation for this

hrms/hrms/hr/doctype/leave_application/leave_application.py

Lines 376 to 392 in 797231c

	if self.from_date and self.to_date:
	self.total_leave_days = get_number_of_leave_days(
	self.employee,
	self.leave_type,
	self.from_date,
	self.to_date,
	self.half_day,
	self.half_day_date,
	)
	if self.total_leave_days <= 0:
	frappe.throw(
	_(
	"The day(s) on which you are applying for leave are holidays. You need not apply for leave."
	)
	)

get_number_of_leave_days should return 0 in that case, if its not then problem is in that check

[Hemil-Sangani]

There is already a validation for this

hrms/hrms/hr/doctype/leave_application/leave_application.py

Lines 376 to 392 in 797231c

	if self.from_date and self.to_date:
	self.total_leave_days = get_number_of_leave_days(
	self.employee,
	self.leave_type,
	self.from_date,
	self.to_date,
	self.half_day,
	self.half_day_date,
	)
	if self.total_leave_days <= 0:
	frappe.throw(
	_(
	"The day(s) on which you are applying for leave are holidays. You need not apply for leave."
	)
	)

get_number_of_leave_days should return 0 in that case, if its not then problem is in that check

The get_number_of_leave_days function currently subtracts holidays from the total days between the leave start and end. When a holiday is also chosen as a half-day date, it gets subtracted twice, causing the leave calculation to be incorrect.
For example: a leave from 7 Dec (Sunday) to 10 Dec should count as 3 days (7th is a holiday). But if the employee also selects 7 Dec as a half-day, the function yields 2.5 days instead of 3.

To prevent this double-counting, I added a validation that disallows selecting holiday dates as half-day dates