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

Author: ruchamahabal

hrms/__init__.py

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

hrms/hr/doctype/leave_application/leave_application.py

@@ -864,6 +864,7 @@ def get_leave_allocation_records(employee, date, leave_type=None):
 			Min(Ledger.from_date).as_("from_date"),
 			Max(Ledger.to_date).as_("to_date"),
 			Ledger.leave_type,
+			Ledger.employee,
 		)
 		.where(
 			(Ledger.from_date <= date)
@@ -903,6 +904,7 @@ def get_leave_allocation_records(employee, date, leave_type=None):
 					"unused_leaves": d.cf_leaves,
 					"new_leaves_allocated": d.new_leaves,
 					"leave_type": d.leave_type,
+					"employee": d.employee,
 				}
 			),
 		)
@@ -941,26 +943,51 @@ def _get_remaining_leaves(remaining_leaves, end_date):
 
 		return remaining_leaves
 
-	leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt(
-		leaves_taken
-	)
-
-	# balance for carry forwarded leaves
 	if cf_expiry and allocation.unused_leaves:
+		# allocation contains both carry forwarded and new leaves
+		new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(allocation, cf_expiry)
+
 		if getdate(date) > getdate(cf_expiry):
-			# carry forwarded leave expiry date passed
+			# carry forwarded leaves have expired
 			cf_leaves = remaining_cf_leaves = 0
 		else:
-			cf_leaves = flt(allocation.unused_leaves) + flt(leaves_taken)
+			cf_leaves = flt(allocation.unused_leaves) + flt(cf_leaves_taken)
 			remaining_cf_leaves = _get_remaining_leaves(cf_leaves, cf_expiry)
 
-		leave_balance = flt(allocation.new_leaves_allocated) + flt(cf_leaves)
-		leave_balance_for_consumption = flt(allocation.new_leaves_allocated) + flt(remaining_cf_leaves)
+		# new leaves allocated - new leaves taken + cf leave balance
+		# Note: `new_leaves_taken` is added here because its already a -ve number in the ledger
+		leave_balance = (flt(allocation.new_leaves_allocated) + flt(new_leaves_taken)) + flt(cf_leaves)
+		leave_balance_for_consumption = (
+			flt(allocation.new_leaves_allocated) + flt(new_leaves_taken)
+		) + flt(remaining_cf_leaves)
+	else:
+		# allocation only contains newly allocated leaves
+		leave_balance = leave_balance_for_consumption = flt(allocation.total_leaves_allocated) + flt(
+			leaves_taken
+		)
 
 	remaining_leaves = _get_remaining_leaves(leave_balance_for_consumption, allocation.to_date)
 	return frappe._dict(leave_balance=leave_balance, leave_balance_for_consumption=remaining_leaves)
 
 
+def get_new_and_cf_leaves_taken(allocation: Dict, cf_expiry: str) -> Tuple[float, float]:
+	"""returns new leaves taken and carry forwarded leaves taken within an allocation period based on cf leave expiry"""
+	cf_leaves_taken = get_leaves_for_period(
+		allocation.employee, allocation.leave_type, allocation.from_date, cf_expiry
+	)
+	new_leaves_taken = get_leaves_for_period(
+		allocation.employee, allocation.leave_type, add_days(cf_expiry, 1), allocation.to_date
+	)
+
+	# using abs because leaves taken is a -ve number in the ledger
+	if abs(cf_leaves_taken) > allocation.unused_leaves:
+		# adjust the excess leaves in new_leaves_taken
+		new_leaves_taken += -(abs(cf_leaves_taken) - allocation.unused_leaves)
+		cf_leaves_taken = -allocation.unused_leaves
+
+	return new_leaves_taken, cf_leaves_taken
+
+
 def get_leaves_for_period(
 	employee: str,
 	leave_type: str,

hrms/hr/doctype/leave_application/test_leave_application.py

@@ -29,6 +29,7 @@
 	get_leave_allocation_records,
 	get_leave_balance_on,
 	get_leave_details,
+	get_new_and_cf_leaves_taken,
 )
 from hrms.hr.doctype.leave_policy_assignment.leave_policy_assignment import (
 	create_assignment_for_multiple_employees,
@@ -97,6 +98,9 @@ def setUp(self):
 		from_date = get_year_start(getdate())
 		to_date = get_year_ending(getdate())
 		self.holiday_list = make_holiday_list(from_date=from_date, to_date=to_date)
+		list_without_weekly_offs = make_holiday_list(
+			"Holiday List w/o Weekly Offs", from_date=from_date, to_date=to_date, add_weekly_offs=False
+		)
 
 		if not frappe.db.exists("Leave Type", "_Test Leave Type"):
 			frappe.get_doc(
@@ -922,8 +926,12 @@ def test_get_leave_details_for_dashboard(self):
 		self.assertEqual(leave_allocation["leaves_pending_approval"], 1)
 		self.assertEqual(leave_allocation["remaining_leaves"], 26)
 
-	@set_holiday_list("Salary Slip Test Holiday List", "_Test Company")
+	@set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company")
 	def test_leave_details_with_expired_cf_leaves(self):
+		"""Tests leave details:
+		Case 1: All leaves available before cf leave expiry
+		Case 2: Remaining Leaves after cf leave expiry
+		"""
 		employee = get_employee()
 		leave_type = create_leave_type(
 			leave_type_name="_Test_CF_leave_expiry",
@@ -936,11 +944,11 @@ def test_leave_details_with_expired_cf_leaves(self):
 			"Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date"
 		)
 
-		# all leaves available before cf leave expiry
+		# case 1: all leaves available before cf leave expiry
 		leave_details = get_leave_details(employee.name, add_days(cf_expiry, -1))
 		self.assertEqual(leave_details["leave_allocation"][leave_type.name]["remaining_leaves"], 30.0)
 
-		# cf leaves expired
+		# case 2: cf leaves expired
 		leave_details = get_leave_details(employee.name, add_days(cf_expiry, 1))
 		expected_data = {
 			"total_leaves": 30.0,
@@ -949,6 +957,119 @@ def test_leave_details_with_expired_cf_leaves(self):
 			"leaves_pending_approval": 0.0,
 			"remaining_leaves": 15.0,
 		}
+
+		self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data)
+
+	@set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company")
+	def test_leave_details_with_application_across_cf_expiry(self):
+		"""Tests leave details with leave application across cf expiry, such that:
+		cf leaves are partially expired and partially consumed
+		"""
+		employee = get_employee()
+		leave_type = create_leave_type(
+			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(
+			"Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date"
+		)
+
+		# leave application across cf expiry
+		application = make_leave_application(
+			employee.name,
+			cf_expiry,
+			add_days(cf_expiry, 3),
+			leave_type.name,
+		)
+
+		leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4))
+		expected_data = {
+			"total_leaves": 30.0,
+			"expired_leaves": 14.0,
+			"leaves_taken": 4.0,
+			"leaves_pending_approval": 0.0,
+			"remaining_leaves": 12.0,
+		}
+
+		self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data)
+
+	@set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company")
+	def test_leave_details_with_application_across_cf_expiry_2(self):
+		"""Tests the same case as above but with leave days greater than cf leaves allocated"""
+		employee = get_employee()
+		leave_type = create_leave_type(
+			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(
+			"Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date"
+		)
+
+		# leave application across cf expiry, 20 days leave
+		application = make_leave_application(
+			employee.name,
+			add_days(cf_expiry, -16),
+			add_days(cf_expiry, 3),
+			leave_type.name,
+		)
+
+		# 15 cf leaves and 5 new leaves should be consumed
+		# after adjustment of the actual days breakup (17 and 3) because only 15 cf leaves have been allocated
+		new_leaves_taken, cf_leaves_taken = get_new_and_cf_leaves_taken(leave_alloc, cf_expiry)
+		self.assertEqual(new_leaves_taken, -5.0)
+		self.assertEqual(cf_leaves_taken, -15.0)
+
+		leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4))
+		expected_data = {
+			"total_leaves": 30.0,
+			"expired_leaves": 0,
+			"leaves_taken": 20.0,
+			"leaves_pending_approval": 0.0,
+			"remaining_leaves": 10.0,
+		}
+
+		self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data)
+
+	@set_holiday_list("Holiday List w/o Weekly Offs", "_Test Company")
+	def test_leave_details_with_application_after_cf_expiry(self):
+		"""Tests leave details with leave application after cf expiry, such that:
+		cf leaves are completely expired and only newly allocated leaves are consumed
+		"""
+		employee = get_employee()
+		leave_type = create_leave_type(
+			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(
+			"Leave Ledger Entry", {"transaction_name": leave_alloc.name, "is_carry_forward": 1}, "to_date"
+		)
+
+		# leave application after cf expiry
+		application = make_leave_application(
+			employee.name,
+			add_days(cf_expiry, 1),
+			add_days(cf_expiry, 4),
+			leave_type.name,
+		)
+
+		leave_details = get_leave_details(employee.name, add_days(cf_expiry, 4))
+		expected_data = {
+			"total_leaves": 30.0,
+			"expired_leaves": 15.0,
+			"leaves_taken": 4.0,
+			"leaves_pending_approval": 0.0,
+			"remaining_leaves": 11.0,
+		}
+
 		self.assertEqual(leave_details["leave_allocation"][leave_type.name], expected_data)
 
 	@set_holiday_list("Salary Slip Test Holiday List", "_Test Company")
@@ -975,6 +1096,7 @@ def test_get_leave_allocation_records(self):
 			"unused_leaves": 15.0,
 			"new_leaves_allocated": 15.0,
 			"leave_type": leave_type.name,
+			"employee": employee.name,
 		}
 		self.assertEqual(details.get(leave_type.name), expected_data)
 

hrms/hr/doctype/shift_type/shift_type.py

@@ -95,16 +95,18 @@ def get_attendance(self, logs):
 		):
 			early_exit = True
 
-		if (
-			self.working_hours_threshold_for_half_day
-			and total_working_hours < self.working_hours_threshold_for_half_day
-		):
-			return "Half Day", total_working_hours, late_entry, early_exit, in_time, out_time
 		if (
 			self.working_hours_threshold_for_absent
 			and total_working_hours < self.working_hours_threshold_for_absent
 		):
 			return "Absent", total_working_hours, late_entry, early_exit, in_time, out_time
+
+		if (
+			self.working_hours_threshold_for_half_day
+			and total_working_hours < self.working_hours_threshold_for_half_day
+		):
+			return "Half Day", total_working_hours, late_entry, early_exit, in_time, out_time
+
 		return "Present", total_working_hours, late_entry, early_exit, in_time, out_time
 
 	def mark_absent_for_dates_with_no_attendance(self, employee):

hrms/hr/doctype/shift_type/test_shift_type.py

@@ -141,8 +141,8 @@ def test_working_hours_threshold_for_absent_and_half_day_1(self):
 		employee = make_employee("test_employee_checkin@example.com", company="_Test Company")
 		shift_type = setup_shift_type(
 			shift_type="Half Day + Absent Test",
-			working_hours_threshold_for_half_day=1,
-			working_hours_threshold_for_absent=2,
+			working_hours_threshold_for_half_day=2,
+			working_hours_threshold_for_absent=1,
 		)
 		date = getdate()
 		make_shift_assignment(shift_type.name, employee, date)
@@ -151,7 +151,7 @@ def test_working_hours_threshold_for_absent_and_half_day_1(self):
 		log_in = make_checkin(employee, timestamp)
 		self.assertEqual(log_in.shift, shift_type.name)
 
-		timestamp = datetime.combine(date, get_time("08:45:00"))
+		timestamp = datetime.combine(date, get_time("09:30:00"))
 		log_out = make_checkin(employee, timestamp)
 		self.assertEqual(log_out.shift, shift_type.name)
 
@@ -161,7 +161,7 @@ def test_working_hours_threshold_for_absent_and_half_day_1(self):
 			"Attendance", {"shift": shift_type.name}, ["status", "working_hours"], as_dict=True
 		)
 		self.assertEqual(attendance.status, "Half Day")
-		self.assertEqual(attendance.working_hours, 0.75)
+		self.assertEqual(attendance.working_hours, 1.5)
 
 	def test_working_hours_threshold_for_absent_and_half_day_2(self):
 		# considers absent over half day
@@ -170,8 +170,8 @@ def test_working_hours_threshold_for_absent_and_half_day_2(self):
 		employee = make_employee("test_employee_checkin@example.com", company="_Test Company")
 		shift_type = setup_shift_type(
 			shift_type="Half Day + Absent Test",
-			working_hours_threshold_for_half_day=1,
-			working_hours_threshold_for_absent=2,
+			working_hours_threshold_for_half_day=2,
+			working_hours_threshold_for_absent=1,
 		)
 		date = getdate()
 		make_shift_assignment(shift_type.name, employee, date)
@@ -180,7 +180,7 @@ def test_working_hours_threshold_for_absent_and_half_day_2(self):
 		log_in = make_checkin(employee, timestamp)
 		self.assertEqual(log_in.shift, shift_type.name)
 
-		timestamp = datetime.combine(date, get_time("09:30:00"))
+		timestamp = datetime.combine(date, get_time("08:45:00"))
 		log_out = make_checkin(employee, timestamp)
 		self.assertEqual(log_out.shift, shift_type.name)
 

hrms/payroll/doctype/payroll_entry/payroll_entry.py

@@ -690,7 +690,6 @@ def create_journal_entry(self, je_payment_amount, user_remark):
 
 		if self.employee_based_payroll_payable_entries:
 			for employee, employee_details in self.employee_based_payroll_payable_entries.items():
-				je_payment_amount = employee_details["earnings"] - employee_details["deduction"]
 				je_payment_amount = employee_details.get("earnings") - (
 					employee_details.get("deductions") or 0
 				)