Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: employee status server-side validation #26615

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion erpnext/hr/doctype/appraisal/appraisal.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from frappe import _
from frappe.model.mapper import get_mapped_doc
from frappe.model.document import Document
from erpnext.hr.utils import set_employee_name
from erpnext.hr.utils import set_employee_name, validate_active_employee

class Appraisal(Document):
def validate(self):
Expand All @@ -19,6 +19,7 @@ def validate(self):
if not self.goals:
frappe.throw(_("Goals cannot be empty"))

validate_active_employee(self.employee)
set_employee_name(self)
self.validate_dates()
self.validate_existing_appraisal()
Expand Down
2 changes: 2 additions & 0 deletions erpnext/hr/doctype/attendance/attendance.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
from frappe import _
from frappe.model.document import Document
from frappe.utils import cstr, get_datetime, formatdate
from erpnext.hr.utils import validate_active_employee

class Attendance(Document):
def validate(self):
from erpnext.controllers.status_updater import validate_status
validate_status(self.status, ["Present", "Absent", "On Leave", "Half Day", "Work From Home"])
validate_active_employee(self.employee)
self.validate_attendance_date()
self.validate_duplicate_record()
self.validate_employee_status()
Expand Down
3 changes: 2 additions & 1 deletion erpnext/hr/doctype/attendance_request/attendance_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
from frappe.model.document import Document
from frappe.utils import date_diff, add_days, getdate
from erpnext.hr.doctype.employee.employee import is_holiday
from erpnext.hr.utils import validate_dates
from erpnext.hr.utils import validate_dates, validate_active_employee

class AttendanceRequest(Document):
def validate(self):
validate_active_employee(self.employee)
validate_dates(self, self.from_date, self.to_date)
if self.half_day:
if not getdate(self.from_date)<=getdate(self.half_day_date)<=getdate(self.to_date):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
from frappe import _
from frappe.utils import date_diff, add_days, getdate, cint, format_date
from frappe.model.document import Document
from erpnext.hr.utils import validate_dates, validate_overlap, get_leave_period, \
from erpnext.hr.utils import validate_dates, validate_overlap, get_leave_period, validate_active_employee, \
get_holidays_for_employee, create_additional_leave_ledger_entry

class CompensatoryLeaveRequest(Document):

def validate(self):
validate_active_employee(self.employee)
validate_dates(self, self.work_from_date, self.work_end_date)
if self.half_day:
if not self.half_day_date:
Expand Down
8 changes: 5 additions & 3 deletions erpnext/hr/doctype/employee/employee.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
from erpnext.utilities.transaction_base import delete_events
from frappe.utils.nestedset import NestedSet

class EmployeeUserDisabledError(frappe.ValidationError): pass
class EmployeeLeftValidationError(frappe.ValidationError): pass
class EmployeeUserDisabledError(frappe.ValidationError):
pass
class InactiveEmployeeStatusError(frappe.ValidationError):
pass

class Employee(NestedSet):
nsm_parent_field = 'reports_to'
Expand Down Expand Up @@ -196,7 +198,7 @@ def validate_status(self):
message += "<br><br><ul><li>" + "</li><li>".join(link_to_employees)
message += "</li></ul><br>"
message += _("Please make sure the employees above report to another Active employee.")
throw(message, EmployeeLeftValidationError, _("Cannot Relieve Employee"))
throw(message, InactiveEmployeeStatusError, _("Cannot Relieve Employee"))
if not self.relieving_date:
throw(_("Please enter relieving date."))

Expand Down
30 changes: 27 additions & 3 deletions erpnext/hr/doctype/employee/test_employee.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import erpnext
import unittest
import frappe.utils
from erpnext.hr.doctype.employee.employee import EmployeeLeftValidationError
from erpnext.hr.doctype.employee.employee import InactiveEmployeeStatusError

test_records = frappe.get_test_records('Employee')

Expand Down Expand Up @@ -45,10 +45,33 @@ def test_employee_status_left(self):
employee2_doc.save()
employee1_doc.reload()
employee1_doc.status = 'Left'
self.assertRaises(EmployeeLeftValidationError, employee1_doc.save)
self.assertRaises(InactiveEmployeeStatusError, employee1_doc.save)

def test_employee_status_inactive(self):
from erpnext.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure
from erpnext.payroll.doctype.salary_structure.salary_structure import make_salary_slip
from erpnext.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list

employee = make_employee("test_employee_status@company.com")
employee_doc = frappe.get_doc("Employee", employee)
employee_doc.status = "Inactive"
employee_doc.save()
employee_doc.reload()

make_holiday_list()
frappe.db.set_value("Company", erpnext.get_default_company(), "default_holiday_list", "Salary Slip Test Holiday List")

frappe.db.sql("""delete from `tabSalary Structure` where name='Test Inactive Employee Salary Slip'""")
salary_structure = make_salary_structure("Test Inactive Employee Salary Slip", "Monthly",
employee=employee_doc.name, company=employee_doc.company)
salary_slip = make_salary_slip(salary_structure.name, employee=employee_doc.name)

self.assertRaises(InactiveEmployeeStatusError, salary_slip.save)

def tearDown(self):
frappe.db.rollback()

def make_employee(user, company=None, **kwargs):
""
if not frappe.db.get_value("User", user):
frappe.get_doc({
"doctype": "User",
Expand Down Expand Up @@ -80,4 +103,5 @@ def make_employee(user, company=None, **kwargs):
employee.insert()
return employee.name
else:
frappe.db.set_value("Employee", {"employee_name":user}, "status", "Active")
return frappe.get_value("Employee", {"employee_name":user}, "name")
6 changes: 4 additions & 2 deletions erpnext/hr/doctype/employee_advance/employee_advance.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from frappe.model.document import Document
from frappe.utils import flt, nowdate
from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account
from erpnext.hr.utils import validate_active_employee

class EmployeeAdvanceOverPayment(frappe.ValidationError):
pass
Expand All @@ -18,6 +19,7 @@ def onload(self):
'make_payment_via_journal_entry')

def validate(self):
validate_active_employee(self.employee)
self.set_status()

def on_cancel(self):
Expand Down Expand Up @@ -183,9 +185,9 @@ def make_return_entry(employee, company, employee_advance_name, return_amount,
bank_cash_account = get_default_bank_cash_account(company, account_type='Cash', mode_of_payment = mode_of_payment)
if not bank_cash_account:
frappe.throw(_("Please set a Default Cash Account in Company defaults"))

advance_account_currency = frappe.db.get_value('Account', advance_account, 'account_currency')

je = frappe.new_doc('Journal Entry')
je.posting_date = nowdate()
je.voucher_type = get_voucher_type(mode_of_payment)
Expand Down
4 changes: 3 additions & 1 deletion erpnext/hr/doctype/employee_checkin/employee_checkin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
from frappe import _

from erpnext.hr.doctype.shift_assignment.shift_assignment import get_actual_start_end_datetime_of_shift
from erpnext.hr.utils import validate_active_employee

class EmployeeCheckin(Document):
def validate(self):
validate_active_employee(self.employee)
self.validate_duplicate_log()
self.fetch_shift()

Expand Down Expand Up @@ -122,7 +124,7 @@ def mark_attendance_and_link_log(logs, attendance_status, attendance_date, worki
def calculate_working_hours(logs, check_in_out_type, working_hours_calc_type):
"""Given a set of logs in chronological order calculates the total working hours based on the parameters.
Zero is returned for all invalid cases.

:param logs: The List of 'Employee Checkin'.
:param check_in_out_type: One of: 'Alternating entries as IN and OUT during the same shift', 'Strictly based on Log Type in Employee Checkin'
:param working_hours_calc_type: One of: 'First Check-in and Last Check-out', 'Every Valid Check-in and Check-out'
Expand Down
5 changes: 2 additions & 3 deletions erpnext/hr/doctype/employee_promotion/employee_promotion.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@
from frappe import _
from frappe.model.document import Document
from frappe.utils import getdate
from erpnext.hr.utils import update_employee
from erpnext.hr.utils import update_employee, validate_active_employee

class EmployeePromotion(Document):
def validate(self):
if frappe.get_value("Employee", self.employee, "status") != "Active":
frappe.throw(_("Cannot promote Employee with status Left or Inactive"))
validate_active_employee(self.employee)

def before_submit(self):
if getdate(self.promotion_date) > getdate():
Expand Down
2 changes: 2 additions & 0 deletions erpnext/hr/doctype/employee_referral/employee_referral.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
from frappe import _
from frappe.utils import get_link_to_form
from frappe.model.document import Document
from erpnext.hr.utils import validate_active_employee

class EmployeeReferral(Document):
def validate(self):
validate_active_employee(self.referrer)
self.set_full_name()
self.set_referral_bonus_payment_status()

Expand Down
4 changes: 0 additions & 4 deletions erpnext/hr/doctype/employee_transfer/employee_transfer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
from erpnext.hr.utils import update_employee

class EmployeeTransfer(Document):
def validate(self):
if frappe.get_value("Employee", self.employee, "status") != "Active":
frappe.throw(_("Cannot transfer Employee with status Left or Inactive"))

def before_submit(self):
if getdate(self.transfer_date) > getdate():
frappe.throw(_("Employee Transfer cannot be submitted before Transfer Date"),
Expand Down
3 changes: 2 additions & 1 deletion erpnext/hr/doctype/expense_claim/expense_claim.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from frappe import _
from frappe.utils import get_fullname, flt, cstr, get_link_to_form
from frappe.model.document import Document
from erpnext.hr.utils import set_employee_name, share_doc_with_approver
from erpnext.hr.utils import set_employee_name, share_doc_with_approver, validate_active_employee
from erpnext.accounts.party import get_party_account
from erpnext.accounts.general_ledger import make_gl_entries
from erpnext.accounts.doctype.sales_invoice.sales_invoice import get_bank_cash_account
Expand All @@ -23,6 +23,7 @@ def onload(self):
'make_payment_via_journal_entry')

def validate(self):
validate_active_employee(self.employee)
self.validate_advances()
self.validate_sanctioned_amount()
self.calculate_total_amount()
Expand Down
3 changes: 2 additions & 1 deletion erpnext/hr/doctype/leave_application/leave_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import frappe
from frappe import _
from frappe.utils import cint, cstr, date_diff, flt, formatdate, getdate, get_link_to_form, get_fullname, add_days, nowdate
from erpnext.hr.utils import set_employee_name, get_leave_period, share_doc_with_approver
from erpnext.hr.utils import set_employee_name, get_leave_period, share_doc_with_approver, validate_active_employee
from erpnext.hr.doctype.leave_block_list.leave_block_list import get_applicable_block_dates
from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
from erpnext.buying.doctype.supplier_scorecard.supplier_scorecard import daterange
Expand All @@ -22,6 +22,7 @@ def get_feed(self):
return _("{0}: From {0} of type {1}").format(self.employee_name, self.leave_type)

def validate(self):
validate_active_employee(self.employee)
set_employee_name(self)
self.validate_dates()
self.validate_balance_leaves()
Expand Down
3 changes: 2 additions & 1 deletion erpnext/hr/doctype/leave_encashment/leave_encashment.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
from frappe import _
from frappe.model.document import Document
from frappe.utils import getdate, nowdate, flt
from erpnext.hr.utils import set_employee_name
from erpnext.hr.utils import set_employee_name, validate_active_employee
from erpnext.payroll.doctype.salary_structure_assignment.salary_structure_assignment import get_assigned_salary_structure
from erpnext.hr.doctype.leave_ledger_entry.leave_ledger_entry import create_leave_ledger_entry
from erpnext.hr.doctype.leave_allocation.leave_allocation import get_unused_leaves

class LeaveEncashment(Document):
def validate(self):
set_employee_name(self)
validate_active_employee(self.employee)
self.get_leave_details_for_encashment()
self.validate_salary_structure()

Expand Down
2 changes: 2 additions & 0 deletions erpnext/hr/doctype/shift_assignment/shift_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
from frappe.utils import cint, cstr, date_diff, flt, formatdate, getdate, now_datetime, nowdate
from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
from erpnext.hr.doctype.holiday_list.holiday_list import is_holiday
from erpnext.hr.utils import validate_active_employee
from datetime import timedelta, datetime

class ShiftAssignment(Document):
def validate(self):
validate_active_employee(self.employee)
self.validate_overlapping_dates()

if self.end_date and self.end_date <= self.start_date:
Expand Down
3 changes: 2 additions & 1 deletion erpnext/hr/doctype/shift_request/shift_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
from frappe import _
from frappe.model.document import Document
from frappe.utils import formatdate, getdate
from erpnext.hr.utils import share_doc_with_approver
from erpnext.hr.utils import share_doc_with_approver, validate_active_employee

class OverlapError(frappe.ValidationError): pass

class ShiftRequest(Document):
def validate(self):
validate_active_employee(self.employee)
self.validate_dates()
self.validate_shift_request_overlap_dates()
self.validate_approver()
Expand Down
4 changes: 3 additions & 1 deletion erpnext/hr/doctype/travel_request/travel_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
from __future__ import unicode_literals
import frappe
from frappe.model.document import Document
from erpnext.hr.utils import validate_active_employee

class TravelRequest(Document):
pass
def validate(self):
validate_active_employee(self.employee)
11 changes: 8 additions & 3 deletions erpnext/hr/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@

import erpnext
import frappe
from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee
from erpnext.hr.doctype.employee.employee import get_holiday_list_for_employee, InactiveEmployeeStatusError
from frappe import _
from frappe.desk.form import assign_to
from frappe.model.document import Document
from frappe.utils import (add_days, cstr, flt, format_datetime, formatdate,
get_datetime, getdate, nowdate, today, unique)

get_datetime, getdate, nowdate, today, unique, get_link_to_form)

class DuplicateDeclarationError(frappe.ValidationError): pass

Expand All @@ -20,6 +19,7 @@ class EmployeeBoardingController(Document):
Assign to the concerned person and roles as per the onboarding/separation template
'''
def validate(self):
validate_active_employee(self.employee)
# remove the task if linked before submitting the form
if self.amended_from:
for activity in self.activities:
Expand Down Expand Up @@ -522,3 +522,8 @@ def share_doc_with_approver(doc, user):
approver = approvers.get(doc.doctype)
if doc_before_save.get(approver) != doc.get(approver):
frappe.share.remove(doc.doctype, doc.name, doc_before_save.get(approver))

def validate_active_employee(employee):
if frappe.db.get_value("Employee", employee, "status") == "Inactive":
frappe.throw(_("Transactions cannot be created for an Inactive Employee {0}.").format(
get_link_to_form("Employee", employee)), InactiveEmployeeStatusError)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from frappe.model.document import Document
from frappe import _, bold
from frappe.utils import getdate, date_diff, comma_and, formatdate
from erpnext.hr.utils import validate_active_employee

class AdditionalSalary(Document):
def on_submit(self):
Expand All @@ -19,6 +20,7 @@ def on_cancel(self):
self.update_employee_referral(cancel=True)

def validate(self):
validate_active_employee(self.employee)
self.validate_dates()
self.validate_salary_structure()
self.validate_recurring_additional_salary_overlap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
from frappe.model.document import Document
from erpnext.payroll.doctype.payroll_period.payroll_period import get_payroll_period_days, get_period_factor
from erpnext.payroll.doctype.salary_structure_assignment.salary_structure_assignment import get_assigned_salary_structure
from erpnext.hr.utils import get_sal_slip_total_benefit_given, get_holidays_for_employee, get_previous_claimed_amount
from erpnext.hr.utils import get_sal_slip_total_benefit_given, get_holidays_for_employee, get_previous_claimed_amount, validate_active_employee

class EmployeeBenefitApplication(Document):
def validate(self):
validate_active_employee(self.employee)
self.validate_duplicate_on_payroll_period()
if not self.max_benefits:
self.max_benefits = get_max_benefits_remaining(self.employee, self.date, self.payroll_period)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
from frappe.utils import flt
from frappe.model.document import Document
from erpnext.payroll.doctype.employee_benefit_application.employee_benefit_application import get_max_benefits
from erpnext.hr.utils import get_previous_claimed_amount
from erpnext.hr.utils import get_previous_claimed_amount, validate_active_employee
from erpnext.payroll.doctype.payroll_period.payroll_period import get_payroll_period
from erpnext.payroll.doctype.salary_structure_assignment.salary_structure_assignment import get_assigned_salary_structure

class EmployeeBenefitClaim(Document):
def validate(self):
validate_active_employee(self.employee)
max_benefits = get_max_benefits(self.employee, self.claim_date)
if not max_benefits or max_benefits <= 0:
frappe.throw(_("Employee {0} has no maximum benefit amount").format(self.employee))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
import frappe
from frappe import _
from frappe.model.document import Document
from erpnext.hr.utils import validate_active_employee

class EmployeeIncentive(Document):
def validate(self):
validate_active_employee(self.employee)
self.validate_salary_structure()

def validate_salary_structure(self):
Expand Down