From 217cc1a00462232d97b3a4eace9b3ebcb9adf8af Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 22 Jan 2025 11:11:55 +0530 Subject: [PATCH 1/5] refactor: fetch all employees that may have shift type as active shift to be considered for attendance (cherry picked from commit 9bd360fd0ed311fe80f439bf5e4dafcce3b32999) --- hrms/hr/doctype/shift_type/shift_type.py | 36 +++++++----------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 77174a03ab..0370f24476 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/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): @@ -235,14 +234,21 @@ 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]: + """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 unnecessary employees who have another shift assigned that may have started or ended befor 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) + or_filters = [["end_date", ">=", from_date], ["end_date", "in", ("", None)]] - assigned_employees = frappe.get_all("Shift Assignment", filters=filters, pluck="employee") + 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 @@ -250,28 +256,6 @@ def get_assigned_employees(self, from_date=None, consider_default_shift=False) - 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 From d416bef4d1123600d3c2eb921b7e38037ace8207 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 22 Jan 2025 11:34:42 +0530 Subject: [PATCH 2/5] chore: typos in the comment (cherry picked from commit 91c29a828421430f556845148f3c8a654d347f4c) --- hrms/hr/doctype/shift_type/shift_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 0370f24476..29c03d1ec4 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -235,7 +235,7 @@ def get_marked_attendance_dates_between(self, employee: str, start_date: str, en def get_assigned_employees(self, from_date=None, 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 unnecessary employees who have another shift assigned that may have started or ended befor or after the + 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: From 7fe95740aa4ca5cf4d8f8b586808f6a885d02750 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 22 Jan 2025 13:10:11 +0530 Subject: [PATCH 3/5] chore: filter syntax chore: better test scenario (cherry picked from commit bb80a0e5bd15dff6cb4d534119944ace5e1e1bd5) --- hrms/hr/doctype/shift_type/shift_type.py | 2 +- hrms/hr/doctype/shift_type/test_shift_type.py | 67 +++++++++++++++++-- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 29c03d1ec4..7435e75322 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -239,7 +239,7 @@ def get_assigned_employees(self, from_date=None, consider_default_shift=False) - 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: - or_filters = [["end_date", ">=", from_date], ["end_date", "in", ("", None)]] + 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" diff --git a/hrms/hr/doctype/shift_type/test_shift_type.py b/hrms/hr/doctype/shift_type/test_shift_type.py index a5c3dd284a..49d5b4c4b8 100644 --- a/hrms/hr/doctype/shift_type/test_shift_type.py +++ b/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 From 2961109e79537d3ff69a319bddd837485571d8b0 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 22 Jan 2025 16:43:03 +0530 Subject: [PATCH 4/5] refactor: None check for process_attendance_after is unnecessary since the field is made mandatory for auto attendance (cherry picked from commit 11623cc236a6cc38eed44d4e0f6779ba70109481) --- hrms/hr/doctype/shift_type/shift_type.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 7435e75322..295bf91d7f 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -238,8 +238,8 @@ def get_assigned_employees(self, from_date=None, consider_default_shift=False) - 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: - or_filters = [["end_date", ">=", from_date], ["end_date", "is", "not set"]] + + 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" From 8d545ad9fdde2e49d8f91189f674d4c33e361867 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 22 Jan 2025 17:01:29 +0530 Subject: [PATCH 5/5] refactor: type check for process_attendance_date (cherry picked from commit d0f6488fdaaec31b41d9469a3e7dd52e1a6916a2) --- hrms/hr/doctype/shift_type/shift_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 295bf91d7f..6dc8cefb56 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -233,7 +233,7 @@ 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."""