From bc8e3522fab7313929e0354f5189252637970fbe Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 12 May 2023 14:53:33 +0530 Subject: [PATCH 1/6] perf: index shift type and employee in checkins and assignment to avoid full table scans --- hrms/hr/doctype/employee_checkin/employee_checkin.json | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hrms/hr/doctype/employee_checkin/employee_checkin.json b/hrms/hr/doctype/employee_checkin/employee_checkin.json index d34316dc0f..0eb63c1bdc 100644 --- a/hrms/hr/doctype/employee_checkin/employee_checkin.json +++ b/hrms/hr/doctype/employee_checkin/employee_checkin.json @@ -26,7 +26,8 @@ "fieldtype": "Link", "label": "Employee", "options": "Employee", - "reqd": 1 + "reqd": 1, + "search_index": 1 }, { "fetch_from": "employee.employee_name", @@ -48,7 +49,8 @@ "fieldtype": "Link", "label": "Shift", "options": "Shift Type", - "read_only": 1 + "read_only": 1, + "search_index": 1 }, { "fieldname": "column_break_4", @@ -107,10 +109,11 @@ } ], "links": [], - "modified": "2020-07-08 11:02:32.660986", + "modified": "2023-05-12 14:52:22.660264", "modified_by": "Administrator", "module": "HR", "name": "Employee Checkin", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -203,6 +206,7 @@ ], "sort_field": "modified", "sort_order": "ASC", + "states": [], "title_field": "employee_name", "track_changes": 1 } \ No newline at end of file From 2971dc11323ff7b4a676110ecdd35c2359b58e0f Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Fri, 12 May 2023 17:48:21 +0530 Subject: [PATCH 2/6] perf: use cache for most used queries - `get_doc` -> `get_cached_doc` / `get_value` - `get_value` to `db.value_cache` --- .../doctype/employee_checkin/employee_checkin.py | 4 ++-- .../doctype/shift_assignment/shift_assignment.py | 15 +++++++++++++-- hrms/hr/doctype/shift_type/shift_type.py | 6 +++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/hrms/hr/doctype/employee_checkin/employee_checkin.py b/hrms/hr/doctype/employee_checkin/employee_checkin.py index a46a556e78..3a7e578733 100644 --- a/hrms/hr/doctype/employee_checkin/employee_checkin.py +++ b/hrms/hr/doctype/employee_checkin/employee_checkin.py @@ -136,7 +136,7 @@ def mark_attendance_and_link_log( return None elif attendance_status in ("Present", "Absent", "Half Day"): - employee_doc = frappe.get_doc("Employee", employee) + company = frappe.db.get_value("Employee", employee, "company", cache=True) duplicate = get_duplicate_attendance_record(employee, attendance_date, shift) overlapping = get_overlapping_shift_attendance(employee, attendance_date, shift) @@ -147,7 +147,7 @@ def mark_attendance_and_link_log( "attendance_date": attendance_date, "status": attendance_status, "working_hours": working_hours, - "company": employee_doc.company, + "company": company, "shift": shift, "late_entry": late_entry, "early_exit": early_exit, diff --git a/hrms/hr/doctype/shift_assignment/shift_assignment.py b/hrms/hr/doctype/shift_assignment/shift_assignment.py index 71de523004..070e463534 100644 --- a/hrms/hr/doctype/shift_assignment/shift_assignment.py +++ b/hrms/hr/doctype/shift_assignment/shift_assignment.py @@ -286,7 +286,7 @@ def get_employee_shift( shift_details = get_shift_for_timestamp(employee, for_timestamp) # if shift assignment is not found, consider default shift - default_shift = frappe.db.get_value("Employee", employee, "default_shift") + default_shift = frappe.db.get_value("Employee", employee, "default_shift", cache=True) if not shift_details and consider_default_shift: shift_details = get_shift_details(default_shift, for_timestamp) @@ -462,7 +462,18 @@ def get_shift_details(shift_type_name: str, for_timestamp: datetime = None) -> D if for_timestamp is None: for_timestamp = now_datetime() - shift_type = frappe.get_doc("Shift Type", shift_type_name) + shift_type = frappe.get_cached_value( + "Shift Type", + shift_type_name, + [ + "name", + "start_time", + "end_time", + "begin_check_in_before_shift_start_time", + "allow_check_out_after_shift_end_time", + ], + as_dict=1, + ) shift_actual_start = shift_type.start_time - timedelta( minutes=shift_type.begin_check_in_before_shift_start_time ) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 22047ba3eb..d1da34a101 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -151,7 +151,7 @@ def get_start_and_end_dates(self, employee): return: start date = max of `process_attendance_after` and DOJ return: end date = min of shift before `last_sync_of_checkin` and Relieving Date """ - date_of_joining, relieving_date, employee_creation = frappe.db.get_value( + date_of_joining, relieving_date, employee_creation = frappe.get_cached_value( "Employee", employee, ["date_of_joining", "relieving_date", "creation"] ) @@ -235,7 +235,7 @@ def should_mark_attendance(self, employee: str, attendance_date: str) -> bool: def process_auto_attendance_for_all_shifts(): - shift_list = frappe.get_all("Shift Type", "name", {"enable_auto_attendance": "1"}, as_list=True) + shift_list = frappe.get_all("Shift Type", filters={"enable_auto_attendance": "1"}, pluck="name") for shift in shift_list: - doc = frappe.get_doc("Shift Type", shift[0]) + doc = frappe.get_cached_doc("Shift Type", shift) doc.process_auto_attendance() From 2a51ae30f7ea6a10398416d01c6bea763998002d Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 16 May 2023 14:55:36 +0530 Subject: [PATCH 3/6] fix: fetch default shift for active employees only --- hrms/hr/doctype/shift_type/shift_type.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index d1da34a101..0dca005677 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -188,7 +188,6 @@ def get_assigned_employee(self, from_date=None, consider_default_shift=False): assigned_employees = frappe.get_all("Shift Assignment", filters=filters, pluck="employee") if consider_default_shift: - filters = {"default_shift": self.name, "status": ["!=", "Inactive"]} default_shift_employees = self.get_employees_with_default_shift(from_date) return list(set(assigned_employees + default_shift_employees)) @@ -196,7 +195,7 @@ def get_assigned_employee(self, from_date=None, consider_default_shift=False): def get_employees_with_default_shift(self, from_date=None): default_shift_employees = frappe.get_all( - "Employee", filters={"default_shift": self.name, "status": ("!=", "Inactive")}, pluck="name" + "Employee", filters={"default_shift": self.name, "status": "Active"}, pluck="name" ) # exclude employees from default shift list if any other valid shift assignment exists From 1e6be1942f24dcd0f2a6b02e8e60b2961c32f295 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 16 May 2023 15:39:12 +0530 Subject: [PATCH 4/6] perf: use `get_all` instead of `get_list` for fetching checkins - get_list makes no sense here since its a scheduled job and perms should not be checked - only select required fields --- hrms/hr/doctype/shift_type/shift_type.py | 36 +++++++++++++++++------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 0dca005677..09848b175a 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -31,16 +31,7 @@ def process_auto_attendance(self): ): return - filters = { - "skip_auto_attendance": 0, - "attendance": ("is", "not set"), - "time": (">=", self.process_attendance_after), - "shift_actual_end": ("<", self.last_sync_of_checkin), - "shift": self.name, - } - logs = frappe.db.get_list( - "Employee Checkin", fields="*", filters=filters, order_by="employee,time" - ) + logs = self.get_employee_checkins() for key, group in itertools.groupby(logs, key=lambda x: (x["employee"], x["shift_start"])): single_shift_logs = list(group) @@ -73,6 +64,31 @@ def process_auto_attendance(self): for employee in self.get_assigned_employee(self.process_attendance_after, True): self.mark_absent_for_dates_with_no_attendance(employee) + def get_employee_checkins(self) -> list[dict]: + return frappe.get_all( + "Employee Checkin", + fields=[ + "name", + "employee", + "log_type", + "time", + "shift", + "shift_start", + "shift_end", + "shift_actual_start", + "shift_actual_end", + "device_id", + ], + filters={ + "skip_auto_attendance": 0, + "attendance": ("is", "not set"), + "time": (">=", self.process_attendance_after), + "shift_actual_end": ("<", self.last_sync_of_checkin), + "shift": self.name, + }, + order_by="employee,time", + ) + def get_attendance(self, logs): """Return attendance_status, working_hours, late_entry, early_exit, in_time, out_time for a set of logs belonging to a single shift. From 224b885a9ca9ca56ca908b82205ba372535cb1f5 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 16 May 2023 18:56:59 +0530 Subject: [PATCH 5/6] test: rectify fallback shift test --- hrms/hr/doctype/shift_type/test_shift_type.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hrms/hr/doctype/shift_type/test_shift_type.py b/hrms/hr/doctype/shift_type/test_shift_type.py index ee9b0a1a78..fdda97ca0e 100644 --- a/hrms/hr/doctype/shift_type/test_shift_type.py +++ b/hrms/hr/doctype/shift_type/test_shift_type.py @@ -341,7 +341,7 @@ def test_skip_marking_absent_for_a_fallback_default_shift(self): default_shift = setup_shift_type() employee = make_employee( - "test_employee_checkin@example.com", company="_Test Company", shift_type=default_shift.name + "test_employee_checkin@example.com", company="_Test Company", default_shift=default_shift.name ) assigned_shift = setup_shift_type(shift_type="Test Absent with no Attendance") @@ -355,11 +355,15 @@ def test_skip_marking_absent_for_a_fallback_default_shift(self): log_out = make_checkin(employee, timestamp) default_shift.process_auto_attendance() - attendance = frappe.db.get_value("Attendance", {"employee": employee}, "status") + attendance = frappe.db.get_value( + "Attendance", {"employee": employee, "shift": default_shift.name}, "status" + ) self.assertIsNone(attendance) assigned_shift.process_auto_attendance() - attendance = frappe.db.get_value("Attendance", {"employee": employee}, "status") + attendance = frappe.db.get_value( + "Attendance", {"employee": employee, "shift": assigned_shift.name}, "status" + ) self.assertEqual(attendance, "Present") def test_get_start_and_end_dates(self): From b8de418c39b39d3c43578b95b3010370b4482afd Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 16 May 2023 18:58:43 +0530 Subject: [PATCH 6/6] perf: optimize fn to fetch default shift employees - early return if no employee found - reuse filters --- hrms/hr/doctype/shift_type/shift_type.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 09848b175a..818c9768e2 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -197,32 +197,29 @@ def get_start_and_end_dates(self, employee): return start_date, end_date def get_assigned_employee(self, from_date=None, consider_default_shift=False): - filters = {"shift_type": self.name, "docstatus": "1"} + filters = {"shift_type": self.name, "docstatus": "1", "status": "Active"} if from_date: - filters["start_date"] = (">", from_date) + filters["start_date"] = (">=", from_date) assigned_employees = frappe.get_all("Shift Assignment", filters=filters, pluck="employee") if consider_default_shift: - default_shift_employees = self.get_employees_with_default_shift(from_date) + default_shift_employees = self.get_employees_with_default_shift(filters) return list(set(assigned_employees + default_shift_employees)) return assigned_employees - def get_employees_with_default_shift(self, from_date=None): + def get_employees_with_default_shift(self, filters: dict) -> list: default_shift_employees = frappe.get_all( "Employee", filters={"default_shift": self.name, "status": "Active"}, pluck="name" ) - # exclude employees from default shift list if any other valid shift assignment exists - filters = { - "docstatus": "1", - "status": "Active", - "shift_type": ["!=", self.name], - } + if not default_shift_employees: + return [] - if from_date: - filters["start_date"] = (">", from_date) + # exclude employees from default shift list if any other valid shift assignment exists + del filters["shift_type"] + filters["employee"] = ("in", default_shift_employees) active_shift_assignments = frappe.get_all( "Shift Assignment",