From 5322baae5a19e3c8b1a37cefb140498341b00092 Mon Sep 17 00:00:00 2001 From: crosspolar <18083323+crosspolar@users.noreply.github.com> Date: Sun, 5 Jul 2026 10:47:12 +0200 Subject: [PATCH 1/5] fix: notify also in case someone with that capability has registered already --- .../services/shift_watch_creation_service.py | 26 +++--- .../tests/test_shiftwatch_notification.py | 92 ++++++++++++++----- 2 files changed, 83 insertions(+), 35 deletions(-) diff --git a/tapir/shifts/services/shift_watch_creation_service.py b/tapir/shifts/services/shift_watch_creation_service.py index b5f92c486..6a589e2d2 100644 --- a/tapir/shifts/services/shift_watch_creation_service.py +++ b/tapir/shifts/services/shift_watch_creation_service.py @@ -1,3 +1,5 @@ +from collections import Counter + from django.db.models import Q from tapir.shifts.models import ( @@ -208,8 +210,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 +218,24 @@ def get_capability_status_changes( "required_capabilities", flat=True ) - current_capabilities_set = set() - for capabilities in current_slots: - current_capabilities_set.update(capabilities) + current_capabilities = [cap for caps in current_slots for cap in caps] + last_capabilities = [cap for caps in last_slots for cap in caps] - last_capabilities_set = set() - for capabilities in last_slots: - last_capabilities_set.update(capabilities) + current_counter = Counter(current_capabilities) + last_counter = Counter(last_capabilities) + notifications = [] for capability in watched_capabilities: - has_now = capability in current_capabilities_set - had_before = capability in last_capabilities_set + current_count = current_counter[capability] + print(f"current_count: {current_count}") + last_count = last_counter[capability] + print(f"last_count: {last_count}") - if has_now and not had_before: + if current_count > last_count: notifications.append(f"Member with capability added: {capability}") - elif not has_now and had_before: + elif current_count < last_count: notifications.append( f"Member with capability unregistered: {capability}" ) + return notifications diff --git a/tapir/shifts/tests/test_shiftwatch_notification.py b/tapir/shifts/tests/test_shiftwatch_notification.py index 795fb8866..a289f3ee2 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,36 @@ def test_handle_recurring_noInitialMailIsSent(self): Command().handle() self.assertEqual(len(mail.outbox), 0) + + def test_handle_registerAnotherUserToSameCapability_MailSent(self): + slot_to_register = ShiftSlot.objects.create( + shift=self.shift_ok_first, name="cheese-making" + ) + self.slots.append(slot_to_register) + 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], + ) + # TODO assert first and second shift have attendance and third shift has no attendance + Command().handle() + self.assertEqual(0, len(mail.outbox)) + + # register user to third slot + user = TapirUserFactory.create() + ShiftAttendance.objects.create(user=user, slot=slot_to_register) + slot_to_register.save() + + self.assertEqual( + slot_to_register.required_capabilities, + [ShiftUserCapability.SHIFT_COORDINATOR], + ) + + Command().handle() + self.assertEqual(1, len(mail.outbox)) From dc7bca37505d6951992e8d705b4e9437a0214d31 Mon Sep 17 00:00:00 2001 From: crosspolar <18083323+crosspolar@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:15:21 +0200 Subject: [PATCH 2/5] easier counting --- tapir/shifts/services/shift_watch_creation_service.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tapir/shifts/services/shift_watch_creation_service.py b/tapir/shifts/services/shift_watch_creation_service.py index 6a589e2d2..3d8a6f612 100644 --- a/tapir/shifts/services/shift_watch_creation_service.py +++ b/tapir/shifts/services/shift_watch_creation_service.py @@ -1,5 +1,3 @@ -from collections import Counter - from django.db.models import Q from tapir.shifts.models import ( @@ -221,15 +219,10 @@ def get_capability_status_changes( current_capabilities = [cap for caps in current_slots for cap in caps] last_capabilities = [cap for caps in last_slots for cap in caps] - current_counter = Counter(current_capabilities) - last_counter = Counter(last_capabilities) - notifications = [] for capability in watched_capabilities: - current_count = current_counter[capability] - print(f"current_count: {current_count}") - last_count = last_counter[capability] - print(f"last_count: {last_count}") + current_count = current_capabilities.count(capability) + last_count = last_capabilities.count(capability) if current_count > last_count: notifications.append(f"Member with capability added: {capability}") From 3edd49399d32c52a8dc22f6f0618ebb31d06b581 Mon Sep 17 00:00:00 2001 From: crosspolar <18083323+crosspolar@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:36:32 +0200 Subject: [PATCH 3/5] use word "registered" --- .../services/shift_watch_creation_service.py | 2 +- .../tests/test_get_capability_status_change.py | 8 ++++++-- .../tests/test_shiftwatch_notification.py | 18 +++++++++++------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/tapir/shifts/services/shift_watch_creation_service.py b/tapir/shifts/services/shift_watch_creation_service.py index 3d8a6f612..1f32ae98d 100644 --- a/tapir/shifts/services/shift_watch_creation_service.py +++ b/tapir/shifts/services/shift_watch_creation_service.py @@ -225,7 +225,7 @@ def get_capability_status_changes( last_count = last_capabilities.count(capability) if current_count > last_count: - notifications.append(f"Member with capability added: {capability}") + notifications.append(f"Member with capability registered: {capability}") elif current_count < last_count: notifications.append( f"Member with capability unregistered: {capability}" 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 a289f3ee2..f5b4cf13d 100644 --- a/tapir/shifts/tests/test_shiftwatch_notification.py +++ b/tapir/shifts/tests/test_shiftwatch_notification.py @@ -211,10 +211,6 @@ def test_handle_recurring_noInitialMailIsSent(self): self.assertEqual(len(mail.outbox), 0) def test_handle_registerAnotherUserToSameCapability_MailSent(self): - slot_to_register = ShiftSlot.objects.create( - shift=self.shift_ok_first, name="cheese-making" - ) - self.slots.append(slot_to_register) for slot in self.slots: slot.required_capabilities = [ShiftUserCapability.SHIFT_COORDINATOR] slot.save() @@ -226,13 +222,21 @@ def test_handle_registerAnotherUserToSameCapability_MailSent(self): 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) + # TODO assert first and second shift have attendance and third shift has no attendance Command().handle() self.assertEqual(0, len(mail.outbox)) # register user to third slot - user = TapirUserFactory.create() - ShiftAttendance.objects.create(user=user, slot=slot_to_register) + ShiftAttendance.objects.create( + user=TapirUserFactory.create(), slot=slot_to_register + ) slot_to_register.save() self.assertEqual( @@ -241,4 +245,4 @@ def test_handle_registerAnotherUserToSameCapability_MailSent(self): ) Command().handle() - self.assertEqual(1, len(mail.outbox)) + self.assert_email_sent("registered") From 2a5f63d04ff0f5a7b316c935f5c42af7853e5fd5 Mon Sep 17 00:00:00 2001 From: crosspolar <18083323+crosspolar@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:40:55 +0200 Subject: [PATCH 4/5] assert first and second shift have attendance and third shift has no attendance --- tapir/shifts/tests/test_shiftwatch_notification.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tapir/shifts/tests/test_shiftwatch_notification.py b/tapir/shifts/tests/test_shiftwatch_notification.py index f5b4cf13d..971b4ef7c 100644 --- a/tapir/shifts/tests/test_shiftwatch_notification.py +++ b/tapir/shifts/tests/test_shiftwatch_notification.py @@ -229,7 +229,12 @@ def test_handle_registerAnotherUserToSameCapability_MailSent(self): slot_to_register.required_capabilities = [ShiftUserCapability.SHIFT_COORDINATOR] self.slots.append(slot_to_register) - # TODO assert first and second shift have attendance and third shift has no attendance + # 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)) From 27b67c944130ffd922652160c9534a41c7919b70 Mon Sep 17 00:00:00 2001 From: Frederik <18083323+crosspolar@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:16:04 +0200 Subject: [PATCH 5/5] Update tapir/shifts/services/shift_watch_creation_service.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Théophile MADET --- tapir/shifts/services/shift_watch_creation_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tapir/shifts/services/shift_watch_creation_service.py b/tapir/shifts/services/shift_watch_creation_service.py index 1f32ae98d..e6c143d16 100644 --- a/tapir/shifts/services/shift_watch_creation_service.py +++ b/tapir/shifts/services/shift_watch_creation_service.py @@ -216,7 +216,7 @@ def get_capability_status_changes( "required_capabilities", flat=True ) - current_capabilities = [cap for caps in current_slots for cap in caps] + 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 = []