diff --git a/club/forms.py b/club/forms.py index 5d6a3b10..27b5054c 100644 --- a/club/forms.py +++ b/club/forms.py @@ -26,12 +26,16 @@ from django import forms from django.conf import settings 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 core.models import User -from core.views.forms import SelectDate, SelectDateTime -from core.views.widgets.ajax_select import AutoCompleteSelectMultipleUser +from core.views.forms import SelectDateTime +from core.views.widgets.ajax_select import ( + AutoCompleteSelectMultipleUser, + AutoCompleteSelectUser, +) from counter.models import Counter, Selling @@ -188,105 +192,113 @@ class SellingsForm(forms.Form): ) -class ClubMemberForm(forms.Form): - """Form handling the members of a club.""" +class ClubOldMemberForm(forms.Form): + members_old = forms.ModelMultipleChoiceField( + Membership.objects.none(), + label=_("Mark as old"), + widget=forms.CheckboxSelectMultiple, + required=False, + ) + + 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) + ) + + +class ClubMemberForm(forms.ModelForm): + """Form to add a member to the club, as a board member.""" error_css_class = "error" required_css_class = "required" - users = forms.ModelMultipleChoiceField( - label=_("Users to add"), - help_text=_("Search users to add (one or more)."), - required=False, - widget=AutoCompleteSelectMultipleUser, - queryset=User.objects.all(), - ) + class Meta: + model = Membership + fields = ["role", "description"] - def __init__(self, *args, **kwargs): - self.club = kwargs.pop("club") - self.request_user = kwargs.pop("request_user") - self.club_members = kwargs.pop("club_members", None) - if not self.club_members: - self.club_members = self.club.members.ongoing().order_by("-role").all() + def __init__(self, *args, club: Club, request_user: User, **kwargs): + self.club = club + 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.instance.club = club - # Using a ModelForm binds too much the form with the model and we don't want that - # We want the view to process the model creation since they are multiple users - # We also want the form to handle bulk deletion - self.fields.update( - forms.fields_for_model( - Membership, - fields=("role", "start_date", "description"), - widgets={"start_date": SelectDate}, - ) - ) + @property + def max_available_role(self): + """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 - # Role is required only if users is specified - self.fields["role"].required = False - # Start date and description are never really required - self.fields["start_date"].required = False - self.fields["description"].required = False +class ClubAddMemberForm(ClubMemberForm): + """Form to add a member to the club, as a board member.""" - self.fields["users_old"] = forms.ModelMultipleChoiceField( - User.objects.filter( - id__in=[ - ms.user.id - for ms in self.club_members - if ms.can_be_edited_by(self.request_user) - ] - ).all(), - label=_("Mark as old"), - required=False, - widget=forms.CheckboxSelectMultiple, - ) - if not self.request_user.is_root: - self.fields.pop("start_date") + class Meta(ClubMemberForm.Meta): + fields = ["user", *ClubMemberForm.Meta.fields] + widgets = {"user": AutoCompleteSelectUser} - def clean_users(self): - """Check that the user is not trying to add an user already in the club. + @cached_property + def max_available_role(self): + """The greatest role 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 + """ + if self.request_user.has_perm("club.add_subscription"): + return settings.SITH_CLUB_ROLES_ID["President"] + 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 + + def clean_user(self): + """Check that the user is not trying to add a user already in the club. Also check that the user is valid and has a valid subscription. """ - cleaned_data = super().clean() - users = [] - for user in cleaned_data["users"]: - if not user.is_subscribed: - raise forms.ValidationError( - _("User must be subscriber to take part to a club"), code="invalid" - ) - if self.club.get_membership_for(user): - raise forms.ValidationError( - _("You can not add the same user twice"), code="invalid" - ) - users.append(user) - return users + user = self.cleaned_data["user"] + if not user.is_subscribed: + raise forms.ValidationError( + _("User must be subscriber to take part to a club"), code="invalid" + ) + if self.club.get_membership_for(user): + raise forms.ValidationError( + _("You can not add the same user twice"), code="invalid" + ) + return user + + +class JoinClubForm(ClubMemberForm): + """Form to join a club.""" + + 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 clean(self): - """Check user rights for adding an user.""" - cleaned_data = super().clean() - - if "start_date" in cleaned_data and not cleaned_data["start_date"]: - # Drop start_date if allowed to edition but not specified - cleaned_data.pop("start_date") - - if not cleaned_data.get("users"): - # No user to add equals no check needed - return cleaned_data - - if cleaned_data.get("role", "") == "": - # Role is required if users exists - self.add_error("role", _("You should specify a role")) - return cleaned_data - - request_user = self.request_user - membership = self.request_user_membership - if not ( - cleaned_data["role"] <= settings.SITH_MAXIMUM_FREE_ROLE - or (membership is not None and membership.role >= cleaned_data["role"]) - or request_user.is_board_member - or request_user.is_root - ): - raise forms.ValidationError(_("You do not have the permission to do that")) - return cleaned_data + """Check that the user is subscribed and isn't already in the club.""" + if not self.request_user.is_subscribed: + raise forms.ValidationError( + _("You must be subscribed to join a club"), code="invalid" + ) + if self.club.get_membership_for(self.request_user): + raise forms.ValidationError( + _("You are already a member of this club"), code="invalid" + ) + return super().clean() diff --git a/club/models.py b/club/models.py index 800f67c2..3c0f720f 100644 --- a/club/models.py +++ b/club/models.py @@ -30,7 +30,8 @@ from django.core.cache import cache 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 +from django.db.models import Exists, F, OuterRef, Q, Value +from django.db.models.functions import Greatest from django.urls import reverse from django.utils import timezone from django.utils.functional import cached_property @@ -209,10 +210,6 @@ class Club(models.Model): """Method to see if that object can be edited by the given user.""" return self.has_rights_in_club(user) - def can_be_viewed_by(self, user: User) -> bool: - """Method to see if that object can be seen by the given user.""" - return user.was_subscribed - def get_membership_for(self, user: User) -> Membership | None: """Return the current membership the given user. @@ -252,6 +249,44 @@ class MembershipQuerySet(models.QuerySet): """ return self.filter(role__gt=settings.SITH_MAXIMUM_FREE_ROLE) + def editable_by(self, user: User) -> Self: + """Filter Memberships that this user can edit. + + Users with the `club.change_membership` permission can edit all Membership. + The other users can edit : + - their own membership + - if they are board members, ongoing memberships with a role lower than their own + + For example, let's suppose the following users : + - A : board member + - B : board member + - C : simple member + - D : curious + - E : old member + + A will be able to edit the memberships of A, C and D ; + C and D will be able to edit only their own membership ; + nobody will be able to edit E's membership. + """ + if user.has_perm("club.change_membership"): + return self.all() + return self.filter( + Q(user=user) + | Exists( + Membership.objects.filter( + Q( + role__gt=Greatest( + OuterRef("role"), Value(settings.SITH_MAXIMUM_FREE_ROLE) + ) + ), + user=user, + end_date=None, + club=OuterRef("club"), + ) + ), + end_date=None, + ) + def update(self, **kwargs) -> int: """Refresh the cache and edit group ownership. @@ -328,16 +363,12 @@ class Membership(models.Model): User, verbose_name=_("user"), related_name="memberships", - null=False, - blank=False, on_delete=models.CASCADE, ) club = models.ForeignKey( Club, verbose_name=_("club"), related_name="members", - null=False, - blank=False, on_delete=models.CASCADE, ) start_date = models.DateField(_("start date"), default=timezone.now) diff --git a/club/static/club/members.scss b/club/static/club/members.scss new file mode 100644 index 00000000..9f7c8b39 --- /dev/null +++ b/club/static/club/members.scss @@ -0,0 +1,24 @@ +#club_members_table { + tbody label { + margin: 0; + padding: 0; + } +} + +#add_club_members_form { + fieldset { + display: flex; + flex-direction: row; + column-gap: 2em; + row-gap: 1em; + flex-wrap: wrap; + + @media (max-width: 1100px) { + justify-content: space-evenly; + } + + .errorlist { + max-width: 300px; + } + } +} \ No newline at end of file diff --git a/club/templates/club/club_members.jinja b/club/templates/club/club_members.jinja index 0778b486..3aa43d56 100644 --- a/club/templates/club/club_members.jinja +++ b/club/templates/club/club_members.jinja @@ -1,15 +1,33 @@ {% extends "core/base.jinja" %} {% from 'core/macros.jinja' import user_profile_link, select_all_checkbox %} +{% block additional_js %} + +{% endblock %} +{% block additional_css %} + + +{% endblock %} + {% block content %} + {% block notifications %} + {# Notifications are moved a little bit below #} + {% endblock %} +

{% trans %}Club members{% endtrans %}

+ + {% if add_member_fragment %} +
+ {{ add_member_fragment }} +
+ {% endif %} + {% include "core/base/notifications.jinja" %} {% if members %} -
+ {% csrf_token %} - {% set users_old = dict(form.users_old | groupby("choice_label")) %} - {% if users_old %} - {{ select_all_checkbox("users_old") }} -

+ {% if can_end_membership %} + {{ select_all_checkbox("members_old") }} +
{% endif %} @@ -18,7 +36,7 @@ - {% if users_old %} + {% if can_end_membership %} {% endif %} @@ -30,20 +48,24 @@ - {% if users_old %} + {%- if can_end_membership -%} - {% endif %} + {%- endif -%} {% endfor %}
{% trans %}Role{% endtrans %} {% trans %}Description{% endtrans %} {% trans %}Since{% endtrans %}{% trans %}Mark as old{% endtrans %}
{{ settings.SITH_CLUB_ROLES[m.role] }} {{ m.description }} {{ m.start_date }} - {% set user_old = users_old[m.user.get_display_name()] %} - {% if user_old %} - {{ user_old[0].tag() }} - {% endif %} + {%- if m.is_editable -%} + + + {%- endif -%}
- {{ form.users_old.errors }} - {% if users_old %} + {% if can_end_membership %}

{% endif %} @@ -51,32 +73,4 @@ {% else %}

{% trans %}There are no members in this club.{% endtrans %}

{% endif %} - - {% csrf_token %} - {{ form.non_field_errors() }} -

- {{ form.users.errors }} - - {{ form.users }} - {{ form.users.help_text }} -

-

- {{ form.role.errors }} - - {{ form.role }} -

- {% if form.start_date %} -

- {{ form.start_date.errors }} - - {{ form.start_date }} -

- {% endif %} -

- {{ form.description.errors }} - - {{ form.description }} -

-

-
{% endblock %} diff --git a/club/templates/club/club_old_members.jinja b/club/templates/club/club_old_members.jinja index 887da079..75603f9e 100644 --- a/club/templates/club/club_old_members.jinja +++ b/club/templates/club/club_old_members.jinja @@ -5,20 +5,22 @@

{% trans %}Club old members{% endtrans %}

- - - - - + + + + + + + - {% for m in club.members.exclude(end_date=None).order_by('-role', 'description', '-end_date').all() %} + {% for member in old_members %} - - - - - + + + + + {% endfor %} diff --git a/club/templates/club/fragments/add_member.jinja b/club/templates/club/fragments/add_member.jinja new file mode 100644 index 00000000..8efd878d --- /dev/null +++ b/club/templates/club/fragments/add_member.jinja @@ -0,0 +1,46 @@ +
+ {% if form.user %} +

{% trans %}Add a new member{% endtrans %}

+ {% else %} +

{% trans %}Join club{% endtrans %}

+ {% endif %} + +
+ {% csrf_token %} + {{ form.non_field_errors() }} +
+ {% if form.user %} +
+ {{ form.user.label_tag() }} + {{ form.user.help_text }} + {{ form.user }} + {{ form.user.errors }} +
+ {% endif %} +
+ {{ form.role.label_tag() }} + {{ form.role }} + {{ form.role.errors }} +
+
+ {{ form.description.label_tag() }} + {{ form.description }} + {{ form.description.errors }} +
+
+ + +
diff --git a/club/tests/base.py b/club/tests/base.py index 8ae8f3f4..ca4fc6cf 100644 --- a/club/tests/base.py +++ b/club/tests/base.py @@ -43,6 +43,9 @@ class TestClub(TestCase): cls.ae = Club.objects.get(pk=settings.SITH_MAIN_CLUB_ID) cls.club = baker.make(Club) + cls.new_members_url = reverse( + "club:club_new_members", kwargs={"club_id": cls.club.id} + ) cls.members_url = reverse("club:club_members", kwargs={"club_id": cls.club.id}) a_month_ago = now() - timedelta(days=30) yesterday = now() - timedelta(days=1) diff --git a/club/tests/test_membership.py b/club/tests/test_membership.py index ff668498..a3c0be50 100644 --- a/club/tests/test_membership.py +++ b/club/tests/test_membership.py @@ -1,13 +1,20 @@ +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 +from django.test import TestCase from django.urls import reverse from django.utils.timezone import localdate, localtime, now from model_bakery import baker +from pytest_django.asserts import assertRedirects -from club.forms import ClubMemberForm -from club.models import Membership +from club.forms import ClubAddMemberForm, JoinClubForm +from club.models import Club, Membership from club.tests.base import TestClub from core.baker_recipes import subscriber_user from core.models import AnonymousUser, User @@ -137,6 +144,38 @@ class TestMembershipQuerySet(TestClub): assert set(user.groups.all()).isdisjoint(club_groups) +class TestMembershipEditableBy(TestCase): + @classmethod + def setUpTestData(cls): + Membership.objects.all().delete() + cls.club_a, cls.club_b = baker.make(Club, _quantity=2) + 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 + ), + ] + + def test_admin_user(self): + perm = Permission.objects.get(codename="change_membership") + user = baker.make(User, user_permissions=[perm]) + qs = Membership.objects.editable_by(user).values_list("id", flat=True) + assert set(qs) == set(Membership.objects.values_list("id", flat=True)) + + def test_simple_subscriber_user(self): + user = subscriber_user.make() + assert not Membership.objects.editable_by(user).exists() + + def test_board_member(self): + # a board member can end lower memberships and its own one + user = self.memberships[2].user + qs = Membership.objects.editable_by(user).values_list("id", flat=True) + expected = {self.memberships[2].id, self.memberships[3].id} + assert set(qs) == expected + + class TestMembership(TestClub): def assert_membership_started_today(self, user: User, role: int): """Assert that the given membership is active and started today.""" @@ -151,7 +190,7 @@ class TestMembership(TestClub): def assert_membership_ended_today(self, user: User): """Assert that the given user have a membership which ended today.""" - today = localtime(now()).date() + today = localdate() assert user.memberships.filter(club=self.club, end_date=today).exists() assert self.club.get_membership_for(user) is None @@ -160,7 +199,9 @@ class TestMembership(TestClub): cannot see the page. """ response = self.client.post(self.members_url) - assert response.status_code == 403 + assertRedirects( + response, reverse("core:login", query={"next": self.members_url}) + ) self.client.force_login(self.public) response = self.client.post(self.members_url) @@ -171,7 +212,9 @@ class TestMembership(TestClub): information are displayed. """ self.client.force_login(self.simple_board_member) - response = self.client.get(self.members_url) + response = self.client.get( + reverse("club:club_members", kwargs={"club_id": self.club.id}) + ) assert response.status_code == 200 soup = BeautifulSoup(response.text, "lxml") table = soup.find("table", id="club_members_table") @@ -197,59 +240,45 @@ class TestMembership(TestClub): assert cols[2].text == membership.description assert cols[3].text == str(membership.start_date) - if membership.role <= 3: # 3 is the role of simple_board_member + if membership.role < 3 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 = { "type": "checkbox", - "name": "users_old", - "value": str(user.id), + "name": "members_old", + "value": str(membership.id), } assert form_input.attrs.items() >= expected_attrs.items() else: assert cols[4].find_all() == [] def test_root_add_one_club_member(self): - """Test that root users can add members to clubs, one at a time.""" + """Test that root users can add members to clubs""" self.client.force_login(self.root) response = self.client.post( - self.members_url, - {"users": [self.subscriber.id], "role": 3}, + self.new_members_url, {"user": self.subscriber.id, "role": 3} + ) + assert response.status_code == 200 + assert response.headers.get("HX-Redirect", "") == reverse( + "club:club_members", kwargs={"club_id": self.club.id} ) - self.assertRedirects(response, self.members_url) self.subscriber.refresh_from_db() self.assert_membership_started_today(self.subscriber, role=3) - def test_root_add_multiple_club_member(self): - """Test that root users can add multiple members at once to clubs.""" - self.client.force_login(self.root) - response = self.client.post( - self.members_url, - { - "users": (self.subscriber.id, self.krophil.id), - "role": 3, - }, - ) - self.assertRedirects(response, self.members_url) - self.subscriber.refresh_from_db() - self.assert_membership_started_today(self.subscriber, role=3) - self.assert_membership_started_today(self.krophil, role=3) - def test_add_unauthorized_members(self): """Test that users who are not currently subscribed cannot be members of clubs. """ for user in self.public, self.old_subscriber: - form = ClubMemberForm( - data={"users": [user.id], "role": 1}, + form = ClubAddMemberForm( + data={"user": user.id, "role": 1}, request_user=self.root, club=self.club, ) assert not form.is_valid() assert form.errors == { - "users": [ - "L'utilisateur doit être cotisant pour faire partie d'un club" - ] + "user": ["L'utilisateur doit être cotisant pour faire partie d'un club"] } def test_add_members_already_members(self): @@ -281,16 +310,16 @@ class TestMembership(TestClub): nb_memberships = self.club.members.count() max_id = User.objects.aggregate(id=Max("id"))["id"] for members in [max_id + 1], [max_id + 1, self.subscriber.id]: - form = ClubMemberForm( - data={"users": members, "role": 1}, + form = ClubAddMemberForm( + data={"user": members, "role": 1}, request_user=self.root, club=self.club, ) assert not form.is_valid() assert form.errors == { - "users": [ + "user": [ "Sélectionnez un choix valide. " - f"{max_id + 1} n\u2019en fait pas partie." + "Ce choix ne fait pas partie de ceux disponibles." ] } self.club.refresh_from_db() @@ -303,10 +332,12 @@ class TestMembership(TestClub): nb_subscriber_memberships = self.subscriber.memberships.count() self.client.force_login(president) response = self.client.post( - self.members_url, - {"users": self.subscriber.id, "role": 9}, + self.new_members_url, {"user": self.subscriber.id, "role": 9} + ) + assert response.status_code == 200 + assert response.headers.get("HX-Redirect", "") == reverse( + "club:club_members", kwargs={"club_id": self.club.id} ) - self.assertRedirects(response, self.members_url) self.club.refresh_from_db() self.subscriber.refresh_from_db() assert self.club.members.count() == nb_club_membership + 1 @@ -317,8 +348,8 @@ class TestMembership(TestClub): """Test that a member of the club member cannot create a membership with a greater role than its own. """ - form = ClubMemberForm( - data={"users": [self.subscriber.id], "role": 10}, + form = ClubAddMemberForm( + data={"user": self.subscriber.id, "role": 10}, request_user=self.simple_board_member, club=self.club, ) @@ -326,7 +357,7 @@ class TestMembership(TestClub): assert not form.is_valid() assert form.errors == { - "__all__": ["Vous n'avez pas la permission de faire cela"] + "role": ["Sélectionnez un choix valide. 10 n\u2019en fait pas partie."] } self.club.refresh_from_db() assert nb_memberships == self.club.members.count() @@ -334,23 +365,53 @@ class TestMembership(TestClub): def test_add_member_without_role(self): """Test that trying to add members without specifying their role fails.""" - self.client.force_login(self.root) - form = ClubMemberForm( - data={"users": [self.subscriber.id]}, - request_user=self.simple_board_member, - club=self.club, + form = ClubAddMemberForm( + data={"user": self.subscriber.id}, request_user=self.root, club=self.club ) assert not form.is_valid() - assert form.errors == {"role": ["Vous devez choisir un rôle"]} + assert form.errors == {"role": ["Ce champ est obligatoire."]} + + def test_add_member_already_there(self): + form = ClubAddMemberForm( + data={"user": self.simple_board_member, "role": 3}, + request_user=self.root, + club=self.club, + ) + assert not form.is_valid() + assert form.errors == { + "user": ["Vous ne pouvez pas ajouter deux fois le même utilisateur"] + } + + def test_add_other_member_forbidden(self): + non_member = subscriber_user.make() + simple_member = baker.make(Membership, club=self.club, role=1).user + for user in non_member, simple_member: + form = ClubAddMemberForm( + data={"user": subscriber_user.make(), "role": 1}, + 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."] + } + + 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) + self.client.force_login(user) + res = self.client.get(self.members_url) + assert res.status_code == 200 + soup = BeautifulSoup(res.text, "lxml") + assert not soup.find(id="add_club_members_form") def test_end_membership_self(self): """Test that a member can end its own membership.""" self.client.force_login(self.simple_board_member) - self.client.post( - self.members_url, - {"users_old": self.simple_board_member.id}, - ) + membership = self.club.members.get(end_date=None, user=self.simple_board_member) + self.client.post(self.members_url, {"members_old": [membership.id]}) self.simple_board_member.refresh_from_db() self.assert_membership_ended_today(self.simple_board_member) @@ -358,15 +419,13 @@ class TestMembership(TestClub): """Test that board members of the club can end memberships of users with lower roles. """ - # remainder : simple_board_member has role 3, president has role 10, richard has role 1 + # reminder : simple_board_member has role 3 self.client.force_login(self.simple_board_member) - response = self.client.post( - self.members_url, - {"users_old": self.richard.id}, - ) + membership = baker.make(Membership, club=self.club, role=2, end_date=None) + response = self.client.post(self.members_url, {"members_old": [membership.id]}) self.assertRedirects(response, self.members_url) self.club.refresh_from_db() - self.assert_membership_ended_today(self.richard) + self.assert_membership_ended_today(membership.user) def test_end_membership_higher_role(self): """Test that board members of the club cannot end memberships @@ -374,46 +433,30 @@ class TestMembership(TestClub): """ membership = self.president.memberships.filter(club=self.club).first() self.client.force_login(self.simple_board_member) - self.client.post( - self.members_url, - {"users_old": self.president.id}, - ) + self.client.post(self.members_url, {"members_old": [membership.id]}) self.club.refresh_from_db() new_membership = self.club.get_membership_for(self.president) assert new_membership is not None assert new_membership == membership - membership = self.president.memberships.filter(club=self.club).first() + membership.refresh_from_db() assert membership.end_date is None - def test_end_membership_as_main_club_board(self): - """Test that board members of the main club can end the membership - of anyone. - """ + def test_end_membership_with_permission(self): + """Test that users with permission can end any membership.""" # make subscriber a board member - subscriber = subscriber_user.make() - Membership.objects.create(club=self.ae, user=subscriber, role=3) - nb_memberships = self.club.members.ongoing().count() - self.client.force_login(subscriber) + self.client.force_login( + subscriber_user.make( + user_permissions=[Permission.objects.get(codename="change_membership")] + ) + ) + president_membership = self.club.president response = self.client.post( - self.members_url, - {"users_old": self.president.id}, + self.members_url, {"members_old": [president_membership.id]} ) self.assertRedirects(response, self.members_url) - self.assert_membership_ended_today(self.president) - assert self.club.members.ongoing().count() == nb_memberships - 1 - - def test_end_membership_as_root(self): - """Test that root users can end the membership of anyone.""" - nb_memberships = self.club.members.ongoing().count() - self.client.force_login(self.root) - response = self.client.post( - self.members_url, - {"users_old": [self.president.id]}, - ) - self.assertRedirects(response, self.members_url) - self.assert_membership_ended_today(self.president) + self.assert_membership_ended_today(president_membership.user) assert self.club.members.ongoing().count() == nb_memberships - 1 def test_end_membership_as_foreigner(self): @@ -421,14 +464,11 @@ class TestMembership(TestClub): nb_memberships = self.club.members.count() membership = self.richard.memberships.filter(club=self.club).first() self.client.force_login(self.subscriber) - self.client.post( - self.members_url, - {"users_old": [self.richard.id]}, - ) + self.client.post(self.members_url, {"members_old": [self.richard.id]}) # nothing should have changed - new_mem = self.club.get_membership_for(self.richard) + membership.refresh_from_db() assert self.club.members.count() == nb_memberships - assert membership == new_mem + assert membership.end_date is None def test_remove_from_club_group(self): """Test that when a membership ends, the user is removed from club groups.""" @@ -490,3 +530,85 @@ class TestMembership(TestClub): new_board = set(self.club.board_group.users.values_list("id", flat=True)) assert new_members == initial_members assert new_board == initial_board + + +@pytest.mark.django_db +class TestJoinClub: + @pytest.fixture(autouse=True) + def clear_cache(self): + cache.clear() + + @pytest.mark.parametrize( + ("user_factory", "role", "errors"), + [ + ( + subscriber_user.make, + 2, + { + "role": [ + "Sélectionnez un choix valide. 2 n\u2019en fait pas partie." + ] + }, + ), + ( + lambda: baker.make(User), + 1, + {"__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 + ): + club = baker.make(Club) + user = user_factory() + form = JoinClubForm(club=club, request_user=user, data={"role": role}) + 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}) + 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}) + assert form.is_valid() + form.save() + assert Membership.objects.ongoing().filter(user=user, club=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] + cls.memberships = baker.make( + Membership, + role=iter(roles), + club=club, + start_date=now() - timedelta(days=14), + end_date=now() - timedelta(days=7), + _quantity=len(roles), + _bulk_create=True, + ) + cls.url = reverse("club:club_old_members", kwargs={"club_id": club.id}) + + def test_ok(self): + user = subscriber_user.make() + self.client.force_login(user) + res = self.client.get(self.url) + assert res.status_code == 200 + + def test_access_forbidden(self): + res = self.client.get(self.url) + assertRedirects(res, reverse("core:login", query={"next": self.url})) + + self.client.force_login(baker.make(User)) + res = self.client.get(self.url) + assert res.status_code == 403 diff --git a/club/urls.py b/club/urls.py index 664d93a6..e33c2a26 100644 --- a/club/urls.py +++ b/club/urls.py @@ -25,6 +25,7 @@ from django.urls import path from club.views import ( + ClubAddMembersFragment, ClubCreateView, ClubEditView, ClubListView, @@ -60,6 +61,11 @@ urlpatterns = [ path("/edit/", ClubEditView.as_view(), name="club_edit"), path("/edit/page/", ClubPageEditView.as_view(), name="club_edit_page"), path("/members/", ClubMembersView.as_view(), name="club_members"), + path( + "fragment//members/", + ClubAddMembersFragment.as_view(), + name="club_new_members", + ), path( "/elderlies/", ClubOldMembersView.as_view(), diff --git a/club/views.py b/club/views.py index b10c4a09..0af2db9d 100644 --- a/club/views.py +++ b/club/views.py @@ -23,12 +23,14 @@ # import csv +from typing import Any from django.conf import settings from django.contrib.auth.mixins import PermissionRequiredMixin +from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied, ValidationError from django.core.paginator import InvalidPage, Paginator -from django.db.models import Sum +from django.db.models import Q, Sum from django.http import ( Http404, HttpResponseRedirect, @@ -37,20 +39,28 @@ from django.http import ( from django.shortcuts import get_object_or_404, redirect from django.urls import reverse, reverse_lazy from django.utils import timezone -from django.utils.functional import cached_property +from django.utils.safestring import SafeString +from django.utils.timezone import now from django.utils.translation import gettext as _t from django.utils.translation import gettext_lazy as _ from django.views.generic import DetailView, ListView, View from django.views.generic.edit import CreateView, DeleteView, UpdateView from club.forms import ( + ClubAddMemberForm, ClubAdminEditForm, ClubEditForm, - ClubMemberForm, + ClubOldMemberForm, + JoinClubForm, MailingForm, SellingsForm, ) -from club.models import Club, Mailing, MailingSubscription, Membership +from club.models import ( + Club, + Mailing, + MailingSubscription, + Membership, +) from com.models import Poster from com.views import ( PosterCreateBaseView, @@ -60,11 +70,10 @@ from com.views import ( ) from core.auth.mixins import ( CanEditMixin, - CanViewMixin, ) from core.models import PageRev -from core.views import DetailFormView, PageEditViewBase -from core.views.mixins import TabedViewMixin +from core.views import DetailFormView, PageEditViewBase, UseFragmentsMixin +from core.views.mixins import FragmentMixin, FragmentRenderer, TabedViewMixin from counter.models import Selling @@ -86,7 +95,7 @@ class ClubTabsMixin(TabedViewMixin): "name": _("Infos"), } ] - if self.request.user.can_view(self.object): + if self.request.user.has_perm("club.view_club"): tab_list.extend( [ { @@ -105,16 +114,16 @@ class ClubTabsMixin(TabedViewMixin): }, ] ) - if self.object.page: - tab_list.append( - { - "url": reverse( - "club:club_hist", kwargs={"club_id": self.object.id} - ), - "slug": "history", - "name": _("History"), - } - ) + if self.object.page: + tab_list.append( + { + "url": reverse( + "club:club_hist", kwargs={"club_id": self.object.id} + ), + "slug": "history", + "name": _("History"), + } + ) if self.request.user.can_edit(self.object): tab_list.extend( [ @@ -235,13 +244,14 @@ class ClubPageEditView(ClubTabsMixin, PageEditViewBase): return reverse_lazy("club:club_view", kwargs={"club_id": self.club.id}) -class ClubPageHistView(ClubTabsMixin, CanViewMixin, DetailView): +class ClubPageHistView(ClubTabsMixin, PermissionRequiredMixin, DetailView): """Modification hostory of the page.""" model = Club pk_url_kwarg = "club_id" template_name = "club/page_history.jinja" current_tab = "history" + permission_required = "club.view_club" class ClubToolsView(ClubTabsMixin, CanEditMixin, DetailView): @@ -253,57 +263,121 @@ class ClubToolsView(ClubTabsMixin, CanEditMixin, DetailView): current_tab = "tools" -class ClubMembersView(ClubTabsMixin, CanViewMixin, DetailFormView): +class ClubAddMembersFragment( + FragmentMixin, PermissionRequiredMixin, SuccessMessageMixin, CreateView +): + template_name = "club/fragments/add_member.jinja" + model = Membership + object = None + reload_on_redirect = True + permission_required = "club.view_club" + + def dispatch(self, *args, **kwargs): + self.club = get_object_or_404(Club, pk=kwargs.get("club_id")) + return super().dispatch(*args, **kwargs) + + def get_form_class(self): + user = self.request.user + if user.has_perm("club.add_membership") or self.club.get_membership_for(user): + return ClubAddMemberForm + return JoinClubForm + + def get_form_kwargs(self): + return super().get_form_kwargs() | { + "request_user": self.request.user, + "club": self.club, + } + + def render_fragment(self, request, **kwargs) -> SafeString: + self.club = kwargs.get("club") + return super().render_fragment(request, **kwargs) + + def get_success_url(self): + return reverse("club:club_members", kwargs={"club_id": self.club.id}) + + def get_context_data(self, **kwargs): + return super().get_context_data(**kwargs) | {"club": self.club} + + def get_success_message(self, cleaned_data): + if "user" not in cleaned_data or cleaned_data["user"] == self.request.user: + return _("You are now a member of this club.") + return _("%(user)s has been added to club.") % cleaned_data + + +class ClubMembersView( + ClubTabsMixin, UseFragmentsMixin, PermissionRequiredMixin, DetailFormView +): """View of a club's members.""" model = Club pk_url_kwarg = "club_id" - form_class = ClubMemberForm + form_class = ClubOldMemberForm template_name = "club/club_members.jinja" current_tab = "members" + permission_required = "club.view_club" - @cached_property - def members(self) -> list[Membership]: - return list(self.object.members.ongoing().order_by("-role")) + def get_fragments(self) -> dict[str, type[FragmentMixin] | FragmentRenderer]: + membership = self.object.get_membership_for(self.request.user) + if ( + membership + and membership.role <= settings.SITH_MAXIMUM_FREE_ROLE + and not self.request.user.has_perm("club.add_membership") + ): + # Simple club members won't see the form anymore. + # Even if they saw it, they couldn't add anyone to the club anyway + return {} + return {"add_member_fragment": ClubAddMembersFragment} + + def get_fragment_data(self) -> dict[str, Any]: + return {"add_member_fragment": {"club": self.object}} def get_form_kwargs(self): - kwargs = super().get_form_kwargs() - kwargs["request_user"] = self.request.user - kwargs["club"] = self.object - kwargs["club_members"] = self.members - return kwargs + return super().get_form_kwargs() | { + "user": self.request.user, + "club": self.object, + } def get_context_data(self, **kwargs): kwargs = super().get_context_data(**kwargs) - kwargs["members"] = self.members + editable = list( + kwargs["form"].fields["members_old"].queryset.values_list("id", flat=True) + ) + kwargs["members"] = list( + self.object.members.ongoing() + .annotate(is_editable=Q(id__in=editable)) + .order_by("-role") + .select_related("user") + ) + kwargs["can_end_membership"] = len(editable) > 0 return kwargs def form_valid(self, form): - """Check user rights.""" - resp = super().form_valid(form) - - data = form.clean() - users = data.pop("users", []) - users_old = data.pop("users_old", []) - for user in users: - Membership(club=self.object, user=user, **data).save() - for user in users_old: - membership = self.object.get_membership_for(user) - membership.end_date = timezone.now() + for membership in form.cleaned_data.get("members_old"): + membership.end_date = now() membership.save() - return resp + return super().form_valid(form) def get_success_url(self, **kwargs): return self.request.path -class ClubOldMembersView(ClubTabsMixin, CanViewMixin, DetailView): +class ClubOldMembersView(ClubTabsMixin, PermissionRequiredMixin, DetailView): """Old members of a club.""" model = Club pk_url_kwarg = "club_id" template_name = "club/club_old_members.jinja" current_tab = "elderlies" + permission_required = "club.view_club" + + def get_context_data(self, **kwargs): + return super().get_context_data(**kwargs) | { + "old_members": ( + self.object.members.exclude(end_date=None) + .order_by("-role", "description", "-end_date") + .select_related("user") + ) + } class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): diff --git a/core/static/core/components/ajax-select.scss b/core/static/core/components/ajax-select.scss index 59d146fc..2089ecac 100644 --- a/core/static/core/components/ajax-select.scss +++ b/core/static/core/components/ajax-select.scss @@ -36,6 +36,7 @@ > .ts-control { box-shadow: none; max-width: 300px; + width: 300px; background-color: var(--nf-input-background-color); &::after { diff --git a/core/static/core/forms.scss b/core/static/core/forms.scss index 9982e77f..3fb1685f 100644 --- a/core/static/core/forms.scss +++ b/core/static/core/forms.scss @@ -47,6 +47,7 @@ } input, + select, textarea[type="text"], [type="number"], .ts-control { @@ -240,6 +241,23 @@ form { } } } + input[type="text"], + input[type="email"], + input[type="tel"], + input[type="url"], + input[type="password"], + input[type="number"], + input[type="date"], + input[type="datetime-local"], + input[type="week"], + input[type="time"], + input[type="month"], + input[type="search"], + textarea, + select, + .ts-control { + min-height: calc(var(--nf-input-size) * 2.5); + } input[type="text"], input[type="checkbox"], diff --git a/core/static/core/style.scss b/core/static/core/style.scss index 771ca5e2..4ceb4bb4 100644 --- a/core/static/core/style.scss +++ b/core/static/core/style.scss @@ -506,6 +506,10 @@ th { >ul { margin-top: 0; } + + >input[type="checkbox"] { + padding: unset; + } } td { diff --git a/eboutic/static/bundled/eboutic/eboutic-index.ts b/eboutic/static/bundled/eboutic/eboutic-index.ts index 434d839a..93afc81b 100644 --- a/eboutic/static/bundled/eboutic/eboutic-index.ts +++ b/eboutic/static/bundled/eboutic/eboutic-index.ts @@ -17,7 +17,6 @@ document.addEventListener("alpine:init", () => { this.$watch("basket", () => { this.saveBasket(); }); - // Invalidate basket if a purchase was made if (lastPurchaseTime !== null && localStorage.basketTimestamp !== undefined) { if ( diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index f4bbbb45..aa135d87 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: 2025-09-25 15:33+0200\n" +"POT-Creation-Date: 2025-09-26 17:36+0200\n" "PO-Revision-Date: 2016-07-18\n" "Last-Translator: Maréchal \n" @@ -174,12 +174,12 @@ msgid "You can not add the same user twice" msgstr "Vous ne pouvez pas ajouter deux fois le même utilisateur" #: club/forms.py -msgid "You should specify a role" -msgstr "Vous devez choisir un rôle" +msgid "You must be subscribed to join a club" +msgstr "Vous devez être cotisant pour faire partie d'un club" -#: club/forms.py sas/forms.py -msgid "You do not have the permission to do that" -msgstr "Vous n'avez pas la permission de faire cela" +#: club/forms.py +msgid "You are already a member of this club" +msgstr "Vous êtes déjà membre de ce club." #: club/models.py msgid "slug name" @@ -350,11 +350,6 @@ msgstr "Depuis" msgid "There are no members in this club." msgstr "Il n'y a pas de membres dans ce club." -#: club/templates/club/club_members.jinja core/templates/core/file_detail.jinja -#: core/views/forms.py trombi/templates/trombi/detail.jinja -msgid "Add" -msgstr "Ajouter" - #: club/templates/club/club_old_members.jinja msgid "Club old members" msgstr "Anciens membres du club" @@ -569,6 +564,24 @@ msgstr "" msgid "Save" msgstr "Sauver" +#: club/templates/club/fragments/add_member.jinja +msgid "Add a new member" +msgstr "Ajouter un nouveau membre" + +#: club/templates/club/fragments/add_member.jinja +msgid "Join club" +msgstr "Rejoindre le club" + +#: club/templates/club/fragments/add_member.jinja +#: core/templates/core/file_detail.jinja core/views/forms.py +#: trombi/templates/trombi/detail.jinja +msgid "Add" +msgstr "Ajouter" + +#: club/templates/club/fragments/add_member.jinja +msgid "Join" +msgstr "Rejoindre" + #: club/templates/club/mailing.jinja msgid "Mailing lists" msgstr "Mailing listes" @@ -675,6 +688,15 @@ msgstr "Vente" msgid "Mailing list" msgstr "Listes de diffusion" +#: club/views.py +#, python-format +msgid "%(user)s has been added to club." +msgstr "%(user)s a été ajouté au club." + +#: club/views.py +msgid "You are now a member of this club." +msgstr "Vous êtes maintenant membre de ce club." + #: com/forms.py msgid "Format: 16:9 | Resolution: 1920x1080" msgstr "Format : 16:9 | Résolution : 1920x1080" @@ -4645,6 +4667,10 @@ msgstr "Pas de ban actif" msgid "Add a new album" msgstr "Ajouter un nouvel album" +#: sas/forms.py +msgid "You do not have the permission to do that" +msgstr "Vous n'avez pas la permission de faire cela" + #: sas/forms.py msgid "Upload images" msgstr "Envoyer les images" @@ -5546,4 +5572,4 @@ msgstr "Vous ne pouvez plus écrire de commentaires, la date est passée." #: trombi/views.py #, python-format msgid "Maximum characters: %(max_length)s" -msgstr "Nombre de caractères max: %(max_length)s" \ No newline at end of file +msgstr "Nombre de caractères max: %(max_length)s"
{% trans %}User{% endtrans %}{% trans %}Role{% endtrans %}{% trans %}Description{% endtrans %}{% trans %}From{% endtrans %}{% trans %}To{% endtrans %}
{% trans %}User{% endtrans %}{% trans %}Role{% endtrans %}{% trans %}Description{% endtrans %}{% trans %}From{% endtrans %}{% trans %}To{% endtrans %}
{{ user_profile_link(m.user) }}{{ settings.SITH_CLUB_ROLES[m.role] }}{{ m.description }}{{ m.start_date }}{{ m.end_date }}{{ user_profile_link(member.user) }}{{ settings.SITH_CLUB_ROLES[member.role] }}{{ member.description }}{{ member.start_date }}{{ member.end_date }}