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

hrms/__init__.py

@@ -1 +1 @@
-__version__ = "14.1.1"
+__version__ = "14.1.2"

hrms/hr/doctype/leave_allocation/leave_allocation.py

@@ -90,6 +90,7 @@ def on_cancel(self):
 		if self.carry_forward:
 			self.set_carry_forwarded_leaves_in_previous_allocation(on_cancel=True)
 
+	# nosemgrep: frappe-semgrep-rules.rules.frappe-modifying-but-not-comitting
 	def on_update_after_submit(self):
 		if self.has_value_changed("new_leaves_allocated"):
 			self.validate_against_leave_applications()
@@ -99,7 +100,11 @@ def on_update_after_submit(self):
 			# run required validations again since total leaves are being updated
 			self.validate_leave_days_and_dates()
 
-			leaves_to_be_added = self.new_leaves_allocated - self.get_existing_leave_count()
+			leaves_to_be_added = flt(
+				(self.new_leaves_allocated - self.get_existing_leave_count()),
+				self.precision("new_leaves_allocated"),
+			)
+
 			args = {
 				"leaves": leaves_to_be_added,
 				"from_date": self.from_date,
@@ -118,14 +123,13 @@ def get_existing_leave_count(self):
 				"employee": self.employee,
 				"company": self.company,
 				"leave_type": self.leave_type,
+				"is_carry_forward": 0,
+				"docstatus": 1,
 			},
-			pluck="leaves",
+			fields=["SUM(leaves) as total_leaves"],
 		)
-		total_existing_leaves = 0
-		for entry in ledger_entries:
-			total_existing_leaves += entry
 
-		return total_existing_leaves
+		return ledger_entries[0].total_leaves if ledger_entries else 0
 
 	def validate_against_leave_applications(self):
 		leaves_taken = get_approved_leaves_for_period(

hrms/hr/doctype/leave_allocation/test_leave_allocation.py

@@ -71,7 +71,6 @@ def test_invalid_period(self):
 
 	def test_validation_for_over_allocation(self):
 		leave_type = create_leave_type(leave_type_name="Test Over Allocation", is_carry_forward=1)
-		leave_type.save()
 
 		doc = frappe.get_doc(
 			{
@@ -139,9 +138,9 @@ def test_validation_for_over_allocation_based_on_leave_setup(self):
 			)
 		).insert()
 
-		leave_type = create_leave_type(leave_type_name="_Test Allocation Validation", is_carry_forward=1)
-		leave_type.max_leaves_allowed = 25
-		leave_type.save()
+		leave_type = create_leave_type(
+			leave_type_name="_Test Allocation Validation", is_carry_forward=1, max_leaves_allowed=25
+		)
 
 		# 15 leaves allocated in this period
 		allocation = create_leave_allocation(
@@ -176,9 +175,9 @@ def test_validation_for_over_allocation_based_on_leave_setup_post_submission(sel
 			)
 		).insert()
 
-		leave_type = create_leave_type(leave_type_name="_Test Allocation Validation", is_carry_forward=1)
-		leave_type.max_leaves_allowed = 30
-		leave_type.save()
+		leave_type = create_leave_type(
+			leave_type_name="_Test Allocation Validation", is_carry_forward=1, max_leaves_allowed=30
+		)
 
 		# 15 leaves allocated
 		allocation = create_leave_allocation(
@@ -209,7 +208,6 @@ def test_validation_for_over_allocation_based_on_leave_setup_post_submission(sel
 
 	def test_validate_back_dated_allocation_update(self):
 		leave_type = create_leave_type(leave_type_name="_Test_CF_leave", is_carry_forward=1)
-		leave_type.save()
 
 		# initial leave allocation = 15
 		leave_allocation = create_leave_allocation(
@@ -237,10 +235,12 @@ def test_validate_back_dated_allocation_update(self):
 		self.assertRaises(BackDatedAllocationError, leave_allocation.save)
 
 	def test_carry_forward_calculation(self):
-		leave_type = create_leave_type(leave_type_name="_Test_CF_leave", is_carry_forward=1)
-		leave_type.maximum_carry_forwarded_leaves = 10
-		leave_type.max_leaves_allowed = 30
-		leave_type.save()
+		leave_type = create_leave_type(
+			leave_type_name="_Test_CF_leave",
+			is_carry_forward=1,
+			maximum_carry_forwarded_leaves=10,
+			max_leaves_allowed=30,
+		)
 
 		# initial leave allocation = 15
 		leave_allocation = create_leave_allocation(
@@ -288,7 +288,6 @@ def test_carry_forward_leaves_expiry(self):
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
 		)
-		leave_type.save()
 
 		# initial leave allocation
 		leave_allocation = create_leave_allocation(
@@ -354,25 +353,101 @@ def test_leave_addition_after_submit(self):
 		)
 		leave_allocation.submit()
 		leave_allocation.reload()
-		self.assertTrue(leave_allocation.total_leaves_allocated, 15)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 15)
 
 		leave_allocation.new_leaves_allocated = 40
-		leave_allocation.submit()
+		leave_allocation.save()
+		leave_allocation.reload()
+
+		updated_entry = frappe.db.get_all(
+			"Leave Ledger Entry",
+			{"transaction_name": leave_allocation.name},
+			pluck="leaves",
+			order_by="creation desc",
+			limit=1,
+		)
+
+		self.assertEqual(updated_entry[0], 25)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 40)
+
+	def test_leave_addition_after_submit_with_carry_forward(self):
+		from hrms.hr.doctype.leave_application.test_leave_application import (
+			create_carry_forwarded_allocation,
+		)
+
+		leave_type = create_leave_type(
+			leave_type_name="_Test_CF_leave_expiry",
+			is_carry_forward=1,
+			include_holiday=True,
+		)
+
+		leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type)
+		# 15 new leaves, 15 carry forwarded leaves
+		self.assertEqual(leave_allocation.total_leaves_allocated, 30)
+
+		leave_allocation.new_leaves_allocated = 32
+		leave_allocation.save()
 		leave_allocation.reload()
-		self.assertTrue(leave_allocation.total_leaves_allocated, 40)
+
+		updated_entry = frappe.db.get_all(
+			"Leave Ledger Entry",
+			{"transaction_name": leave_allocation.name},
+			pluck="leaves",
+			order_by="creation desc",
+			limit=1,
+		)
+		self.assertEqual(updated_entry[0], 17)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 47)
 
 	def test_leave_subtraction_after_submit(self):
 		leave_allocation = create_leave_allocation(
 			employee=self.employee.name, employee_name=self.employee.employee_name
 		)
 		leave_allocation.submit()
 		leave_allocation.reload()
-		self.assertTrue(leave_allocation.total_leaves_allocated, 15)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 15)
 
 		leave_allocation.new_leaves_allocated = 10
 		leave_allocation.submit()
 		leave_allocation.reload()
-		self.assertTrue(leave_allocation.total_leaves_allocated, 10)
+
+		updated_entry = frappe.db.get_all(
+			"Leave Ledger Entry",
+			{"transaction_name": leave_allocation.name},
+			pluck="leaves",
+			order_by="creation desc",
+			limit=1,
+		)
+
+		self.assertEqual(updated_entry[0], -5)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 10)
+
+	def test_leave_subtraction_after_submit_with_carry_forward(self):
+		from hrms.hr.doctype.leave_application.test_leave_application import (
+			create_carry_forwarded_allocation,
+		)
+
+		leave_type = create_leave_type(
+			leave_type_name="_Test_CF_leave_expiry",
+			is_carry_forward=1,
+			include_holiday=True,
+		)
+
+		leave_allocation = create_carry_forwarded_allocation(self.employee, leave_type)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 30)
+
+		leave_allocation.new_leaves_allocated = 8
+		leave_allocation.save()
+
+		updated_entry = frappe.db.get_all(
+			"Leave Ledger Entry",
+			{"transaction_name": leave_allocation.name},
+			pluck="leaves",
+			order_by="creation desc",
+			limit=1,
+		)
+		self.assertEqual(updated_entry[0], -7)
+		self.assertEqual(leave_allocation.total_leaves_allocated, 23)
 
 	def test_validation_against_leave_application_after_submit(self):
 		from hrms.payroll.doctype.salary_slip.test_salary_slip import make_holiday_list

hrms/hr/doctype/leave_application/test_leave_application.py

@@ -703,7 +703,7 @@ def test_leave_balance_near_allocaton_expiry(self):
 			leave_type_name="_Test_CF_leave_expiry",
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
-		).insert()
+		)
 
 		create_carry_forwarded_allocation(employee, leave_type)
 		details = get_leave_balance_on(
@@ -754,7 +754,6 @@ def test_creation_of_leave_ledger_entry_on_submit(self):
 		employee = get_employee()
 
 		leave_type = create_leave_type(leave_type_name="Test Leave Type 1")
-		leave_type.save()
 
 		leave_allocation = create_leave_allocation(
 			employee=employee.name, employee_name=employee.employee_name, leave_type=leave_type.name
@@ -797,7 +796,6 @@ def test_ledger_entry_creation_on_intermediate_allocation_expiry(self):
 			expire_carry_forwarded_leaves_after_days=90,
 			include_holiday=True,
 		)
-		leave_type.submit()
 
 		create_carry_forwarded_allocation(employee, leave_type)
 
@@ -836,7 +834,6 @@ def test_leave_application_creation_after_expiry(self):
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
 		)
-		leave_type.submit()
 
 		create_carry_forwarded_allocation(employee, leave_type)
 
@@ -937,7 +934,7 @@ def test_leave_details_with_expired_cf_leaves(self):
 			leave_type_name="_Test_CF_leave_expiry",
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
-		).insert()
+		)
 
 		leave_alloc = create_carry_forwarded_allocation(employee, leave_type)
 		cf_expiry = frappe.db.get_value(
@@ -970,7 +967,7 @@ def test_leave_details_with_application_across_cf_expiry(self):
 			leave_type_name="_Test_CF_leave_expiry",
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
-		).insert()
+		)
 
 		leave_alloc = create_carry_forwarded_allocation(employee, leave_type)
 		cf_expiry = frappe.db.get_value(
@@ -1004,7 +1001,7 @@ def test_leave_details_with_application_across_cf_expiry_2(self):
 			leave_type_name="_Test_CF_leave_expiry",
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
-		).insert()
+		)
 
 		leave_alloc = create_carry_forwarded_allocation(employee, leave_type)
 		cf_expiry = frappe.db.get_value(
@@ -1046,7 +1043,7 @@ def test_leave_details_with_application_after_cf_expiry(self):
 			leave_type_name="_Test_CF_leave_expiry",
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
-		).insert()
+		)
 
 		leave_alloc = create_carry_forwarded_allocation(employee, leave_type)
 		cf_expiry = frappe.db.get_value(
@@ -1080,7 +1077,7 @@ def test_get_leave_allocation_records(self):
 			leave_type_name="_Test_CF_leave_expiry",
 			is_carry_forward=1,
 			expire_carry_forwarded_leaves_after_days=90,
-		).insert()
+		)
 
 		leave_alloc = create_carry_forwarded_allocation(employee, leave_type)
 		cf_expiry = frappe.db.get_value(

hrms/hr/doctype/leave_type/test_leave_type.py

@@ -9,7 +9,8 @@
 def create_leave_type(**args):
 	args = frappe._dict(args)
 	if frappe.db.exists("Leave Type", args.leave_type_name):
-		return frappe.get_doc("Leave Type", args.leave_type_name)
+		frappe.delete_doc("Leave Type", args.leave_type_name)
+
 	leave_type = frappe.get_doc(
 		{
 			"doctype": "Leave Type",
@@ -23,10 +24,14 @@ def create_leave_type(**args):
 			"expire_carry_forwarded_leaves_after_days": args.expire_carry_forwarded_leaves_after_days or 0,
 			"encashment_threshold_days": args.encashment_threshold_days or 5,
 			"earning_component": "Leave Encashment",
+			"max_leaves_allowed": args.max_leaves_allowed,
+			"maximum_carry_forwarded_leaves": args.maximum_carry_forwarded_leaves,
 		}
 	)
 
 	if leave_type.is_ppl:
 		leave_type.fraction_of_daily_salary_per_leave = args.fraction_of_daily_salary_per_leave or 0.5
 
+	leave_type.insert()
+
 	return leave_type

hrms/hr/report/employee_leave_balance/test_employee_leave_balance.py

@@ -155,7 +155,6 @@ def test_opening_balance_on_alloc_boundary_dates(self):
 	@set_holiday_list("_Test Emp Balance Holiday List", "_Test Company")
 	def test_opening_balance_considers_carry_forwarded_leaves(self):
 		leave_type = create_leave_type(leave_type_name="_Test_CF_leave_expiry", is_carry_forward=1)
-		leave_type.insert()
 
 		# 30 leaves allocated for first half of the year
 		allocation1 = make_allocation_record(

hrms/patches/post_install/set_salary_details_submittable.py

@@ -2,10 +2,13 @@
 
 
 def execute():
-	frappe.db.sql(
-		"""
-		update `tabSalary Structure` ss, `tabSalary Detail` sd
-		set sd.docstatus=1
-		where ss.name=sd.parent and ss.docstatus=1 and sd.parenttype='Salary Structure'
-	"""
-	)
+	ss = frappe.qb.DocType("Salary Structure").as_("ss")
+	sd = frappe.qb.DocType("Salary Detail").as_("sd")
+
+	(
+		frappe.qb.update(sd)
+		.inner_join(ss)
+		.on(ss.name == sd.parent)
+		.set(sd.docstatus, 1)
+		.where((ss.docstatus == 1) & (sd.parenttype == "Salary Structure"))
+	).run()

hrms/payroll/doctype/salary_slip/test_salary_slip.py

@@ -301,7 +301,6 @@ def test_payment_days_based_on_leave_application(self):
 		make_leave_application(emp_id, first_sunday, add_days(first_sunday, 3), "Leave Without Pay")
 
 		leave_type_ppl = create_leave_type(leave_type_name="Test Partially Paid Leave", is_ppl=1)
-		leave_type_ppl.save()
 
 		alloc = create_leave_allocation(
 			employee=emp_id,