Merge pull request #909 from frappe/version-14-hotfix

frappe · Sep 25, 2023 · 7cb26e4 · 7cb26e4
2 parents ff071e9 + 4f4bbe0
commit 7cb26e4
Show file tree

Hide file tree

Showing 5 changed files with 140 additions and 15 deletions.
diff --git a/hrms/hooks.py b/hrms/hooks.py
@@ -4,7 +4,7 @@
 app_description = "Modern HR and Payroll Software"
 app_email = "[email protected]"
 app_license = "GNU General Public License (v3)"
-required_apps = ["erpnext"]
+required_apps = ["frappe/erpnext"]
 
 
 # Includes in <head>

diff --git a/hrms/hr/doctype/compensatory_leave_request/compensatory_leave_request.py b/hrms/hr/doctype/compensatory_leave_request/compensatory_leave_request.py
@@ -35,18 +35,27 @@ def validate(self):
 			frappe.throw(_("Leave Type is madatory"))
 
 	def validate_attendance(self):
-		attendance = frappe.get_all(
+		attendance_records = frappe.get_all(
 			"Attendance",
 			filters={
 				"attendance_date": ["between", (self.work_from_date, self.work_end_date)],
-				"status": "Present",
+				"status": ("in", ["Present", "Work From Home", "Half Day"]),
 				"docstatus": 1,
 				"employee": self.employee,
 			},
 			fields=["attendance_date", "status"],
 		)
 
-		if len(attendance) < date_diff(self.work_end_date, self.work_from_date) + 1:
+		half_days = [entry.attendance_date for entry in attendance_records if entry.status == "Half Day"]
+
+		if half_days and (not self.half_day or getdate(self.half_day_date) not in half_days):
+			frappe.throw(
+				_(
+					"You were only present for Half Day on {}. Cannot apply for a full day compensatory leave"
+				).format(", ".join([frappe.bold(format_date(half_day)) for half_day in half_days]))
+			)
+
+		if len(attendance_records) < date_diff(self.work_end_date, self.work_from_date) + 1:
 			frappe.throw(_("You are not present all day(s) between compensatory leave request days"))
 
 	def validate_holidays(self):

diff --git a/hrms/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py b/hrms/hr/doctype/compensatory_leave_request/test_compensatory_leave_request.py
@@ -14,14 +14,12 @@
 
 class TestCompensatoryLeaveRequest(FrappeTestCase):
 	def setUp(self):
-		frappe.db.sql(""" delete from `tabCompensatory Leave Request`""")
-		frappe.db.sql(""" delete from `tabLeave Ledger Entry`""")
-		frappe.db.sql(""" delete from `tabLeave Allocation`""")
-		frappe.db.sql(
-			""" delete from `tabAttendance` where attendance_date in {0} """.format(
-				(today(), add_days(today(), -1))
-			)
-		)  # nosec
+		frappe.db.delete("Compensatory Leave Request")
+		frappe.db.delete("Leave Ledger Entry")
+		frappe.db.delete("Leave Allocation")
+		frappe.db.delete("Attendance")
+		frappe.db.delete("Leave Period")
+
 		create_leave_period(add_months(today(), -3), add_months(today(), 3), "_Test Company")
 		create_holiday_list()
 
@@ -99,6 +97,37 @@ def test_creation_of_leave_ledger_entry_on_submit(self):
 		self.assertEqual(leave_ledger_entry[0].leave_type, compensatory_leave_request.leave_type)
 		self.assertEqual(leave_ledger_entry[0].leaves, -1)
 
+	def test_half_day_compensatory_leave(self):
+		employee = get_employee()
+		mark_attendance(employee, status="Half Day")
+		date = today()
+		compensatory_leave_request = frappe.new_doc("Compensatory Leave Request")
+		compensatory_leave_request.update(
+			dict(
+				employee=employee.name,
+				leave_type="Compensatory Off",
+				work_from_date=date,
+				work_end_date=date,
+				reason="test",
+			)
+		)
+
+		# cannot apply for full day compensatory leave for a half day attendance
+		self.assertRaises(frappe.ValidationError, compensatory_leave_request.submit)
+
+		compensatory_leave_request.half_day = 1
+		compensatory_leave_request.half_day_date = date
+		compensatory_leave_request.submit()
+
+		# check creation of leave ledger entry on submission of leave request
+		leave_ledger_entry = frappe.get_all(
+			"Leave Ledger Entry",
+			fields="*",
+			filters={"transaction_name": compensatory_leave_request.leave_allocation},
+		)
+
+		self.assertEqual(leave_ledger_entry[0].leaves, 0.5)
+
 
 def get_compensatory_leave_request(employee, leave_date=today()):
 	prev_comp_leave_req = frappe.db.get_value(

diff --git a/hrms/payroll/doctype/salary_slip/salary_slip.py b/hrms/payroll/doctype/salary_slip/salary_slip.py
@@ -2,6 +2,7 @@
 # License: GNU General Public License v3. See license.txt
 
 
+import unicodedata
 from datetime import date
 
 import frappe
@@ -1047,14 +1048,14 @@ def eval_condition_and_formula(self, struct_row, data):
 		try:
 			condition = sanitize_expression(struct_row.condition)
 			if condition:
-				if not frappe.safe_eval(condition, self.whitelisted_globals, data):
+				if not _safe_eval(condition, self.whitelisted_globals, data):
 					return None
 			amount = struct_row.amount
 			if struct_row.amount_based_on_formula:
 				formula = sanitize_expression(struct_row.formula)
 				if formula:
 					amount = flt(
-						frappe.safe_eval(formula, self.whitelisted_globals, data), struct_row.precision("amount")
+						_safe_eval(formula, self.whitelisted_globals, data), struct_row.precision("amount")
 					)
 			if amount:
 				data[struct_row.abbr] = amount
@@ -2235,3 +2236,51 @@ def throw_error_message(row, error, title, description=None):
 	).format(**data)
 
 	frappe.throw(message, title=title)
+
+
+def _safe_eval(code: str, eval_globals: dict | None = None, eval_locals: dict | None = None):
+	"""Old version of safe_eval from framework.
+
+	Note: current frappe.safe_eval transforms code so if you have nested
+	iterations with too much depth then it can hit recursion limit of python.
+	There's no workaround for this and people need large formulas in some
+	countries so this is alternate implementation for that.
+
+	WARNING: DO NOT use this function anywhere else outside of this file.
+	"""
+	code = unicodedata.normalize("NFKC", code)
+
+	_check_attributes(code)
+
+	whitelisted_globals = {"int": int, "float": float, "long": int, "round": round}
+	if not eval_globals:
+		eval_globals = {}
+
+	eval_globals["__builtins__"] = {}
+	eval_globals.update(whitelisted_globals)
+	return eval(code, eval_globals, eval_locals)  # nosemgrep
+
+
+def _check_attributes(code: str) -> None:
+	import ast
+
+	from frappe.utils.safe_exec import UNSAFE_ATTRIBUTES
+
+	unsafe_attrs = set(UNSAFE_ATTRIBUTES).union(["__"]) - {"format"}
+
+	for attribute in unsafe_attrs:
+		if attribute in code:
+			raise SyntaxError(f'Illegal rule {frappe.bold(code)}. Cannot use "{attribute}"')
+
+	BLOCKED_NODES = (ast.NamedExpr,)
+
+	tree = ast.parse(code, mode="eval")
+	for node in ast.walk(tree):
+		if isinstance(node, BLOCKED_NODES):
+			raise SyntaxError(f"Operation not allowed: line {node.lineno} column {node.col_offset}")
+		if (
+			isinstance(node, ast.Attribute)
+			and isinstance(node.attr, str)
+			and node.attr in UNSAFE_ATTRIBUTES
+		):
+			raise SyntaxError(f'Illegal rule {frappe.bold(code)}. Cannot use "{node.attr}"')
diff --git a/hrms/payroll/doctype/salary_slip/test_salary_slip.py b/hrms/payroll/doctype/salary_slip/test_salary_slip.py
@@ -35,7 +35,10 @@
 	create_payroll_period,
 )
 from hrms.payroll.doctype.payroll_entry.payroll_entry import get_month_details
-from hrms.payroll.doctype.salary_slip.salary_slip import make_salary_slip_from_timesheet
+from hrms.payroll.doctype.salary_slip.salary_slip import (
+	_safe_eval,
+	make_salary_slip_from_timesheet,
+)
 from hrms.payroll.doctype.salary_structure.salary_structure import make_salary_slip
 from hrms.tests.test_utils import get_first_sunday
 
@@ -1387,6 +1390,41 @@ def test_variable_tax_component(self):
 		self.assertListEqual(tax_component, ["_Test TDS"])
 
 
+class TestSalarySlipSafeEval(FrappeTestCase):
+	def test_safe_eval_for_salary_slip(self):
+		TEST_CASES = {
+			"1+1": 2,
+			'"abc" in "abl"': False,
+			'"a" in "abl"': True,
+			'"a" in ("a", "b")': True,
+			'"a" in {"a", "b"}': True,
+			'"a" in {"a": 1, "b": 2}': True,
+			'"a" in ["a" ,"b"]': True,
+		}
+
+		for code, result in TEST_CASES.items():
+			self.assertEqual(_safe_eval(code), result)
+
+		self.assertRaises(NameError, _safe_eval, "frappe.utils.os.path", {})
+
+		# Doc/dict objects
+		user = frappe.new_doc("User")
+		user.user_type = "System User"
+		user.enabled = 1
+		self.assertTrue(_safe_eval("user_type == 'System User'", eval_locals=user.as_dict()))
+		self.assertEqual(
+			"System User Test", _safe_eval("user_type + ' Test'", eval_locals=user.as_dict())
+		)
+		self.assertEqual(1, _safe_eval("int(enabled)", eval_locals=user.as_dict()))
+
+		# Walrus not allowed
+		self.assertRaises(SyntaxError, _safe_eval, "(x := (40+2))")
+
+		# Format check but saner
+		self.assertTrue(_safe_eval("'x' != 'Information Techonology'"))
+		self.assertRaises(SyntaxError, _safe_eval, "'blah'.format(1)")
+
+
 def make_income_tax_components():
 	tax_components = [
 		{