Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validation for circular shift times #2783

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix: validation for circular shift times #2783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion hrms/hr/doctype/shift_type/shift_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -42,6 +43,31 @@ def unlinked_checkins_exist(self):
{"shift": self.name, "attendance": ["is", "not set"], "skip_auto_attendance": 0, "is_invalid": 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."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message is unclear. Point to the exact fields we need to change. "buffers" is unclear

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe explicitly say, shift timings are overlapping with itself
Overlapping with what is ambiguous

)

@frappe.whitelist()
def process_auto_attendance(self):
if (
Expand Down Expand Up @@ -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
38 changes: 38 additions & 0 deletions hrms/hr/doctype/shift_type/test_shift_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading