Allocation of 2x half day of leave for different leave types on the same day

Frappe Framework Customization
1

Hi All

In setting up the HR module in V5, I have being going back through some standard tests to see how things work.

I have been testing and setting up the Leave Allocation tools where I came across a small issue which I think I have resolved. Advice from the team would be appreciated to ensure that I haven’t overlooked anything though.

The scenario happened during the New Year holiday where I have been entering employee’s leave. My company gave 1/2 day complementary leave on 31st Dec and the employees had to take 1/2 day out of their normal allocation. When I entered the 1/2 from the normal allocation everything was fine. However when I then tried to enter the complementary leave the system would not allow it as the person already had a 1/2 booked although it was a different leave type.

To solve the situation I ensured that there were a few days in the complementary leave allocation and then adjusted the SQL in the validate_leave_overlap method of leave_application.py (as shown below)

		for d in frappe.db.sql("""select name, leave_type, posting_date,
		from_date, to_date
		from `tabLeave Application`
		where
		employee = %(employee)s
		and docstatus < 2
		and status in ("Open", "Approved")
		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)s between from_date and to_date)
		and name != %(name)s
		and leave_type = %(leave_type)s""", {
			"employee": self.employee,
			"from_date": self.from_date,
			"to_date": self.to_date,
			"name": self.name,
			"leave_type": self.leave_type
		}, as_dict = 1):

The new parts to the query are to add in a condition in the where clause which includes a match against the leave type:

spaces`and leave_type = %(leave_type)s

and the value passed in here:

"leave_type": self.leave_type

There are two leave types set up for this situation, normal Annual Leave @ 23 days and Comlementary Leave @ 5 days.

Does this sound like a reasonable customisation? Could it be included in the core product?

Many thanks

2

@simondawson thanks for your intent to contribute :slight_smile: wish more users / devs had the same intent.

By doing this a user may accidentally book 2 separate leave types on the same day.

Ideally we should only allow overlap in case of half day and that too only twice. Maybe another condition could be written for that or for now you can just give the complimentary leave on the day before?

3

@mehta thanks for the response. What I have put in place is what I need to happen. We would allow someone to book two different leave types for the same day.

i.e For example New Year just gone, the UK had a public holiday on the 1st Jan (Thursday this year). The company ‘gave’ employees an extra half a day’s leave for the 2nd Jan as it was not worth opening the office but the employee also had to take half a day from their allocation. Therefore the need for the concessionary leave allocation (1/2 day) and annual leave (1/2 day) on the same day.

Does that explain my logic?

4

Hi @rmehta,

I had another look at your response and understood it a little better with a re-read. What you said did make some sense - ie a user could book any number of half days, as long as they were all of different leave types.

Therefore I set about solving that problem and think that this solution resolves the issue. Basically by putting a UNION in the query and ensuring that the validation gets thrown if there are already 2 half days booked for any leave type, or the whole day has already been booked then you cannot book more time off.

def validate_leave_overlap(self):
	if not self.name:
		self.name = "New Leave Application"

	for d in frappe.db.sql("""select name, leave_type, posting_date,
		from_date, to_date
		from `tabLeave Application`
		where
		employee = %(employee)s
		and docstatus < 2
		and status in ("Open", "Approved")
		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)s between from_date and to_date)
		and name != %(name)s
		and leave_type = %(leave_type)s
		
		union 
		
		select name, leave_type, posting_date,
		from_date, to_date
		from `tabLeave Application`
		where
		employee = %(employee)s
		and docstatus < 2
		and status in ("Open", "Approved")
		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)s between from_date and to_date)
		and name != %(name)s
		and (select count(half_day) 
			from `tabLeave Application` 
			where employee = %(employee)s 
			and docstatus < 2 
			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)s between from_date and to_date)
			and name != %(name)s)>1; """, {
			"employee": self.employee,
			"from_date": self.from_date,
			"to_date": self.to_date,
			"name": self.name,
			"leave_type": self.leave_type
		}, as_dict = 1):

		frappe.msgprint(_("Employee {0} has already applied for {1} between {2} and {3} or have 2 half days booked.").format(self.employee,
			cstr(d['leave_type']), formatdate(d['from_date']), formatdate(d['to_date'])))
		frappe.throw('<a href="#Form/Leave Application/{0}">{0}</a>'.format(d["name"]), OverlapError)

How does that seem?

5

@simondawson thats a heck of a query.

I personally don’t like to make complex queries beyond a point, they get very hard to debug later.

I would still go for a work-around.

6

@rmehta - aah, that’s the difference between us. My view is if the database can do, let it do so.

The logic here is relatively simple - all we are seeing is if one or more rows get returned…and using a union keeps the two elements of the query separate.

7

@simondawson I my experience, if you are changing the business logic later, its much better to standardize in the ORM. We like queries too (much to the chargin of some people) but there don’t want them to be too complex :smile: