fix: Checks for status in employee and for date range in upload attendance. (#16279)

* fix: checks for status in employee and for date range in upload attendance.

1. Earlier the status of an amployee can be set to 'Left' even though other employees are reporting to this employee, added checks to prevent this behaviour.
2. Earlier the range of dates added to the template were not checked for date of joining and relieving date. Now the range of dates added in the template are between the date of joining and relieving date.

* remove "import erpnext"

* fix: replace frappe.db.sql with frappe.db.get_all

* fix: refactored using list comprehension

* fix: query refactoring

* fix: combining list comprehensions

* travis debugging

* fix: doc.save

erpnext/hr/doctype/employee/employee.py

@@ -13,8 +13,8 @@ from frappe.model.document import Document
 from erpnext.utilities.transaction_base import delete_events
 from frappe.utils.nestedset import NestedSet
 
-class EmployeeUserDisabledError(frappe.ValidationError):
-	pass
+class EmployeeUserDisabledError(frappe.ValidationError): pass
+class EmployeeLeftValidationError(frappe.ValidationError): pass
 
 class Employee(NestedSet):
 	nsm_parent_field = 'reports_to'
@@ -147,8 +147,16 @@ class Employee(NestedSet):
 			validate_email_add(self.personal_email, True)
 
 	def validate_status(self):
-		if self.status == 'Left' and not self.relieving_date:
-			throw(_("Please enter relieving date."))
+		if self.status == 'Left':
+			reports_to = frappe.db.get_all('Employee',
+				filters={'reports_to': self.name}
+			)
+			if reports_to:
+				link_to_employees = [frappe.utils.get_link_to_form('Employee', employee.name) for employee in reports_to]
+				throw(_("Employee status cannot be set to 'Left' as following employees are currently reporting to this employee: ")
+					+ ', '.join(link_to_employees), EmployeeLeftValidationError)
+			if not self.relieving_date:
+				throw(_("Please enter relieving date."))
 
 	def validate_for_enabled_user_id(self, enabled):
 		if not self.status == 'Active':

erpnext/hr/doctype/employee/test_employee.py

@@ -7,6 +7,7 @@ import frappe
 import erpnext
 import unittest
 import frappe.utils
+from erpnext.hr.doctype.employee.employee import EmployeeLeftValidationError
 
 test_records = frappe.get_test_records('Employee')
 
@@ -32,6 +33,18 @@ class TestEmployee(unittest.TestCase):
 		email_queue = frappe.db.sql("""select * from `tabEmail Queue`""", as_dict=True)
 		self.assertTrue("Subject: Birthday Reminder" in email_queue[0].message)
 
+	def test_employee_status_left(self):
+		employee1 = make_employee("test_employee_1@company.com")
+		employee2 = make_employee("test_employee_2@company.com")
+		employee1_doc = frappe.get_doc("Employee", employee1)
+		employee2_doc = frappe.get_doc("Employee", employee2)
+		employee2_doc.reload()
+		employee2_doc.reports_to = employee1_doc.name
+		employee2_doc.save()
+		employee1_doc.reload()
+		employee1_doc.status = 'Left'
+		self.assertRaises(EmployeeLeftValidationError, employee1_doc.save)
+
 def make_employee(user):
 	if not frappe.db.get_value("User", user):
 		frappe.get_doc({

erpnext/hr/doctype/upload_attendance/test_upload_attendance.py

@@ -0,0 +1,34 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
+# See license.txt
+from __future__ import unicode_literals
+
+import frappe
+import unittest
+from frappe.utils import getdate
+from erpnext.hr.doctype.upload_attendance.upload_attendance import get_data
+from erpnext.hr.doctype.employee.test_employee import make_employee
+
+class TestUploadAttendance(unittest.TestCase):
+	def test_date_range(self):
+		employee = make_employee("test_employee@company.com")
+		employee_doc = frappe.get_doc("Employee", employee)
+		date_of_joining = "2018-01-02"
+		relieving_date = "2018-01-03"
+		from_date = "2018-01-01"
+		to_date = "2018-01-04"
+		employee_doc.date_of_joining = date_of_joining
+		employee_doc.relieving_date = relieving_date
+		employee_doc.save()
+		args = {
+			"from_date": from_date,
+			"to_date": to_date
+		}
+		data = get_data(args)
+		filtered_data = []
+		for row in data:
+			if row[1] == employee:
+				filtered_data.append(row)
+		print(filtered_data)
+		for row in filtered_data:
+			self.assertTrue(getdate(row[3]) >= getdate(date_of_joining) and getdate(row[3]) <= getdate(relieving_date))

erpnext/hr/doctype/upload_attendance/upload_attendance.py

@@ -41,16 +41,28 @@ def add_header(w):
 	return w
 
 def add_data(w, args):
+	data = get_data(args)
+	writedata(w, data)
+	return w
+
+def get_data(args):
 	dates = get_dates(args)
 	employees = get_active_employees()
 	existing_attendance_records = get_existing_attendance_records(args)
+	data = []
 	for date in dates:
 		for employee in employees:
+			if getdate(date) < getdate(employee.date_of_joining):
+				continue
+			if employee.relieving_date:
+				if getdate(date) > getdate(employee.relieving_date):
+					continue
 			existing_attendance = {}
 			if existing_attendance_records \
-				and tuple([getdate(date), employee.name]) in existing_attendance_records:
+				and tuple([getdate(date), employee.name]) in existing_attendance_records \
+				and getdate(employee.date_of_joining) >= getdate(date) \
+				and getdate(employee.relieving_date) <= getdate(date):
 					existing_attendance = existing_attendance_records[tuple([getdate(date), employee.name])]
-
 			row = [
 				existing_attendance and existing_attendance.name or "",
 				employee.name, employee.employee_name, date,
@@ -58,8 +70,12 @@ def add_data(w, args):
 				existing_attendance and existing_attendance.leave_type or "", employee.company,
 				existing_attendance and existing_attendance.naming_series or get_naming_series(),
 			]
-			w.writerow(row)
-	return w
+			data.append(row)
+	return data
+
+def writedata(w, data):
+	for row in data:
+		w.writerow(row)
 
 def get_dates(args):
 	"""get list of dates in between from date and to date"""
@@ -68,8 +84,13 @@ def get_dates(args):
 	return dates
 
 def get_active_employees():
-	employees = frappe.db.sql("""select name, employee_name, company
-		from tabEmployee where docstatus < 2 and status = 'Active'""", as_dict=1)
+	employees = frappe.db.get_all('Employee',
+		fields=['name', 'employee_name', 'date_of_joining', 'company', 'relieving_date'],
+		filters={
+			'docstatus': ['<', 2],
+			'status': 'Active'
+		}
+	)
 	return employees
 
 def get_existing_attendance_records(args):