From b2b8d24003f69fe82a2029cd37a287fe8be2f312 Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 28 Sep 2025 12:37:52 +0200 Subject: [PATCH] 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 dcd270e7..a18851a0 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 083b9760..89eadd7f 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 edd86d7e..1e75bbf1 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 3aa43d56..18c14e84 100644 --- a/club/templates/club/club_members.jinja +++ b/club/templates/club/club_members.jinja @@ -45,7 +45,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 9077d0d7..46b601f8 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 @@ -318,7 +317,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. @@ -343,8 +342,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 @@ -372,8 +371,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") ) } @@ -724,9 +723,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..c821b362 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.ongoing().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 29ecad2f..e51636e1 100644 --- a/counter/models.py +++ b/counter/models.py @@ -583,7 +583,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 66193c4f..6f7c02fc 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -574,30 +574,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 ""