From 710bf56c10cdf167a1f8a18fadcdda85afc471e2 Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 17 Apr 2026 18:13:13 +0200 Subject: [PATCH] add tests --- .../0015_clubrole_alter_membership_role.py | 3 + club/models.py | 20 +- club/tests/test_club_controller.py | 2 +- club/tests/test_clubrole.py | 217 +++++++++++++++++- 4 files changed, 220 insertions(+), 22 deletions(-) diff --git a/club/migrations/0015_clubrole_alter_membership_role.py b/club/migrations/0015_clubrole_alter_membership_role.py index f776b20c..b25b02de 100644 --- a/club/migrations/0015_clubrole_alter_membership_role.py +++ b/club/migrations/0015_clubrole_alter_membership_role.py @@ -128,6 +128,9 @@ class Migration(migrations.Migration): ("is_presidency", False), ("is_board", True), _connector="OR" ), name="clubrole_presidency_implies_board", + violation_error_message=( + "A role cannot be in the presidency while not being in the board" + ), ), ), migrations.RunPython(migrate_roles, migrations.RunPython.noop), diff --git a/club/models.py b/club/models.py index 317d625a..d13de8aa 100644 --- a/club/models.py +++ b/club/models.py @@ -208,7 +208,9 @@ class Club(models.Model): """Return True if the given user can edit the roles of this club""" return ( user.has_perm("club.change_clubrole") - or self.members.ongoing().filter(user=user, role__is_presidency=True).exists() + or self.members.ongoing() + .filter(user=user, role__is_presidency=True) + .exists() ) @cached_property @@ -258,6 +260,9 @@ class ClubRole(OrderedModel): models.CheckConstraint( condition=Q(is_presidency=False) | Q(is_board=True), name="clubrole_presidency_implies_board", + violation_error_message=_( + "A role cannot be in the presidency while not being in the board" + ), ) ] @@ -267,21 +272,8 @@ class ClubRole(OrderedModel): def get_display_name(self): return f"{self.name} - {self.club.name}" - def get_absolute_url(self): - return reverse("club:club_roles", kwargs={"club_id": self.club_id}) - def clean(self): errors = [] - if self.is_presidency and not self.is_board: - errors.append( - ValidationError( - _( - "Role %(name)s was declared as a presidency role " - "without being a board role" - ) - % {"name": self.name} - ) - ) roles = list(self.club.roles.all()) if ( self.is_board diff --git a/club/tests/test_club_controller.py b/club/tests/test_club_controller.py index 3bf0b75f..b6248e01 100644 --- a/club/tests/test_club_controller.py +++ b/club/tests/test_club_controller.py @@ -9,7 +9,7 @@ from model_bakery import baker from model_bakery.recipe import Recipe from pytest_django.asserts import assertNumQueries -from club.models import Club, ClubRole, Membership, ClubRole +from club.models import Club, ClubRole, Membership from core.baker_recipes import subscriber_user from core.models import Group, Page, User diff --git a/club/tests/test_clubrole.py b/club/tests/test_clubrole.py index 6956ac8c..81a98372 100644 --- a/club/tests/test_clubrole.py +++ b/club/tests/test_clubrole.py @@ -1,25 +1,48 @@ +from collections.abc import Callable + import pytest +from django.contrib.auth.models import Permission +from django.test import Client, TestCase +from django.urls import reverse from model_bakery import baker, seq from model_bakery.recipe import Recipe +from pytest_django.asserts import assertRedirects -from club.models import Club, ClubRole +from club.forms import ClubRoleFormSet +from club.models import Club, ClubRole, Membership +from core.baker_recipes import subscriber_user +from core.models import User -@pytest.mark.django_db -def test_order_auto(): - """Test that newly created roles are put in the right place.""" +def make_club(): + # unittest-style tests cannot use fixture, so we create a function + # that will be callable either by a pytest fixture or inside + # a TestCase.setUpTestData method. club = baker.make(Club) recipe = Recipe(ClubRole, club=club, name=seq("role ")) - # bulk create initial roles (1 presidency, 1 board, 1 member) - roles = recipe.make( + recipe.make( is_board=iter([True, True, False]), is_presidency=iter([True, False, False]), order=iter([1, 2, 3]), _quantity=3, _bulk_create=True, ) - # then create the remaining roles one by one (like they will be in prod) + return club + + +@pytest.fixture +def club(db): + """A club with a presidency role, a board role and a member role""" + return make_club() + + +@pytest.mark.django_db +def test_order_auto(club): + """Test that newly created roles are put in the right place.""" + roles = list(club.roles.all()) + # create new roles one by one (like they will be in prod) # each new role should be placed at the end of its category + recipe = Recipe(ClubRole, club=club, name=seq("new role ")) role_a = recipe.make(is_board=True, is_presidency=True, order=None) role_b = recipe.make(is_board=True, is_presidency=False, order=None) role_c = recipe.make(is_board=False, is_presidency=False, order=None) @@ -31,3 +54,183 @@ def test_order_auto(): roles[2], role_c, ] + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ("user_factory", "is_allowed"), + [ + ( + lambda club: baker.make( + User, + user_permissions=[Permission.objects.get(codename="change_clubrole")], + ), + True, + ), + ( # user with presidency roles can edit the club roles + lambda club: subscriber_user.make( + memberships=[ + baker.make( + Membership, + club=club, + role=club.roles.filter(is_presidency=True).first(), + ) + ] + ), + True, + ), + ( # user in the board but not in the presidency cannot edit roles + lambda club: subscriber_user.make( + memberships=[ + baker.make( + Membership, + club=club, + role=club.roles.filter( + is_presidency=False, is_board=True + ).first(), + ) + ] + ), + False, + ), + ], +) +def test_can_roles_be_edited_by( + club: Club, user_factory: Callable[[Club], User], is_allowed +): + """Test that `Club.can_roles_be_edited_by` return the right value""" + user = user_factory(club) + assert club.can_roles_be_edited_by(user) == is_allowed + + +@pytest.mark.django_db +@pytest.mark.parametrize( + ["route", "is_presidency", "is_board"], + [ + ("club:new_role_president", True, True), + ("club:new_role_board", False, True), + ("club:new_role_member", False, False), + ], +) +def test_create_role_view(client: Client, route: str, is_presidency, is_board): + """Test that the role creation views work.""" + club = baker.make(Club) + role = baker.make(ClubRole, club=club, is_presidency=True, is_board=True) + user = subscriber_user.make() + baker.make(Membership, club=club, role=role, user=user, end_date=None) + url = reverse(route, kwargs={"club_id": club.id}) + client.force_login(user) + + res = client.get(url) + assert res.status_code == 200 + + res = client.post(url, data={"name": "foo"}) + assertRedirects(res, reverse("club:club_roles", kwargs={"club_id": club.id})) + new_role = club.roles.last() + assert new_role.name == "foo" + assert new_role.is_presidency == is_presidency + assert new_role.is_board == is_board + + +class TestClubRoleUpdate(TestCase): + @classmethod + def setUpTestData(cls): + cls.club = make_club() + cls.roles = list(cls.club.roles.all()) + cls.user = subscriber_user.make() + baker.make( + Membership, club=cls.club, role=cls.roles[0], user=cls.user, end_date=None + ) + cls.url = reverse("club:club_roles", kwargs={"club_id": cls.club.id}) + + def setUp(self): + self.payload = { + "roles-TOTAL_FORMS": 3, + "roles-INITIAL_FORMS": 3, + "roles-MIN_NUM_FORMS": 0, + "roles-MAX_NUM_FORMS": 1000, + "roles-0-ORDER": self.roles[0].order, + "roles-0-id": self.roles[0].id, + "roles-0-club": self.club.id, + "roles-0-is_presidency": True, + "roles-0-is_board": True, + "roles-0-name": self.roles[0].name, + "roles-0-description": self.roles[0].description, + "roles-0-is_active": True, + "roles-1-ORDER": self.roles[1].order, + "roles-1-id": self.roles[1].id, + "roles-1-club": self.club.id, + "roles-1-is_presidency": False, + "roles-1-is_board": True, + "roles-1-name": self.roles[1].name, + "roles-1-description": self.roles[1].description, + "roles-1-is_active": True, + "roles-2-ORDER": self.roles[2].order, + "roles-2-id": self.roles[2].id, + "roles-2-club": self.club.id, + "roles-2-is_presidency": False, + "roles-2-is_board": False, + "roles-2-name": self.roles[2].name, + "roles-2-description": self.roles[2].description, + "roles-2-is_active": True, + } + + def test_view_ok(self): + """Basic test to check that the view works.""" + self.client.force_login(self.user) + res = self.client.get(self.url) + assert res.status_code == 200 + self.payload["roles-2-name"] = "foo" + res = self.client.post(self.url, data=self.payload) + assertRedirects(res, self.url) + self.roles[2].refresh_from_db() + assert self.roles[2].name == "foo" + + def test_incoherent_order(self): + """Test that placing a member role over a board role fails.""" + self.payload["roles-0-ORDER"] = 4 + formset = ClubRoleFormSet(data=self.payload, instance=self.club) + assert not formset.is_valid() + assert formset.errors == [ + { + "__all__": [ + f"Le rôle {self.roles[0].name} ne peut pas " + "être placé en-dessous d'un rôle de membre.", + f"Le rôle {self.roles[0].name} ne peut pas être placé " + "en-dessous d'un rôle qui n'est pas de la présidence.", + ] + }, + {}, + {}, + ] + + def test_change_order_ok(self): + """Test that changing order the intended way works""" + self.payload["roles-1-ORDER"] = 3 + self.payload["roles-1-is_board"] = False + self.payload["roles-2-ORDER"] = 2 + formset = ClubRoleFormSet(data=self.payload, instance=self.club) + assert formset.is_valid() + formset.save() + assert list(self.club.roles.order_by("order")) == [ + self.roles[0], + self.roles[2], + self.roles[1], + ] + self.roles[1].refresh_from_db() + assert not self.roles[1].is_board + + def test_non_board_presidency_is_forbidden(self): + """Test that a role cannot be in the presidency without being in the board.""" + self.payload["roles-0-is_board"] = False + formset = ClubRoleFormSet(data=self.payload, instance=self.club) + assert not formset.is_valid() + assert formset.errors == [ + { + "__all__": [ + "Un rôle ne peut pas appartenir à la présidence sans être dans le bureau", + ] + }, + {}, + {}, + ]