Merge pull request #4763 from rmehta/leave-app-cleanup

[cleanup] leave application

erpnext/hr/doctype/leave_allocation/leave_allocation.js

@@ -6,36 +6,36 @@ cur_frm.add_fetch('employee','employee_name','employee_name');
 frappe.ui.form.on("Leave Allocation", {
 	onload: function(frm) {
 		if(!frm.doc.from_date) frm.set_value("from_date", get_today());
-		
+
 		frm.set_query("employee", function() {
 			return {
 				query: "erpnext.controllers.queries.employee_query"
 			}
 		})
 	},
-	
+
 	employee: function(frm) {
 		frm.trigger("calculate_total_leaves_allocated");
 	},
-	
+
 	leave_type: function(frm) {
 		frm.trigger("calculate_total_leaves_allocated");
 	},
-	
+
 	carry_forward: function(frm) {
 		frm.trigger("calculate_total_leaves_allocated");
 	},
-	
+
 	carry_forwarded_leaves: function(frm) {
-		frm.set_value("total_leaves_allocated", 
+		frm.set_value("total_leaves_allocated",
 			flt(frm.doc.carry_forwarded_leaves) + flt(frm.doc.new_leaves_allocated));
 	},
-	
+
 	new_leaves_allocated: function(frm) {
-		frm.set_value("total_leaves_allocated", 
+		frm.set_value("total_leaves_allocated",
 			flt(frm.doc.carry_forwarded_leaves) + flt(frm.doc.new_leaves_allocated));
 	},
-	
+
 	calculate_total_leaves_allocated: function(frm) {
 		if (cint(frm.doc.carry_forward) == 1 && frm.doc.leave_type && frm.doc.employee) {
 			return frappe.call({
@@ -49,7 +49,7 @@ frappe.ui.form.on("Leave Allocation", {
 				callback: function(r) {
 					if (!r.exc && r.message) {
 						frm.set_value('carry_forwarded_leaves', r.message);
-						frm.set_value("total_leaves_allocated", 
+						frm.set_value("total_leaves_allocated",
 							flt(r.message) + flt(frm.doc.new_leaves_allocated));
 					}
 				}
@@ -59,4 +59,4 @@ frappe.ui.form.on("Leave Allocation", {
 			frm.set_value("total_leaves_allocated", flt(frm.doc.new_leaves_allocated));
 		}
 	}
-})
+})

erpnext/hr/doctype/leave_application/leave_application.py

@@ -62,7 +62,7 @@ class LeaveApplication(Document):
 	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 not is_lwp(self.leave_type):
 			self.validate_dates_acorss_allocation()
 			self.validate_back_dated_application()
@@ -118,7 +118,7 @@ class LeaveApplication(Document):
 				frappe.throw(_("The day(s) on which you are applying for leave are holidays. You need not apply for leave."))
 
 			if not is_lwp(self.leave_type):
-				self.leave_balance = get_leave_balance_on(self.employee, self.leave_type, self.from_date, 
+				self.leave_balance = get_leave_balance_on(self.employee, self.leave_type, self.from_date,
 					consider_all_leaves_in_the_allocation_period=True)
 
 				if self.status != "Rejected" and self.leave_balance < self.total_leave_days:
@@ -145,28 +145,31 @@ class LeaveApplication(Document):
 			}, as_dict = 1):
 
 			if d['total_leave_days']==0.5 and cint(self.half_day)==1:
-                    		sum_leave_days=frappe.db.sql("""select sum(total_leave_days) from `tabLeave Application`
+				sum_leave_days = self.get_total_leaves_on_half_day()
-			                        where employee = %(employee)s
+				if sum_leave_days==1:
-			                        and docstatus < 2
+					self.throw_overlap_error(d)
-			                        and status in ("Open", "Approved")
+			else:
-			                        and (from_date between %(from_date)s and %(to_date)s
+				self.throw_overlap_error(d)
-				                    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""", {
-				                        "employee": self.employee,
-				                        "from_date": self.from_date,
-				                        "to_date": self.to_date,
-				                        "name": self.name
-			                            })[0][0]
-                        	if sum_leave_days==1:
-                                	frappe.msgprint(_("Employee {0} has already applied this day").format(self.employee))
-			        	frappe.throw('<a href="#Form/Leave Application/{0}">{0}</a>'.format(d["name"]), OverlapError)
-                    	else:
-			        frappe.msgprint(_("Employee {0} has already applied for {1} between {2} and {3}")
-					.format(self.employee, cstr(d['leave_type']),formatdate(d['from_date']), formatdate(d['to_date'])))
 
-				frappe.throw("""Exising Application: <a href="#Form/Leave Application/{0}">{0}</a>""".format(d["name"]), OverlapError)
+	def throw_overlap_error(self, d):
+		msg = _("Employee {0} has already applied for {1} between {2} and {3}").format(self.employee,
+			d['leave_type'], formatdate(d['from_date']), formatdate(d['to_date'])) \
+			+ """ <br><b><a href="#Form/Leave Application/{0}">{0}</a></b>""".format(d["name"])
+		frappe.throw(msg, OverlapError)
 
+	def get_total_leaves_on_half_day(self):
+		return frappe.db.sql("""select sum(total_leave_days) from `tabLeave Application`
+			where employee = %(employee)s
+			and docstatus < 2
+			and status in ("Open", "Approved")
+			and from_date = %(from_date)s
+			and to_date = %(to_date)s
+			and name != %(name)s""", {
+				"employee": self.employee,
+				"from_date": self.from_date,
+				"to_date": self.to_date,
+				"name": self.name
+			})[0][0]
 
 	def validate_max_days(self):
 		max_days = frappe.db.get_value("Leave Type", self.leave_type, "max_days_allowed")
@@ -259,13 +262,13 @@ def get_number_of_leave_days(employee, leave_type, from_date, to_date, half_day=
 	return number_of_days
 
 @frappe.whitelist()
-def get_leave_balance_on(employee, leave_type, date, allocation_records=None, 
+def get_leave_balance_on(employee, leave_type, date, allocation_records=None,
 		consider_all_leaves_in_the_allocation_period=False):
 	if allocation_records == None:
 		allocation_records = get_leave_allocation_records(date, employee).get(employee, frappe._dict())
 
 	allocation = allocation_records.get(leave_type, frappe._dict())
-	
+
 	if consider_all_leaves_in_the_allocation_period:
 		date = allocation.to_date
 	leaves_taken = get_approved_leaves_for_period(employee, leave_type, allocation.from_date, date)

erpnext/hr/doctype/leave_application/test_leave_application.py

@@ -44,11 +44,11 @@ _test_records = [
 ]
 
 
-class TestLeaveApplication(unittest.TestCase):	
+class TestLeaveApplication(unittest.TestCase):
 	def setUp(self):
 		for dt in ["Leave Application", "Leave Allocation", "Salary Slip"]:
 			frappe.db.sql("delete from `tab%s`" % dt)
-	
+
 	def tearDown(self):
 		frappe.set_user("Administrator")
 
@@ -101,7 +101,7 @@ class TestLeaveApplication(unittest.TestCase):
 
 		frappe.db.set_value("Department", "_Test Department",
 			"leave_block_list", "_Test Leave Block List")
-			
+
 		make_allocation_record()
 
 		application = self.get_application(_test_records[0])
@@ -128,7 +128,7 @@ class TestLeaveApplication(unittest.TestCase):
 		frappe.set_user("test@example.com")
 
 		make_allocation_record()
-		
+
 		application = self.get_application(_test_records[0])
 		application.leave_approver = "test2@example.com"
 		application.insert()
@@ -137,6 +137,60 @@ class TestLeaveApplication(unittest.TestCase):
 		application.leave_approver = "test2@example.com"
 		self.assertRaises(OverlapError, application.insert)
 
+	def test_overlap_with_half_day(self):
+		self._clear_roles()
+		self._clear_applications()
+
+		from frappe.utils.user import add_role
+		add_role("test@example.com", "Employee")
+		add_role("test2@example.com", "Leave Approver")
+
+		frappe.set_user("test@example.com")
+
+		make_allocation_record()
+
+		# allow second half-day on the same day if available
+		application = self.get_application(_test_records[0])
+		application.leave_approver = "test2@example.com"
+		application.half_day = 1
+		application.insert()
+
+		# allow second half-day on the same day if available
+		application = self.get_application(_test_records[0])
+		application.leave_approver = "test2@example.com"
+		application.half_day = 1
+		application.insert()
+
+		application = self.get_application(_test_records[0])
+		application.leave_approver = "test2@example.com"
+		application.half_day = 1
+
+		self.assertRaises(OverlapError, application.insert)
+
+	def test_overlap_with_half_day_not_applicable(self):
+		self._clear_roles()
+		self._clear_applications()
+
+		from frappe.utils.user import add_role
+		add_role("test@example.com", "Employee")
+		add_role("test2@example.com", "Leave Approver")
+
+		frappe.set_user("test@example.com")
+
+		make_allocation_record()
+
+		# allow second half-day on the same day if available
+		application = self.get_application(_test_records[0])
+		application.leave_approver = "test2@example.com"
+		application.insert()
+
+		# allow second half-day on the same day if available
+		application = self.get_application(_test_records[0])
+		application.leave_approver = "test2@example.com"
+		application.half_day = 1
+
+		self.assertRaises(OverlapError, application.insert)
+
 	def test_global_block_list(self):
 		self._clear_roles()
 
@@ -144,9 +198,9 @@ class TestLeaveApplication(unittest.TestCase):
 		add_role("test1@example.com", "Employee")
 		add_role("test@example.com", "Leave Approver")
 		self._add_employee_leave_approver("_T-Employee-0002", "test@example.com")
-		
+
 		make_allocation_record(employee="_T-Employee-0002")
-		
+
 		application = self.get_application(_test_records[1])
 		application.leave_approver = "test@example.com"
 
@@ -186,9 +240,9 @@ class TestLeaveApplication(unittest.TestCase):
 
 		# create leave application as Employee
 		frappe.set_user("test@example.com")
-		
+
 		make_allocation_record()
-		
+
 		application = self.get_application(_test_records[0])
 		application.leave_approver = "test1@example.com"
 		application.insert()
@@ -228,9 +282,9 @@ class TestLeaveApplication(unittest.TestCase):
 		# create leave application as employee
 		# but submit as invalid leave approver - should raise exception
 		frappe.set_user("test@example.com")
-		
+
 		make_allocation_record()
-		
+
 		application = self.get_application(_test_records[0])
 		application.leave_approver = "test2@example.com"
 		application.insert()
@@ -251,9 +305,9 @@ class TestLeaveApplication(unittest.TestCase):
 		frappe.db.set_value("Employee", "_T-Employee-0001", "department", None)
 
 		frappe.set_user("test@example.com")
-		
+
 		make_allocation_record()
-		
+
 		application = self.get_application(_test_records[0])
 		application.leave_approver = "test2@example.com"
 		application.insert()
@@ -269,10 +323,10 @@ class TestLeaveApplication(unittest.TestCase):
 			"_T-Employee-0001")
 
 		frappe.db.set_value("Employee", "_T-Employee-0001", "department", original_department)
-		
+
 def make_allocation_record(employee=None, leave_type=None):
 	frappe.db.sql("delete from `tabLeave Allocation`")
-	
+
 	allocation = frappe.get_doc({
 		"doctype": "Leave Allocation",
 		"employee": employee or "_T-Employee-0001",
@@ -281,6 +335,6 @@ def make_allocation_record(employee=None, leave_type=None):
 		"to_date": "2015-12-31",
 		"new_leaves_allocated": 30
 	})
-	
+
 	allocation.insert(ignore_permissions=True)
 	allocation.submit()

erpnext/hr/doctype/leave_type/leave_type.js

@@ -0,0 +1,8 @@
+frappe.ui.form.on("Leave Type", {
+	refresh: function(frm) {
+		frm.add_custom_button(__("Allocations"), function() {
+			frappe.set_route("List", "Leave Allocation",
+			{"leave_type": frm.doc.name});
+		});
+	}
+});