Fix removing suspended badge when absolving earlier warning

Author: gdaviet

collectives/models/user/badge.py

@@ -194,23 +194,25 @@ def increment_warning_badges(self, registration: Registration):
                     level=num_suspended_badges + 1,
                     registration=registration,
                 )
-            else:
-                badge_id = BadgeIds.UnjustifiedAbsenceWarning
-                expiration_date = date(
-                    (
-                        date.today().year
-                        if date.today().month < Configuration.LICENSE_EXPIRY_MONTH
-                        else date.today().year + 1
-                    ),
-                    Configuration.LICENSE_EXPIRY_MONTH,
-                    1,
-                ) - timedelta(days=1)
-                self.assign_badge(
-                    badge_id,
-                    expiration_date=expiration_date,
-                    level=num_warning_badges + 1,
-                    registration=registration,
-                )
+
+            # always add a warning badge as well
+            # this simplifies logic when removing badge due to status change
+            badge_id = BadgeIds.UnjustifiedAbsenceWarning
+            expiration_date = date(
+                (
+                    date.today().year
+                    if date.today().month < Configuration.LICENSE_EXPIRY_MONTH
+                    else date.today().year + 1
+                ),
+                Configuration.LICENSE_EXPIRY_MONTH,
+                1,
+            ) - timedelta(days=1)
+            self.assign_badge(
+                badge_id,
+                expiration_date=expiration_date,
+                level=num_warning_badges + 1,
+                registration=registration,
+            )
         except ValueError:
             # Badges update should not break unregistration logic
             pass
@@ -220,11 +222,32 @@ def remove_warning_badges(self, registration: Registration):
         Removes warning badges associated to this registration
         """
 
-        badges = [
+        registration_badges = [
             badge
             for badge in registration.badges
             if badge.badge_id
             in (BadgeIds.Suspended, BadgeIds.UnjustifiedAbsenceWarning)
         ]
-        for badge in badges:
+
+        if not any(
+            badge
+            for badge in registration_badges
+            if badge.badge_id == BadgeIds.Suspended
+        ):
+            # If this badge was counted towards a later suspension,
+            # Remove the first suspended badge from a later registration
+            max_id = max(badge.id for badge in registration_badges)
+
+            later_suspended_badges = [
+                badge
+                for badge in self.matching_badges([BadgeIds.Suspended], valid_only=True)
+                if badge.id > max_id
+            ]
+            if later_suspended_badges:
+                first_suspended_badge = min(
+                    later_suspended_badges, key=lambda badge: badge.id
+                )
+                db.session.delete(first_suspended_badge)
+
+        for badge in registration_badges:
             db.session.delete(badge)

tests/events/test_registrations.py

@@ -357,7 +357,7 @@ def test_unregister_lately_valid_second_warning(
 
     assert event.is_late_unregistered(user)
     assert user.has_a_valid_badge([BadgeIds.UnjustifiedAbsenceWarning])
-    assert user.number_of_valid_warning_badges() == 2
+    assert user.number_of_valid_warning_badges() == 3
     assert user.has_a_valid_badge([BadgeIds.Suspended])
 
 

tests/unit/test_badges.py

@@ -3,8 +3,14 @@
 from datetime import date, timedelta
 from collectives.models.activity_type import ActivityType
 from collectives.models.badge import Badge, BadgeIds
-from collectives.models import db
 from collectives.models.user import User
+from collectives.models import (
+    db,
+    Event,
+    Registration,
+    RegistrationLevels,
+    RegistrationStatus,
+)
 
 
 def test_add_a_valid_badge(user1):
@@ -143,3 +149,77 @@ def test_increment_warning_badges_no_updates(
     assert len(session_monkeypatch["add"]) == 2  # No badge should be added
     assert session_monkeypatch["commit"] == 2  # No commit should occur
     assert session_monkeypatch["rollback"] == 0  # No rollback should occur
+
+
+def test_remove_sanction_badge(user1: User, event: Event):
+    """Test that removing a warning badge removes suspension
+    if that warning was counted towards suspension"""
+
+    db.session.add(event)
+    db.session.commit()
+
+    expiration_date = date.today() + timedelta(days=1)
+
+    reg1 = Registration(
+        user_id=user1.id,
+        status=RegistrationStatus.UnJustifiedAbsentee,
+        level=RegistrationLevels.Normal,
+        is_self=True,
+    )
+    reg2 = Registration(
+        user_id=user1.id,
+        status=RegistrationStatus.UnJustifiedAbsentee,
+        level=RegistrationLevels.Normal,
+        is_self=True,
+    )
+    reg3 = Registration(
+        user_id=user1.id,
+        status=RegistrationStatus.UnJustifiedAbsentee,
+        level=RegistrationLevels.Normal,
+        is_self=True,
+    )
+    event.registrations.append(reg1)
+    event.registrations.append(reg2)
+    event.registrations.append(reg3)
+
+    user1.assign_badge(
+        BadgeIds.UnjustifiedAbsenceWarning,
+        expiration_date=expiration_date,
+        registration=reg1,
+    )
+    user1.assign_badge(
+        BadgeIds.UnjustifiedAbsenceWarning,
+        expiration_date=expiration_date,
+        registration=reg2,
+    )
+    user1.assign_badge(
+        BadgeIds.Suspended,
+        expiration_date=expiration_date,
+        registration=reg2,
+    )
+    user1.assign_badge(
+        BadgeIds.UnjustifiedAbsenceWarning,
+        expiration_date=expiration_date,
+        registration=reg3,
+    )
+    user1.assign_badge(
+        BadgeIds.Suspended,
+        expiration_date=expiration_date,
+        registration=reg3,
+    )
+    db.session.commit()
+
+    assert user1.has_a_valid_suspended_badge()
+    assert user1.number_of_valid_warning_badges() == 3
+
+    user1.remove_warning_badges(reg1)
+    db.session.commit()
+
+    assert user1.has_a_valid_suspended_badge()
+    assert user1.number_of_valid_warning_badges() == 2
+
+    user1.remove_warning_badges(reg2)
+    db.session.commit()
+
+    assert not user1.has_a_valid_suspended_badge()
+    assert user1.number_of_valid_warning_badges() == 1