adapt club members pages to new club roles framework

club/forms.py

@@ -23,13 +23,12 @@
 #
 
 from django import forms
-from django.conf import settings
+from django.db.models import Exists, OuterRef, Q, QuerySet
-from django.db.models import Exists, OuterRef, Q
 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 = (
+        self.fields["members_old"].queryset = club.members.ongoing().editable_by(user)
-            Membership.objects.ongoing().filter(club=club).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"].queryset = self.available_roles
-        self.fields["role"].choices = [
-            (value, name)
-            for value, name in settings.SITH_CLUB_ROLES.items()
-            if value <= self.max_available_role
-        ]
         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:
+        if membership is None or not membership.role.is_board:
-            return -1
+            return ClubRole.objects.none()
-        if membership.role == settings.SITH_CLUB_ROLES_ID["President"]:
+        if membership.role.is_presidency:
-            return membership.role
+            return self.club.roles.all()
-        return membership.role - 1
+        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):
+    def available_roles(self):
-        return settings.SITH_MAXIMUM_FREE_ROLE
+        return self.club.roles.filter(is_board=False)
 
     def clean(self):
         """Check that the user is subscribed and isn't already in the club."""

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

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(
+        return self.members.filter(end_date=None).order_by("role__order").first()
-            role=settings.SITH_CLUB_ROLES_ID["President"], end_date=None
-        ).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(
+            errors.append(
+                ValidationError(
                     _(
                         "Role %(name)s was declared as a presidency role "
                         "without being a board role"
                     )
                     % {"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"),
                 )
             )
         )

club/templates/club/club_members.jinja

@@ -45,7 +45,7 @@
           {% for m in members %}
             <tr>
               <td>{{ user_profile_link(m.user) }}</td>
-              <td>{{ settings.SITH_CLUB_ROLES[m.role] }}</td>
+              <td>{{ m.role.name }}</td>
               <td>{{ m.description }}</td>
               <td>{{ m.start_date }}</td>
               {%- if can_end_membership -%}

club/templates/club/club_old_members.jinja

@@ -17,7 +17,7 @@
       {% for member in old_members %}
         <tr>
           <td>{{ user_profile_link(member.user) }}</td>
-          <td>{{ settings.SITH_CLUB_ROLES[member.role] }}</td>
+          <td>{{ member.role.name }}</td>
           <td>{{ member.description }}</td>
           <td>{{ member.start_date }}</td>
           <td>{{ member.end_date }}</td>

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
 

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")
+            .order_by("role__order")
-            .select_related("user")
+            .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")
+                .order_by("role__order", "description", "-end_date")
-                .select_related("user")
+                .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(
+        members = club.members.ongoing().filter(role__is_board=True)
-            role__gte=settings.SITH_CLUB_ROLES_ID["Board member"]
-        ).exclude(end_date__lte=timezone.now())
         for member in members.all():
             MailingSubscription(user=member.user, mailing=self.mailing).save()
         return redirect("club:mailing", club_id=club.id)

core/templates/core/user_clubs.jinja

@@ -23,10 +23,10 @@
       </tr>
     </thead>
     <tbody>
-      {% for m in profile.memberships.filter(end_date=None).all() %}
+      {% for m in profile.memberships.ongoing().select_related("role") %}
         <tr>
           <td><a href="{{ url('club:club_members', club_id=m.club.id) }}">{{ m.club }}</a></td>
-          <td>{{ settings.SITH_CLUB_ROLES[m.role] }}</td>
+          <td>{{ m.role.name }}</td>
           <td>{{ m.description }}</td>
           <td>{{ m.start_date }}</td>
           {% if m.can_be_edited_by(user) %}
@@ -65,10 +65,10 @@
       </tr>
     </thead>
     <tbody>
-      {% for m in profile.memberships.exclude(end_date=None).all() %}
+      {% for m in profile.memberships.ongoing().select_related("role") %}
         <tr>
           <td><a href="{{ url('club:club_members', club_id=m.club.id) }}">{{ m.club }}</a></td>
-          <td>{{ settings.SITH_CLUB_ROLES[m.role] }}</td>
+          <td>{{ m.role.name }}</td>
           <td>{{ m.description }}</td>
           <td>{{ m.start_date }}</td>
           <td>{{ m.end_date }}</td>

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)
 

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

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(
+        for m in (
-            role__gt=settings.SITH_MAXIMUM_FREE_ROLE
+            self.user.memberships.filter(role__is_board=True)
-        ).order_by("end_date"):
+            .select_related("role")
-            role = str(settings.SITH_CLUB_ROLES[m.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 ""