From f64980a411691e0c6ba2f542103519ebe007a2c9 Mon Sep 17 00:00:00 2001
From: Lucas Brandstaetter <lucas@brandstaetter.tech>
Date: Tue, 24 Dec 2024 15:35:41 +0100
Subject: [PATCH] Fix TeamMember removal with one only manager

---
 src/backoffice/tests/teams/members.py | 27 ++++++++++++++++++++++++---
 src/core/models/teams/team_member.py  |  6 ++++--
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/backoffice/tests/teams/members.py b/src/backoffice/tests/teams/members.py
index 56d974488..a2fbe91c5 100644
--- a/src/backoffice/tests/teams/members.py
+++ b/src/backoffice/tests/teams/members.py
@@ -88,6 +88,7 @@ class TeamMemberDeleteViewTestCase(BackOfficeTestCase):
         self.client.force_login(self.admin)
         activity_log_count = ActivityLogEntry.objects.count()
 
+        # Remove the first manager
         response = self.client.post(
             reverse('backoffice:team-member-delete', kwargs={'pk': self.team_member_staff_2.pk, 'team': self.team.uuid}),
             {'confirmation': 'true'},
@@ -96,20 +97,30 @@ class TeamMemberDeleteViewTestCase(BackOfficeTestCase):
         self.assertFalse(TeamMember.objects.filter(user=self.staff_2).exists())
         self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 1)
 
+        # Remove a normal user
+        response = self.client.post(
+            reverse('backoffice:team-member-delete', kwargs={'pk': self.team_member_user.pk, 'team': self.team.uuid}),
+            {'confirmation': 'true'},
+        )
+        self.assertRedirects(response, reverse('backoffice:team', kwargs={'uuid': self.team.uuid}))
+        self.assertFalse(TeamMember.objects.filter(user=self.user).exists())
+        self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 2)
+
+        # Try to remove the last manager
         response = self.client.post(
             reverse('backoffice:team-member-delete', kwargs={'pk': self.team_member_staff.pk, 'team': self.team.uuid}),
             {'confirmation': 'true'},
         )
         self.assertTemplateUsed(response, 'backoffice/components/confirmation_modal.html')
         self.assertTrue(TeamMember.objects.filter(user=self.staff).exists())
-        self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 1)
+        self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 2)
         self.assertContains(response, _('TeamMember__delete__cannot_delete_last_manager'))
 
     def test_cannot_leave_last_manager(self):
         activate('en')
-        self.client.force_login(self.staff)
         activity_log_count = ActivityLogEntry.objects.count()
 
+        self.client.force_login(self.staff_2)
         response = self.client.post(
             reverse('backoffice:team-member-delete', kwargs={'pk': self.team_member_staff_2.pk, 'team': self.team.uuid}),
             {'confirmation': 'true'},
@@ -118,13 +129,23 @@ class TeamMemberDeleteViewTestCase(BackOfficeTestCase):
         self.assertFalse(TeamMember.objects.filter(user=self.staff_2).exists())
         self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 1)
 
+        self.client.force_login(self.user)
+        response = self.client.post(
+            reverse('backoffice:team-member-delete', kwargs={'pk': self.team_member_user.pk, 'team': self.team.uuid}),
+            {'confirmation': 'true'},
+        )
+        self.assertRedirects(response, reverse('backoffice:team', kwargs={'uuid': self.team.uuid}))
+        self.assertFalse(TeamMember.objects.filter(user=self.user).exists())
+        self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 2)
+
+        self.client.force_login(self.staff)
         response = self.client.post(
             reverse('backoffice:team-member-delete', kwargs={'pk': self.team_member_staff.pk, 'team': self.team.uuid}),
             {'confirmation': 'true'},
         )
         self.assertTemplateUsed(response, 'backoffice/components/confirmation_modal.html')
         self.assertTrue(TeamMember.objects.filter(user=self.staff).exists())
-        self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 1)
+        self.assertEqual(ActivityLogEntry.objects.count(), activity_log_count + 2)
         self.assertContains(response, _('TeamMember__delete__cannot_delete_last_manager'))
 
 
diff --git a/src/core/models/teams/team_member.py b/src/core/models/teams/team_member.py
index 5523f45c7..0fb4ce95b 100644
--- a/src/core/models/teams/team_member.py
+++ b/src/core/models/teams/team_member.py
@@ -63,11 +63,13 @@ class TeamMember(RulesModel):
         return isinstance(obj, cls)
 
     def clean(self) -> None:
-        if not self.can_manage and self.team.members.filter(can_manage=True).count() == 1:
+        # Here we clan the new values of the object. So we have to check if it can_manage is False (e.g. remove)
+        if not self.can_manage and not self.team.members.exclude(user=self.user).filter(can_manage=True).exists():
             raise LastManagerError(_('TeamMember__clean__cannot_remove_last_manager'))
         return super().clean()
 
     def delete(self, using: Any = None, keep_parents: bool = False) -> tuple[int, dict[str, int]]:
-        if self.can_manage and self.team.members.filter(can_manage=True).count() == 1:
+        # Here we check for the current value of the object. So we have to check if it can_manage is True
+        if self.can_manage and not self.team.members.exclude(user=self.user).filter(can_manage=True).exists():
             raise LastManagerError(_('TeamMember__delete__cannot_delete_last_manager'))
         return super().delete(using, keep_parents)
-- 
GitLab