diff --git a/club/api.py b/club/api.py index 3ed425bf..84d60161 100644 --- a/club/api.py +++ b/club/api.py @@ -39,7 +39,8 @@ class ClubController(ControllerBase): ) def fetch_club(self, club_id: int): prefetch = Prefetch( - "members", queryset=Membership.objects.ongoing().select_related("user") + "members", + queryset=Membership.objects.ongoing().select_related("user", "role"), ) return self.get_object_or_exception( Club.objects.prefetch_related(prefetch), id=club_id @@ -61,5 +62,5 @@ class UserClubController(ControllerBase): return ( Membership.objects.ongoing() .filter(user=user) - .select_related("club", "user") + .select_related("club", "user", "role") ) diff --git a/club/models.py b/club/models.py index a2617d37..a5b1cc2f 100644 --- a/club/models.py +++ b/club/models.py @@ -444,7 +444,11 @@ class Membership(models.Model): if user.is_root or user.is_board_member: return True membership = self.club.get_membership_for(user) - return membership is not None and membership.role.order < self.role.order + if not membership: + return False + return membership.user_id == user.id or ( + membership.is_board and membership.role.order < self.role.order + ) def delete(self, *args, **kwargs): self._remove_club_groups([self]) diff --git a/club/schemas.py b/club/schemas.py index 08488c31..ab13aac5 100644 --- a/club/schemas.py +++ b/club/schemas.py @@ -3,7 +3,7 @@ from typing import Annotated from django.db.models import Q from ninja import FilterLookup, FilterSchema, ModelSchema -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.schemas import NonEmptyStr, SimpleUserSchema @@ -39,14 +39,21 @@ class ClubProfileSchema(ModelSchema): return obj.get_absolute_url() +class ClubRoleSchema(ModelSchema): + class Meta: + model = ClubRole + fields = ["id", "name", "is_presidency", "is_board"] + + class ClubMemberSchema(ModelSchema): """A schema to represent all memberships in a club.""" class Meta: model = Membership - fields = ["start_date", "end_date", "role", "description"] + fields = ["start_date", "end_date", "description"] user: SimpleUserSchema + role: ClubRoleSchema class ClubSchema(ModelSchema): @@ -62,6 +69,7 @@ class UserMembershipSchema(ModelSchema): class Meta: model = Membership - fields = ["id", "start_date", "role", "description"] + fields = ["id", "start_date", "description"] club: SimpleClubSchema + role: ClubRoleSchema diff --git a/club/tests/base.py b/club/tests/base.py index ca4fc6cf..52e04fca 100644 --- a/club/tests/base.py +++ b/club/tests/base.py @@ -8,7 +8,7 @@ from django.utils.timezone import now from model_bakery import baker from model_bakery.recipe import Recipe -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.baker_recipes import old_subscriber_user, subscriber_user from core.models import User @@ -43,6 +43,11 @@ class TestClub(TestCase): cls.ae = Club.objects.get(pk=settings.SITH_MAIN_CLUB_ID) cls.club = baker.make(Club) + cls.president_role = baker.make( + ClubRole, club=cls.club, is_board=True, is_presidency=True, order=0 + ) + cls.board_role = baker.make(ClubRole, club=cls.club, is_board=True, order=1) + cls.member_role = baker.make(ClubRole, club=cls.club, order=2) cls.new_members_url = reverse( "club:club_new_members", kwargs={"club_id": cls.club.id} ) @@ -51,12 +56,17 @@ class TestClub(TestCase): yesterday = now() - timedelta(days=1) membership_recipe = Recipe(Membership, club=cls.club) membership_recipe.make( - user=cls.simple_board_member, start_date=a_month_ago, role=3 + user=cls.simple_board_member, start_date=a_month_ago, role=cls.board_role + ) + membership_recipe.make(user=cls.richard, role=cls.member_role) + membership_recipe.make( + user=cls.president, start_date=a_month_ago, role=cls.president_role ) - membership_recipe.make(user=cls.richard, role=1) - membership_recipe.make(user=cls.president, start_date=a_month_ago, role=10) membership_recipe.make( # sli was a member but isn't anymore - user=cls.sli, start_date=a_month_ago, end_date=yesterday, role=2 + user=cls.sli, + start_date=a_month_ago, + end_date=yesterday, + role=cls.board_role, ) def setUp(self): diff --git a/club/tests/test_club.py b/club/tests/test_club.py index 2a232b19..c33fa612 100644 --- a/club/tests/test_club.py +++ b/club/tests/test_club.py @@ -5,7 +5,7 @@ from django.utils.timezone import localdate from model_bakery import baker from model_bakery.recipe import Recipe -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.baker_recipes import subscriber_user @@ -16,11 +16,19 @@ def test_club_queryset_having_board_member(): membership_recipe = Recipe( Membership, user=user, start_date=localdate() - timedelta(days=3) ) - membership_recipe.make(club=clubs[0], role=1) - membership_recipe.make(club=clubs[1], role=3) - membership_recipe.make(club=clubs[2], role=7) membership_recipe.make( - club=clubs[3], role=3, end_date=localdate() - timedelta(days=1) + club=clubs[0], role=baker.make(ClubRole, club=clubs[0], is_board=False) + ) + membership_recipe.make( + club=clubs[1], role=baker.make(ClubRole, club=clubs[1], is_board=True) + ) + membership_recipe.make( + club=clubs[2], role=baker.make(ClubRole, club=clubs[2], is_board=True) + ) + membership_recipe.make( + club=clubs[3], + role=baker.make(ClubRole, club=clubs[3], is_board=True), + end_date=localdate() - timedelta(days=1), ) club_ids = Club.objects.having_board_member(user).values_list("id", flat=True) diff --git a/club/tests/test_club_controller.py b/club/tests/test_club_controller.py index 500dbe6a..e084109c 100644 --- a/club/tests/test_club_controller.py +++ b/club/tests/test_club_controller.py @@ -1,6 +1,7 @@ from datetime import date, timedelta import pytest +from django.conf import settings from django.contrib.auth.models import Permission from django.test import Client, TestCase from django.urls import reverse @@ -8,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, Membership +from club.models import Club, Membership, ClubRole from core.baker_recipes import subscriber_user from core.models import Group, Page, User @@ -26,8 +27,10 @@ class TestClubSearch(TestCase): "id", flat=True ) ) - Page.objects.exclude(club=None).delete() + Membership.objects.all().delete() + ClubRole.objects.all().delete() Club.objects.all().delete() + Page.objects.exclude(name=settings.SITH_CLUB_ROOT_PAGE).delete() Group.objects.filter(id__in=groups).delete() cls.clubs = baker.make( diff --git a/club/tests/test_edit.py b/club/tests/test_edit.py index 9da327ea..b4d9e2b1 100644 --- a/club/tests/test_edit.py +++ b/club/tests/test_edit.py @@ -4,7 +4,7 @@ from django.urls import reverse from model_bakery import baker from pytest_django.asserts import assertRedirects -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.baker_recipes import subscriber_user @@ -12,7 +12,12 @@ from core.baker_recipes import subscriber_user def test_club_board_member_cannot_edit_club_properties(client: Client): user = subscriber_user.make() club = baker.make(Club, name="old name", is_active=True, address="old address") - baker.make(Membership, club=club, user=user, role=7) + baker.make( + Membership, + club=club, + user=user, + role=baker.make(ClubRole, club=club, is_board=True), + ) client.force_login(user) res = client.post( reverse("club:club_edit", kwargs={"club_id": club.id}), @@ -32,7 +37,12 @@ def test_edit_club_page_doesnt_crash(client: Client): """crash test for club:club_edit""" club = baker.make(Club) user = subscriber_user.make() - baker.make(Membership, club=club, user=user, role=3) + baker.make( + Membership, + club=club, + user=user, + role=baker.make(ClubRole, club=club, is_board=True), + ) client.force_login(user) res = client.get(reverse("club:club_edit", kwargs={"club_id": club.id})) assert res.status_code == 200 diff --git a/club/tests/test_mailing.py b/club/tests/test_mailing.py index e8ea3a9b..2217281c 100644 --- a/club/tests/test_mailing.py +++ b/club/tests/test_mailing.py @@ -3,9 +3,10 @@ from django.test import TestCase from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext as _ +from model_bakery import baker from club.forms import MailingForm -from club.models import Club, Mailing, Membership +from club.models import Club, ClubRole, Mailing, Membership from core.models import User @@ -25,7 +26,7 @@ class TestMailingForm(TestCase): user=cls.rbatsbak, club=cls.club, start_date=timezone.now(), - role=settings.SITH_CLUB_ROLES_ID["Board member"], + role=baker.make(ClubRole, club=cls.club, is_board=True), ).save() def test_mailing_list_add_no_moderation(self): diff --git a/club/tests/test_membership.py b/club/tests/test_membership.py index e24fe9d0..19b0d685 100644 --- a/club/tests/test_membership.py +++ b/club/tests/test_membership.py @@ -1,9 +1,9 @@ +import itertools from collections.abc import Callable from datetime import timedelta import pytest from bs4 import BeautifulSoup -from django.conf import settings from django.contrib.auth.models import Permission from django.core.cache import cache from django.db.models import Max @@ -14,7 +14,7 @@ from model_bakery import baker from pytest_django.asserts import assertRedirects from club.forms import ClubAddMemberForm, JoinClubForm -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from club.tests.base import TestClub from core.baker_recipes import subscriber_user from core.models import AnonymousUser, User @@ -75,17 +75,22 @@ class TestMembershipQuerySet(TestClub): def test_update_change_club_groups(self): """Test that `update` set the user groups accordingly.""" user = baker.make(User) - membership = baker.make(Membership, end_date=None, user=user, role=5) + board_role, member_role = baker.make( + ClubRole, is_board=iter([True, False]), _quantity=2, _bulk_create=True + ) + membership = baker.make( + Membership, end_date=None, user=user, role=board_role, club=board_role.club + ) members_group = membership.club.members_group board_group = membership.club.board_group assert user.groups.contains(members_group) assert user.groups.contains(board_group) - user.memberships.update(role=1) # from board to simple member + user.memberships.update(role=member_role) # from board to simple member assert user.groups.contains(members_group) assert not user.groups.contains(board_group) - user.memberships.update(role=5) # from member to board + user.memberships.update(role=board_role) # from member to board assert user.groups.contains(members_group) assert user.groups.contains(board_group) @@ -96,7 +101,17 @@ class TestMembershipQuerySet(TestClub): def test_delete_remove_from_groups(self): """Test that `delete` removes from club groups""" user = baker.make(User) - memberships = baker.make(Membership, role=iter([1, 5]), user=user, _quantity=2) + club = baker.make(Club) + roles = baker.make( + ClubRole, + is_board=iter([False, True]), + club=club, + _quantity=2, + _bulk_create=True, + ) + memberships = baker.make( + Membership, club=club, role=iter(roles), user=user, _quantity=2 + ) club_groups = { memberships[0].club.members_group, memberships[1].club.members_group, @@ -112,13 +127,20 @@ class TestMembershipEditableBy(TestCase): def setUpTestData(cls): Membership.objects.all().delete() cls.club_a, cls.club_b = baker.make(Club, _quantity=2) + roles = baker.make( + ClubRole, + is_presidency=itertools.cycle([True, False, False, False]), + is_board=itertools.cycle([True, True, True, False]), + order=itertools.cycle(range(4)), + club=iter( + [*itertools.repeat(cls.club_a, 4), *itertools.repeat(cls.club_b, 4)] + ), + _quantity=8, + _bulk_create=True, + ) cls.memberships = [ - *baker.make( - Membership, role=iter([7, 3, 3, 1]), club=cls.club_a, _quantity=4 - ), - *baker.make( - Membership, role=iter([7, 3, 3, 1]), club=cls.club_b, _quantity=4 - ), + *baker.make(Membership, role=iter(roles[:4]), club=cls.club_a, _quantity=4), + *baker.make(Membership, role=iter(roles[4:]), club=cls.club_b, _quantity=4), ] def test_admin_user(self): @@ -140,7 +162,7 @@ class TestMembershipEditableBy(TestCase): class TestMembership(TestClub): - def assert_membership_started_today(self, user: User, role: int): + def assert_membership_started_today(self, user: User, role: ClubRole): """Assert that the given membership is active and started today.""" membership = user.memberships.ongoing().filter(club=self.club).first() assert membership is not None @@ -189,21 +211,27 @@ class TestMembership(TestClub): "Marquer comme ancien", ] rows = table.find("tbody").find_all("tr") - memberships = self.club.members.ongoing().order_by("-role") - for row, membership in zip( - rows, memberships.select_related("user"), strict=False - ): + memberships = ( + self.club.members.ongoing() + .order_by("role__order") + .select_related("user", "role") + ) + user_role = ClubRole.objects.get(members__user=self.simple_board_member) + for row, membership in zip(rows, memberships, strict=False): user = membership.user user_url = reverse("core:user_profile", args=[user.id]) cols = row.find_all("td") user_link = cols[0].find("a") assert user_link.attrs["href"] == user_url assert user_link.text == user.get_display_name() - assert cols[1].text == settings.SITH_CLUB_ROLES[membership.role] + assert cols[1].text == membership.role.name assert cols[2].text == membership.description assert cols[3].text == str(membership.start_date) - if membership.role < 3 or membership.user_id == self.simple_board_member.id: + if ( + membership.role.order > user_role.order + or membership.user_id == self.simple_board_member.id + ): # 3 is the role of simple_board_member form_input = cols[4].find("input") expected_attrs = { @@ -219,14 +247,15 @@ class TestMembership(TestClub): """Test that root users can add members to clubs""" self.client.force_login(self.root) response = self.client.post( - self.new_members_url, {"user": self.subscriber.id, "role": 3} + self.new_members_url, + {"user": self.subscriber.id, "role": self.board_role.id}, ) assert response.status_code == 200 assert response.headers.get("HX-Redirect", "") == reverse( "club:club_members", kwargs={"club_id": self.club.id} ) self.subscriber.refresh_from_db() - self.assert_membership_started_today(self.subscriber, role=3) + self.assert_membership_started_today(self.subscriber, role=self.board_role) def test_add_unauthorized_members(self): """Test that users who are not currently subscribed @@ -234,7 +263,7 @@ class TestMembership(TestClub): """ for user in self.public, self.old_subscriber: form = ClubAddMemberForm( - data={"user": user.id, "role": 1}, + data={"user": user.id, "role": self.member_role}, request_user=self.root, club=self.club, ) @@ -255,7 +284,7 @@ class TestMembership(TestClub): nb_memberships = self.simple_board_member.memberships.count() self.client.post( self.members_url, - {"users": self.simple_board_member.id, "role": current_membership.role + 1}, + {"users": self.simple_board_member.id, "role": self.member_role}, ) self.simple_board_member.refresh_from_db() assert nb_memberships == self.simple_board_member.memberships.count() @@ -274,7 +303,7 @@ class TestMembership(TestClub): max_id = User.objects.aggregate(id=Max("id"))["id"] for members in [max_id + 1], [max_id + 1, self.subscriber.id]: form = ClubAddMemberForm( - data={"user": members, "role": 1}, + data={"user": members, "role": self.member_role}, request_user=self.root, club=self.club, ) @@ -290,12 +319,13 @@ class TestMembership(TestClub): def test_president_add_members(self): """Test that the president of the club can add members.""" - president = self.club.members.get(role=10).user + 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": 9} + 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( @@ -305,14 +335,17 @@ class TestMembership(TestClub): 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=9) + 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": 10}, + data={"user": self.subscriber.id, "role": other_role.id}, request_user=self.simple_board_member, club=self.club, ) @@ -320,7 +353,10 @@ class TestMembership(TestClub): assert not form.is_valid() assert form.errors == { - "role": ["Sélectionnez un choix valide. 10 n\u2019en fait pas partie."] + "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() @@ -336,8 +372,9 @@ class TestMembership(TestClub): assert form.errors == {"role": ["Ce champ est obligatoire."]} def test_add_member_already_there(self): + role = ClubRole.objects.get(members__user=self.simple_board_member) form = ClubAddMemberForm( - data={"user": self.simple_board_member, "role": 3}, + data={"user": self.simple_board_member, "role": role.id}, request_user=self.root, club=self.club, ) @@ -348,22 +385,27 @@ class TestMembership(TestClub): def test_add_other_member_forbidden(self): non_member = subscriber_user.make() - simple_member = baker.make(Membership, club=self.club, role=1).user + simple_member = baker.make( + Membership, club=self.club, role=self.member_role + ).user for user in non_member, simple_member: form = ClubAddMemberForm( - data={"user": subscriber_user.make(), "role": 1}, + data={"user": subscriber_user.make(), "role": self.member_role.id}, request_user=user, club=self.club, ) assert not form.is_valid() assert form.errors == { - "role": ["Sélectionnez un choix valide. 1 n\u2019en fait pas partie."] + "role": [ + "Sélectionnez un choix valide. " + "Ce choix ne fait pas partie de ceux disponibles." + ] } 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) + baker.make(Membership, club=self.club, user=user, role=self.member_role) self.client.force_login(user) res = self.client.get(self.members_url) assert res.status_code == 200 @@ -382,9 +424,10 @@ class TestMembership(TestClub): """Test that board members of the club can end memberships of users with lower roles. """ - # reminder : simple_board_member has role 3 self.client.force_login(self.simple_board_member) - membership = baker.make(Membership, club=self.club, role=2, end_date=None) + role = baker.make(ClubRole, club=self.club, is_board=True) + role.below(self.board_role) + membership = baker.make(Membership, club=self.club, role=role) response = self.client.post(self.members_url, {"members_old": [membership.id]}) self.assertRedirects(response, self.members_url) self.club.refresh_from_db() @@ -394,7 +437,9 @@ class TestMembership(TestClub): """Test that board members of the club cannot end memberships of users with higher roles. """ - membership = self.president.memberships.filter(club=self.club).first() + membership = self.president.memberships.filter( + club=self.club, end_date=None + ).first() self.client.force_login(self.simple_board_member) self.client.post(self.members_url, {"members_old": [membership.id]}) self.club.refresh_from_db() @@ -436,7 +481,9 @@ class TestMembership(TestClub): def test_remove_from_club_group(self): """Test that when a membership ends, the user is removed from club groups.""" user = baker.make(User) - baker.make(Membership, user=user, club=self.club, end_date=None, role=3) + baker.make( + Membership, user=user, club=self.club, end_date=None, role=self.board_role + ) assert user.groups.contains(self.club.members_group) assert user.groups.contains(self.club.board_group) user.memberships.update(end_date=localdate()) @@ -447,18 +494,20 @@ class TestMembership(TestClub): """Test that when a membership begins, the user is added to the club group.""" assert not self.subscriber.groups.contains(self.club.members_group) assert not self.subscriber.groups.contains(self.club.board_group) - baker.make(Membership, club=self.club, user=self.subscriber, role=3) + baker.make( + Membership, club=self.club, user=self.subscriber, role=self.board_role + ) assert self.subscriber.groups.contains(self.club.members_group) assert self.subscriber.groups.contains(self.club.board_group) def test_change_position_in_club(self): """Test that when moving from board to members, club group change""" membership = baker.make( - Membership, club=self.club, user=self.subscriber, role=3 + Membership, club=self.club, user=self.subscriber, role=self.board_role ) assert self.subscriber.groups.contains(self.club.members_group) assert self.subscriber.groups.contains(self.club.board_group) - membership.role = 1 + membership.role = self.member_role membership.save() assert self.subscriber.groups.contains(self.club.members_group) assert not self.subscriber.groups.contains(self.club.board_group) @@ -471,7 +520,11 @@ class TestMembership(TestClub): # make sli a board member self.sli.memberships.all().delete() - Membership(club=self.ae, user=self.sli, role=3).save() + Membership( + club=self.ae, + user=self.sli, + role=baker.make(ClubRole, club=self.ae, is_board=True), + ).save() assert self.club.is_owned_by(self.sli) def test_change_club_name(self): @@ -497,7 +550,7 @@ class TestMembership(TestClub): @pytest.mark.django_db def test_membership_set_old(client: Client): - membership = baker.make(Membership, end_date=None, user=(subscriber_user.make())) + membership = baker.make(Membership, end_date=None, user=subscriber_user.make()) client.force_login(membership.user) response = client.post( reverse("club:membership_set_old", kwargs={"membership_id": membership.id}) @@ -531,55 +584,63 @@ class TestJoinClub: cache.clear() @pytest.mark.parametrize( - ("user_factory", "role", "errors"), + ("user_factory", "board_role", "errors"), [ ( subscriber_user.make, - 2, + True, { "role": [ - "Sélectionnez un choix valide. 2 n\u2019en fait pas partie." + "Sélectionnez un choix valide. " + "Ce choix ne fait pas partie de ceux disponibles." ] }, ), ( lambda: baker.make(User), - 1, + False, {"__all__": ["Vous devez être cotisant pour faire partie d'un club"]}, ), ], ) def test_join_club_errors( - self, user_factory: Callable[[], User], role: int, errors: dict + self, user_factory: Callable[[], User], board_role, errors: dict ): club = baker.make(Club) user = user_factory() - form = JoinClubForm(club=club, request_user=user, data={"role": role}) + role = baker.make(ClubRole, club=club, is_board=board_role) + form = JoinClubForm(club=club, request_user=user, data={"role": role.id}) assert not form.is_valid() assert form.errors == errors def test_user_already_in_club(self): - club = baker.make(Club) user = subscriber_user.make() - baker.make(Membership, user=user, club=club) - form = JoinClubForm(club=club, request_user=user, data={"role": 1}) + role = baker.make(ClubRole, is_board=False) + baker.make(Membership, user=user, club=role.club) + form = JoinClubForm(club=role.club, request_user=user, data={"role": role.id}) assert not form.is_valid() assert form.errors == {"__all__": ["Vous êtes déjà membre de ce club."]} def test_ok(self): - club = baker.make(Club) user = subscriber_user.make() - form = JoinClubForm(club=club, request_user=user, data={"role": 1}) + role = baker.make(ClubRole, is_board=False) + form = JoinClubForm(club=role.club, request_user=user, data={"role": role.id}) assert form.is_valid() form.save() - assert Membership.objects.ongoing().filter(user=user, club=club).exists() + assert Membership.objects.ongoing().filter(user=user, club=role.club).exists() class TestOldMembersView(TestCase): @classmethod def setUpTestData(cls): club = baker.make(Club) - roles = [1, 1, 1, 2, 2, 4, 4, 5, 7, 9, 10] + roles = baker.make( + ClubRole, + club=club, + is_board=itertools.cycle([True, True, False]), + _quantity=10, + _bulk_create=True, + ) cls.memberships = baker.make( Membership, role=iter(roles), diff --git a/club/tests/test_page.py b/club/tests/test_page.py index c368735d..699a3e9e 100644 --- a/club/tests/test_page.py +++ b/club/tests/test_page.py @@ -5,7 +5,7 @@ from django.urls import reverse from model_bakery import baker from pytest_django.asserts import assertHTMLEqual, assertRedirects -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.baker_recipes import subscriber_user from core.markdown import markdown from core.models import PageRev, User @@ -59,7 +59,12 @@ def test_page_revision(client: Client): def test_edit_page(client: Client): club = baker.make(Club) user = subscriber_user.make() - baker.make(Membership, user=user, club=club, role=3) + baker.make( + Membership, + user=user, + club=club, + role=baker.make(ClubRole, club=club, is_board=True), + ) client.force_login(user) url = reverse("club:club_edit_page", kwargs={"club_id": club.id}) content = "# foo\nLorem ipsum dolor sit amet" diff --git a/club/tests/test_user_club_controller.py b/club/tests/test_user_club_controller.py index 2aba7225..dd851d14 100644 --- a/club/tests/test_user_club_controller.py +++ b/club/tests/test_user_club_controller.py @@ -6,7 +6,7 @@ from django.utils.timezone import localdate from model_bakery import baker from model_bakery.recipe import Recipe -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from club.schemas import UserMembershipSchema from core.baker_recipes import subscriber_user from core.models import Page @@ -19,7 +19,10 @@ class TestFetchClub(TestCase): pages = baker.make(Page, _quantity=3, _bulk_create=True) clubs = baker.make(Club, page=iter(pages), _quantity=3, _bulk_create=True) recipe = Recipe( - Membership, user=cls.user, start_date=localdate() - timedelta(days=2) + Membership, + user=cls.user, + start_date=localdate() - timedelta(days=2), + role=baker.make(ClubRole), ) cls.members = Membership.objects.bulk_create( [ diff --git a/com/tests/test_views.py b/com/tests/test_views.py index 0e48f033..0ef3a5e2 100644 --- a/com/tests/test_views.py +++ b/com/tests/test_views.py @@ -28,7 +28,7 @@ from django.utils.translation import gettext as _ from model_bakery import baker from pytest_django.asserts import assertNumQueries, assertRedirects -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from com.models import News, NewsDate, Poster, Sith, Weekmail, WeekmailArticle from core.baker_recipes import subscriber_user from core.models import AnonymousUser, Group, User @@ -214,7 +214,8 @@ class TestNewsCreation(TestCase): def setUpTestData(cls): cls.club = baker.make(Club) cls.user = subscriber_user.make() - baker.make(Membership, user=cls.user, club=cls.club, role=5) + role = baker.make(ClubRole, club=cls.club, is_board=True) + baker.make(Membership, user=cls.user, club=cls.club, role=role) def setUp(self): self.client.force_login(self.user) diff --git a/com/views.py b/com/views.py index ea5b742d..3eeabe34 100644 --- a/com/views.py +++ b/com/views.py @@ -504,7 +504,7 @@ class WeekmailArticleCreateView(CreateView): self.object = form.instance form.is_valid() # Valid a first time to populate club field m = form.instance.club.get_membership_for(request.user) - if m is None or m.role <= settings.SITH_MAXIMUM_FREE_ROLE: + if m is None or not m.role.is_board: form.add_error( "club", ValidationError( diff --git a/core/baker_recipes.py b/core/baker_recipes.py index 4b873b0f..30f5d66f 100644 --- a/core/baker_recipes.py +++ b/core/baker_recipes.py @@ -4,9 +4,9 @@ from dateutil.relativedelta import relativedelta from django.conf import settings from django.utils.timezone import localdate, now from model_bakery import seq -from model_bakery.recipe import Recipe, related +from model_bakery.recipe import Recipe, foreign_key, related -from club.models import Membership +from club.models import ClubRole, Membership from core.models import Group, User from subscription.models import Subscription @@ -52,7 +52,9 @@ ae_board_membership = Recipe( Membership, start_date=now() - timedelta(days=30), club_id=settings.SITH_MAIN_CLUB_ID, - role=settings.SITH_CLUB_ROLES_ID["Board member"], + role=foreign_key( + Recipe(ClubRole, club_id=settings.SITH_MAIN_CLUB_ID, is_board=True) + ), ) board_user = Recipe( diff --git a/core/tests/test_page.py b/core/tests/test_page.py index 8dcfa19c..b530ba18 100644 --- a/core/tests/test_page.py +++ b/core/tests/test_page.py @@ -11,7 +11,7 @@ from django.utils.timezone import now from model_bakery import baker from pytest_django.asserts import assertHTMLEqual, assertRedirects -from club.models import Club +from club.models import Club, Membership from core.baker_recipes import board_user, subscriber_user from core.markdown import markdown from core.models import AnonymousUser, Page, PageRev, User @@ -122,6 +122,9 @@ def test_page_revision_club_redirection(client: Client): @pytest.mark.django_db def test_viewable_by(): # remove existing pages to prevent side effect + # club pages are protected, so we must delete clubs first + Membership.objects.all().delete() + Club.objects.all().delete() Page.objects.all().delete() view_groups = [ [settings.SITH_GROUP_PUBLIC_ID], diff --git a/counter/tests/test_counter.py b/counter/tests/test_counter.py index 4f3e2df6..be49c185 100644 --- a/counter/tests/test_counter.py +++ b/counter/tests/test_counter.py @@ -32,7 +32,7 @@ from model_bakery import baker from model_bakery.recipe import Recipe from pytest_django.asserts import assertRedirects -from club.models import Membership +from club.models import ClubRole, Membership from core.baker_recipes import board_user, subscriber_user, very_old_subscriber_user from core.models import BanGroup, User from counter.baker_recipes import product_recipe, sale_recipe @@ -88,7 +88,7 @@ class TestFullClickBase(TestCase): Membership, start_date=now() - timedelta(days=30), club=cls.club_counter.club, - role=settings.SITH_CLUB_ROLES_ID["Board member"], + role=baker.make(ClubRole, club=cls.club_counter.club, is_board=True), user=cls.club_admin, ) @@ -782,7 +782,13 @@ class TestClubCounterClickAccess(TestCase): "counter:click", kwargs={"counter_id": cls.counter.id, "user_id": cls.customer.id}, ) - + cls.board_role, cls.member_role = baker.make( + ClubRole, + club=cls.counter.club, + is_board=iter([True, False]), + _quantity=2, + _bulk_create=True, + ) cls.user = subscriber_user.make() def setUp(self): @@ -797,13 +803,17 @@ class TestClubCounterClickAccess(TestCase): res = self.client.get(self.click_url) assert res.status_code == 403 # being a member of the club, without being in the board, isn't enough - baker.make(Membership, club=self.counter.club, user=self.user, role=1) + baker.make( + Membership, club=self.counter.club, user=self.user, role=self.member_role + ) res = self.client.get(self.click_url) assert res.status_code == 403 def test_board_member(self): """By default, board members should be able to click on office counters""" - baker.make(Membership, club=self.counter.club, user=self.user, role=3) + baker.make( + Membership, club=self.counter.club, user=self.user, role=self.board_role + ) self.client.force_login(self.user) res = self.client.get(self.click_url) assert res.status_code == 200 @@ -818,7 +828,9 @@ class TestClubCounterClickAccess(TestCase): def test_both_barman_and_board_member(self): """If the user is barman and board member, he should be authorized as well.""" self.counter.sellers.add(self.user) - baker.make(Membership, club=self.counter.club, user=self.user, role=3) + baker.make( + Membership, club=self.counter.club, user=self.user, role=self.board_role + ) self.client.force_login(self.user) res = self.client.get(self.click_url) assert res.status_code == 200 diff --git a/counter/tests/test_customer.py b/counter/tests/test_customer.py index 311e48ed..5b712f5c 100644 --- a/counter/tests/test_customer.py +++ b/counter/tests/test_customer.py @@ -3,14 +3,13 @@ import string from datetime import timedelta import pytest -from django.conf import settings from django.contrib.auth.base_user import make_password from django.test import Client, TestCase from django.urls import reverse from django.utils.timezone import now from model_bakery import baker -from club.models import Membership +from club.models import ClubRole, Membership from core.baker_recipes import board_user, subscriber_user from core.models import User from counter.baker_recipes import product_recipe, refill_recipe, sale_recipe @@ -42,11 +41,12 @@ class TestStudentCard(TestCase): cls.counter.sellers.add(cls.barmen) cls.club_counter = baker.make(Counter) + role = baker.make(ClubRole, club=cls.club_counter.club, is_board=True) baker.make( Membership, start_date=now() - timedelta(days=30), club=cls.club_counter.club, - role=settings.SITH_CLUB_ROLES_ID["Board member"], + role=role, user=cls.club_admin, ) diff --git a/forum/models.py b/forum/models.py index 85c487e9..dc50968d 100644 --- a/forum/models.py +++ b/forum/models.py @@ -183,7 +183,7 @@ class Forum(models.Model): Forum._club_memberships[self.id] = {} Forum._club_memberships[self.id][user.id] = m if m: - return m.role > settings.SITH_MAXIMUM_FREE_ROLE + return m.role.is_board return False def check_loop(self): diff --git a/galaxy/management/commands/generate_galaxy_test_data.py b/galaxy/management/commands/generate_galaxy_test_data.py index 966697a2..fae4a939 100644 --- a/galaxy/management/commands/generate_galaxy_test_data.py +++ b/galaxy/management/commands/generate_galaxy_test_data.py @@ -29,8 +29,9 @@ from django.conf import settings from django.core.files.base import ContentFile from django.core.management.base import BaseCommand from django.utils import timezone +from model_bakery import baker -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.models import Group, Page, SithFile, User from core.utils import RED_PIXEL_PNG from sas.models import Album, PeoplePictureRelation, Picture @@ -217,11 +218,19 @@ class Command(BaseCommand): "The `make_clubs()` method must be called before `make_club_memberships()`" ) memberships = [] + roles = { + r.club_id: r.id + for r in baker.make( + ClubRole, + club=iter(self.clubs), + _quantity=len(self.clubs), + _bulk_create=True, + ) + } for i in range(1, 11): # users can be in up to 20 clubs self.logger.info(f"Club membership, pass {i}") - for uid in range( - i, self.NB_USERS, i - ): # Pass #1 will make sure every user is at least in one club + for uid in range(i, self.NB_USERS, i): + # Pass #1 will make sure every user is at least in one club user = self.users[uid] club = self.clubs[(uid + i**2) % self.NB_CLUBS] @@ -236,7 +245,7 @@ class Command(BaseCommand): Membership( user=user, club=club, - role=(uid + i) % 10 + 1, # spread the different roles + role_id=roles[club.id], start_date=start, end_date=end, ) @@ -259,7 +268,7 @@ class Command(BaseCommand): Membership( user=user, club=club, - role=((uid // 10) + i) % 10 + 1, # spread the different roles + role_id=roles[club.id], start_date=start, end_date=end, ) diff --git a/subscription/tests/test_permissions.py b/subscription/tests/test_permissions.py index fcc290e9..fb0483fd 100644 --- a/subscription/tests/test_permissions.py +++ b/subscription/tests/test_permissions.py @@ -4,7 +4,7 @@ from django.urls import reverse from model_bakery import baker from pytest_django.asserts import assertRedirects -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.baker_recipes import subscriber_user from core.models import User @@ -15,7 +15,8 @@ class TestSubscriptionPermission(TestCase): cls.user: User = subscriber_user.make() cls.admin = baker.make(User, is_superuser=True) cls.club = baker.make(Club) - baker.make(Membership, user=cls.user, club=cls.club, role=7) + role = baker.make(ClubRole, club=cls.club, is_board=True) + baker.make(Membership, user=cls.user, club=cls.club, role=role) def test_give_permission(self): self.client.force_login(self.admin)