From 50f0a99330ae7cbf4354874baa06132f9bdcdc97 Mon Sep 17 00:00:00 2001 From: sahil28297 <37302950+sahil28297@users.noreply.github.com> Date: Fri, 28 Dec 2018 17:15:56 +0530 Subject: [PATCH] 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 | 16 ++++++--- erpnext/hr/doctype/employee/test_employee.py | 13 +++++++ .../test_upload_attendance.py | 34 +++++++++++++++++++ .../upload_attendance/upload_attendance.py | 33 ++++++++++++++---- 4 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 erpnext/hr/doctype/upload_attendance/test_upload_attendance.py diff --git a/erpnext/hr/doctype/employee/employee.py b/erpnext/hr/doctype/employee/employee.py index 66d9badd2ac..d518cd89957 100755 --- a/erpnext/hr/doctype/employee/employee.py +++ b/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': diff --git a/erpnext/hr/doctype/employee/test_employee.py b/erpnext/hr/doctype/employee/test_employee.py index 1afb8f40b47..5a63beb2836 100644 --- a/erpnext/hr/doctype/employee/test_employee.py +++ b/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({ diff --git a/erpnext/hr/doctype/upload_attendance/test_upload_attendance.py b/erpnext/hr/doctype/upload_attendance/test_upload_attendance.py new file mode 100644 index 00000000000..5ab2847dde1 --- /dev/null +++ b/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)) diff --git a/erpnext/hr/doctype/upload_attendance/upload_attendance.py b/erpnext/hr/doctype/upload_attendance/upload_attendance.py index 3d080a725eb..db74b102a70 100644 --- a/erpnext/hr/doctype/upload_attendance/upload_attendance.py +++ b/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):