From 5cf54729ae90ea34379286acfe6af5fb3fc8e1bc Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 22 Jun 2025 16:59:07 +0200 Subject: [PATCH 01/11] add `ClubRole` model --- club/admin.py | 16 ++- .../0015_clubrole_alter_membership_role.py | 118 ++++++++++++++++++ .../0016_clubrole_alter_membership_role.py | 25 ++++ club/models.py | 95 ++++++++++---- 4 files changed, 232 insertions(+), 22 deletions(-) create mode 100644 club/migrations/0015_clubrole_alter_membership_role.py create mode 100644 club/migrations/0016_clubrole_alter_membership_role.py diff --git a/club/admin.py b/club/admin.py index 0622cb16..bff21208 100644 --- a/club/admin.py +++ b/club/admin.py @@ -14,7 +14,7 @@ # from django.contrib import admin -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership @admin.register(Club) @@ -30,6 +30,20 @@ class ClubAdmin(admin.ModelAdmin): ) +@admin.register(ClubRole) +class ClubRoleAdmin(admin.ModelAdmin): + list_display = ("name", "club", "is_board", "is_presidency") + search_fields = ("name",) + autocomplete_fields = ("club",) + list_select_related = ("club",) + list_filter = ( + "is_board", + "is_presidency", + ("club", admin.RelatedOnlyFieldListFilter), + ) + show_facets = admin.ModelAdmin.show_facets.ALWAYS + + @admin.register(Membership) class MembershipAdmin(admin.ModelAdmin): list_display = ("user", "club", "role", "start_date", "end_date") diff --git a/club/migrations/0015_clubrole_alter_membership_role.py b/club/migrations/0015_clubrole_alter_membership_role.py new file mode 100644 index 00000000..74387af9 --- /dev/null +++ b/club/migrations/0015_clubrole_alter_membership_role.py @@ -0,0 +1,118 @@ +# Generated by Django 5.2.3 on 2025-06-21 21:59 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models +from django.db.migrations.state import StateApps +from django.db.models import Case, When +from django.utils import translation + + +def migrate_roles(apps: StateApps, schema_editor): + ClubRole = apps.get_model("club", "ClubRole") + Membership = apps.get_model("club", "Membership") + + translation.activate("fr") + + updates = [] + presidency = settings.SITH_CLUB_ROLES_ID["President"] + for club_id, role in Membership.objects.values_list("club", "role").distinct(): + new_role = ClubRole.objects.create( + name=SITH_CLUB_ROLES[role], + is_board=role > settings.SITH_MAXIMUM_FREE_ROLE, + is_presidency=role == presidency, + club_id=club_id, + order=presidency - role, + ) + updates.append(When(role=role, then=new_role.id)) + # all updates must happen at the same time + # otherwise, the 10 first created ClubRole would be + # re-modified after their initial creation, and it would + # result in an incoherent state. + # To avoid that, all updates are wrapped in a single giant Case(When) statement + # cf. https://docs.djangoproject.com/fr/stable/ref/models/conditional-expressions/#conditional-update + Membership.objects.update(role=Case(*updates)) + + +class Migration(migrations.Migration): + dependencies = [ + ("club", "0014_alter_club_options_rename_unix_name_club_slug_name_and_more") + ] + + operations = [ + migrations.CreateModel( + name="ClubRole", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "order", + models.PositiveIntegerField( + db_index=True, editable=False, verbose_name="order" + ), + ), + ( + "club", + models.ForeignKey( + help_text="The club with which this role is associated", + on_delete=django.db.models.deletion.CASCADE, + related_name="roles", + to="club.club", + verbose_name="club", + ), + ), + ("name", models.CharField(max_length=50, verbose_name="name")), + ( + "description", + models.TextField( + default="", blank=True, verbose_name="description" + ), + ), + ( + "is_board", + models.BooleanField(default=False, verbose_name="Board role"), + ), + ( + "is_presidency", + models.BooleanField(default=False, verbose_name="Presidency role"), + ), + ( + "is_active", + models.BooleanField( + default=True, + help_text=( + "If the role is inactive, people joining the club " + "won't be able to get it." + ), + verbose_name="is active", + ), + ), + ], + options={ + "ordering": ("order",), + "verbose_name": "club role", + "verbose_name_plural": "club roles", + }, + ), + migrations.AddConstraint( + model_name="clubrole", + constraint=models.CheckConstraint( + condition=models.Q( + ("is_presidency", False), ("is_board", True), _connector="OR" + ), + name="clubrole_presidency_implies_board", + ), + ), + migrations.RunPython(migrate_roles, migrations.RunPython.noop), + # because Postgres migrations run in a single transaction, + # we cannot change the actual values of Membership.role + # and apply the FOREIGN KEY constraint in the same migration. + # The constraint is created in the next migration + ] diff --git a/club/migrations/0016_clubrole_alter_membership_role.py b/club/migrations/0016_clubrole_alter_membership_role.py new file mode 100644 index 00000000..2031911c --- /dev/null +++ b/club/migrations/0016_clubrole_alter_membership_role.py @@ -0,0 +1,25 @@ +# Generated by Django 5.2.3 on 2025-09-27 09:57 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("club", "0015_clubrole_alter_membership_role")] + + operations = [ + # because Postgres migrations run in a single transaction, + # we cannot change the actual values of Membership.role + # and apply the FOREIGN KEY constraint in the same migration. + # The data migration was made in the previous migration. + migrations.AlterField( + model_name="membership", + name="role", + field=models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="members", + to="club.clubrole", + verbose_name="role", + ), + ), + ] diff --git a/club/models.py b/club/models.py index f6695812..d58bc6fe 100644 --- a/club/models.py +++ b/club/models.py @@ -29,14 +29,14 @@ from django.conf import settings from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import RegexValidator, validate_email from django.db import models, transaction -from django.db.models import Exists, F, OuterRef, Q, Value -from django.db.models.functions import Greatest +from django.db.models import Exists, F, OuterRef, Q from django.urls import reverse from django.utils import timezone from django.utils.functional import cached_property from django.utils.text import slugify from django.utils.timezone import localdate from django.utils.translation import gettext_lazy as _ +from ordered_model.models import OrderedModel from core.fields import ResizedImageField from core.models import Group, Notification, Page, SithFile, User @@ -220,6 +220,62 @@ class Club(models.Model): return user.is_in_group(pk=self.board_group_id) +class ClubRole(OrderedModel): + club = models.ForeignKey( + Club, + verbose_name=_("club"), + help_text=_("The club with which this role is associated"), + related_name="roles", + on_delete=models.CASCADE, + ) + name = models.CharField(_("name"), max_length=50) + description = models.TextField(_("description"), blank=True, default="") + is_board = models.BooleanField(_("Board role"), default=False) + is_presidency = models.BooleanField(_("Presidency role"), default=False) + is_active = models.BooleanField( + _("is active"), + default=True, + help_text=_( + "If the role is inactive, people joining the club won't be able to get it." + ), + ) + + order_with_respect_to = "club" + + class Meta(OrderedModel.Meta): + verbose_name = _("club role") + verbose_name_plural = _("club roles") + abstract = False + constraints = [ + # presidency IMPLIES board <=> NOT presidency OR board + # cf. MT1 :) + models.CheckConstraint( + condition=Q(is_presidency=False) | Q(is_board=True), + name="clubrole_presidency_implies_board", + ) + ] + + def __str__(self): + return f"{self.name} - {self.club.name}" + + 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): + if self.is_presidency and not self.is_board: + raise ValidationError( + _( + "Role %(name)s was declared as a presidency role " + "without being a board role" + ) + % {"name": self.name} + ) + return super().clean() + + class MembershipQuerySet(models.QuerySet): def ongoing(self) -> Self: """Filter all memberships which are not finished yet.""" @@ -232,9 +288,10 @@ class MembershipQuerySet(models.QuerySet): are included, even if there are no more members. If you want to get the users who are currently in the board, - mind combining this with the `ongoing` queryset method + mind combining this with the [MembershipQuerySet.ongoing][] + queryset method """ - return self.filter(role__gt=settings.SITH_MAXIMUM_FREE_ROLE) + return self.filter(role__is_board=True) def editable_by(self, user: User) -> Self: """Filter Memberships that this user can edit. @@ -257,21 +314,16 @@ class MembershipQuerySet(models.QuerySet): """ if user.has_perm("club.change_membership"): return self.all() - return self.filter( + return self.ongoing().filter( Q(user=user) | Exists( - Membership.objects.filter( - Q( - role__gt=Greatest( - OuterRef("role"), Value(settings.SITH_MAXIMUM_FREE_ROLE) - ) - ), + Membership.objects.ongoing().filter( user=user, - end_date=None, club=OuterRef("club"), + role__is_board=True, + role__order__gt=OuterRef("role__order"), ) - ), - end_date=None, + ) ) def update(self, **kwargs) -> int: @@ -341,10 +393,11 @@ class Membership(models.Model): ) start_date = models.DateField(_("start date"), default=timezone.now) end_date = models.DateField(_("end date"), null=True, blank=True) - role = models.IntegerField( - _("role"), - choices=sorted(settings.SITH_CLUB_ROLES.items()), - default=sorted(settings.SITH_CLUB_ROLES.items())[0][0], + role = models.ForeignKey( + ClubRole, + verbose_name=_("role"), + related_name="members", + on_delete=models.PROTECT, ) description = models.CharField( _("description"), max_length=128, null=False, blank=True @@ -362,7 +415,7 @@ class Membership(models.Model): def __str__(self): return ( f"{self.club.name} - {self.user.username} " - f"- {settings.SITH_CLUB_ROLES[self.role]} " + f"- {self.role.name} " f"- {str(_('past member')) if self.end_date is not None else ''}" ) @@ -391,7 +444,7 @@ 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 >= self.role + return membership is not None and membership.role.order < self.role.order def delete(self, *args, **kwargs): self._remove_club_groups([self]) @@ -467,7 +520,7 @@ class Membership(models.Model): group_id=membership.club.members_group_id, ) ) - if membership.role > settings.SITH_MAXIMUM_FREE_ROLE: + if membership.role.is_board: club_groups.append( User.groups.through( user_id=membership.user_id, From 11d9e912e19d04a67e953af69c065fe3e8e26667 Mon Sep 17 00:00:00 2001 From: imperosol Date: Sat, 27 Sep 2025 16:10:22 +0200 Subject: [PATCH 02/11] adapt populate and populate_more --- core/management/commands/populate.py | 163 +++++++++++++--------- core/management/commands/populate_more.py | 28 ++-- 2 files changed, 109 insertions(+), 82 deletions(-) diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 22541841..d4eeaa5a 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -36,7 +36,7 @@ from django.utils import timezone from django.utils.timezone import localdate from PIL import Image -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from com.ics_calendar import IcsCalendar from com.models import News, NewsDate, Sith, Weekmail from core.models import BanGroup, Group, Page, PageRev, SithFile, User @@ -69,6 +69,13 @@ class PopulatedGroups(NamedTuple): campus_admin: Group +class PopulatedClubs(NamedTuple): + ae: Club + troll: Club + pdf: Club + refound: Club + + class Command(BaseCommand): ROOT_PATH: ClassVar[Path] = Path(__file__).parent.parent.parent.parent SAS_FIXTURE_PATH: ClassVar[Path] = ( @@ -120,32 +127,16 @@ class Command(BaseCommand): sas = SithFile.objects.create( name="SAS", owner=root, id=settings.SITH_SAS_ROOT_DIR_ID ) - main_club = Club.objects.create( - id=1, name="AE", address="6 Boulevard Anatole France, 90000 Belfort" - ) - main_club.board_group.permissions.add( - *Permission.objects.filter( - codename__in=[ - "view_subscription", - "add_subscription", - "view_hidden_user", - ] - ) - ) - bar_club = Club.objects.create( - id=settings.SITH_PDF_CLUB_ID, - name="PdF", - address="6 Boulevard Anatole France, 90000 Belfort", - ) + clubs = self._create_clubs() self.reset_index("club") for bar_id, bar_name in settings.SITH_COUNTER_BARS: - Counter(id=bar_id, name=bar_name, club=bar_club, type="BAR").save() + Counter(id=bar_id, name=bar_name, club=clubs.pdf, type="BAR").save() self.reset_index("counter") counters = [ - Counter(name="Eboutic", club=main_club, type="EBOUTIC"), - Counter(name="AE", club=main_club, type="OFFICE"), - Counter(name="Vidage comptes AE", club=main_club, type="OFFICE"), + Counter(name="Eboutic", club=clubs.ae, type="EBOUTIC"), + Counter(name="AE", club=clubs.ae, type="OFFICE"), + Counter(name="Vidage comptes AE", club=clubs.ae, type="OFFICE"), ] Counter.objects.bulk_create(counters) bar_groups = [] @@ -328,62 +319,49 @@ class Command(BaseCommand): self._create_subscription(tutu) StudentCard(uid="9A89B82018B0A0", customer=sli.customer).save() - # Clubs - Club.objects.create( - name="Bibo'UT", address="46 de la Boustifaille", parent=main_club + Membership.objects.create( + user=skia, club=clubs.ae, role=clubs.ae.roles.get(name="Respo Info") ) - guyut = Club.objects.create( - name="Guy'UT", address="42 de la Boustifaille", parent=main_club - ) - Club.objects.create(name="Woenzel'UT", address="Woenzel", parent=guyut) - troll = Club.objects.create( - name="Troll Penché", address="Terre Du Milieu", parent=main_club - ) - refound = Club.objects.create( - name="Carte AE", address="Jamais imprimée", parent=main_club - ) - - Membership.objects.create(user=skia, club=main_club, role=3) Membership.objects.create( user=comunity, - club=bar_club, + club=clubs.pdf, start_date=localdate(), - role=settings.SITH_CLUB_ROLES_ID["Board member"], + role=clubs.pdf.roles.get(name="Membre du bureau"), ) Membership.objects.create( user=sli, - club=troll, - role=9, + club=clubs.troll, + role=clubs.troll.roles.get(name="Vice-Président⸱e"), description="Padawan Troll", start_date=localdate() - timedelta(days=17), ) Membership.objects.create( user=krophil, - club=troll, - role=10, + club=clubs.troll, + role=clubs.troll.roles.get(name="Président⸱e"), description="Maitre Troll", start_date=localdate() - timedelta(days=200), ) Membership.objects.create( user=skia, - club=troll, - role=2, + club=clubs.troll, + role=clubs.troll.roles.get(name="Membre du bureau"), description="Grand Ancien Troll", start_date=localdate() - timedelta(days=400), end_date=localdate() - timedelta(days=86), ) Membership.objects.create( user=richard, - club=troll, - role=2, + club=clubs.troll, + role=clubs.troll.roles.get(name="Membre du bureau"), description="", start_date=localdate() - timedelta(days=200), end_date=localdate() - timedelta(days=100), ) - self._create_products(groups, main_club, refound) + self._create_products(groups, clubs) - Counter.objects.create(name="Carte AE", club=refound, type="OFFICE") + Counter.objects.create(name="Carte AE", club=clubs.refound, type="OFFICE") # Add barman to counter Counter.sellers.through.objects.bulk_create( @@ -403,7 +381,7 @@ class Command(BaseCommand): end_date="7942-06-12 10:28:45+01", ) el.view_groups.add(groups.public) - el.edit_groups.add(main_club.board_group) + el.edit_groups.add(clubs.ae.board_group) el.candidature_groups.add(groups.subscribers) el.vote_groups.add(groups.subscribers) liste = ElectionList.objects.create(title="Candidature Libre", election=el) @@ -476,7 +454,7 @@ class Command(BaseCommand): title="Apero barman", summary="Viens boire un coup avec les barmans", content="Glou glou glou glou glou glou glou", - club=bar_club, + club=clubs.pdf, author=subscriber, is_published=True, moderator=skia, @@ -494,7 +472,7 @@ class Command(BaseCommand): content=( "Viens donc t'enjailler avec les autres barmans aux frais du BdF! \\o/" ), - club=bar_club, + club=clubs.pdf, author=subscriber, is_published=True, moderator=skia, @@ -510,7 +488,7 @@ class Command(BaseCommand): title="Repas fromager", summary="Wien manger du l'bon fromeug'", content="Fô viendre mangey d'la bonne fondue!", - club=bar_club, + club=clubs.pdf, author=subscriber, is_published=True, moderator=skia, @@ -526,7 +504,7 @@ class Command(BaseCommand): title="SdF", summary="Enjoy la fin des finaux!", content="Viens faire la fête avec tout plein de gens!", - club=bar_club, + club=clubs.pdf, author=subscriber, is_published=True, moderator=skia, @@ -544,7 +522,7 @@ class Command(BaseCommand): summary="Viens jouer!", content="Rejoins la fine équipe du Troll Penché et viens " "t'amuser le Vendredi soir!", - club=troll, + club=clubs.troll, author=subscriber, is_published=True, moderator=skia, @@ -644,9 +622,7 @@ class Command(BaseCommand): ] ) - def _create_products( - self, groups: PopulatedGroups, main_club: Club, refound_club: Club - ): + def _create_products(self, groups: PopulatedGroups, clubs: PopulatedClubs): beers_type, cotis_type, refill_type, verre_type = ( ProductType.objects.bulk_create( [ @@ -662,28 +638,28 @@ class Command(BaseCommand): code="1SCOTIZ", product_type=cotis_type, purchase_price=15, - club=main_club, + club=clubs.ae, ) cotis2 = Product.objects.create( name="Cotis 2 semestres", code="2SCOTIZ", product_type=cotis_type, purchase_price="28", - club=main_club, + club=clubs.ae, ) refill = Product.objects.create( name="Rechargement 15 €", code="15REFILL", product_type=refill_type, purchase_price=15, - club=main_club, + club=clubs.ae, ) barb = Product.objects.create( name="Barbar", code="BARB", product_type=beers_type, purchase_price="1.50", - club=main_club, + club=clubs.pdf, limit_age=18, ) cble = Product.objects.create( @@ -691,7 +667,7 @@ class Command(BaseCommand): code="CBLE", product_type=beers_type, purchase_price="1.50", - club=main_club, + club=clubs.pdf, limit_age=18, ) cons = Product.objects.create( @@ -699,21 +675,21 @@ class Command(BaseCommand): code="CONS", product_type=verre_type, purchase_price="1", - club=main_club, + club=clubs.pdf, ) dcons = Product.objects.create( name="Déconsigne Eco-cup", code="DECO", product_type=verre_type, purchase_price="-1", - club=main_club, + club=clubs.pdf, ) cors = Product.objects.create( name="Corsendonk", code="CORS", product_type=beers_type, purchase_price="1.50", - club=main_club, + club=clubs.pdf, limit_age=18, ) carolus = Product.objects.create( @@ -721,14 +697,14 @@ class Command(BaseCommand): code="CARO", product_type=beers_type, purchase_price="1.50", - club=main_club, + club=clubs.pdf, limit_age=18, ) Product.objects.create( name="remboursement", code="REMBOURS", purchase_price=0, - club=refound_club, + club=clubs.refound, ) ReturnableProduct.objects.create( product=cons, returned_product=dcons, max_return=3 @@ -805,6 +781,57 @@ class Command(BaseCommand): ) s.save() + def _create_clubs(self) -> PopulatedClubs: + ae = Club.objects.create( + id=1, name="AE", address="6 Boulevard Anatole France, 90000 Belfort" + ) + ae.board_group.permissions.add( + *Permission.objects.filter( + codename__in=[ + "view_subscription", + "add_subscription", + "add_membership", + "view_hidden_user", + ] + ) + ) + pdf = Club.objects.create( + id=settings.SITH_PDF_CLUB_ID, + name="PdF", + address="6 Boulevard Anatole France, 90000 Belfort", + ) + troll = Club.objects.create( + name="Troll Penché", address="Terre Du Milieu", parent=ae + ) + refound = Club.objects.create( + name="Carte AE", address="Jamais imprimée", parent=ae + ) + roles = [] + presidency_roles = ["Président⸱e", "Vice-Président⸱e"] + board_roles = [ + "Trésorier⸱e", + "Secrétaire", + "Respo Info", + "Respo Com", + "Membre du bureau", + ] + simple_roles = ["Membre actif⸱ve", "Curieux⸱euse"] + for club in ae, pdf, troll, refound: + for i, role in enumerate(presidency_roles): + roles.append( + ClubRole( + club=club, order=i, name=role, is_presidency=True, is_board=True + ) + ) + for i, role in enumerate(board_roles, start=len(presidency_roles)): + roles.append(ClubRole(club=club, order=i, name=role, is_board=True)) + for i, role in enumerate( + simple_roles, start=len(presidency_roles) + len(board_roles) + ): + roles.append(ClubRole(club=club, order=i, name=role)) + ClubRole.objects.bulk_create(roles) + return PopulatedClubs(ae=ae, troll=troll, pdf=pdf, refound=refound) + def _create_groups(self) -> PopulatedGroups: perms = Permission.objects.all() diff --git a/core/management/commands/populate_more.py b/core/management/commands/populate_more.py index 47cb75d5..14b1c59a 100644 --- a/core/management/commands/populate_more.py +++ b/core/management/commands/populate_more.py @@ -11,7 +11,7 @@ from django.db.models import Count, Exists, Min, OuterRef, Subquery from django.utils.timezone import localdate, make_aware, now from faker import Faker -from club.models import Club, Membership +from club.models import Club, ClubRole, Membership from core.models import Group, User, UserBan from counter.models import ( Counter, @@ -173,20 +173,25 @@ class Command(BaseCommand): Customer.objects.bulk_create(customers, ignore_conflicts=True) def make_club(self, club: Club, members: list[User], old_members: list[User]): - def zip_roles(users: list[User]) -> Iterator[tuple[User, int]]: - roles = iter(sorted(settings.SITH_CLUB_ROLES.keys(), reverse=True)) + roles: list[ClubRole] = list(club.roles.all()) + + def zip_roles(users: list[User]) -> Iterator[tuple[User, ClubRole]]: + important_roles = [r for r in roles if r.is_board] + important_roles.sort(key=lambda r: r.order) + simple_board_role = important_roles.pop() + member_roles = [r for r in roles if not r.is_board] user_idx = 0 - while (role := next(roles)) > 2: + for _role in important_roles: # one member for each major role - yield users[user_idx], role + yield users[user_idx], _role user_idx += 1 for _ in range(int(0.3 * (len(users) - user_idx))): # 30% of the remaining in the board - yield users[user_idx], 2 + yield users[user_idx], simple_board_role user_idx += 1 for remaining in users[user_idx + 1 :]: # everything else is a simple member - yield remaining, 1 + yield remaining, random.choices(member_roles, weights=(0.8, 0.2))[0] memberships = [] old_members = old_members.copy() @@ -198,19 +203,14 @@ class Command(BaseCommand): start_date=start, end_date=self.faker.past_date(start), user=old, - role=random.choice(list(settings.SITH_CLUB_ROLES.keys())), + role=random.choice(roles), club=club, ) ) for member, role in zip_roles(members): start = self.faker.past_date("-1y") memberships.append( - Membership( - start_date=start, - user=member, - role=role, - club=club, - ) + Membership(start_date=start, user=member, role=role, club=club) ) memberships = Membership.objects.bulk_create(memberships) Membership._add_club_groups(memberships) From 74d8a374b9f63405203be74e9a66f4e53a3e3ca5 Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 28 Sep 2025 11:20:58 +0200 Subject: [PATCH 03/11] change on_delete constraint for club pages --- .../0015_clubrole_alter_membership_role.py | 13 ++++++++++++- club/models.py | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/club/migrations/0015_clubrole_alter_membership_role.py b/club/migrations/0015_clubrole_alter_membership_role.py index 74387af9..12ec01d9 100644 --- a/club/migrations/0015_clubrole_alter_membership_role.py +++ b/club/migrations/0015_clubrole_alter_membership_role.py @@ -36,10 +36,21 @@ def migrate_roles(apps: StateApps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ("club", "0014_alter_club_options_rename_unix_name_club_slug_name_and_more") + ("club", "0014_alter_club_options_rename_unix_name_club_slug_name_and_more"), + ("core", "0047_alter_notification_date_alter_notification_type"), ] operations = [ + migrations.AlterField( + model_name="club", + name="page", + field=models.OneToOneField( + blank=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="club", + to="core.page", + ), + ), migrations.CreateModel( name="ClubRole", fields=[ diff --git a/club/models.py b/club/models.py index d58bc6fe..a2617d37 100644 --- a/club/models.py +++ b/club/models.py @@ -89,7 +89,7 @@ class Club(models.Model): on_delete=models.SET_NULL, ) page = models.OneToOneField( - Page, related_name="club", blank=True, on_delete=models.CASCADE + Page, related_name="club", blank=True, on_delete=models.PROTECT ) members_group = models.OneToOneField( Group, related_name="club", on_delete=models.PROTECT From 898ce48cc8c573542640981f8e3be18a25ff3f1d Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 28 Sep 2025 11:52:08 +0200 Subject: [PATCH 04/11] adapt tests to new club roles framework --- club/api.py | 5 +- club/models.py | 6 +- club/schemas.py | 14 +- club/tests/base.py | 20 +- club/tests/test_club.py | 18 +- club/tests/test_club_controller.py | 7 +- club/tests/test_edit.py | 16 +- club/tests/test_mailing.py | 5 +- club/tests/test_membership.py | 173 ++++++++++++------ club/tests/test_page.py | 9 +- club/tests/test_user_club_controller.py | 7 +- com/tests/test_views.py | 5 +- com/views.py | 2 +- core/baker_recipes.py | 8 +- core/tests/test_page.py | 5 +- counter/tests/test_counter.py | 24 ++- counter/tests/test_customer.py | 6 +- forum/models.py | 2 +- .../commands/generate_galaxy_test_data.py | 21 ++- subscription/tests/test_permissions.py | 5 +- 20 files changed, 250 insertions(+), 108 deletions(-) diff --git a/club/api.py b/club/api.py index cde007c2..d6a8c97d 100644 --- a/club/api.py +++ b/club/api.py @@ -37,7 +37,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 @@ -59,5 +60,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 02622110..aa4aa4e5 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 4c69b2c4..0e25995e 100644 --- a/club/tests/test_club.py +++ b/club/tests/test_club.py @@ -7,7 +7,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 from core.models import User @@ -19,11 +19,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 456ba6ec..1d29d6ce 100644 --- a/com/views.py +++ b/com/views.py @@ -503,7 +503,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 a195c1f9..b5a34e22 100644 --- a/counter/tests/test_counter.py +++ b/counter/tests/test_counter.py @@ -31,7 +31,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, Group, User from counter.baker_recipes import price_recipe, product_recipe, sale_recipe @@ -87,7 +87,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, ) @@ -800,7 +800,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 test_anonymous(self): @@ -812,13 +818,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 @@ -833,7 +843,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 eb1de534..5a194824 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) From fa190f6909d0dc3e6582d25ad7b4285f706194e1 Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 28 Sep 2025 12:37:52 +0200 Subject: [PATCH 05/11] adapt club members pages to new club roles framework --- club/forms.py | 40 +++++++------------ .../0015_clubrole_alter_membership_role.py | 21 +++++++--- club/models.py | 37 ++++++++++++----- club/templates/club/club_members.jinja | 2 +- club/templates/club/club_old_members.jinja | 2 +- club/tests/test_club_controller.py | 2 +- club/views.py | 15 +++---- core/templates/core/user_clubs.jinja | 8 ++-- counter/models.py | 2 +- sith/settings.py | 24 ----------- trombi/models.py | 11 ++--- 11 files changed, 76 insertions(+), 88 deletions(-) diff --git a/club/forms.py b/club/forms.py index 3c10dfce..5ef27003 100644 --- a/club/forms.py +++ b/club/forms.py @@ -23,13 +23,12 @@ # from django import forms -from django.conf import settings -from django.db.models import Exists, OuterRef, Q +from django.db.models import Exists, OuterRef, Q, QuerySet from django.db.models.functions import Lower from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ -from club.models import Club, Mailing, MailingSubscription, Membership +from club.models import Club, ClubRole, Mailing, MailingSubscription, Membership from core.models import User from core.views.forms import SelectDateTime from core.views.widgets.ajax_select import ( @@ -215,9 +214,7 @@ class ClubOldMemberForm(forms.Form): def __init__(self, *args, user: User, club: Club, **kwargs): super().__init__(*args, **kwargs) - self.fields["members_old"].queryset = ( - Membership.objects.ongoing().filter(club=club).editable_by(user) - ) + self.fields["members_old"].queryset = club.members.ongoing().editable_by(user) class ClubMemberForm(forms.ModelForm): @@ -235,19 +232,14 @@ class ClubMemberForm(forms.ModelForm): self.request_user = request_user self.request_user_membership = self.club.get_membership_for(self.request_user) super().__init__(*args, **kwargs) - self.fields["role"].required = True - self.fields["role"].choices = [ - (value, name) - for value, name in settings.SITH_CLUB_ROLES.items() - if value <= self.max_available_role - ] + self.fields["role"].queryset = self.available_roles self.instance.club = club @property - def max_available_role(self): + def available_roles(self) -> QuerySet[ClubRole]: """The greatest role that will be obtainable with this form.""" # this is unreachable, because it will be overridden by subclasses - return -1 # pragma: no cover + return ClubRole.objects.none() # pragma: no cover class ClubAddMemberForm(ClubMemberForm): @@ -258,7 +250,7 @@ class ClubAddMemberForm(ClubMemberForm): widgets = {"user": AutoCompleteSelectUser} @cached_property - def max_available_role(self): + def available_roles(self): """The greatest role that will be obtainable with this form. Admins and the club president can attribute any role. @@ -266,13 +258,13 @@ class ClubAddMemberForm(ClubMemberForm): Other users cannot attribute roles with this form """ if self.request_user.has_perm("club.add_membership"): - return settings.SITH_CLUB_ROLES_ID["President"] + return self.club.roles.all() membership = self.request_user_membership - if membership is None or membership.role <= settings.SITH_MAXIMUM_FREE_ROLE: - return -1 - if membership.role == settings.SITH_CLUB_ROLES_ID["President"]: - return membership.role - return membership.role - 1 + 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) def clean_user(self): """Check that the user is not trying to add a user already in the club. @@ -296,13 +288,11 @@ class JoinClubForm(ClubMemberForm): def __init__(self, *args, club: Club, request_user: User, **kwargs): super().__init__(*args, club=club, request_user=request_user, **kwargs) - # this form doesn't manage the user who will join the club, - # so we must set this here to avoid errors self.instance.user = self.request_user @cached_property - def max_available_role(self): - return settings.SITH_MAXIMUM_FREE_ROLE + def available_roles(self): + return self.club.roles.filter(is_board=False) def clean(self): """Check that the user is subscribed and isn't already in the club.""" diff --git a/club/migrations/0015_clubrole_alter_membership_role.py b/club/migrations/0015_clubrole_alter_membership_role.py index 12ec01d9..08431ea1 100644 --- a/club/migrations/0015_clubrole_alter_membership_role.py +++ b/club/migrations/0015_clubrole_alter_membership_role.py @@ -5,24 +5,33 @@ from django.conf import settings from django.db import migrations, models from django.db.migrations.state import StateApps from django.db.models import Case, When -from django.utils import translation + +PRESIDENT_ROLE = 10 +SITH_CLUB_ROLES = { + 10: "Président⸱e", + 9: "Vice-Président⸱e", + 7: "Trésorier⸱e", + 5: "Responsable communication", + 4: "Secrétaire", + 3: "Responsable info", + 2: "Membre du bureau", + 1: "Membre actif⸱ve", + 0: "Curieux⸱euse", +} def migrate_roles(apps: StateApps, schema_editor): ClubRole = apps.get_model("club", "ClubRole") Membership = apps.get_model("club", "Membership") - translation.activate("fr") - updates = [] - presidency = settings.SITH_CLUB_ROLES_ID["President"] for club_id, role in Membership.objects.values_list("club", "role").distinct(): new_role = ClubRole.objects.create( name=SITH_CLUB_ROLES[role], is_board=role > settings.SITH_MAXIMUM_FREE_ROLE, - is_presidency=role == presidency, + is_presidency=role == PRESIDENT_ROLE, club_id=club_id, - order=presidency - role, + order=PRESIDENT_ROLE - role, ) updates.append(When(role=role, then=new_role.id)) # all updates must happen at the same time diff --git a/club/models.py b/club/models.py index a5b1cc2f..58d9c139 100644 --- a/club/models.py +++ b/club/models.py @@ -138,9 +138,7 @@ class Club(models.Model): @cached_property def president(self) -> Membership | None: """Fetch the membership of the current president of this club.""" - return self.members.filter( - role=settings.SITH_CLUB_ROLES_ID["President"], end_date=None - ).first() + return self.members.filter(end_date=None).order_by("role__order").first() def check_loop(self): """Raise a validation error when a loop is found within the parent list.""" @@ -208,7 +206,9 @@ class Club(models.Model): @cached_property def current_members(self) -> list[Membership]: - return list(self.members.ongoing().select_related("user").order_by("-role")) + return list( + self.members.ongoing().select_related("user", "role").order_by("-role") + ) def get_membership_for(self, user: User) -> Membership | None: """Return the current membership of the given user.""" @@ -256,7 +256,7 @@ class ClubRole(OrderedModel): ] def __str__(self): - return f"{self.name} - {self.club.name}" + return self.name def get_display_name(self): return f"{self.name} - {self.club.name}" @@ -265,14 +265,29 @@ class ClubRole(OrderedModel): return reverse("club:club_roles", kwargs={"club_id": self.club_id}) def clean(self): + errors = [] if self.is_presidency and not self.is_board: - raise ValidationError( - _( - "Role %(name)s was declared as a presidency role " - "without being a board role" + errors.append( + ValidationError( + _( + "Role %(name)s was declared as a presidency role " + "without being a board role" + ) + % {"name": self.name} ) - % {"name": self.name} ) + if ( + self.is_board + and self.club.roles.filter(is_board=False, order__lt=self.order).exists() + ): + errors.append( + ValidationError( + _("Board role %(role)s cannot be placed below a member role") + % {"role": self.name} + ) + ) + if errors: + raise ValidationError(errors) return super().clean() @@ -321,7 +336,7 @@ class MembershipQuerySet(models.QuerySet): user=user, club=OuterRef("club"), role__is_board=True, - role__order__gt=OuterRef("role__order"), + role__order__lt=OuterRef("role__order"), ) ) ) diff --git a/club/templates/club/club_members.jinja b/club/templates/club/club_members.jinja index 7e37b04e..3e134442 100644 --- a/club/templates/club/club_members.jinja +++ b/club/templates/club/club_members.jinja @@ -41,7 +41,7 @@ {% for m in members %} {{ user_profile_link(m.user) }} - {{ settings.SITH_CLUB_ROLES[m.role] }} + {{ m.role.name }} {{ m.description }} {{ m.start_date }} {%- if can_end_membership -%} diff --git a/club/templates/club/club_old_members.jinja b/club/templates/club/club_old_members.jinja index 75603f9e..216a7437 100644 --- a/club/templates/club/club_old_members.jinja +++ b/club/templates/club/club_old_members.jinja @@ -17,7 +17,7 @@ {% for member in old_members %} {{ user_profile_link(member.user) }} - {{ settings.SITH_CLUB_ROLES[member.role] }} + {{ member.role.name }} {{ member.description }} {{ member.start_date }} {{ member.end_date }} diff --git a/club/tests/test_club_controller.py b/club/tests/test_club_controller.py index e084109c..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, 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/views.py b/club/views.py index 6a9963a9..92de52d2 100644 --- a/club/views.py +++ b/club/views.py @@ -28,7 +28,6 @@ import csv import itertools from typing import TYPE_CHECKING, Any -from django.conf import settings from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied, ValidationError @@ -355,7 +354,7 @@ class ClubMembersView( membership = self.object.get_membership_for(self.request.user) if ( membership - and membership.role <= settings.SITH_MAXIMUM_FREE_ROLE + and not membership.role.is_board and not self.request.user.has_perm("club.add_membership") ): # Simple club members won't see the form anymore. @@ -380,8 +379,8 @@ class ClubMembersView( kwargs["members"] = list( self.object.members.ongoing() .annotate(is_editable=Q(id__in=editable)) - .order_by("-role") - .select_related("user") + .order_by("role__order") + .select_related("user", "role") ) kwargs["can_end_membership"] = len(editable) > 0 return kwargs @@ -409,8 +408,8 @@ class ClubOldMembersView(ClubTabsMixin, PermissionRequiredMixin, DetailView): return super().get_context_data(**kwargs) | { "old_members": ( self.object.members.exclude(end_date=None) - .order_by("-role", "description", "-end_date") - .select_related("user") + .order_by("role__order", "description", "-end_date") + .select_related("user", "role") ) } @@ -761,9 +760,7 @@ class MailingAutoGenerationView(View): def get(self, request, *args, **kwargs): club = self.mailing.club self.mailing.subscriptions.all().delete() - members = club.members.filter( - role__gte=settings.SITH_CLUB_ROLES_ID["Board member"] - ).exclude(end_date__lte=timezone.now()) + members = club.members.ongoing().filter(role__is_board=True) for member in members.all(): MailingSubscription(user=member.user, mailing=self.mailing).save() return redirect("club:mailing", club_id=club.id) diff --git a/core/templates/core/user_clubs.jinja b/core/templates/core/user_clubs.jinja index bd365239..afc6748e 100644 --- a/core/templates/core/user_clubs.jinja +++ b/core/templates/core/user_clubs.jinja @@ -23,10 +23,10 @@ - {% for m in profile.memberships.filter(end_date=None).all() %} + {% for m in profile.memberships.ongoing().select_related("role") %} {{ m.club }} - {{ settings.SITH_CLUB_ROLES[m.role] }} + {{ m.role.name }} {{ m.description }} {{ m.start_date }} {% if m.can_be_edited_by(user) %} @@ -65,10 +65,10 @@ - {% for m in profile.memberships.exclude(end_date=None).all() %} + {% for m in profile.memberships.exclude(end_date=None).select_related("role") %} {{ m.club }} - {{ settings.SITH_CLUB_ROLES[m.role] }} + {{ m.role.name }} {{ m.description }} {{ m.start_date }} {{ m.end_date }} diff --git a/counter/models.py b/counter/models.py index 0c7790c9..fbc208f3 100644 --- a/counter/models.py +++ b/counter/models.py @@ -600,7 +600,7 @@ class Counter(models.Model): if user.is_anonymous: return False mem = self.club.get_membership_for(user) - if mem and mem.role >= settings.SITH_CLUB_ROLES_ID["Treasurer"]: + if mem and mem.role.is_presidency: return True return user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) diff --git a/sith/settings.py b/sith/settings.py index 5f51a0db..82ef9643 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -560,30 +560,6 @@ SITH_SUBSCRIPTIONS = { # To be completed.... } -SITH_CLUB_ROLES_ID = { - "President": 10, - "Vice-President": 9, - "Treasurer": 7, - "Communication supervisor": 5, - "Secretary": 4, - "IT supervisor": 3, - "Board member": 2, - "Active member": 1, - "Curious": 0, -} - -SITH_CLUB_ROLES = { - 10: _("President"), - 9: _("Vice-President"), - 7: _("Treasurer"), - 5: _("Communication supervisor"), - 4: _("Secretary"), - 3: _("IT supervisor"), - 2: _("Board member"), - 1: _("Active member"), - 0: _("Curious"), -} - # This corresponds to the maximum role a user can freely subscribe to # In this case, SITH_MAXIMUM_FREE_ROLE=1 means that a user can # set himself as "Membre actif" or "Curieux", but not higher diff --git a/trombi/models.py b/trombi/models.py index 8771778b..8ce96447 100644 --- a/trombi/models.py +++ b/trombi/models.py @@ -23,7 +23,6 @@ from datetime import date -from django.conf import settings from django.core.exceptions import ValidationError from django.db import models from django.urls import reverse @@ -152,10 +151,12 @@ class TrombiUser(models.Model): def make_memberships(self): self.memberships.all().delete() - for m in self.user.memberships.filter( - role__gt=settings.SITH_MAXIMUM_FREE_ROLE - ).order_by("end_date"): - role = str(settings.SITH_CLUB_ROLES[m.role]) + for m in ( + self.user.memberships.filter(role__is_board=True) + .select_related("role") + .order_by("end_date") + ): + role = m.role.name if m.description: role += " (%s)" % m.description end_date = get_semester_code(m.end_date) if m.end_date else "" From aaaaeb204fb1f551b47143d253a255d610a46513 Mon Sep 17 00:00:00 2001 From: imperosol Date: Sat, 20 Dec 2025 09:58:26 +0100 Subject: [PATCH 06/11] remove settings.SITH_MAXIMUM_FREE_ROLE --- .../0012_club_board_group_club_members_group.py | 10 +++++----- club/migrations/0015_clubrole_alter_membership_role.py | 10 +++++----- sith/settings.py | 5 ----- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/club/migrations/0012_club_board_group_club_members_group.py b/club/migrations/0012_club_board_group_club_members_group.py index e436bcd4..a9ad8d3a 100644 --- a/club/migrations/0012_club_board_group_club_members_group.py +++ b/club/migrations/0012_club_board_group_club_members_group.py @@ -2,12 +2,15 @@ import django.db.models.deletion import django.db.models.functions.datetime -from django.conf import settings from django.db import migrations, models from django.db.migrations.state import StateApps from django.db.models import Q from django.utils.timezone import localdate +# Before the club role rework, the maximum free role +# was the hardcoded highest non-board role +MAXIMUM_FREE_ROLE = 1 + def migrate_meta_groups(apps: StateApps, schema_editor): """Attach the existing meta groups to the clubs. @@ -46,10 +49,7 @@ def migrate_meta_groups(apps: StateApps, schema_editor): ).select_related("user") club.members_group.users.set([m.user for m in memberships]) club.board_group.users.set( - [ - m.user - for m in memberships.filter(role__gt=settings.SITH_MAXIMUM_FREE_ROLE) - ] + [m.user for m in memberships.filter(role__gt=MAXIMUM_FREE_ROLE)] ) diff --git a/club/migrations/0015_clubrole_alter_membership_role.py b/club/migrations/0015_clubrole_alter_membership_role.py index 08431ea1..f776b20c 100644 --- a/club/migrations/0015_clubrole_alter_membership_role.py +++ b/club/migrations/0015_clubrole_alter_membership_role.py @@ -1,12 +1,12 @@ # Generated by Django 5.2.3 on 2025-06-21 21:59 import django.db.models.deletion -from django.conf import settings from django.db import migrations, models from django.db.migrations.state import StateApps from django.db.models import Case, When -PRESIDENT_ROLE = 10 +PRESIDENCY_ROLES = [10, 9] +MAXIMUM_FREE_ROLE = 1 SITH_CLUB_ROLES = { 10: "Président⸱e", 9: "Vice-Président⸱e", @@ -28,10 +28,10 @@ def migrate_roles(apps: StateApps, schema_editor): for club_id, role in Membership.objects.values_list("club", "role").distinct(): new_role = ClubRole.objects.create( name=SITH_CLUB_ROLES[role], - is_board=role > settings.SITH_MAXIMUM_FREE_ROLE, - is_presidency=role == PRESIDENT_ROLE, + is_board=role > MAXIMUM_FREE_ROLE, + is_presidency=role in PRESIDENCY_ROLES, club_id=club_id, - order=PRESIDENT_ROLE - role, + order=max(SITH_CLUB_ROLES) - role, ) updates.append(When(role=role, then=new_role.id)) # all updates must happen at the same time diff --git a/sith/settings.py b/sith/settings.py index 82ef9643..96eba926 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -560,11 +560,6 @@ SITH_SUBSCRIPTIONS = { # To be completed.... } -# This corresponds to the maximum role a user can freely subscribe to -# In this case, SITH_MAXIMUM_FREE_ROLE=1 means that a user can -# set himself as "Membre actif" or "Curieux", but not higher -SITH_MAXIMUM_FREE_ROLE = 1 - # Minutes to timeout the logged barmen SITH_BARMAN_TIMEOUT = 30 From c9caa3f32440076b99783b6d963ddb6dfa04365c Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 22 Mar 2026 11:21:20 +0100 Subject: [PATCH 07/11] add translations --- locale/fr/LC_MESSAGES/django.po | 87 +++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index 67925d29..66c3f83b 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -233,15 +233,58 @@ msgstr "home" msgid "You can not make loops in clubs" msgstr "Vous ne pouvez pas faire de boucles dans les clubs" +#: club/models.py com/models.py counter/models.py trombi/models.py +msgid "club" +msgstr "club" + +#: club/models.py +msgid "The club with which this role is associated" +msgstr "Le club auquel ce rôle est attaché." + +#: club/models.py core/models.py counter/models.py election/models.py +#: forum/models.py +msgid "description" +msgstr "description" + +#: club/models.py +msgid "Board role" +msgstr "Rôle du bureau" + +#: club/models.py +msgid "Presidency role" +msgstr "Rôle de la présidence" + +#: club/models.py +msgid "" +"If the role is inactive, people joining the club won't be able to get it." +msgstr "" +"Si ce rôle est inactif, il ne pourra pas être attribué aux gens qui rejoignent le club." + +#: club/models.py +msgid "club role" +msgstr "rôle de club" + +#: club/models.py +msgid "club roles" +msgstr "rôles de club" + +#: club/models.py +#, python-format +msgid "" +"Role %(name)s was declared as a presidency role without being a board role" +msgstr "" +"Le rôle %(name)s a été déclaré comme rôle de présidence sans être un rôle du bureau." + +#: club/models.py +#, python-format +msgid "Board role %(role)s cannot be placed below a member role" +msgstr "Le rôle du bureau %(role)s ne peut pas être placé en-dessous d'un rôle de membre." + #: club/models.py core/models.py counter/models.py eboutic/models.py #: election/models.py pedagogy/models.py sas/models.py trombi/models.py msgid "user" msgstr "utilisateur" -#: club/models.py com/models.py counter/models.py trombi/models.py -msgid "club" -msgstr "club" - #: club/models.py counter/models.py election/models.py msgid "start date" msgstr "date de début" @@ -5376,40 +5419,8 @@ msgid "Alternating cursus (-20%)" msgstr "Cursus alternant (-20%)" #: sith/settings.py -msgid "President" -msgstr "Président⸱e" - -#: sith/settings.py -msgid "Vice-President" -msgstr "Vice-Président⸱e" - -#: sith/settings.py -msgid "Treasurer" -msgstr "Trésorier⸱e" - -#: sith/settings.py -msgid "Communication supervisor" -msgstr "Responsable communication" - -#: sith/settings.py -msgid "Secretary" -msgstr "Secrétaire" - -#: sith/settings.py -msgid "IT supervisor" -msgstr "Responsable info" - -#: sith/settings.py -msgid "Board member" -msgstr "Membre du bureau" - -#: sith/settings.py -msgid "Active member" -msgstr "Membre actif⸱ve" - -#: sith/settings.py -msgid "Curious" -msgstr "Curieux⸱euse" +msgid "One year for free(CA offer)" +msgstr "Une année offerte (Offre CA)" #: sith/settings.py msgid "A new poster needs to be moderated" From 44e8ab4fb9d61af72c8e02140da9dccc0a8219d6 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 31 Mar 2026 16:58:26 +0200 Subject: [PATCH 08/11] put roles at the right place when they are created --- club/models.py | 17 +++++++++++++++++ club/tests/test_clubrole.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 club/tests/test_clubrole.py diff --git a/club/models.py b/club/models.py index 58d9c139..0f536b58 100644 --- a/club/models.py +++ b/club/models.py @@ -278,6 +278,7 @@ class ClubRole(OrderedModel): ) if ( self.is_board + and self.order and self.club.roles.filter(is_board=False, order__lt=self.order).exists() ): errors.append( @@ -290,6 +291,22 @@ class ClubRole(OrderedModel): raise ValidationError(errors) return super().clean() + def save(self, *args, **kwargs): + auto_order = self.order is None and self.is_board + if not auto_order: + super().save(*args, **kwargs) + return + # get the role that should be placed after the role we are dealing with. + # So, if this is role is presidency, get the first board role ; + # if it is a board role, get the first member role ; + # and if it is a member role, get nothing (OrderedModel.save will + # automatically put it in the last position anyway) + filters = {"is_board": self.is_presidency, "is_presidency": False} + next_role = self.club.roles.filter(**filters).order_by("order").first() + super().save(*args, **kwargs) + if next_role: + self.above(next_role) + class MembershipQuerySet(models.QuerySet): def ongoing(self) -> Self: diff --git a/club/tests/test_clubrole.py b/club/tests/test_clubrole.py new file mode 100644 index 00000000..54067862 --- /dev/null +++ b/club/tests/test_clubrole.py @@ -0,0 +1,33 @@ +import pytest +from model_bakery import baker, seq +from model_bakery.recipe import Recipe + +from club.models import Club, ClubRole + + +@pytest.mark.django_db +def test_order_auto(): + """Test that newly created roles are put in the right place.""" + club = baker.make(Club) + recipe = Recipe(ClubRole, club=club, name=seq("role ")) + # bulk create initial roles + roles = 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) + # each new role should be placed at the end of its category + 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) + assert list(club.roles.order_by("order")) == [ + roles[0], + role_a, + roles[1], + role_b, + roles[2], + role_c, + ] From a40c43204affe16e328cbcdb3a3a6e54731d11e1 Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 7 Apr 2026 11:03:49 +0200 Subject: [PATCH 09/11] 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 5ef27003..76687103 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:] From 8876c64f549e258ff9a5f84c3f5f5b2f0295069b Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 10 Apr 2026 18:53:29 +0200 Subject: [PATCH 10/11] add forgotten check --- club/models.py | 17 +++++++-- club/tests/test_clubrole.py | 2 +- locale/fr/LC_MESSAGES/django.po | 64 +++++++++++++-------------------- 3 files changed, 40 insertions(+), 43 deletions(-) diff --git a/club/models.py b/club/models.py index 0f536b58..5e072853 100644 --- a/club/models.py +++ b/club/models.py @@ -245,7 +245,6 @@ class ClubRole(OrderedModel): class Meta(OrderedModel.Meta): verbose_name = _("club role") verbose_name_plural = _("club roles") - abstract = False constraints = [ # presidency IMPLIES board <=> NOT presidency OR board # cf. MT1 :) @@ -276,14 +275,26 @@ class ClubRole(OrderedModel): % {"name": self.name} ) ) + roles = list(self.club.roles.all()) if ( self.is_board and self.order - and self.club.roles.filter(is_board=False, order__lt=self.order).exists() + and any(r.order < self.order and not r.is_board for r in roles) ): errors.append( ValidationError( - _("Board role %(role)s cannot be placed below a member role") + _("Role %(role)s cannot be placed below a member role") + % {"role": self.name} + ) + ) + if ( + self.is_presidency + and self.order + and any(r.order < self.order and not r.is_presidency for r in roles) + ): + errors.append( + ValidationError( + _("Role %(role)s cannot be placed below a non-presidency role") % {"role": self.name} ) ) diff --git a/club/tests/test_clubrole.py b/club/tests/test_clubrole.py index 54067862..6956ac8c 100644 --- a/club/tests/test_clubrole.py +++ b/club/tests/test_clubrole.py @@ -10,7 +10,7 @@ def test_order_auto(): """Test that newly created roles are put in the right place.""" club = baker.make(Club) recipe = Recipe(ClubRole, club=club, name=seq("role ")) - # bulk create initial roles + # bulk create initial roles (1 presidency, 1 board, 1 member) roles = recipe.make( is_board=iter([True, True, False]), is_presidency=iter([True, False, False]), diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index 66c3f83b..7791ae3f 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2026-05-10 20:27+0200\n" +"POT-Creation-Date: 2026-05-12 09:48+0200\n" "PO-Revision-Date: 2016-07-18\n" "Last-Translator: Maréchal \n" @@ -258,7 +258,8 @@ msgstr "Rôle de la présidence" msgid "" "If the role is inactive, people joining the club won't be able to get it." msgstr "" -"Si ce rôle est inactif, il ne pourra pas être attribué aux gens qui rejoignent le club." +"Si ce rôle est inactif, il ne pourra pas être attribué aux gens qui " +"rejoignent le club." #: club/models.py msgid "club role" @@ -273,12 +274,20 @@ msgstr "rôles de club" msgid "" "Role %(name)s was declared as a presidency role without being a board role" msgstr "" -"Le rôle %(name)s a été déclaré comme rôle de présidence sans être un rôle du bureau." +"Le rôle %(name)s a été déclaré comme rôle de présidence sans être un rôle du " +"bureau." #: club/models.py #, python-format -msgid "Board role %(role)s cannot be placed below a member role" -msgstr "Le rôle du bureau %(role)s ne peut pas être placé en-dessous d'un rôle de membre." +msgid "Role %(role)s cannot be placed below a member role" +msgstr "" +"Le rôle %(role)s ne peut pas être placé en-dessous d'un rôle de membre." + +#: club/models.py +#, python-format +msgid "Role %(role)s cannot be placed below a non-presidency role" +msgstr "" +"Le rôle %(role)s ne peut pas être placé en-dessous d'un rôle de membre." #: club/models.py core/models.py counter/models.py eboutic/models.py #: election/models.py pedagogy/models.py sas/models.py trombi/models.py @@ -297,10 +306,9 @@ msgstr "date de fin" msgid "role" msgstr "rôle" -#: club/models.py core/models.py counter/models.py election/models.py -#: forum/models.py -msgid "description" -msgstr "description" +#: club/models.py +msgid "past member" +msgstr "ancien membre" #: club/models.py com/templates/com/mailing_admin.jinja #: com/templates/com/news_admin_list.jinja com/templates/com/weekmail.jinja @@ -2932,8 +2940,8 @@ msgstr "Nom d'utilisateur, email, ou numéro de compte AE" #: core/views/forms.py msgid "" -"Profile: you need to be visible on the picture, in order to be recognized (e." -"g. by the barmen)" +"Profile: you need to be visible on the picture, in order to be recognized " +"(e.g. by the barmen)" msgstr "" "Photo de profil: vous devez être visible sur la photo afin d'être reconnu " "(par exemple par les barmen)" @@ -3733,10 +3741,10 @@ msgid "" "orders a sandwich and a soft drink, the formula will be applied and the " "basket will then contain a sandwich formula instead." msgstr "" -"Par exemple s'il existe une formule associant un produit « Formule sandwich " -"» aux produits « Sandwich » et « Soft », alors, si une personne commande un " -"sandwich et un soft, la formule sera appliquée et le panier contiendra alors " -"une formule sandwich à la place." +"Par exemple s'il existe une formule associant un produit « Formule " +"sandwich » aux produits « Sandwich » et « Soft », alors, si une personne " +"commande un sandwich et un soft, la formule sera appliquée et le panier " +"contiendra alors une formule sandwich à la place." #: counter/templates/counter/formula_list.jinja msgid "New formula" @@ -3798,8 +3806,8 @@ msgstr "" #: counter/templates/counter/mails/account_dump.jinja msgid "If you think this was a mistake, please mail us at ae@utbm.fr." msgstr "" -"Si vous pensez qu'il s'agit d'une erreur, veuillez envoyer un mail à ae@utbm." -"fr." +"Si vous pensez qu'il s'agit d'une erreur, veuillez envoyer un mail à " +"ae@utbm.fr." #: counter/templates/counter/mails/account_dump.jinja msgid "" @@ -5418,10 +5426,6 @@ msgstr "Cursus branche (-20%)" msgid "Alternating cursus (-20%)" msgstr "Cursus alternant (-20%)" -#: sith/settings.py -msgid "One year for free(CA offer)" -msgstr "Une année offerte (Offre CA)" - #: sith/settings.py msgid "A new poster needs to be moderated" msgstr "Une nouvelle affiche a besoin d'être modérée" @@ -5899,21 +5903,3 @@ msgstr "Vous ne pouvez plus écrire de commentaires, la date est passée." #, python-format msgid "Maximum characters: %(max_length)s" msgstr "Nombre de caractères max: %(max_length)s" - -#~ msgid "past member" -#~ msgstr "ancien membre" - -#~ msgid "One semester Welcome Week" -#~ msgstr "Un semestre Welcome Week" - -#~ msgid "Eurok's volunteer" -#~ msgstr "Bénévole Eurockéennes" - -#~ msgid "Six weeks for free" -#~ msgstr "6 semaines gratuites" - -#~ msgid "GA staff member" -#~ msgstr "Membre staff GA" - -#~ msgid "One year for free(CA offer)" -#~ msgstr "Une année offerte (Offre CA)" From 54d261142dc49de24800330b7cf476a11cc180aa Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 19 Apr 2026 14:43:37 +0200 Subject: [PATCH 11/11] create default club roles on club creation --- club/admin.py | 13 ++++ .../0015_clubrole_alter_membership_role.py | 20 +++++ club/models.py | 40 +++++++++- club/tests/test_club.py | 78 +++++++++++++++++++ club/views.py | 5 ++ 5 files changed, 153 insertions(+), 3 deletions(-) diff --git a/club/admin.py b/club/admin.py index bff21208..08883913 100644 --- a/club/admin.py +++ b/club/admin.py @@ -13,6 +13,8 @@ # # from django.contrib import admin +from django.forms.models import ModelForm +from django.http import HttpRequest from club.models import Club, ClubRole, Membership @@ -29,6 +31,17 @@ class ClubAdmin(admin.ModelAdmin): "page", ) + def save_model( + self, + request: HttpRequest, + obj: Club, + form: ModelForm, + change: bool, # noqa: FBT001 + ): + super().save_model(request, obj, form, change) + if not change: + obj.create_default_roles() + @admin.register(ClubRole) class ClubRoleAdmin(admin.ModelAdmin): diff --git a/club/migrations/0015_clubrole_alter_membership_role.py b/club/migrations/0015_clubrole_alter_membership_role.py index f776b20c..6a1b58ee 100644 --- a/club/migrations/0015_clubrole_alter_membership_role.py +++ b/club/migrations/0015_clubrole_alter_membership_role.py @@ -121,6 +121,26 @@ class Migration(migrations.Migration): "verbose_name_plural": "club roles", }, ), + migrations.AlterField( + model_name="club", + name="board_group", + field=models.OneToOneField( + editable=False, + on_delete=django.db.models.deletion.PROTECT, + related_name="club_board", + to="core.group", + ), + ), + migrations.AlterField( + model_name="club", + name="members_group", + field=models.OneToOneField( + editable=False, + on_delete=django.db.models.deletion.PROTECT, + related_name="club", + to="core.group", + ), + ), migrations.AddConstraint( model_name="clubrole", constraint=models.CheckConstraint( diff --git a/club/models.py b/club/models.py index 5e072853..f8698d6b 100644 --- a/club/models.py +++ b/club/models.py @@ -28,7 +28,7 @@ from typing import Iterable, Self from django.conf import settings from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.core.validators import RegexValidator, validate_email -from django.db import models, transaction +from django.db import ProgrammingError, models, transaction from django.db.models import Exists, F, OuterRef, Q from django.urls import reverse from django.utils import timezone @@ -92,10 +92,10 @@ class Club(models.Model): Page, related_name="club", blank=True, on_delete=models.PROTECT ) members_group = models.OneToOneField( - Group, related_name="club", on_delete=models.PROTECT + Group, related_name="club", on_delete=models.PROTECT, editable=False ) board_group = models.OneToOneField( - Group, related_name="club_board", on_delete=models.PROTECT + Group, related_name="club_board", on_delete=models.PROTECT, editable=False ) objects = ClubQuerySet.as_manager() @@ -183,6 +183,40 @@ class Club(models.Model): self.page.parent = self.parent.page self.page.save(force_lock=True) + def create_default_roles(self): + """Create some roles that should exist by default for this club. + + The created roles are : president, treasurer, active member and curious. + + Warnings: + When calling this method, no club must exist yet for this club. + """ + if self.roles.exists(): + raise ProgrammingError( + "Default roles can be created only for clubs " + "that don't have associated roles yet" + ) + # The names are written in French, because there is no gettext involved + # for strings stored in database, and the majority of users are french. + roles = [ + ClubRole(name="Président⸱e", is_board=True, is_presidency=True), + ClubRole(name="Trésorier⸱e", is_board=True, is_presidency=False), + ClubRole(name="Membre actif⸱ve", is_board=False, is_presidency=False), + ClubRole( + name="Curieux⸱euse", + description=( + "Les gens qui suivent l'activité " + "du club sans forcément y participer" + ), + is_board=False, + is_presidency=False, + ), + ] + for i, role in enumerate(roles): + role.club = self + role.order = i + ClubRole.objects.bulk_create(roles) + def delete(self, *args, **kwargs) -> tuple[int, dict[str, int]]: self.board_group.delete() self.members_group.delete() diff --git a/club/tests/test_club.py b/club/tests/test_club.py index 0e25995e..e64b4ce2 100644 --- a/club/tests/test_club.py +++ b/club/tests/test_club.py @@ -1,11 +1,14 @@ from datetime import timedelta import pytest +from django.conf import settings +from django.db import ProgrammingError from django.test import Client from django.urls import reverse from django.utils.timezone import localdate from model_bakery import baker from model_bakery.recipe import Recipe +from pytest_django.asserts import assertRedirects from club.models import Club, ClubRole, Membership from core.baker_recipes import subscriber_user @@ -47,3 +50,78 @@ def test_club_list(client: Client, nb_additional_clubs: int, is_fragment): headers = {"HX-Request": True} if is_fragment else {} res = client.get(reverse("club:club_list"), headers=headers) assert res.status_code == 200 + + +def assert_club_created(club_name: str): + club = Club.objects.last() + assert club.name == club_name + assert club.board_group.name == f"{club_name} - Bureau" + assert club.members_group.name == f"{club_name} - Membres" + # default roles should be added on club creation, + # whether the creation happens on the admin site or on the user site + assert list(club.roles.values("name", "is_presidency", "is_board")) == [ + {"name": "Président⸱e", "is_presidency": True, "is_board": True}, + {"name": "Trésorier⸱e", "is_presidency": False, "is_board": True}, + {"name": "Membre actif⸱ve", "is_presidency": False, "is_board": False}, + {"name": "Curieux⸱euse", "is_presidency": False, "is_board": False}, + ] + + +@pytest.mark.django_db +def test_create_view(admin_client: Client): + """Test that the club creation view works well""" + res = admin_client.get(reverse("club:club_new")) + assert res.status_code == 200 + res = admin_client.post( + reverse("club:club_new"), + data={"name": "foo", "parent": settings.SITH_MAIN_CLUB_ID}, + ) + club = Club.objects.last() + assertRedirects(res, club.get_absolute_url()) + assert_club_created("foo") + + +@pytest.mark.django_db +def test_default_roles_for_club_with_roles_fails(): + """Test that an Error is raised if trying to create + default roles for a club that already has roles. + """ + club = baker.make(Club) + baker.make(ClubRole, club=club) + with pytest.raises(ProgrammingError): + club.create_default_roles() + + +@pytest.mark.django_db +class TestAdminInterface: + def test_create(self, admin_client: Client): + """Test the creation of a club via the admin interface.""" + res = admin_client.post( + reverse("admin:club_club_add"), + data={ + "name": "foo", + "parent": settings.SITH_MAIN_CLUB_ID, + "address": "Rome", + }, + ) + assertRedirects(res, reverse("admin:club_club_changelist")) + assert_club_created("foo") + + def test_change(self, admin_client: Client): + """Test the edition of a club via the admin interface.""" + club = baker.make(Club) + res = admin_client.post( + reverse("admin:club_club_change", kwargs={"object_id": club.id}), + data={ + "name": "foo", + "page": club.page_id, + "home": club.home_id, + "address": club.address, + }, + ) + assertRedirects(res, reverse("admin:club_club_changelist")) + club.refresh_from_db() + assert club.name == "foo" + # Club roles shouldn't be modified when editing the club on the admin interface + # This club had no roles beforehand, therefore it shouldn't have roles now. + assert not club.roles.exists() diff --git a/club/views.py b/club/views.py index 92de52d2..cce60e8c 100644 --- a/club/views.py +++ b/club/views.py @@ -580,6 +580,11 @@ class ClubCreateView(PermissionRequiredMixin, CreateView): template_name = "core/create.jinja" permission_required = "club.add_club" + def form_valid(self, form): + res = super().form_valid(form) + self.object.create_default_roles() + return res + class MembershipSetOldView(CanEditMixin, SingleObjectMixin, View): """Set a membership as being old."""