diff --git a/tapir/shifts/services/shift_watch_creation_service.py b/tapir/shifts/services/shift_watch_creation_service.py index b5f92c486..e6c143d16 100644 --- a/tapir/shifts/services/shift_watch_creation_service.py +++ b/tapir/shifts/services/shift_watch_creation_service.py @@ -208,8 +208,6 @@ def get_capability_status_changes( if not watched_capabilities: return [] - notifications = [] - current_slots = ShiftSlot.objects.filter( id__in=this_valid_slot_ids ).values_list("required_capabilities", flat=True) @@ -218,22 +216,19 @@ def get_capability_status_changes( "required_capabilities", flat=True ) - current_capabilities_set = set() - for capabilities in current_slots: - current_capabilities_set.update(capabilities) - - last_capabilities_set = set() - for capabilities in last_slots: - last_capabilities_set.update(capabilities) + current_capabilities = [capability for capabilities in capabilities_by_current_slot for capability in capabilities] + last_capabilities = [cap for caps in last_slots for cap in caps] + notifications = [] for capability in watched_capabilities: - has_now = capability in current_capabilities_set - had_before = capability in last_capabilities_set + current_count = current_capabilities.count(capability) + last_count = last_capabilities.count(capability) - if has_now and not had_before: - notifications.append(f"Member with capability added: {capability}") - elif not has_now and had_before: + if current_count > last_count: + notifications.append(f"Member with capability registered: {capability}") + elif current_count < last_count: notifications.append( f"Member with capability unregistered: {capability}" ) + return notifications diff --git a/tapir/shifts/tests/test_get_capability_status_change.py b/tapir/shifts/tests/test_get_capability_status_change.py index 1e2a494cd..cd08de3f8 100644 --- a/tapir/shifts/tests/test_get_capability_status_change.py +++ b/tapir/shifts/tests/test_get_capability_status_change.py @@ -37,7 +37,9 @@ def set_up_slots(db): [1, 2], [2], [ShiftUserCapability.SHIFT_COORDINATOR], - [f"Member with capability added: {ShiftUserCapability.SHIFT_COORDINATOR}"], + [ + f"Member with capability registered: {ShiftUserCapability.SHIFT_COORDINATOR}" + ], ), ( [2], @@ -52,7 +54,9 @@ def set_up_slots(db): [3], [2], [ShiftUserCapability.HANDLING_CHEESE], - [f"Member with capability added: {ShiftUserCapability.HANDLING_CHEESE}"], + [ + f"Member with capability registered: {ShiftUserCapability.HANDLING_CHEESE}" + ], ), ( [3], diff --git a/tapir/shifts/tests/test_shiftwatch_notification.py b/tapir/shifts/tests/test_shiftwatch_notification.py index 795fb8866..971b4ef7c 100644 --- a/tapir/shifts/tests/test_shiftwatch_notification.py +++ b/tapir/shifts/tests/test_shiftwatch_notification.py @@ -10,6 +10,7 @@ RecurringShiftWatch, ShiftAttendance, ShiftSlot, + ShiftUserCapability, StaffingStatusChoices, get_staffingstatus_choices, ) @@ -30,25 +31,33 @@ def create_shift_with_attendance(num_attendances): slot = ShiftSlot.objects.create(shift=shift, name="cheese-making") user = TapirUserFactory.create() ShiftAttendance.objects.create(user=user, slot=slot) - slots.append(slot.pk) + slots.append(slot) return shift, slots def create_shift_watch( - user, shift, slots, last_staffing_status=None, staffing_status=None + user, + shift, + last_valid_slot_ids, + last_staffing_status=None, + staffing_status=None, + watched_capabilities=None, ): if last_staffing_status is None: last_staffing_status = ShiftWatchCreator.get_initial_staffing_status_for_shift( shift=shift ) if staffing_status is None: - staffing_status = [event.value for event in get_staffingstatus_choices()] + staffing_status = [] + if watched_capabilities is None: + watched_capabilities = [] return ShiftWatchFactory( user=user, shift=shift, - last_valid_slot_ids=slots, + last_valid_slot_ids=[slot.pk for slot in last_valid_slot_ids], staffing_status=staffing_status, last_staffing_status=last_staffing_status, + watched_capabilities=watched_capabilities, ) @@ -62,15 +71,16 @@ def setUp(self): self.NUM_REQUIRED_ATTENDANCE ) - def unregister_first_slot(self): - first_slot = self.slots[0] - first_shift_attendance = ShiftAttendance.objects.filter(slot=first_slot).first() + def unregister_slot(self, slot: ShiftSlot | None = None): + if slot is None: + slot = self.slots[0] + first_shift_attendance = ShiftAttendance.objects.filter(slot=slot).first() first_shift_attendance.state = ShiftAttendance.State.LOOKING_FOR_STAND_IN first_shift_attendance.save() - def assert_email_sent(self, expected_status_choice): + def assert_email_sent(self, expected_status_choice: str): self.assertEqual(len(mail.outbox), 1) - self.assertIn(str(expected_status_choice.label), mail.outbox[0].body) + self.assertIn(str(expected_status_choice), mail.outbox[0].body) self.assertEmailOfClass_GotSentTo( ShiftWatchEmailBuilder, self.USER_EMAIL_ADDRESS, mail.outbox[0] ) @@ -79,21 +89,22 @@ def test_handle_watchedShiftIsUnderstaffed_correctNotificationIsSent(self): self.shift_watch = create_shift_watch( user=self.user, shift=self.shift_ok_first, - slots=self.slots, + last_valid_slot_ids=self.slots, staffing_status=[StaffingStatusChoices.UNDERSTAFFED], ) Command().handle() self.assertEqual(0, len(mail.outbox)) - self.unregister_first_slot() + self.unregister_slot() Command().handle() - self.assert_email_sent(StaffingStatusChoices.UNDERSTAFFED) + self.assertEqual(1, len(mail.outbox)) + self.assert_email_sent(StaffingStatusChoices.UNDERSTAFFED.label) def test_handle_watchedShiftIsAlright_noNotificationIsSent(self): self.shift_watch = create_shift_watch( user=self.user, shift=self.shift_ok_first, - slots=self.slots, + last_valid_slot_ids=self.slots, staffing_status=list(get_staffingstatus_choices()), ) Command().handle() @@ -110,10 +121,11 @@ def test_handle_initialWatchUnderstaffedShift_noInitialMailIsSent(self): create_shift_watch( user=user, shift=shift_understaffed, - slots=slots, + last_valid_slot_ids=slots, last_staffing_status=ShiftWatchCreator.get_initial_staffing_status_for_shift( shift=shift_understaffed ), + staffing_status=list(get_staffingstatus_choices()), ) Command().handle() @@ -126,24 +138,24 @@ def test_handle_initialWatchUnderstaffedShift_noInitialMailIsSent(self): Command().handle() - self.assert_email_sent(StaffingStatusChoices.ALL_CLEAR) + self.assert_email_sent(StaffingStatusChoices.ALL_CLEAR.label) def test_handle_triggeredMultipleTimes_onlyOneMailIsSent(self): self.shift_watch = create_shift_watch( user=self.user, shift=self.shift_ok_first, - slots=self.slots, + last_valid_slot_ids=self.slots, staffing_status=[StaffingStatusChoices.UNDERSTAFFED], ) - self.unregister_first_slot() + self.unregister_slot() self.assertEqual(len(mail.outbox), 0) for _ in range(3): Command().handle() - self.assert_email_sent(StaffingStatusChoices.UNDERSTAFFED) + self.assert_email_sent(StaffingStatusChoices.UNDERSTAFFED.label) def test_handle_watchedShiftIsCurrentlyRunning_correctNotificationIsSent(self): self.shift_ok_first.start_time = timezone.now() - datetime.timedelta(hours=4) @@ -153,14 +165,14 @@ def test_handle_watchedShiftIsCurrentlyRunning_correctNotificationIsSent(self): self.shift_watch = create_shift_watch( user=self.user, shift=self.shift_ok_first, - slots=self.slots, + last_valid_slot_ids=self.slots, staffing_status=[StaffingStatusChoices.UNDERSTAFFED], ) - self.unregister_first_slot() + self.unregister_slot() Command().handle() - self.assert_email_sent(StaffingStatusChoices.UNDERSTAFFED) + self.assert_email_sent(StaffingStatusChoices.UNDERSTAFFED.label) def test_handle_shiftInThePast_noNotification(self): @@ -173,10 +185,11 @@ def test_handle_shiftInThePast_noNotification(self): self.shift_watch = create_shift_watch( user=self.user, shift=self.shift_ok_first, - slots=self.slots, + last_valid_slot_ids=self.slots, + staffing_status=list(get_staffingstatus_choices()), ) - self.unregister_first_slot() + self.unregister_slot() Command().handle() @@ -196,3 +209,45 @@ def test_handle_recurring_noInitialMailIsSent(self): Command().handle() self.assertEqual(len(mail.outbox), 0) + + def test_handle_registerAnotherUserToSameCapability_MailSent(self): + for slot in self.slots: + slot.required_capabilities = [ShiftUserCapability.SHIFT_COORDINATOR] + slot.save() + + self.shift_watch = create_shift_watch( + user=self.user, + shift=self.shift_ok_first, + last_valid_slot_ids=self.slots, + staffing_status=[], + watched_capabilities=[ShiftUserCapability.SHIFT_COORDINATOR], + ) + + slot_to_register = ShiftSlot.objects.create( + shift=self.shift_ok_first, name="cheese-making" + ) + slot_to_register.required_capabilities = [ShiftUserCapability.SHIFT_COORDINATOR] + self.slots.append(slot_to_register) + + # assert first and second shift have attendance and third shift has no attendance + self.assertTrue( + ShiftAttendance.objects.filter(slot__in=self.slots[:2]).exists() + ) + self.assertFalse(ShiftAttendance.objects.filter(slot=self.slots[2]).exists()) + + Command().handle() + self.assertEqual(0, len(mail.outbox)) + + # register user to third slot + ShiftAttendance.objects.create( + user=TapirUserFactory.create(), slot=slot_to_register + ) + slot_to_register.save() + + self.assertEqual( + slot_to_register.required_capabilities, + [ShiftUserCapability.SHIFT_COORDINATOR], + ) + + Command().handle() + self.assert_email_sent("registered")