fix: multiple attendances per capability

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

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],

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")