From 7bcba1f70b26cb64cdf12d4fdef028772f677afc Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Wed, 12 Feb 2025 10:29:18 +0530 Subject: [PATCH 1/3] fix: validation for circular shift times test: validation for circular shift times --- hrms/hr/doctype/shift_type/shift_type.py | 32 +++++++++++++++- hrms/hr/doctype/shift_type/test_shift_type.py | 38 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index 47b609e5d6..b4003fc454 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -8,7 +8,7 @@ import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import cint, create_batch, get_datetime, get_time, getdate +from frappe.utils import add_days, cint, create_batch, get_datetime, get_time, getdate, time_diff from erpnext.setup.doctype.employee.employee import get_holiday_list_for_employee from erpnext.setup.doctype.holiday_list.holiday_list import is_holiday @@ -32,6 +32,7 @@ def validate(self): title=_("Unmarked Check-in Logs Found"), msg=_("Mark attendance for existing check-in/out logs before changing shift settings"), ) + self.validate_circular_shift() def is_field_modified(self, fieldname): return not self.is_new() and self.has_value_changed(fieldname) @@ -42,6 +43,31 @@ def unlinked_checkins_exist(self): {"shift": self.name, "attendance": ["is", "not set"], "skip_auto_attendance": 0, "offshift": 0}, ) + def validate_circular_shift(self): + start = get_time(self.start_time) + end = get_time(self.end_time) + shift_start = datetime.combine(getdate(), start) + if start < end: + shift_end = datetime.combine(getdate(), end) + elif start > end: + shift_end = datetime.combine(add_days(getdate(), 1), end) + else: + frappe.throw( + title=_("Invalid Shift Times"), + msg=_("Start time and end time cannot be same."), + ) + total_minutes = ( + time_diff_in_minutes(shift_end, shift_start) + + self.allow_check_out_after_shift_end_time + + self.begin_check_in_before_shift_start_time + ) + + if total_minutes >= 1440: + frappe.throw( + title=_("Invalid Shift Times"), + msg=_("Please change shift times or buffers to avoid overlap."), + ) + @frappe.whitelist() def process_auto_attendance(self): if ( @@ -296,3 +322,7 @@ def process_auto_attendance_for_all_shifts(): for shift in shift_list: doc = frappe.get_cached_doc("Shift Type", shift) doc.process_auto_attendance() + + +def time_diff_in_minutes(string_ed_date, string_st_date): + return time_diff(string_ed_date, string_st_date).total_seconds() / 60 diff --git a/hrms/hr/doctype/shift_type/test_shift_type.py b/hrms/hr/doctype/shift_type/test_shift_type.py index 139f35be86..d97504e505 100644 --- a/hrms/hr/doctype/shift_type/test_shift_type.py +++ b/hrms/hr/doctype/shift_type/test_shift_type.py @@ -724,6 +724,44 @@ def test_validation_for_unlinked_logs_before_changing_important_shift_configurat get_time(frappe.get_value("Shift Type", shift.name, "start_time")), get_time("10:15:00") ) + def test_circular_shift_times(self): + # single day shift + shift_type = frappe.get_doc( + { + "doctype": "Shift Type", + "__newname": "Test Shift Validation", + "start_time": "09:00:00", + "end_time": "18:00:00", + "enable_auto_attendance": 1, + "determine_check_in_and_check_out": "Alternating entries as IN and OUT during the same shift", + "working_hours_calculation_based_on": "First Check-in and Last Check-out", + "begin_check_in_before_shift_start_time": 500, + "allow_check_out_after_shift_end_time": 500, + "process_attendance_after": add_days(getdate(), -2), + "last_sync_of_checkin": now_datetime() + timedelta(days=1), + } + ) + + self.assertRaises(frappe.ValidationError, shift_type.save) + + # two day shift + shift_type = frappe.get_doc( + { + "doctype": "Shift Type", + "__newname": "Test Shift Validation", + "start_time": "18:00:00", + "end_time": "03:00:00", + "enable_auto_attendance": 1, + "determine_check_in_and_check_out": "Alternating entries as IN and OUT during the same shift", + "working_hours_calculation_based_on": "First Check-in and Last Check-out", + "begin_check_in_before_shift_start_time": 500, + "allow_check_out_after_shift_end_time": 500, + "process_attendance_after": add_days(getdate(), -2), + "last_sync_of_checkin": now_datetime() + timedelta(days=1), + } + ) + self.assertRaises(frappe.ValidationError, shift_type.save) + def setup_shift_type(**args): args = frappe._dict(args) From 1a310ac08ef522278ff54354ccf7197e61f047c5 Mon Sep 17 00:00:00 2001 From: Asmita Hase Date: Mon, 17 Feb 2025 16:27:31 +0530 Subject: [PATCH 2/3] refactor: validation for circular shift times --- hrms/hr/doctype/shift_type/shift_type.py | 83 +++++++++++++++--------- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/hrms/hr/doctype/shift_type/shift_type.py b/hrms/hr/doctype/shift_type/shift_type.py index b4003fc454..516fe802f5 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -27,12 +27,64 @@ class ShiftType(Document): def validate(self): + start = get_time(self.start_time) + end = get_time(self.end_time) + self.validate_same_start_and_end(start, end) + self.validate_circular_shift(start, end) + self.validate_unlinked_logs() + + def validate_same_start_and_end(self, start_time: datetime.time, end_time: datetime.time): + if start_time == end_time: + frappe.throw( + title=_("Invalid Shift Times"), + msg=_("Start time and end time cannot be same."), + ) + + def validate_circular_shift(self, start_time: datetime.time, end_time: datetime.time): + shift_start, shift_end = self.get_shift_start_and_shift_end(start_time, end_time) + if self.get_total_shift_duration_in_minutes(shift_start, shift_end) >= 1440: + max_label = self.get_max_shift_buffer_label() + frappe.throw( + title=_("Invalid Shift Times"), + msg=_("Please change {0} to avoid shift time overlapping with itself").format( + frappe.bold(max_label) + ), + ) + + def get_shift_start_and_shift_end(self, start_time: datetime.time, end_time: datetime.time): + shift_start = datetime.combine(getdate(), start_time) + if start_time < end_time: + shift_end = datetime.combine(getdate(), end_time) + elif start_time > end_time: + shift_end = datetime.combine(add_days(getdate(), 1), end_time) + return shift_start, shift_end + + def get_total_shift_duration_in_minutes( + self, shift_start: datetime.time, shift_end: datetime.time + ) -> int: + return ( + (time_diff(shift_end, shift_start).total_seconds() / 60) + + self.allow_check_out_after_shift_end_time + + self.begin_check_in_before_shift_start_time + ) + + def get_max_shift_buffer_label(self) -> str: + labels = { + self.meta.get_label( + "allow_check_out_after_shift_end_time" + ): self.allow_check_out_after_shift_end_time, + self.meta.get_label( + "begin_check_in_before_shift_start_time" + ): self.begin_check_in_before_shift_start_time, + } + return max(labels, key=labels.get) + + def validate_unlinked_logs(self): if self.is_field_modified("start_time") and self.unlinked_checkins_exist(): frappe.throw( title=_("Unmarked Check-in Logs Found"), msg=_("Mark attendance for existing check-in/out logs before changing shift settings"), ) - self.validate_circular_shift() def is_field_modified(self, fieldname): return not self.is_new() and self.has_value_changed(fieldname) @@ -43,31 +95,6 @@ def unlinked_checkins_exist(self): {"shift": self.name, "attendance": ["is", "not set"], "skip_auto_attendance": 0, "offshift": 0}, ) - def validate_circular_shift(self): - start = get_time(self.start_time) - end = get_time(self.end_time) - shift_start = datetime.combine(getdate(), start) - if start < end: - shift_end = datetime.combine(getdate(), end) - elif start > end: - shift_end = datetime.combine(add_days(getdate(), 1), end) - else: - frappe.throw( - title=_("Invalid Shift Times"), - msg=_("Start time and end time cannot be same."), - ) - total_minutes = ( - time_diff_in_minutes(shift_end, shift_start) - + self.allow_check_out_after_shift_end_time - + self.begin_check_in_before_shift_start_time - ) - - if total_minutes >= 1440: - frappe.throw( - title=_("Invalid Shift Times"), - msg=_("Please change shift times or buffers to avoid overlap."), - ) - @frappe.whitelist() def process_auto_attendance(self): if ( @@ -322,7 +349,3 @@ def process_auto_attendance_for_all_shifts(): for shift in shift_list: doc = frappe.get_cached_doc("Shift Type", shift) doc.process_auto_attendance() - - -def time_diff_in_minutes(string_ed_date, string_st_date): - return time_diff(string_ed_date, string_st_date).total_seconds() / 60 From b51ac227c38e2f58d0556b39cc16289dcdbb3a85 Mon Sep 17 00:00:00 2001 From: Asmita Hase <44727809+asmitahase@users.noreply.github.com> Date: Wed, 19 Feb 2025 16:43:45 +0530 Subject: [PATCH 3/3] refactor: clearer validation message --- 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 516fe802f5..8b70beec0b 100644 --- a/hrms/hr/doctype/shift_type/shift_type.py +++ b/hrms/hr/doctype/shift_type/shift_type.py @@ -46,7 +46,7 @@ def validate_circular_shift(self, start_time: datetime.time, end_time: datetime. max_label = self.get_max_shift_buffer_label() frappe.throw( title=_("Invalid Shift Times"), - msg=_("Please change {0} to avoid shift time overlapping with itself").format( + msg=_("Please reduce {0} to avoid shift time overlapping with itself").format( frappe.bold(max_label) ), )