add tests

club/forms.py

@@ -267,8 +267,6 @@ class ClubMemberForm(forms.ModelForm):
     def clean(self):
         """Check user rights for adding a user."""
         cleaned_data = super().clean()
-        if "role" not in cleaned_data:
-            return cleaned_data
         if (
             self.request_user_membership is None
             or self.request_user_membership.role <= settings.SITH_MAXIMUM_FREE_ROLE
@@ -280,6 +278,4 @@ class ClubMemberForm(forms.ModelForm):
                 ),
                 code="invalid",
             )
-        if cleaned_data["role"] > self.max_available_role:
-            raise forms.ValidationError(_("You do not have the permission to do that"))
         return cleaned_data

club/models.py

@@ -253,35 +253,38 @@ class MembershipQuerySet(models.QuerySet):
         """Filter Memberships that this user can edit.
 
         Users with the `club.change_membership` permission can edit all Membership.
-        The other users can end :
+        The other users can edit :
         - their own membership
-        - if they are board members, memberships with a role lower than their own
+        - if they are board members, ongoing memberships with a role lower than their own
 
         For example, let's suppose the following users :
         - A : board member
         - B : board member
         - C : simple member
         - D : curious
+        - E : old member
 
-        A will be able to end the memberships of A, C and D ;
-        C and D will be able to end only their own membership.
+        A will be able to edit the memberships of A, C and D ;
+        C and D will be able to edit only their own membership ;
+        nobody will be able to edit E's membership.
         """
         if user.has_perm("club.change_membership"):
             return self.all()
         return self.filter(
-            Exists(
+            Q(user=user)
+            | Exists(
                 Membership.objects.filter(
                     Q(
                         role__gt=Greatest(
                             OuterRef("role"), Value(settings.SITH_MAXIMUM_FREE_ROLE)
                         )
-                    )
-                    | Q(pk=OuterRef("pk")),
+                    ),
                     user=user,
                     end_date=None,
                     club=OuterRef("club"),
                 )
-            )
+            ),
+            end_date=None,
         )
 
     def update(self, **kwargs) -> int:

club/tests/test_membership.py

@@ -1,3 +1,5 @@
+from datetime import timedelta
+
 from bs4 import BeautifulSoup
 from django.conf import settings
 from django.contrib.auth.models import Permission
@@ -368,6 +370,44 @@ class TestMembership(TestClub):
         assert not form.is_valid()
         assert form.errors == {"role": ["Ce champ est obligatoire."]}
 
+    def test_add_member_already_there(self):
+        form = ClubMemberForm(
+            data={"user": self.simple_board_member, "role": 3},
+            request_user=self.root,
+            club=self.club,
+        )
+        assert not form.is_valid()
+        assert form.errors == {
+            "user": ["Vous ne pouvez pas ajouter deux fois le même utilisateur"]
+        }
+
+    def test_add_other_member_forbidden(self):
+        non_member = subscriber_user.make()
+        simple_member = baker.make(Membership, club=self.club, role=1).user
+        for user in non_member, simple_member:
+            form = ClubMemberForm(
+                data={"user": subscriber_user.make(), "role": 1},
+                request_user=user,
+                club=self.club,
+            )
+            assert not form.is_valid()
+            assert form.errors == {
+                "__all__": [
+                    "Vous ne pouvez pas ajouter d'autres utilisateurs "
+                    "dans un club si vous ne faites pas partie de son bureau."
+                ]
+            }
+
+    def test_simple_members_dont_see_form_anymore(self):
+        """Test that simple club members don't see the form to add members"""
+        user = subscriber_user.make()
+        baker.make(Membership, club=self.club, user=user, role=1)
+        self.client.force_login(user)
+        res = self.client.get(self.members_url)
+        assert res.status_code == 200
+        soup = BeautifulSoup(res.text, "lxml")
+        assert not soup.find(id="add_club_members_form")
+
     def test_end_membership_self(self):
         """Test that a member can end its own membership."""
         self.client.force_login(self.simple_board_member)
@@ -491,3 +531,34 @@ class TestMembership(TestClub):
         new_board = set(self.club.board_group.users.values_list("id", flat=True))
         assert new_members == initial_members
         assert new_board == initial_board
+
+
+class TestOldMembersView(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        club = baker.make(Club)
+        roles = [1, 1, 1, 2, 2, 4, 4, 5, 7, 9, 10]
+        cls.memberships = baker.make(
+            Membership,
+            role=iter(roles),
+            club=club,
+            start_date=now() - timedelta(days=14),
+            end_date=now() - timedelta(days=7),
+            _quantity=len(roles),
+            _bulk_create=True,
+        )
+        cls.url = reverse("club:club_old_members", kwargs={"club_id": club.id})
+
+    def test_ok(self):
+        user = subscriber_user.make()
+        self.client.force_login(user)
+        res = self.client.get(self.url)
+        assert res.status_code == 200
+
+    def test_access_forbidden(self):
+        res = self.client.get(self.url)
+        assertRedirects(res, reverse("core:login", query={"next": self.url}))
+
+        self.client.force_login(baker.make(User))
+        res = self.client.get(self.url)
+        assert res.status_code == 403

club/views.py

@@ -274,9 +274,6 @@ class ClubAddMembersFragment(
     success_message = _("%(user)s has been added to club.")
 
     def dispatch(self, *args, **kwargs):
-        club_id = self.kwargs.get("club_id")
-        if not club_id:
-            raise Http404
         self.club = get_object_or_404(Club, pk=kwargs.get("club_id"))
         return super().dispatch(*args, **kwargs)
 
@@ -316,6 +313,8 @@ class ClubMembersView(
             and membership.role <= settings.SITH_MAXIMUM_FREE_ROLE
             and not self.request.user.has_perm("club.add_membership")
         ):
+            # Simple club members won't see the form anymore.
+            # Even if they saw it, they couldn't add anyone to the club anyway
             return {}
         return {"add_member_fragment": ClubAddMembersFragment}