From fea06453703f0e80f72b8210922c0db199327066 Mon Sep 17 00:00:00 2001 From: krantheman Date: Sat, 29 Jun 2024 12:07:14 +0530 Subject: [PATCH 1/4] fix(Shift Assignment): timings overlap conditions (cherry picked from commit 981c756d3d2667b60ed2930048cf266e01acd3db) --- .../shift_assignment/shift_assignment.py | 26 ++++++------------- .../hr/doctype/shift_request/shift_request.py | 6 ++--- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/hrms/hr/doctype/shift_assignment/shift_assignment.py b/hrms/hr/doctype/shift_assignment/shift_assignment.py index 995e1265d5..f6a8410428 100644 --- a/hrms/hr/doctype/shift_assignment/shift_assignment.py +++ b/hrms/hr/doctype/shift_assignment/shift_assignment.py @@ -35,9 +35,9 @@ def validate_overlapping_shifts(self): if len(overlapping_dates): self.validate_same_date_multiple_shifts(overlapping_dates) # if dates are overlapping, check if timings are overlapping, else allow - overlapping_timings = has_overlapping_timings(self.shift_type, overlapping_dates[0].shift_type) - if overlapping_timings: - self.throw_overlap_error(overlapping_dates[0]) + for d in overlapping_dates: + if has_overlapping_timings(self.shift_type, d.shift_type): + self.throw_overlap_error(d) def validate_same_date_multiple_shifts(self, overlapping_dates): if cint(frappe.db.get_single_value("HR Settings", "allow_multiple_shift_assignments")): @@ -106,25 +106,15 @@ def has_overlapping_timings(shift_1: str, shift_2: str) -> bool: """ Accepts two shift types and checks whether their timings are overlapping """ - if shift_1 == shift_2: - return True s1 = frappe.db.get_value("Shift Type", shift_1, ["start_time", "end_time"], as_dict=True) s2 = frappe.db.get_value("Shift Type", shift_2, ["start_time", "end_time"], as_dict=True) - if ( - # shift 1 spans across 2 days - (s1.start_time > s1.end_time and s1.start_time < s2.end_time) - or (s1.start_time > s1.end_time and s2.start_time < s1.end_time) - or (s1.start_time > s1.end_time and s2.start_time > s2.end_time) - # both shifts fall on the same day - or (s1.start_time < s2.end_time and s2.start_time < s1.end_time) - # shift 2 spans across 2 days - or (s1.start_time < s2.end_time and s2.start_time > s2.end_time) - or (s2.start_time < s1.end_time and s2.start_time > s2.end_time) - ): - return True - return False + for d in [s1, s2]: + if d.end_time <= d.start_time: + d.end_time += timedelta(days=1) + + return s1.end_time >= s2.start_time and s1.start_time <= s2.end_time @frappe.whitelist() diff --git a/hrms/hr/doctype/shift_request/shift_request.py b/hrms/hr/doctype/shift_request/shift_request.py index 5974768190..7ecb77f3b0 100644 --- a/hrms/hr/doctype/shift_request/shift_request.py +++ b/hrms/hr/doctype/shift_request/shift_request.py @@ -81,9 +81,9 @@ def validate_overlapping_shift_requests(self): overlapping_dates = self.get_overlapping_dates() if len(overlapping_dates): # if dates are overlapping, check if timings are overlapping, else allow - overlapping_timings = has_overlapping_timings(self.shift_type, overlapping_dates[0].shift_type) - if overlapping_timings: - self.throw_overlap_error(overlapping_dates[0]) + for d in overlapping_dates: + if has_overlapping_timings(self.shift_type, d.shift_type): + self.throw_overlap_error(d) def get_overlapping_dates(self): if not self.name: From a48889ac40b9a77a3cff0e27a310539a8f3f4146 Mon Sep 17 00:00:00 2001 From: krantheman Date: Mon, 1 Jul 2024 12:44:31 +0530 Subject: [PATCH 2/4] fix(Shift Assignment): allow shift to start at end_time of another shift (and vice versa) (cherry picked from commit 5e880f23c7e699a2bbcb7fd52a3112f5faa480a4) --- hrms/hr/doctype/shift_assignment/shift_assignment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hrms/hr/doctype/shift_assignment/shift_assignment.py b/hrms/hr/doctype/shift_assignment/shift_assignment.py index f6a8410428..02ada2c92c 100644 --- a/hrms/hr/doctype/shift_assignment/shift_assignment.py +++ b/hrms/hr/doctype/shift_assignment/shift_assignment.py @@ -114,7 +114,7 @@ def has_overlapping_timings(shift_1: str, shift_2: str) -> bool: if d.end_time <= d.start_time: d.end_time += timedelta(days=1) - return s1.end_time >= s2.start_time and s1.start_time <= s2.end_time + return s1.end_time > s2.start_time and s1.start_time < s2.end_time @frappe.whitelist() From 653ff40bf884e2f8c06623359a66c83e7c9592e6 Mon Sep 17 00:00:00 2001 From: krantheman Date: Mon, 1 Jul 2024 13:27:51 +0530 Subject: [PATCH 3/4] test: refactor - fix shift overlapping timings tests - refactor tests to use helper functions and improve readability - remove redundant test test_multiple_shift_assignments_for_same_day (cherry picked from commit 155b8152dafb9d9172af937b8f91c2a17758496c) --- .../shift_assignment/test_shift_assignment.py | 132 +++++++----------- 1 file changed, 47 insertions(+), 85 deletions(-) diff --git a/hrms/hr/doctype/shift_assignment/test_shift_assignment.py b/hrms/hr/doctype/shift_assignment/test_shift_assignment.py index 19bac6a8c8..47175147b2 100644 --- a/hrms/hr/doctype/shift_assignment/test_shift_assignment.py +++ b/hrms/hr/doctype/shift_assignment/test_shift_assignment.py @@ -24,68 +24,48 @@ def setUp(self): frappe.db.delete("Shift Type") def test_overlapping_for_ongoing_shift(self): + shift = "Day Shift" + employee = "_T-Employee-00001" + date = nowdate() + # shift should be Ongoing if Only start_date is present and status = Active - setup_shift_type(shift_type="Day Shift") - make_shift_assignment("Day Shift", "_T-Employee-00001", nowdate()) + setup_shift_type(shift_type=shift) + make_shift_assignment(shift, employee, date) # shift ends before ongoing shift starts - non_overlapping_shift = make_shift_assignment( - "Day Shift", "_T-Employee-00001", add_days(nowdate(), -1), add_days(nowdate(), -1) - ) + non_overlapping_shift = make_shift_assignment(shift, employee, add_days(date, -1), add_days(date, -1)) self.assertEqual(non_overlapping_shift.docstatus, 1) - overlapping_shift = frappe.get_doc( - { - "doctype": "Shift Assignment", - "shift_type": "Day Shift", - "company": "_Test Company", - "employee": "_T-Employee-00001", - "start_date": add_days(nowdate(), 2), - } - ) + overlapping_shift = make_shift_assignment(shift, employee, add_days(date, 2), do_not_submit=True) self.assertRaises(OverlappingShiftError, overlapping_shift.save) def test_multiple_shift_assignments_for_same_date(self): + employee = "_T-Employee-00001" + date = nowdate() + setup_shift_type(shift_type="Day Shift") - make_shift_assignment("Day Shift", "_T-Employee-00001", nowdate(), add_days(nowdate(), 30)) + make_shift_assignment("Day Shift", employee, date) setup_shift_type(shift_type="Night Shift", start_time="19:00:00", end_time="23:00:00") - shift_assignment_2 = frappe.get_doc( - { - "doctype": "Shift Assignment", - "shift_type": "Night Shift", - "company": "_Test Company", - "employee": "_T-Employee-00001", - "start_date": nowdate(), - "end_date": add_days(nowdate(), 30), - "status": "Active", - } - ) + assignment = make_shift_assignment("Night Shift", employee, date, do_not_submit=True) frappe.db.set_single_value("HR Settings", "allow_multiple_shift_assignments", 0) - self.assertRaises(MultipleShiftError, shift_assignment_2.save) + self.assertRaises(MultipleShiftError, assignment.save) frappe.db.set_single_value("HR Settings", "allow_multiple_shift_assignments", 1) - shift_assignment_2.save() # would throw error if multiple shift assignments not allowed + assignment.save() # would throw error if multiple shift assignments not allowed def test_overlapping_for_fixed_period_shift(self): - # shift should is for Fixed period if Only start_date and end_date both are present and status = Active - setup_shift_type(shift_type="Day Shift") - make_shift_assignment("Day Shift", "_T-Employee-00001", nowdate(), add_days(nowdate(), 30)) - - # it should not allowed within period of any shift. - shift_assignment_3 = frappe.get_doc( - { - "doctype": "Shift Assignment", - "shift_type": "Day Shift", - "company": "_Test Company", - "employee": "_T-Employee-00001", - "start_date": add_days(nowdate(), 10), - "end_date": add_days(nowdate(), 35), - "status": "Active", - } - ) + shift = "Day Shift" + employee = "_T-Employee-00001" + date = nowdate() + + setup_shift_type(shift_type=shift) + make_shift_assignment(shift, employee, date, add_days(date, 30)) - self.assertRaises(OverlappingShiftError, shift_assignment_3.save) + assignment = make_shift_assignment( + shift, employee, add_days(date, 10), add_days(date, 35), do_not_submit=True + ) + self.assertRaises(OverlappingShiftError, assignment.save) def test_overlapping_for_a_fixed_period_shift_and_ongoing_shift(self): employee = make_employee("test_shift_assignment@example.com", company="_Test Company") @@ -101,37 +81,30 @@ def test_overlapping_for_a_fixed_period_shift_and_ongoing_shift(self): date = getdate() # shift assignment without end date - shift2 = frappe.get_doc( - { - "doctype": "Shift Assignment", - "shift_type": shift_type.name, - "company": "_Test Company", - "employee": employee, - "start_date": date, - } - ) - self.assertRaises(OverlappingShiftError, shift2.insert) + assignment = make_shift_assignment("Shift 2", employee, date, do_not_submit=True) + self.assertRaises(OverlappingShiftError, assignment.save) def test_overlap_for_shifts_on_same_day_with_overlapping_timeslots(self): employee = make_employee("test_shift_assignment@example.com", company="_Test Company") + date = getdate() # shift setup for 8-12 - shift_type = setup_shift_type(shift_type="Shift 1", start_time="08:00:00", end_time="12:00:00") - date = getdate() - make_shift_assignment(shift_type.name, employee, date) + setup_shift_type(shift_type="Shift 1", start_time="08:00:00", end_time="12:00:00") + make_shift_assignment("Shift 1", employee, date) # shift setup for 11-15 - shift_type = setup_shift_type(shift_type="Shift 2", start_time="11:00:00", end_time="15:00:00") - shift2 = frappe.get_doc( - { - "doctype": "Shift Assignment", - "shift_type": shift_type.name, - "company": "_Test Company", - "employee": employee, - "start_date": date, - } - ) - self.assertRaises(OverlappingShiftError, shift2.insert) + setup_shift_type(shift_type="Shift 2", start_time="11:00:00", end_time="15:00:00") + assignment = make_shift_assignment("Shift 2", employee, date, do_not_submit=True) + self.assertRaises(OverlappingShiftError, assignment.save) + + # shift setup for 12-16 + setup_shift_type(shift_type="Shift 3", start_time="12:00:00", end_time="16:00:00") + make_shift_assignment("Shift 3", employee, date) + + # shift setup for 15-19 + setup_shift_type(shift_type="Shift 4", start_time="15:00:00", end_time="19:00:00") + assignment = make_shift_assignment("Shift 4", employee, date, do_not_submit=True) + self.assertRaises(OverlappingShiftError, assignment.save) def test_overlap_for_midnight_shifts(self): employee = make_employee("test_shift_assignment@example.com", company="_Test Company") @@ -140,10 +113,8 @@ def test_overlap_for_midnight_shifts(self): overlapping_shifts = [ # s1(start, end), s2(start, end) [("22:00:00", "02:00:00"), ("21:00:00", "23:00:00")], - [("22:00:00", "02:00:00"), ("01:00:00", "03:00:00")], [("22:00:00", "02:00:00"), ("20:00:00", "01:00:00")], [("01:00:00", "02:00:00"), ("01:30:00", "03:00:00")], - [("01:00:00", "02:00:00"), ("22:00:00", "03:00:00")], [("21:00:00", "23:00:00"), ("22:00:00", "03:00:00")], ] @@ -166,22 +137,13 @@ def test_overlap_for_midnight_shifts(self): assignment = make_shift_assignment(shift_type.name, employee, date) # no overlap - shift_type.update({"start_time": "02:00:00", "end_time": "3:00:00"}) - shift_type.save() + shift_type = setup_shift_type(shift_type="Shift 3", start_time="01:00:00", end_time="05:00:00") assignment = make_shift_assignment(shift_type.name, employee, date) - def test_multiple_shift_assignments_for_same_day(self): - employee = make_employee("test_shift_assignment@example.com", company="_Test Company") - - # shift setup for 8-12 - shift_type = setup_shift_type(shift_type="Shift 1", start_time="08:00:00", end_time="12:00:00") - date = getdate() - make_shift_assignment(shift_type.name, employee, date) - - # shift setup for 13-15 - shift_type = setup_shift_type(shift_type="Shift 2", start_time="13:00:00", end_time="15:00:00") - date = getdate() - make_shift_assignment(shift_type.name, employee, date) + # overlap + shift_type = setup_shift_type(shift_type="Shift 4", start_time="21:00:00", end_time="02:00:00") + assignment = make_shift_assignment(shift_type.name, employee, date, do_not_submit=True) + self.assertRaises(OverlappingShiftError, assignment.save) def test_calendar(self): employee1 = make_employee("test_shift_assignment1@example.com", company="_Test Company") From f76895780074fb34320ef3ce0e1726d9bce75f1c Mon Sep 17 00:00:00 2001 From: krantheman Date: Mon, 1 Jul 2024 15:14:44 +0530 Subject: [PATCH 4/4] fix(Attendance): overlapping shift attendance (cherry picked from commit 0ea2d125653b03676304e3a7207c4c8e98929eae) --- hrms/hr/doctype/attendance/attendance.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hrms/hr/doctype/attendance/attendance.py b/hrms/hr/doctype/attendance/attendance.py index 7a34db7ee7..f537aff92b 100644 --- a/hrms/hr/doctype/attendance/attendance.py +++ b/hrms/hr/doctype/attendance/attendance.py @@ -141,8 +141,10 @@ def get_overlapping_shift_attendance(self) -> dict: ) ).run(as_dict=True) - if same_date_attendance and has_overlapping_timings(self.shift, same_date_attendance[0].shift): - return same_date_attendance[0] + for d in same_date_attendance: + if has_overlapping_timings(self.shift, d.shift): + return d + return {} def validate_employee_status(self):