Merge pull request #2688 from frappe/mergify/bp/version-15-hotfix/pr-…

…2683

refactor(Shift Type): fetch all employees that may have shift type as active shift (backport #2683)

hrms/hr/doctype/shift_type/shift_type.py

@@ -70,7 +70,6 @@ def process_auto_attendance(self):
 		frappe.db.commit()  # nosemgrep
 
 		assigned_employees = self.get_assigned_employees(self.process_attendance_after, True)
-
 		# mark absent in batches & commit to avoid losing progress since this tries to process remaining attendance
 		# right from "Process Attendance After" to "Last Sync of Checkin"
 		for batch in create_batch(assigned_employees, EMPLOYEE_CHUNK_SIZE):
@@ -234,44 +233,29 @@ def get_marked_attendance_dates_between(self, employee: str, start_date: str, en
 			)
 		).run(pluck=True)
 
-	def get_assigned_employees(self, from_date=None, consider_default_shift=False) -> list[str]:
+	def get_assigned_employees(self, from_date: datetime.date, consider_default_shift=False) -> list[str]:
+		"""Get all such employees who either have this shift assigned that hasn't ended or have this shift as default shift.
+		This may fetch some redundant employees who have another shift assigned that may have started or ended before or after the
+		attendance processing date. But this is done to avoid missing any employee who may have this shift as active shift."""
 		filters = {"shift_type": self.name, "docstatus": "1", "status": "Active"}
-		if from_date:
-			filters["start_date"] = (">=", from_date)
 
-		assigned_employees = frappe.get_all("Shift Assignment", filters=filters, pluck="employee")
+		or_filters = [["end_date", ">=", from_date], ["end_date", "is", "not set"]]
+
+		assigned_employees = frappe.get_all(
+			"Shift Assignment", filters=filters, or_filters=or_filters, pluck="employee"
+		)
 
 		if consider_default_shift:
-			default_shift_employees = self.get_employees_with_default_shift(filters, from_date)
+			default_shift_employees = frappe.get_all(
+				"Employee", filters={"default_shift": self.name, "status": "Active"}, pluck="name"
+			)
 			assigned_employees = set(assigned_employees + default_shift_employees)
 
 		# exclude inactive employees
 		inactive_employees = frappe.db.get_all("Employee", {"status": "Inactive"}, pluck="name")
 
 		return list(set(assigned_employees) - set(inactive_employees))
 
-	def get_employees_with_default_shift(self, filters: dict, from_date) -> list:
-		filters["start_date"] = ("<=", from_date)
-		default_shift_employees = frappe.get_all(
-			"Employee", filters={"default_shift": self.name, "status": "Active"}, pluck="name"
-		)
-
-		if not default_shift_employees:
-			return []
-
-		# exclude employees from default shift list if any other valid shift assignment exists
-		# that starts before the attendance processing date
-		del filters["shift_type"]
-		filters["employee"] = ("in", default_shift_employees)
-
-		active_shift_assignments = frappe.get_all(
-			"Shift Assignment",
-			filters=filters,
-			pluck="employee",
-		)
-
-		return list(set(default_shift_employees) - set(active_shift_assignments))
-
 	def get_holiday_list(self, employee: str) -> str:
 		holiday_list_name = self.holiday_list or get_holiday_list_for_employee(employee, False)
 		return holiday_list_name

hrms/hr/doctype/shift_type/test_shift_type.py

@@ -347,28 +347,83 @@ def test_mark_absent_for_dates_with_no_attendance_for_midnight_shift(self):
 			shift_type="Test Absent with no Attendance",
 			start_time="15:00:00",
 			end_time="23:30:00",
-			process_attendance_after=add_days(today, -6),
+			process_attendance_after=add_days(today, -8),
 			allow_check_out_after_shift_end_time=120,
 			last_sync_of_checkin=f"{today} 15:00:00",
 		)
 		# single day assignment
-		date1 = add_days(today, -5)
+		date1 = add_days(today, -7)
 		make_shift_assignment(shift_type.name, employee, date1, date1)
 
-		# assignment without end date
-		date2 = add_days(today, -4)
+		# assignment after a gap
+		date2 = add_days(today, -5)
 		make_shift_assignment(shift_type.name, employee, date2, date2)
 
+		# assignment without end date
+		date3 = add_days(today, -3)
+		make_shift_assignment(shift_type.name, employee, date3)
+
 		shift_type.process_auto_attendance()
 		absent_records = frappe.get_all(
 			"Attendance",
-			{
+			fields=["name", "employee", "attendance_date", "status", "shift"],
+			filters={
 				"attendance_date": ["between", [date1, today]],
 				"employee": employee,
 				"status": "Absent",
 			},
 		)
-		self.assertEqual(len(absent_records), 2)
+
+		self.assertEqual(len(absent_records), 5)
+		# absent for first assignment
+		self.assertEqual(
+			frappe.db.get_value(
+				"Attendance",
+				{"attendance_date": date1, "shift": shift_type.name, "employee": employee},
+				"status",
+			),
+			"Absent",
+		)
+		# no attendance for day after first assignment
+		self.assertIsNone(
+			frappe.db.get_value(
+				"Attendance",
+				{"attendance_date": add_days(date1, 1), "shift": shift_type.name, "employee": employee},
+			)
+		)
+		# absent for second assignment
+		self.assertEqual(
+			frappe.db.get_value(
+				"Attendance",
+				{"attendance_date": date2, "shift": shift_type.name, "employee": employee},
+				"status",
+			),
+			"Absent",
+		)
+		# no attendance for day after second assignment
+		self.assertIsNone(
+			frappe.db.get_value(
+				"Attendance",
+				{"attendance_date": add_days(date2, 1), "shift": shift_type.name, "employee": employee},
+			)
+		)
+		# absent for third assignment
+		self.assertEqual(
+			frappe.db.get_value(
+				"Attendance",
+				{"attendance_date": date3, "shift": shift_type.name, "employee": employee},
+				"status",
+			),
+			"Absent",
+		)
+		self.assertEqual(
+			frappe.db.get_value(
+				"Attendance",
+				{"attendance_date": add_days(date3, 1), "shift": shift_type.name, "employee": employee},
+				"status",
+			),
+			"Absent",
+		)
 
 	def test_do_not_mark_absent_before_shift_actual_end_time(self):
 		from hrms.hr.doctype.employee_checkin.test_employee_checkin import make_checkin