diff --git a/club/forms.py b/club/forms.py index 5d6a3b10..d2b24dde 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_membership"): + 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/club_sellings.jinja b/club/templates/club/club_sellings.jinja index 4928e356..3733d0c8 100644 --- a/club/templates/club/club_sellings.jinja +++ b/club/templates/club/club_sellings.jinja @@ -83,9 +83,10 @@ TODO : rewrite the pagination used in this template an Alpine one
{% 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 }}
{{ paginate(paginated_result, paginator, "formPagination(this)") }} diff --git a/club/templates/club/fragments/add_member.jinja b/club/templates/club/fragments/add_member.jinja new file mode 100644 index 00000000..d1e6e97c --- /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 4ed40a9c..a14b71cd 100644 --- a/club/views.py +++ b/club/views.py @@ -23,30 +23,31 @@ # 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.http import ( - Http404, - HttpResponseRedirect, - StreamingHttpResponse, -) +from django.db.models import Q, Sum +from django.http import Http404, HttpResponseRedirect, StreamingHttpResponse 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, ) @@ -58,13 +59,10 @@ from com.views import ( PosterEditBaseView, PosterListBaseView, ) -from core.auth.mixins import ( - CanEditMixin, - CanViewMixin, -) +from core.auth.mixins import CanEditMixin 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 +84,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 +103,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 +233,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 +252,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): @@ -344,7 +407,7 @@ class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): form = self.get_form() if form.is_valid(): if not len([v for v in form.cleaned_data.values() if v is not None]): - qs = Selling.objects.filter(id=-1) + qs = Selling.objects.none() if form.cleaned_data["begin_date"]: qs = qs.filter(date__gte=form.cleaned_data["begin_date"]) if form.cleaned_data["end_date"]: @@ -362,7 +425,9 @@ class ClubSellingView(ClubTabsMixin, CanEditMixin, DetailFormView): if len(selected_products) > 0: qs = qs.filter(product__in=selected_products) - kwargs["result"] = qs.all().order_by("-id") + kwargs["result"] = qs.select_related( + "counter", "counter__club", "customer", "customer__user", "seller" + ).order_by("-id") kwargs["total"] = sum([s.quantity * s.unit_price for s in kwargs["result"]]) total_quantity = qs.all().aggregate(Sum("quantity")) if total_quantity["quantity__sum"]: diff --git a/com/static/bundled/com/slideshow-index.ts b/com/static/bundled/com/slideshow-index.ts new file mode 100644 index 00000000..ddd3ebe7 --- /dev/null +++ b/com/static/bundled/com/slideshow-index.ts @@ -0,0 +1,49 @@ +const INTERVAL = 10; + +interface Poster { + url: string; // URL of the poster + displayTime: number; // Number of seconds to display that poster +} + +document.addEventListener("alpine:init", () => { + Alpine.data("slideshow", (posters: Poster[]) => ({ + posters: posters, + progress: 0, + elapsed: 0, + + current: 0, + previous: 0, + + init() { + this.$watch("elapsed", () => { + const displayTime = this.posters[this.current].displayTime * 1000; + if (this.elapsed > displayTime) { + this.previous = this.current; + this.current = this.getNext(); + this.elapsed = 0; + } + if (displayTime === 0) { + this.progress = 100; + } else { + this.progress = (100 * this.elapsed) / displayTime; + } + }); + setInterval(() => { + this.elapsed += INTERVAL; + }, INTERVAL); + }, + + getNext() { + return (this.current + 1) % this.posters.length; + }, + + async toggleFullScreen(event: Event) { + if (document.fullscreenElement) { + await document.exitFullscreen(); + return; + } + const target = event.target as HTMLElement; + await target.requestFullscreen(); + }, + })); +}); diff --git a/com/static/com/css/news-list.scss b/com/static/com/css/news-list.scss index cc423ccc..b53ff784 100644 --- a/com/static/com/css/news-list.scss +++ b/com/static/com/css/news-list.scss @@ -83,7 +83,8 @@ #links_content { overflow: auto; box-shadow: $shadow-color 1px 1px 1px; - height: 20em; + min-height: 20em; + padding-bottom: 1em; h4 { margin-left: 5px; diff --git a/com/static/com/css/posters.scss b/com/static/com/css/posters.scss index 26cf2b91..e3863bdb 100644 --- a/com/static/com/css/posters.scss +++ b/com/static/com/css/posters.scss @@ -111,7 +111,7 @@ top: 0; left: 0; z-index: 10; - content: "Click to expand"; + content: attr(hover); color: white; background-color: rgba(black, 0.5); } diff --git a/com/static/com/js/poster_list.js b/com/static/com/js/poster_list.js deleted file mode 100644 index 8ffb97c4..00000000 --- a/com/static/com/js/poster_list.js +++ /dev/null @@ -1,23 +0,0 @@ -$(document).ready(() => { - $("#poster_list #view").click(() => { - $("#view").removeClass("active"); - }); - - $("#poster_list .poster .image").click((e) => { - let el = $(e.target); - if (el.hasClass("image")) { - el = el.find("img"); - } - $("#poster_list #view #placeholder").html(el.clone()); - - $("#view").addClass("active"); - }); - - $(document).keyup((e) => { - if (e.keyCode === 27) { - // escape key maps to keycode `27` - e.preventDefault(); - $("#view").removeClass("active"); - } - }); -}); diff --git a/com/static/com/js/slideshow.js b/com/static/com/js/slideshow.js deleted file mode 100644 index 952af8b8..00000000 --- a/com/static/com/js/slideshow.js +++ /dev/null @@ -1,98 +0,0 @@ -$(document).ready(() => { - const transitionTime = 1000; - - let i = 0; - const max = $("#slideshow .slide").length; - - function enterFullscreen() { - const element = document.getElementById("slideshow"); - $(element).addClass("fullscreen"); - if (element.requestFullscreen) { - element.requestFullscreen(); - } else if (element.mozRequestFullScreen) { - element.mozRequestFullScreen(); - } else if (element.webkitRequestFullscreen) { - element.webkitRequestFullscreen(); - } else if (element.msRequestFullscreen) { - element.msRequestFullscreen(); - } - } - - function exitFullscreen() { - const element = document.getElementById("slideshow"); - $(element).removeClass("fullscreen"); - if (document.exitFullscreen) { - document.exitFullscreen(); - } else if (document.webkitExitFullscreen) { - document.webkitExitFullscreen(); - } else if (document.mozCancelFullScreen) { - document.mozCancelFullScreen(); - } else if (document.msExitFullscreen) { - document.msExitFullscreen(); - } - } - - function initProgressBar() { - $("#slideshow #progress_bar").css("transition", "none"); - $("#slideshow #progress_bar").removeClass("progress"); - $("#slideshow #progress_bar").addClass("init"); - } - - function startProgressBar(displayTime) { - $("#slideshow #progress_bar").removeClass("init"); - $("#slideshow #progress_bar").addClass("progress"); - $("#slideshow #progress_bar").css("transition", `width ${displayTime}s linear`); - } - - function next() { - initProgressBar(); - const slide = $($("#slideshow .slide").get(i % max)); - slide.removeClass("center"); - slide.addClass("left"); - - const nextSlide = $($("#slideshow .slide").get((i + 1) % max)); - nextSlide.removeClass("right"); - nextSlide.addClass("center"); - const displayTime = nextSlide.attr("display_time") || 2; - - $("#slideshow .bullet").removeClass("active"); - const bullet = $("#slideshow .bullet")[(i + 1) % max]; - $(bullet).addClass("active"); - - i = (i + 1) % max; - - setTimeout(() => { - const othersLeft = $("#slideshow .slide.left"); - othersLeft.removeClass("left"); - othersLeft.addClass("right"); - - startProgressBar(displayTime); - setTimeout(next, displayTime * 1000); - }, transitionTime); - } - - const displayTime = $("#slideshow .center").attr("display_time"); - initProgressBar(); - setTimeout(() => { - if (max > 1) { - startProgressBar(displayTime); - setTimeout(next, displayTime * 1000); - } - }, 10); - - $("#slideshow").click(() => { - if ($("#slideshow").hasClass("fullscreen")) { - exitFullscreen(); - } else { - enterFullscreen(); - } - }); - - $(document).keyup((e) => { - if (e.keyCode === 27) { - // escape key maps to keycode `27` - e.preventDefault(); - exitFullscreen(); - } - }); -}); diff --git a/com/static/css/slideshow.scss b/com/static/css/slideshow.scss index 9ea4fa52..3f127cb7 100644 --- a/com/static/css/slideshow.scss +++ b/com/static/css/slideshow.scss @@ -1,4 +1,4 @@ -body{ +body { position: absolute; width: 100vw; height: 100vh; @@ -7,22 +7,22 @@ body{ margin: 0; } -#slideshow{ +#slideshow { position: relative; background-color: lightgrey; height: 100%; - *{ + * { -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; } - &:hover{ + &:hover { - &::before{ + &::before { position: absolute; width: 100%; @@ -34,7 +34,7 @@ body{ z-index: 10; - content: "Click to expand"; + content: attr(hover); color: white; background-color: rgba(black, 0.5); @@ -43,7 +43,7 @@ body{ } - &.fullscreen{ + &:fullscreen { position: fixed; width: 100%; height: 100%; @@ -51,57 +51,78 @@ body{ left: 0; background: none; - &:before{ - display:none; + &:before { + display: none; } - #slides{ + #slides { height: 100vh; } } - #slides{ + #slides { position: relative; height: 100%; overflow: hidden; + background-color: grey; - .slide{ + .slide { position: absolute; width: 100%; height: 100%; - display: inline-flex; + display: none; justify-content: center; top: 0px; + left: 0%; - background-color: grey; - transition: left 1s ease-out; - - img{ + img { max-width: 100%; max-height: 100%; object-fit: contain; } - } - .slide.left{ - left: -100%; - } + &.current { + display: inline-flex; + left: 0%; + animation: scrolling-in 1s linear; + } - .slide.center{ - left: 0px; - } + &.previous { + display: inline-flex; + animation: scrolling-out 1s linear; + opacity: 0; + transition: opacity 0.1s; + transition-delay: 0.9s; + } + + @keyframes scrolling-in { + 0% { + transform: translateX(100%); + } + + 100% { + transform: translateX(0%); + } + } + + @keyframes scrolling-out { + 0% { + transform: translateX(0%); + } + + 100% { + transform: translateX(-100%); + } + } - .slide.right{ - left: 100%; - transition: none; } } - #progress_bullets{ + #progress_bullets { position: absolute; bottom: 10px; width: 100%; @@ -112,7 +133,7 @@ body{ margin-bottom: 10px; - .bullet{ + .bullet { height: 10px; width: 10px; @@ -123,27 +144,33 @@ body{ background-color: grey; - &.active{ + &.active { background-color: #c99836; } } } - #progress_bar{ + progress { + --color: #304c83; + position: absolute; bottom: 0px; height: 10px; - background-color: #304c83; + color: var(--color); + width: 100%; + margin-bottom: 0px; + border: none; - &.init{ - width: 0px; - transition: none; + &::-moz-progress-bar { + background: var(--color); } - &.progress{ - width: 100%; - transition: width 10s linear; + &::-webkit-progress-value { + background: var(--color); + } + + &[value] { + background-color: transparent; } } -} - +} \ No newline at end of file diff --git a/com/templates/com/macros.jinja b/com/templates/com/macros.jinja index 1e3fb853..233f8cd4 100644 --- a/com/templates/com/macros.jinja +++ b/com/templates/com/macros.jinja @@ -76,18 +76,20 @@ It will stay hidden for other users until it has been published. {% endtrans %}

- {% if user.has_perm("com.moderate_news") %} + {%- if user.has_perm("com.moderate_news") -%} {# This is an additional query for each non-moderated news, but it will be executed only for admin users, and only one time - (if they do their job and moderated news as soon as they see them), + (if they do their job and moderate news as soon as they see them), so it's still reasonable #}