From 29a023140d41046865765c06870332dcb39788b3 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 7 Apr 2026 11:03:49 +0200 Subject: [PATCH] exclude inactive roles from attributable roles --- club/forms.py | 13 +++-- club/tests/test_membership.py | 100 ++++++++++++++++++---------------- 2 files changed, 61 insertions(+), 52 deletions(-) diff --git a/club/forms.py b/club/forms.py index a18851a0..90045112 100644 --- a/club/forms.py +++ b/club/forms.py @@ -237,7 +237,7 @@ class ClubMemberForm(forms.ModelForm): @property def available_roles(self) -> QuerySet[ClubRole]: - """The greatest role that will be obtainable with this form.""" + """The roles that will be obtainable with this form.""" # this is unreachable, because it will be overridden by subclasses return ClubRole.objects.none() # pragma: no cover @@ -251,20 +251,21 @@ class ClubAddMemberForm(ClubMemberForm): @cached_property def available_roles(self): - """The greatest role that will be obtainable with this form. + """The roles that will be obtainable with this form. Admins and the club president can attribute any role. Board members can attribute roles lower than their own. Other users cannot attribute roles with this form """ + qs = self.club.roles.filter(is_active=True) if self.request_user.has_perm("club.add_membership"): - return self.club.roles.all() + return qs.all() membership = self.request_user_membership if membership is None or not membership.role.is_board: return ClubRole.objects.none() if membership.role.is_presidency: - return self.club.roles.all() - return self.club.roles.above_instance(membership.role) + return qs.all() + return qs.above_instance(membership.role) def clean_user(self): """Check that the user is not trying to add a user already in the club. @@ -292,7 +293,7 @@ class JoinClubForm(ClubMemberForm): @cached_property def available_roles(self): - return self.club.roles.filter(is_board=False) + return self.club.roles.filter(is_board=False, is_active=True) def clean(self): """Check that the user is subscribed and isn't already in the club.""" diff --git a/club/tests/test_membership.py b/club/tests/test_membership.py index 19b0d685..e1bafef7 100644 --- a/club/tests/test_membership.py +++ b/club/tests/test_membership.py @@ -10,7 +10,7 @@ from django.db.models import Max from django.test import Client, TestCase from django.urls import reverse from django.utils.timezone import localdate, localtime, now -from model_bakery import baker +from model_bakery import baker, seq from pytest_django.asserts import assertRedirects from club.forms import ClubAddMemberForm, JoinClubForm @@ -317,51 +317,6 @@ class TestMembership(TestClub): self.club.refresh_from_db() assert self.club.members.count() == nb_memberships - def test_president_add_members(self): - """Test that the president of the club can add members.""" - president = self.club.members.get(role=self.president_role).user - nb_club_membership = self.club.members.count() - nb_subscriber_memberships = self.subscriber.memberships.count() - self.client.force_login(president) - response = self.client.post( - self.new_members_url, - {"user": self.subscriber.id, "role": self.president_role.id}, - ) - assert response.status_code == 200 - assert response.headers.get("HX-Redirect", "") == reverse( - "club:club_members", kwargs={"club_id": self.club.id} - ) - self.club.refresh_from_db() - self.subscriber.refresh_from_db() - assert self.club.members.count() == nb_club_membership + 1 - assert self.subscriber.memberships.count() == nb_subscriber_memberships + 1 - self.assert_membership_started_today(self.subscriber, role=self.president_role) - - def test_add_member_greater_role(self): - """Test that a member of the club member cannot create - a membership with a greater role than its own. - """ - user_role = self.simple_board_member.memberships.first().role - other_role = baker.make(ClubRole, club=user_role.club, is_board=True) - other_role.above(user_role) - form = ClubAddMemberForm( - data={"user": self.subscriber.id, "role": other_role.id}, - request_user=self.simple_board_member, - club=self.club, - ) - nb_memberships = self.club.members.count() - - assert not form.is_valid() - assert form.errors == { - "role": [ - "Sélectionnez un choix valide. " - "Ce choix ne fait pas partie de ceux disponibles." - ] - } - self.club.refresh_from_db() - assert nb_memberships == self.club.members.count() - assert not self.subscriber.memberships.filter(club=self.club).exists() - def test_add_member_without_role(self): """Test that trying to add members without specifying their role fails.""" form = ClubAddMemberForm( @@ -577,6 +532,50 @@ def test_membership_delete(client: Client): assert not Membership.objects.filter(id=membership.id).exists() +@pytest.mark.django_db +class TestAddMemberForm(TestCase): + @classmethod + def setUpTestData(cls): + cls.club = baker.make(Club) + cls.roles = baker.make( + ClubRole, + club=cls.club, + is_board=iter([True, True, True, True, False, False]), + is_presidency=iter([True, True, False, False, False, False]), + order=seq(0), + _quantity=6, + _bulk_create=True, + ) + cls.roles[-1].is_active = False + cls.roles[-1].save() + + def test_admin(self): + """Test that admin users can give any active role.""" + user = baker.make( + User, user_permissions=[Permission.objects.get(codename="add_membership")] + ) + form = ClubAddMemberForm(request_user=user, club=self.club) + assert list(form.fields["role"].queryset) == self.roles[:-1] + + def test_president(self): + """Test that someone with a presidency role can give any active role.""" + user = baker.make(Membership, club=self.club, role=self.roles[0]).user + form = ClubAddMemberForm(request_user=user, club=self.club) + assert list(form.fields["role"].queryset) == self.roles[:-1] + + def test_board_member(self): + """Test that someone with a board role can give lower active role.""" + user = baker.make(Membership, club=self.club, role=self.roles[2]).user + form = ClubAddMemberForm(request_user=user, club=self.club) + assert list(form.fields["role"].queryset) == self.roles[3:-1] + + def test_simple_member(self): + """Test that someone with a non-board role cannot give roles.""" + user = baker.make(Membership, club=self.club, role=self.roles[4]).user + form = ClubAddMemberForm(request_user=user, club=self.club) + assert list(form.fields["role"].queryset) == [] + + @pytest.mark.django_db class TestJoinClub: @pytest.fixture(autouse=True) @@ -638,6 +637,7 @@ class TestOldMembersView(TestCase): ClubRole, club=club, is_board=itertools.cycle([True, True, False]), + order=seq(0), _quantity=10, _bulk_create=True, ) @@ -665,3 +665,11 @@ class TestOldMembersView(TestCase): self.client.force_login(baker.make(User)) res = self.client.get(self.url) assert res.status_code == 403 + + def test_context_data(self): + # mark a membership as not ended, to make sure it is excluded from the result + self.memberships[0].end_date = None + self.memberships[0].save() + self.client.force_login(subscriber_user.make()) + res = self.client.get(self.url) + assert list(res.context_data.get("old_members")) == self.memberships[1:]