Merge pull request #1020 from ae-utbm/taiste

RSS feed, subscription creation permisssion, pedagogy permissions and bugfixes

.github/actions/setup_project/action.yml

@@ -4,7 +4,7 @@ runs:
   using: composite
   steps:
     - name: Install apt packages
-      uses: awalsh128/cache-apt-pkgs-action@latest
+      uses: awalsh128/cache-apt-pkgs-action@v1.4.3
       with:
         packages: gettext
         version: 1.0  # increment to reset cache

.github/workflows/ci.yml

@@ -41,7 +41,7 @@ jobs:
           uv run coverage report
           uv run coverage html
       - name: Archive code coverage results
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
-          name: coverage-report
+          name: coverage-report-${{ matrix.pytest-mark }}
           path: coverage_report

club/views.py

@@ -256,7 +256,7 @@ class ClubMembersView(ClubTabsMixin, CanViewMixin, DetailFormView):
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         kwargs["request_user"] = self.request.user
-        kwargs["club"] = self.get_object()
+        kwargs["club"] = self.object
         kwargs["club_members"] = self.members
         return kwargs
 
@@ -273,9 +273,9 @@ class ClubMembersView(ClubTabsMixin, CanViewMixin, DetailFormView):
         users = data.pop("users", [])
         users_old = data.pop("users_old", [])
         for user in users:
-            Membership(club=self.get_object(), user=user, **data).save()
+            Membership(club=self.object, user=user, **data).save()
         for user in users_old:
-            membership = self.get_object().get_membership_for(user)
+            membership = self.object.get_membership_for(user)
             membership.end_date = timezone.now()
             membership.save()
         return resp
@@ -285,9 +285,7 @@ class ClubMembersView(ClubTabsMixin, CanViewMixin, DetailFormView):
         return super().dispatch(request, *args, **kwargs)
 
     def get_success_url(self, **kwargs):
-        return reverse_lazy(
+        return reverse_lazy("club:club_members", kwargs={"club_id": self.object.id})
-            "club:club_members", kwargs={"club_id": self.get_object().id}
-        )
 
 
 class ClubOldMembersView(ClubTabsMixin, CanViewMixin, DetailView):

com/static/bundled/com/components/ics-calendar-index.ts

@@ -152,6 +152,7 @@ export class IcsCalendar extends inheritHtmlElement("div") {
 
   async connectedCallback() {
     super.connectedCallback();
+    const cacheInvalidate = `?invalidate=${Date.now()}`;
     this.calendar = new Calendar(this.node, {
       plugins: [dayGridPlugin, iCalendarPlugin, listPlugin],
       locales: [frLocale, enLocale],
@@ -161,11 +162,11 @@ export class IcsCalendar extends inheritHtmlElement("div") {
       headerToolbar: this.currentToolbar(),
       eventSources: [
         {
-          url: await makeUrl(calendarCalendarInternal),
+          url: `${await makeUrl(calendarCalendarInternal)}${cacheInvalidate}`,
           format: "ics",
         },
         {
-          url: await makeUrl(calendarCalendarExternal),
+          url: `${await makeUrl(calendarCalendarExternal)}${cacheInvalidate}`,
           format: "ics",
         },
       ],

com/static/com/components/ics-calendar.scss

@@ -75,7 +75,7 @@ ics-calendar {
   }
 
   td {
-    overflow-x: visible; // Show events on multiple days
+    overflow: visible; // Show events on multiple days
   }
 
   //Reset from style.scss

com/static/com/css/news-list.scss

@@ -36,6 +36,11 @@
     &:not(:first-of-type) {
       margin: 2em 0 1em 0;
     }
+
+    .feed {
+      float: right;
+      color: #f26522;
+    }
   }
 
   @media screen and (max-width: $small-devices) {

com/templates/com/news_list.jinja

@@ -8,6 +8,9 @@
 {% block additional_css %}
   <link rel="stylesheet" href="{{ static('com/css/news-list.scss') }}">
   <link rel="stylesheet" href="{{ static('com/components/ics-calendar.scss') }}">
+
+  {# Atom feed discovery, not really css but also goes there #}
+  <link rel="alternate" type="application/rss+xml" title="{% trans %}News feed{% endtrans %}" href="{{ url("com:news_feed") }}">
 {% endblock %}
 
 {% block additional_js %}
@@ -19,7 +22,10 @@
   <div id="news">
     <div id="left_column" class="news_column">
       {% set events_dates = NewsDate.objects.filter(end_date__gte=timezone.now(), start_date__lte=timezone.now()+timedelta(days=5), news__is_moderated=True).datetimes('start_date', 'day') %}
-      <h3>{% trans %}Events today and the next few days{% endtrans %}</h3>
+      <h3>
+        {% trans %}Events today and the next few days{% endtrans %}
+        <a target="#" href="{{ url("com:news_feed") }}"><i class="fa fa-rss feed"></i></a>
+      </h3>
       {% if user.is_authenticated and (user.is_com_admin or user.memberships.board().ongoing().exists()) %}
         <a class="btn btn-blue margin-bottom" href="{{ url("com:news_new") }}">
           <i class="fa fa-plus"></i>
@@ -73,7 +79,10 @@
         </div>
       {% endif %}
 
-      <h3>{% trans %}All coming events{% endtrans %}</h3>
+      <h3>
+        {% trans %}All coming events{% endtrans %}
+        <a target="#" href="{{ url("com:news_feed") }}"><i class="fa fa-rss feed"></i></a>
+      </h3>
       <ics-calendar locale="{{ get_language() }}"></ics-calendar>
     </div>
 

com/tests/test_views.py

@@ -17,6 +17,7 @@ from unittest.mock import patch
 
 import pytest
 from django.conf import settings
+from django.contrib.sites.models import Site
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase
 from django.urls import reverse
@@ -24,7 +25,7 @@ from django.utils import html
 from django.utils.timezone import localtime, now
 from django.utils.translation import gettext as _
 from model_bakery import baker
-from pytest_django.asserts import assertRedirects
+from pytest_django.asserts import assertNumQueries, assertRedirects
 
 from club.models import Club, Membership
 from com.models import News, NewsDate, Poster, Sith, Weekmail, WeekmailArticle
@@ -319,3 +320,15 @@ class TestNewsCreation(TestCase):
                 self.valid_payload,
             )
             mocked.assert_called()
+
+
+@pytest.mark.django_db
+def test_feed(client):
+    """Smoke test that checks that the atom feed is working"""
+    Site.objects.clear_cache()
+    with assertNumQueries(2):
+        # get sith domain with Site api: 1 request
+        # get all news and related info: 1 request
+        resp = client.get(reverse("com:news_feed"))
+        assert resp.status_code == 200
+        assert resp.headers["Content-Type"] == "application/rss+xml; charset=utf-8"

com/urls.py

@@ -25,6 +25,7 @@ from com.views import (
     NewsCreateView,
     NewsDeleteView,
     NewsDetailView,
+    NewsFeed,
     NewsListView,
     NewsModerateView,
     NewsUpdateView,
@@ -73,6 +74,7 @@ urlpatterns = [
         name="weekmail_article_edit",
     ),
     path("news/", NewsListView.as_view(), name="news_list"),
+    path("news/feed/", NewsFeed(), name="news_feed"),
     path("news/admin/", NewsAdminListView.as_view(), name="news_admin_list"),
     path("news/create/", NewsCreateView.as_view(), name="news_new"),
     path("news/<int:news_id>/edit/", NewsUpdateView.as_view(), name="news_edit"),

com/views.py

@@ -26,8 +26,10 @@ from datetime import timedelta
 from smtplib import SMTPRecipientsRefused
 from typing import Any
 
+from dateutil.relativedelta import relativedelta
 from django.conf import settings
 from django.contrib.auth.mixins import AccessMixin, PermissionRequiredMixin
+from django.contrib.syndication.views import Feed
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.db.models import Max
 from django.forms.models import modelform_factory
@@ -268,6 +270,34 @@ class NewsDetailView(CanViewMixin, DetailView):
         return super().get_context_data(**kwargs) | {"date": self.object.dates.first()}
 
 
+class NewsFeed(Feed):
+    title = _("News")
+    link = reverse_lazy("com:news_list")
+    description = _("All incoming events")
+
+    def items(self):
+        return (
+            NewsDate.objects.filter(
+                news__is_moderated=True,
+                end_date__gte=timezone.now() - (relativedelta(months=6)),
+            )
+            .select_related("news", "news__author")
+            .order_by("-start_date")
+        )
+
+    def item_title(self, item: NewsDate):
+        return item.news.title
+
+    def item_description(self, item: NewsDate):
+        return item.news.summary
+
+    def item_link(self, item: NewsDate):
+        return item.news.get_absolute_url()
+
+    def item_author_name(self, item: NewsDate):
+        return item.news.author.get_display_name()
+
+
 # Weekmail
 
 

core/auth/api_permissions.py

@@ -37,8 +37,11 @@ Example:
     ```
 """
 
+import operator
+from functools import reduce
 from typing import Any
 
+from django.contrib.auth.models import Permission
 from django.http import HttpRequest
 from ninja_extra import ControllerBase
 from ninja_extra.permissions import BasePermission
@@ -56,6 +59,46 @@ class IsInGroup(BasePermission):
         return request.user.is_in_group(pk=self._group_pk)
 
 
+class HasPerm(BasePermission):
+    """Check that the user has the required perm.
+
+    If multiple perms are given, a comparer function can also be passed,
+    in order to change the way perms are checked.
+
+    Example:
+        ```python
+        # this route will require both permissions
+        @route.put("/foo", permissions=[HasPerm(["foo.change_foo", "foo.add_foo"])]
+        def foo(self): ...
+
+        # This route will require at least one of the perm,
+        # but it's not mandatory to have all of them
+        @route.put(
+            "/bar",
+            permissions=[HasPerm(["foo.change_bar", "foo.add_bar"], op=operator.or_)],
+        )
+        def bar(self): ...
+    """
+
+    def __init__(
+        self, perms: str | Permission | list[str | Permission], op=operator.and_
+    ):
+        """
+        Args:
+            perms: a permission or a list of permissions the user must have
+            op: An operator to combine multiple permissions (in most cases,
+                it will be either `operator.and_` or `operator.or_`)
+        """
+        super().__init__()
+        if not isinstance(perms, (list, tuple, set)):
+            perms = [perms]
+        self._operator = op
+        self._perms = perms
+
+    def has_permission(self, request: HttpRequest, controller: ControllerBase) -> bool:
+        return reduce(self._operator, (request.user.has_perm(p) for p in self._perms))
+
+
 class IsRoot(BasePermission):
     """Check that the user is root."""
 

core/management/commands/populate.py

@@ -92,7 +92,12 @@ class Command(BaseCommand):
             raise Exception("Never call this command in prod. Never.")
 
         Sith.objects.create(weekmail_destinations="etudiants@git.an personnel@git.an")
-        Site.objects.create(domain=settings.SITH_URL, name=settings.SITH_NAME)
+
+        site = Site.objects.get_current()
+        site.domain = settings.SITH_URL
+        site.name = settings.SITH_NAME
+        site.save()
+
         groups = self._create_groups()
         self._create_ban_groups()
 
@@ -120,6 +125,11 @@ class Command(BaseCommand):
             unix_name=settings.SITH_MAIN_CLUB["unix_name"],
             address=settings.SITH_MAIN_CLUB["address"],
         )
+        main_club.board_group.permissions.add(
+            *Permission.objects.filter(
+                codename__in=["view_subscription", "add_subscription"]
+            )
+        )
         bar_club = Club.objects.create(
             id=2,
             name=settings.SITH_BAR_MANAGER["name"],
@@ -895,13 +905,16 @@ Welcome to the wiki page!
 
         subscribers = Group.objects.create(name="Subscribers")
         subscribers.permissions.add(
-            *list(perms.filter(codename__in=["add_news", "add_uvcommentreport"]))
+            *list(perms.filter(codename__in=["add_news", "add_uvcomment"]))
         )
         old_subscribers = Group.objects.create(name="Old subscribers")
         old_subscribers.permissions.add(
             *list(
                 perms.filter(
                     codename__in=[
+                        "view_uv",
+                        "view_uvcomment",
+                        "add_uvcommentreport",
                         "view_user",
                         "view_picture",
                         "view_album",
@@ -973,9 +986,9 @@ Welcome to the wiki page!
         )
         pedagogy_admin.permissions.add(
             *list(
-                perms.filter(content_type__app_label="pedagogy").values_list(
+                perms.filter(content_type__app_label="pedagogy")
-                    "pk", flat=True
+                .exclude(codename__in=["change_uvcomment"])
-                )
+                .values_list("pk", flat=True)
             )
         )
         self.reset_index("core", "auth")

core/models.py

@@ -417,29 +417,6 @@ class User(AbstractUser):
     def is_board_member(self) -> bool:
         return self.groups.filter(club_board=settings.SITH_MAIN_CLUB_ID).exists()
 
-    @cached_property
-    def can_read_subscription_history(self) -> bool:
-        if self.is_root or self.is_board_member:
-            return True
-
-        from club.models import Club
-
-        for club in Club.objects.filter(
-            id__in=settings.SITH_CAN_READ_SUBSCRIPTION_HISTORY
-        ):
-            if club in self.clubs_with_rights:
-                return True
-        return False
-
-    @cached_property
-    def can_create_subscription(self) -> bool:
-        return self.is_root or (
-            self.memberships.board()
-            .ongoing()
-            .filter(club_id__in=settings.SITH_CAN_CREATE_SUBSCRIPTIONS)
-            .exists()
-        )
-
     @cached_property
     def is_launderette_manager(self):
         from club.models import Club
@@ -679,14 +656,6 @@ class AnonymousUser(AuthAnonymousUser):
     def __init__(self):
         super().__init__()
 
-    @property
-    def can_create_subscription(self):
-        return False
-
-    @property
-    def can_read_subscription_history(self):
-        return False
-
     @property
     def was_subscribed(self):
         return False

core/templates/core/edit.jinja

@@ -1,19 +1,40 @@
 {% extends "core/base.jinja" %}
 
+{# if the template context has the `object_name` variable,
+   then this one will be used in the page title,
+   instead of the result of `str(object)` #}
+{% if object and not object_name %}
+  {% set object_name=object %}
+{% endif %}
+
 {% block title %}
-  {% if object %}
+  {% if object_name %}
-    {% trans obj=object %}Edit {{ obj }}{% endtrans %}
+    {% trans name=object_name %}Edit {{ name }}{% endtrans %}
   {% else %}
     {% trans %}Save{% endtrans %}
   {% endif %}
 {% endblock %}
 
 {% block content %}
-  {% if object %}
+  {% if object_name %}
-    <h2>{% trans obj=object %}Edit {{ obj }}{% endtrans %}</h2>
+    <h2>{% trans name=object_name %}Edit {{ name }}{% endtrans %}</h2>
   {% else %}
     <h2>{% trans %}Save{% endtrans %}</h2>
   {% endif %}
+  {% if messages %}
+    <div x-data="{show_alert: true}" class="alert alert-green" x-show="show_alert" x-transition>
+      <span class="alert-main">
+        {% for message in messages %}
+          {% if message.level_tag == "success" %}
+            {{ message }}
+          {% endif %}
+        {% endfor %}
+      </span>
+      <span class="clickable" @click="show_alert = false">
+        <i class="fa fa-close"></i>
+      </span>
+    </div>
+  {% endif %}
   <form action="" method="post" enctype="multipart/form-data">
     {% csrf_token %}
     {{ form.as_p() }}

core/templates/core/user_detail.jinja

@@ -166,7 +166,7 @@
   </div>
 {% endif %}
 <br>
-{% if profile.was_subscribed and (user == profile or user.can_read_subscription_history)%}
+{% if profile.was_subscribed and (user == profile or user.has_perm("subscription.view_subscription")) %}
   <div class="collapse" :class="{'shadow': collapsed}" x-data="{collapsed: false}" x-cloak>
     <div class="collapse-header clickable" @click="collapsed = !collapsed">
       <span class="collapse-header-text">
@@ -197,9 +197,9 @@
       </table>
     </div>
   </div>
+  <hr>
 {% endif %}
 
-<hr>
 <div>
   {% if user.is_root or user.is_board_member %}
     <form class="form-gifts" action="{{ url('core:user_gift_create', user_id=profile.id) }}" method="post">

core/templates/core/user_tools.jinja

@@ -13,7 +13,7 @@
     <h3>{% trans %}User Tools{% endtrans %}</h3>
 
     <div class="container">
-      {% if user.can_create_subscription or user.is_root or user.is_board_member %}
+      {% if user.has_perm("subscription.view_userban") or user.is_root or user.is_board_member %}
         <div>
           <h4>{% trans %}Sith management{% endtrans %}</h4>
           <ul>
@@ -21,16 +21,16 @@
               <li><a href="{{ url('core:group_list') }}">{% trans %}Groups{% endtrans %}</a></li>
               <li><a href="{{ url('rootplace:merge') }}">{% trans %}Merge users{% endtrans %}</a></li>
               <li><a href="{{ url('rootplace:operation_logs') }}">{% trans %}Operation logs{% endtrans %}</a></li>
-              <li><a href="{{ url('rootplace:delete_forum_messages') }}">{% trans %}Delete user's forum messages{% endtrans %}</a></li>
+              <li>
+                <a href="{{ url('rootplace:delete_forum_messages') }}">
+                  {% trans %}Delete user's forum messages{% endtrans %}
+                </a>
+              </li>
             {% endif %}
             {% if user.has_perm("core.view_userban") %}
               <li><a href="{{ url("rootplace:ban_list") }}">{% trans %}Bans{% endtrans %}</a></li>
             {% endif %}
-            {% if user.can_create_subscription or user.is_root %}
-              <li><a href="{{ url('subscription:subscription') }}">{% trans %}Subscriptions{% endtrans %}</a></li>
-            {% endif %}
             {% if user.is_board_member or user.is_root %}
-              <li><a href="{{ url('subscription:stats') }}">{% trans %}Subscription stats{% endtrans %}</a></li>
               <li><a href="{{ url('club:club_new') }}">{% trans %}New club{% endtrans %}</a></li>
             {% endif %}
           </ul>
@@ -42,26 +42,44 @@
         {% set is_admin_on_a_counter = true %}
       {% endfor %}
 
-      {% if
+      {% if is_admin_on_a_counter or user.is_root or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) %}
-      is_admin_on_a_counter
-      or user.is_root
-      or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID)
-      %}
         <div>
           <h4>{% trans %}Counters{% endtrans %}</h4>
           <ul>
-          {% if user.is_root
+            {% if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) %}
-          or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID)
+              <li>
-          %}
+                <a href="{{ url('counter:admin_list') }}">
-          <li><a href="{{ url('counter:admin_list') }}">{% trans %}General counters management{% endtrans %}</a></li>
+                  {% trans %}General counters management{% endtrans %}
-          <li><a href="{{ url('counter:product_list') }}">{% trans %}Products management{% endtrans %}</a></li>
+                </a>
-          <li><a href="{{ url('counter:product_type_list') }}">{% trans %}Product types management{% endtrans %}</a></li>
+              </li>
-          <li><a href="{{ url('counter:cash_summary_list') }}">{% trans %}Cash register summaries{% endtrans %}</a></li>
+              <li>
-          <li><a href="{{ url('counter:invoices_call') }}">{% trans %}Invoices call{% endtrans %}</a></li>
+                <a href="{{ url('counter:product_list') }}">
-          <li><a href="{{ url('counter:eticket_list') }}">{% trans %}Etickets{% endtrans %}</a></li>
+                  {% trans %}Products management{% endtrans %}
-{% endif %}
+                </a>
-</ul>
+              </li>
-<ul>
+              <li>
+                <a href="{{ url('counter:product_type_list') }}">
+                  {% trans %}Product types management{% endtrans %}
+                </a>
+              </li>
+              <li>
+                <a href="{{ url('counter:cash_summary_list') }}">
+                  {% trans %}Cash register summaries{% endtrans %}
+                </a>
+              </li>
+              <li>
+                <a href="{{ url('counter:invoices_call') }}">
+                  {% trans %}Invoices call{% endtrans %}
+                </a>
+              </li>
+              <li>
+                <a href="{{ url('counter:eticket_list') }}">
+                  {% trans %}Etickets{% endtrans %}
+                </a>
+              </li>
+            {% endif %}
+          </ul>
+          <ul>
             {% for b in settings.SITH_COUNTER_BARS %}
               {% if user.is_in_group(name=b[1]+" admin") %}
                 {% set c = Counter.objects.filter(id=b[0]).first() %}
@@ -71,28 +89,26 @@
 
                   <span>
                     <span>
-            <a class="button" href="{{ url('counter:admin', counter_id=b[0]) }}">{% trans %}Edit{% endtrans %}</a>
+                      <a class="button" href="{{ url('counter:admin', counter_id=b[0]) }}">
-            <a class="button" href="{{ url('counter:stats', counter_id=b[0]) }}">{% trans %}Stats{% endtrans %}</a>
+                        {% trans %}Edit{% endtrans %}
+                      </a>
+                      <a class="button" href="{{ url('counter:stats', counter_id=b[0]) }}">
+                        {% trans %}Stats{% endtrans %}
+                      </a>
                     </span>
                   </span>
                 </li>
               {% endif %}
             {% endfor %}
-</ul>
+          </ul>
-</div>
+        </div>
-{% endif %}
+      {% endif %}
 
-{% if
+      {% if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
-user.is_root
+        <div>
-or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
-or user.memberships.ongoing().filter(role__gte=7).count() > 10
-%}
-<div>
           <h4>{% trans %}Accounting{% endtrans %}</h4>
           <ul>
-    {% if user.is_root
+            {% if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
-    or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID)
-    %}
               <li><a href="{{ url('accounting:refound_account') }}">{% trans %}Refound Account{% endtrans %}</a></li>
               <li><a href="{{ url('accounting:bank_list') }}">{% trans %}General accounting{% endtrans %}</a></li>
               <li><a href="{{ url('accounting:co_list') }}">{% trans %}Company list{% endtrans %}</a></li>
@@ -116,15 +132,11 @@ or user.memberships.ongoing().filter(role__gte=7).count() > 10
               {%- endif -%}
             {%- endfor %}
           </ul>
-</div>
+        </div>
-{% endif %}
+      {% endif %}
 
-{% if
+      {% if user.is_root or user.is_com_admin or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %}
-user.is_root
+        <div>
-or user.is_com_admin
-or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
-%}
-<div>
           <h4>{% trans %}Communication{% endtrans %}</h4>
           <ul>
             {% if user.is_com_admin or user.is_root %}
@@ -144,10 +156,39 @@ or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
               <li><a href="{{ url('sas:moderation') }}">{% trans %}Moderate pictures{% endtrans %}</a></li>
             {% endif %}
           </ul>
-</div>
+        </div>
-{% endif %}
+      {% endif %}
 
-{% if user.memberships.filter(end_date=None).all().count() > 0 %}
+      {% if user.has_perm("subscription.add_subscription") or user.has_perm("auth.change_perm") or user.is_root or user.is_board_member %}
+        <div>
+          <h4>{% trans %}Subscriptions{% endtrans %}</h4>
+          <ul>
+            {% if user.has_perm("subscription.add_subscription") %}
+              <li>
+                <a href="{{ url("subscription:subscription") }}">
+                  {% trans %}New subscription{% endtrans %}
+                </a>
+              </li>
+            {% endif %}
+            {% if user.has_perm("auth.change_permission") %}
+              <li>
+                <a href="{{ url("subscription:perms") }}">
+                  {% trans %}Manage permissions{% endtrans %}
+                </a>
+              </li>
+            {% endif %}
+            {% if user.is_root or user.is_board_member %}
+              <li>
+                <a href="{{ url("subscription:stats") }}">
+                  {% trans %}Subscription stats{% endtrans %}
+                </a>
+              </li>
+            {% endif %}
+          </ul>
+        </div>
+      {% endif %}
+
+      {% if user.memberships.filter(end_date=None).all().count() > 0 %}
         <div>
           <h4>{% trans %}Club tools{% endtrans %}</h4>
           <ul>
@@ -156,22 +197,31 @@ or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID)
             {% endfor %}
           </ul>
         </div>
-{% endif %}
+      {% endif %}
 
-{% if
+      {% if user.has_perm("pedagogy.add_uv") or user.has_perm("pedagogy.delete_uvcomment") %}
-user.is_root
+        <div>
-or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)
-%}
-<div>
           <h4>{% trans %}Pedagogy{% endtrans %}</h4>
           <ul>
-    <li><a href="{{ url('pedagogy:uv_create') }}">{% trans %}Create UV{% endtrans %}</a></li>
+            {% if user.has_perm("pedagogy.add_uv") %}
-    <li><a href="{{ url('pedagogy:moderation') }}">{% trans %}Moderate comments{% endtrans %}</a></li>
+              <li>
+                <a href="{{ url("pedagogy:uv_create") }}">
+                  {% trans %}Create UV{% endtrans %}
+                </a>
+              </li>
+            {% endif %}
+            {% if user.has_perm("pedagogy.delete_uvcomment") %}
+              <li>
+                <a href="{{ url("pedagogy:moderation") }}">
+                  {% trans %}Moderate comments{% endtrans %}
+                </a>
+              </li>
+            {% endif %}
           </ul>
-</div>
+        </div>
-{% endif %}
+      {% endif %}
 
-<div>
+      <div>
         <h4>{% trans %}Elections{% endtrans %}</h4>
         <ul>
           <li><a href="{{ url('election:list') }}">{% trans %}See available elections{% endtrans %}</a></li>
@@ -180,14 +230,14 @@ or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)
             <li><a href="{{ url('election:create') }}">{% trans %}Create a new election{% endtrans %}</a></li>
           {%- endif -%}
         </ul>
-</div>
+      </div>
 
-<div>
+      <div>
         <h4>{% trans %}Other tools{% endtrans %}</h4>
         <ul>
           <li><a href="{{ url('trombi:user_tools') }}">{% trans %}Trombi tools{% endtrans %}</a></li>
         </ul>
-</div>
+      </div>
-</div>
+    </div>
-</main>
+  </main>
 {% endblock %}

core/tests/test_user.py

@@ -8,6 +8,7 @@ from django.urls import reverse
 from django.utils.timezone import now
 from model_bakery import baker, seq
 from model_bakery.recipe import Recipe, foreign_key
+from pytest_django.asserts import assertRedirects
 
 from com.models import News
 from core.baker_recipes import (
@@ -15,7 +16,7 @@ from core.baker_recipes import (
     subscriber_user,
     very_old_subscriber_user,
 )
-from core.models import User
+from core.models import Group, User
 from counter.models import Counter, Refilling, Selling
 from eboutic.models import Invoice, InvoiceItem
 
@@ -198,3 +199,23 @@ def test_user_added_to_public_group():
     user = baker.make(User)
     assert user.groups.filter(pk=settings.SITH_GROUP_PUBLIC_ID).exists()
     assert user.is_in_group(pk=settings.SITH_GROUP_PUBLIC_ID)
+
+
+@pytest.mark.django_db
+def test_user_update_groups(client: Client):
+    client.force_login(baker.make(User, is_superuser=True))
+    manageable_groups = baker.make(Group, is_manually_manageable=True, _quantity=3)
+    hidden_groups = baker.make(Group, is_manually_manageable=False, _quantity=4)
+    user = baker.make(User, groups=[*manageable_groups[1:], *hidden_groups[:3]])
+    response = client.post(
+        reverse("core:user_groups", kwargs={"user_id": user.id}),
+        data={"groups": [manageable_groups[0].id, manageable_groups[1].id]},
+    )
+    assertRedirects(response, user.get_absolute_url())
+    # only the manually manageable groups should have changed
+    assert set(user.groups.all()) == {
+        Group.objects.get(pk=settings.SITH_GROUP_PUBLIC_ID),
+        manageable_groups[0],
+        manageable_groups[1],
+        *hidden_groups[:3],
+    }

core/views/__init__.py

@@ -28,8 +28,7 @@ from django.http import (
     HttpResponseServerError,
 )
 from django.shortcuts import render
-from django.utils.functional import cached_property
+from django.views.generic.detail import BaseDetailView
-from django.views.generic.detail import SingleObjectMixin
 from django.views.generic.edit import FormView
 from sentry_sdk import last_event_id
 
@@ -54,17 +53,12 @@ def internal_servor_error(request):
     return HttpResponseServerError(render(request, "core/500.jinja"))
 
 
-class DetailFormView(SingleObjectMixin, FormView):
+class DetailFormView(FormView, BaseDetailView):
     """Class that allow both a detail view and a form view."""
 
-    def get_object(self):
+    def post(self, request, *args, **kwargs):
-        """Get current group from id in url."""
+        self.object = self.get_object()
-        return self.cached_object
+        return super().post(request, *args, **kwargs)
-
-    @cached_property
-    def cached_object(self):
-        """Optimisation on group retrieval."""
-        return super().get_object()
 
 
 # F403: those star-imports would be hellish to refactor

core/views/forms.py

@@ -28,6 +28,7 @@ from captcha.fields import CaptchaField
 from django import forms
 from django.conf import settings
 from django.contrib.auth.forms import AuthenticationForm, UserCreationForm
+from django.contrib.auth.models import Permission
 from django.contrib.staticfiles.management.commands.collectstatic import (
     staticfiles_storage,
 )
@@ -323,6 +324,19 @@ class UserGroupsForm(forms.ModelForm):
         model = User
         fields = ["groups"]
 
+    def save(self, *args, **kwargs) -> User:
+        # make the super method manage error without persisting in db
+        super().save(commit=False)
+        # Don't forget to add the non-manageable groups when setting groups,
+        # or the user would lose all of those when the form is submitted
+        self.instance.groups.set(
+            [
+                *self.cleaned_data["groups"],
+                *self.instance.groups.filter(is_manually_manageable=False),
+            ]
+        )
+        return self.instance
+
 
 class UserGodfathersForm(forms.Form):
     type = forms.ChoiceField(
@@ -427,3 +441,28 @@ class GiftForm(forms.ModelForm):
                 id=user_id
             )
             self.fields["user"].widget = forms.HiddenInput()
+
+
+class PermissionGroupsForm(forms.ModelForm):
+    """Manage the groups that have a specific permission."""
+
+    class Meta:
+        model = Permission
+        fields = []
+
+    groups = forms.ModelMultipleChoiceField(
+        Group.objects.all(),
+        label=_("Groups"),
+        widget=AutoCompleteSelectMultipleGroup,
+        required=False,
+    )
+
+    def __init__(self, instance: Permission, **kwargs):
+        super().__init__(instance=instance, **kwargs)
+        self.fields["groups"].initial = instance.group_set.all()
+
+    def save(self, commit: bool = True):  # noqa FTB001
+        instance = super().save(commit=False)
+        if commit:
+            instance.group_set.set(self.cleaned_data["groups"])
+        return instance

core/views/group.py

@@ -17,6 +17,10 @@
 
 from django import forms
 from django.contrib.auth.mixins import PermissionRequiredMixin
+from django.contrib.auth.models import Permission
+from django.contrib.messages.views import SuccessMessageMixin
+from django.core.exceptions import ImproperlyConfigured
+from django.shortcuts import get_object_or_404
 from django.urls import reverse_lazy
 from django.utils.translation import gettext_lazy as _
 from django.views.generic import ListView
@@ -25,6 +29,7 @@ from django.views.generic.edit import CreateView, DeleteView, UpdateView
 from core.auth.mixins import CanEditMixin
 from core.models import Group, User
 from core.views import DetailFormView
+from core.views.forms import PermissionGroupsForm
 from core.views.widgets.select import AutoCompleteSelectMultipleUser
 
 # Forms
@@ -130,3 +135,62 @@ class GroupDeleteView(CanEditMixin, DeleteView):
     pk_url_kwarg = "group_id"
     template_name = "core/delete_confirm.jinja"
     success_url = reverse_lazy("core:group_list")
+
+
+class PermissionGroupsUpdateView(
+    PermissionRequiredMixin, SuccessMessageMixin, UpdateView
+):
+    """Manage the groups that have a specific permission.
+
+    Notes:
+        This is an `UpdateView`, but unlike typical `UpdateView`,
+        it doesn't accept url arguments to retrieve the object
+        to update.
+        As such, a `PermissionGroupsUpdateView` can only deal with
+        a single hardcoded permission.
+
+        This is not a limitation, but an on-purpose design,
+        mainly for security matters.
+
+    Example:
+        ```python
+        class SubscriptionPermissionView(PermissionGroupsUpdateView):
+            permission = "subscription.add_subscription"
+        ```
+    """
+
+    permission_required = "auth.change_permission"
+    template_name = "core/edit.jinja"
+    form_class = PermissionGroupsForm
+    permission = None
+    success_message = _("Groups have been successfully updated.")
+
+    def get_object(self, *args, **kwargs):
+        if not self.permission:
+            raise ImproperlyConfigured(
+                f"{self.__class__.__name__} is missing the permission attribute. "
+                "Please fill it with either a permission string "
+                "or a Permission object."
+            )
+        if isinstance(self.permission, Permission):
+            return self.permission
+        if isinstance(self.permission, str):
+            try:
+                app_label, codename = self.permission.split(".")
+            except ValueError as e:
+                raise ValueError(
+                    "Permission name should be in the form "
+                    "app_label.permission_codename."
+                ) from e
+            return get_object_or_404(
+                Permission, codename=codename, content_type__app_label=app_label
+            )
+        raise TypeError(
+            f"{self.__class__.__name__}.permission "
+            f"must be a string or a permission instance."
+        )
+
+    def get_success_url(self):
+        # if children classes define a success url, return it,
+        # else stay on the same page
+        return self.success_url or self.request.path

core/views/user.py

@@ -66,7 +66,6 @@ from core.views.forms import (
 )
 from core.views.mixins import QuickNotifMixin, TabedViewMixin
 from counter.models import Refilling, Selling
-from counter.views.student_card import StudentCardFormView
 from eboutic.models import Invoice
 from subscription.models import Subscription
 from trombi.views import UserTrombiForm
@@ -566,6 +565,8 @@ class UserPreferencesView(UserTabsMixin, CanEditMixin, UpdateView):
         if not hasattr(self.object, "trombi_user"):
             kwargs["trombi_form"] = UserTrombiForm()
         if hasattr(self.object, "customer"):
+            from counter.views.student_card import StudentCardFormView
+
             kwargs["student_card_fragment"] = StudentCardFormView.get_template_data(
                 self.object.customer
             ).render(self.request)

counter/models.py

@@ -52,7 +52,8 @@ class CustomerQuerySet(models.QuerySet):
     def update_amount(self) -> int:
         """Update the amount of all customers selected by this queryset.
 
-        The result is given as the sum of all refills minus the sum of all purchases.
+        The result is given as the sum of all refills
+        minus the sum of all purchases paid with the AE account.
 
         Returns:
             The number of updated rows.
@@ -73,7 +74,9 @@ class CustomerQuerySet(models.QuerySet):
             .values("res")
         )
         money_out = Subquery(
-            Selling.objects.filter(customer=OuterRef("pk"))
+            Selling.objects.filter(
+                customer=OuterRef("pk"), payment_method="SITH_ACCOUNT"
+            )
             .values("customer_id")
             .annotate(res=Sum(F("unit_price") * F("quantity"), default=0))
             .values("res")

counter/tests/test_counter.py

@@ -937,13 +937,23 @@ class TestClubCounterClickAccess(TestCase):
         assert res.status_code == 403
 
     def test_board_member(self):
+        """By default, board members should be able to click on office counters"""
         baker.make(Membership, club=self.counter.club, user=self.user, role=3)
         self.client.force_login(self.user)
         res = self.client.get(self.click_url)
         assert res.status_code == 200
 
     def test_barman(self):
+        """Sellers should be able to click on office counters"""
         self.counter.sellers.add(self.user)
         self.client.force_login(self.user)
         res = self.client.get(self.click_url)
-        assert res.status_code == 403
+        assert res.status_code == 200
+
+    def test_both_barman_and_board_member(self):
+        """If the user is barman and board member, he should be authorized as well."""
+        self.counter.sellers.add(self.user)
+        baker.make(Membership, club=self.counter.club, user=self.user, role=3)
+        self.client.force_login(self.user)
+        res = self.client.get(self.click_url)
+        assert res.status_code == 200

counter/tests/test_customer.py

@@ -442,6 +442,7 @@ def test_update_balance():
             _quantity=len(customers),
             unit_price=10,
             quantity=1,
+            payment_method="SITH_ACCOUNT",
             _save_related=True,
         ),
         *sale_recipe.prepare(
@@ -449,10 +450,26 @@ def test_update_balance():
             _quantity=3,
             unit_price=5,
             quantity=2,
+            payment_method="SITH_ACCOUNT",
             _save_related=True,
         ),
         sale_recipe.prepare(
-            customer=customers[4], quantity=1, unit_price=50, _save_related=True
+            customer=customers[4],
+            quantity=1,
+            unit_price=50,
+            payment_method="SITH_ACCOUNT",
+            _save_related=True,
+        ),
+        *sale_recipe.prepare(
+            # all customers also bought products without using their AE account.
+            # All purchases made with another mean than the AE account should
+            # be ignored when updating the account balance.
+            customer=iter(customers),
+            _quantity=len(customers),
+            unit_price=50,
+            quantity=1,
+            payment_method="CARD",
+            _save_related=True,
         ),
     ]
     Selling.objects.bulk_create(sales)

counter/views/click.py

@@ -142,15 +142,16 @@ class CounterClick(CounterTabsMixin, CanViewMixin, SingleObjectMixin, FormView):
     """
 
     model = Counter
-    queryset = Counter.objects.annotate_is_open()
+    queryset = (
+        Counter.objects.exclude(type="EBOUTIC")
+        .annotate_is_open()
+        .select_related("club")
+    )
     form_class = BasketForm
     template_name = "counter/counter_click.jinja"
     pk_url_kwarg = "counter_id"
     current_tab = "counter"
 
-    def get_queryset(self):
-        return super().get_queryset().exclude(type="EBOUTIC").annotate_is_open()
-
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         kwargs["form_kwargs"] = {
@@ -168,9 +169,15 @@ class CounterClick(CounterTabsMixin, CanViewMixin, SingleObjectMixin, FormView):
             return redirect(obj)  # Redirect to counter
 
         if obj.type == "OFFICE" and (
-            obj.sellers.filter(pk=request.user.pk).exists()
+            request.user.is_anonymous
-            or not obj.club.has_rights_in_club(request.user)
+            or not (
+                obj.sellers.contains(request.user)
+                or obj.club.has_rights_in_club(request.user)
+            )
         ):
+            # To be able to click on an office counter,
+            # a user must either be in the board of the club that own the counter
+            # or a seller of this counter.
             raise PermissionDenied
 
         if obj.type == "BAR" and (

docs/tutorial/groups.md

@@ -228,3 +228,38 @@ Les groupes de ban existants sont les suivants :
 - `Banned from buying alcohol` : les utilisateurs interdits de vente d'alcool (non mineurs)
 - `Banned from counters` : les utilisateurs interdits d'utilisation des comptoirs
 - `Banned to subscribe` : les utilisateurs interdits de cotisation
+
+## Groupes liés à une permission
+
+Certaines actions sur le site demandent une permission en particulier,
+que l'on veut donner ou retirer n'importe quand.
+
+Prenons par exemple les cotisations : lors de l'intégration,
+on veut permettre aux membres du bureau de l'Integ
+de créer des cotisations, et pareil pour les membres du bureau 
+de la Welcome Week pendant cette dernière.
+
+Dans ces cas-là, il est pertinent de mettre à disposition
+des administrateurs du site une page leur permettant
+de gérer quels groupes ont une permission donnée.
+Pour ce faire, il existe 
+[PermissionGroupsUpdateView][core.views.PermissionGroupsUpdateView].
+
+Pour l'utiliser, il suffit de créer une vue qui en hérite
+et de lui dire quelle est la permission dont on veut gérer
+les groupes :
+
+```python
+from core.views.group import PermissionGroupsUpdateView
+
+
+class SubscriptionPermissionView(PermissionGroupsUpdateView):
+    permission = "subscription.add_subscription"
+```
+
+Configurez l'url de la vue, et c'est tout !
+La page ainsi générée contiendra un formulaire
+avec un unique champ permettant de sélectionner des groupes.
+Par défaut, seuls les utilisateurs avec la permission
+`auth.change_permission` auront accès à ce formulaire
+(donc, normalement, uniquement les utilisateurs Root).

locale/fr/LC_MESSAGES/django.po

@@ -6,7 +6,7 @@
 msgid ""
 msgstr ""
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2025-01-10 14:52+0100\n"
+"POT-Creation-Date: 2025-02-12 15:55+0100\n"
 "PO-Revision-Date: 2016-07-18\n"
 "Last-Translator: Maréchal <thomas.girod@utbm.fr\n"
 "Language-Team: AE info <ae.info@utbm.fr>\n"
@@ -1447,7 +1447,7 @@ msgid "News admin"
 msgstr "Administration des nouvelles"
 
 #: com/templates/com/news_admin_list.jinja com/templates/com/news_detail.jinja
-#: com/templates/com/news_list.jinja
+#: com/templates/com/news_list.jinja com/views.py
 msgid "News"
 msgstr "Nouvelles"
 
@@ -1525,6 +1525,10 @@ msgstr "Éditer (sera soumise de nouveau à la modération)"
 msgid "Edit news"
 msgstr "Éditer la nouvelle"
 
+#: com/templates/com/news_list.jinja
+msgid "News feed"
+msgstr "Flux d'actualités"
+
 #: com/templates/com/news_list.jinja
 msgid "Events today and the next few days"
 msgstr "Événements aujourd'hui et dans les prochains jours"
@@ -1767,6 +1771,10 @@ msgstr "Message d'alerte"
 msgid "Screens list"
 msgstr "Liste d'écrans"
 
+#: com/views.py
+msgid "All incoming events"
+msgstr "Tous les événements à venir"
+
 #: com/views.py
 msgid "Delete and save to regenerate"
 msgstr "Supprimer et sauver pour régénérer"
@@ -2375,11 +2383,10 @@ msgstr "Confirmation"
 msgid "Cancel"
 msgstr "Annuler"
 
-#: core/templates/core/edit.jinja core/templates/core/file_edit.jinja
+#: core/templates/core/edit.jinja
-#: counter/templates/counter/cash_register_summary.jinja
 #, python-format
-msgid "Edit %(obj)s"
+msgid "Edit %(name)s"
-msgstr "Éditer %(obj)s"
+msgstr "Éditer %(name)s"
 
 #: core/templates/core/file.jinja core/templates/core/file_list.jinja
 msgid "File list"
@@ -2449,6 +2456,12 @@ msgstr "octets"
 msgid "Download"
 msgstr "Télécharger"
 
+#: core/templates/core/file_edit.jinja
+#: counter/templates/counter/cash_register_summary.jinja
+#, python-format
+msgid "Edit %(obj)s"
+msgstr "Éditer %(obj)s"
+
 #: core/templates/core/file_list.jinja
 msgid "There is no file in this website."
 msgstr "Il n'y a pas de fichier sur ce site web."
@@ -2906,7 +2919,7 @@ msgstr "Blouse"
 msgid "Not subscribed"
 msgstr "Non cotisant"
 
-#: core/templates/core/user_detail.jinja
+#: core/templates/core/user_detail.jinja core/templates/core/user_tools.jinja
 #: subscription/templates/subscription/subscription.jinja
 msgid "New subscription"
 msgstr "Nouvelle cotisation"
@@ -3138,15 +3151,6 @@ msgstr "Supprimer les messages forum d'un utilisateur"
 msgid "Bans"
 msgstr "Bans"
 
-#: core/templates/core/user_tools.jinja
-msgid "Subscriptions"
-msgstr "Cotisations"
-
-#: core/templates/core/user_tools.jinja
-#: subscription/templates/subscription/stats.jinja
-msgid "Subscription stats"
-msgstr "Statistiques de cotisation"
-
 #: core/templates/core/user_tools.jinja counter/forms.py
 #: counter/views/mixins.py
 msgid "Counters"
@@ -3219,6 +3223,19 @@ msgstr "Modérer les fichiers"
 msgid "Moderate pictures"
 msgstr "Modérer les photos"
 
+#: core/templates/core/user_tools.jinja
+msgid "Subscriptions"
+msgstr "Cotisations"
+
+#: core/templates/core/user_tools.jinja
+msgid "Manage permissions"
+msgstr "Gérer les permissions"
+
+#: core/templates/core/user_tools.jinja
+#: subscription/templates/subscription/stats.jinja
+msgid "Subscription stats"
+msgstr "Statistiques de cotisation"
+
 #: core/templates/core/user_tools.jinja pedagogy/templates/pedagogy/guide.jinja
 msgid "Create UV"
 msgstr "Créer UV"
@@ -3347,6 +3364,10 @@ msgstr "Utilisateurs à ajouter au groupe"
 msgid "Users to remove from group"
 msgstr "Utilisateurs à retirer du groupe"
 
+#: core/views/group.py
+msgid "Groups have been successfully updated."
+msgstr "Les groupes ont été mis à jour avec succès."
+
 #: core/views/user.py
 msgid "We couldn't verify that this email actually exists"
 msgstr "Nous n'avons pas réussi à vérifier que cette adresse mail existe."
@@ -5668,6 +5689,10 @@ msgstr "Cotisations par type"
 msgid "Existing member"
 msgstr "Membre existant"
 
+#: subscription/views.py
+msgid "the groups that can create subscriptions"
+msgstr "les groupes pouvant créer des cotisations"
+
 #: trombi/models.py
 msgid "subscription deadline"
 msgstr "fin des inscriptions"

pedagogy/api.py

@@ -1,13 +1,13 @@
+import operator
 from typing import Annotated
 
 from annotated_types import Ge
-from django.conf import settings
 from ninja import Query
 from ninja_extra import ControllerBase, api_controller, paginate, route
 from ninja_extra.exceptions import NotFound
 from ninja_extra.pagination import PageNumberPaginationExtra, PaginatedResponseSchema
 
-from core.auth.api_permissions import IsInGroup, IsRoot, IsSubscriber
+from core.auth.api_permissions import HasPerm
 from pedagogy.models import UV
 from pedagogy.schemas import SimpleUvSchema, UvFilterSchema, UvSchema
 from pedagogy.utbm_api import find_uv
@@ -17,7 +17,11 @@ from pedagogy.utbm_api import find_uv
 class UvController(ControllerBase):
     @route.get(
         "/{year}/{code}",
-        permissions=[IsRoot | IsInGroup(settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)],
+        permissions=[
+            # this route will almost always be called in the context
+            # of a UV creation/edition
+            HasPerm(["pedagogy.add_uv", "pedagogy.change_uv"], op=operator.or_)
+        ],
         url_name="fetch_uv_from_utbm",
         response=UvSchema,
     )
@@ -34,8 +38,8 @@ class UvController(ControllerBase):
         "",
         response=PaginatedResponseSchema[SimpleUvSchema],
         url_name="fetch_uvs",
-        permissions=[IsSubscriber | IsInGroup(settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)],
+        permissions=[HasPerm("pedagogy.view_uv")],
     )
     @paginate(PageNumberPaginationExtra, page_size=100)
     def fetch_uv_list(self, search: Query[UvFilterSchema]):
-        return search.filter(UV.objects.all())
+        return search.filter(UV.objects.values())

pedagogy/models.py

@@ -20,10 +20,12 @@
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
+from typing import Self
 
 from django.conf import settings
 from django.core import validators
 from django.db import models
+from django.db.models import Exists, OuterRef
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.functional import cached_property
@@ -145,14 +147,6 @@ class UV(models.Model):
     def get_absolute_url(self):
         return reverse("pedagogy:uv_detail", kwargs={"uv_id": self.id})
 
-    def is_owned_by(self, user):
-        """Can be created by superuser, root or pedagogy admin user."""
-        return user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)
-
-    def can_be_viewed_by(self, user):
-        """Only visible by subscribers."""
-        return user.is_subscribed
-
     def __grade_average_generic(self, field):
         comments = self.comments.filter(**{field + "__gte": 0})
         if not comments.exists():
@@ -191,6 +185,22 @@ class UV(models.Model):
         return self.__grade_average_generic("grade_work_load")
 
 
+class UVCommentQuerySet(models.QuerySet):
+    def viewable_by(self, user: User) -> Self:
+        if user.has_perms(["pedagogy.view_uvcomment", "pedagogy.view_uvcommentreport"]):
+            # the user can view uv comment reports,
+            # so he can view non-moderated comments
+            return self
+        if user.has_perm("pedagogy.view_uvcomment"):
+            return self.filter(reports=None)
+        return self.filter(author=user)
+
+    def annotate_is_reported(self) -> Self:
+        return self.annotate(
+            is_reported=Exists(UVCommentReport.objects.filter(comment=OuterRef("pk")))
+        )
+
+
 class UVComment(models.Model):
     """A comment about an UV."""
 
@@ -243,6 +253,8 @@ class UVComment(models.Model):
     )
     publish_date = models.DateTimeField(_("publish date"), blank=True)
 
+    objects = UVCommentQuerySet.as_manager()
+
     def __str__(self):
         return f"{self.uv} - {self.author}"
 
@@ -251,15 +263,6 @@ class UVComment(models.Model):
             self.publish_date = timezone.now()
         super().save(*args, **kwargs)
 
-    def is_owned_by(self, user):
-        """Is owned by a pedagogy admin, a superuser or the author himself."""
-        return self.author == user or user.is_owner(self.uv)
-
-    @cached_property
-    def is_reported(self):
-        """Return True if someone reported this UV."""
-        return self.reports.exists()
-
 
 # TODO : it seems that some views were meant to be implemented
 #        to use this model.
@@ -323,7 +326,3 @@ class UVCommentReport(models.Model):
     @cached_property
     def uv(self):
         return self.comment.uv
-
-    def is_owned_by(self, user):
-        """Can be created by a pedagogy admin, a superuser or a subscriber."""
-        return user.is_subscribed or user.is_owner(self.comment.uv)

pedagogy/templates/pedagogy/guide.jinja

@@ -19,7 +19,7 @@
 {% endblock head %}
 
 {% block content %}
-  {% if can_create_uv %}
+  {% if user.has_perm("pedagogy.add_uv") %}
     <div class="action-bar">
       <p>
         <a href="{{ url('pedagogy:uv_create') }}">{% trans %}Create UV{% endtrans %}</a>
@@ -94,8 +94,10 @@
           <td>{% trans %}Credit type{% endtrans %}</td>
           <td><i class="fa fa-leaf"></i></td>
           <td><i class="fa-regular fa-sun"></i></td>
-          {% if can_create_uv %}
+          {%- if user.has_perm("pedagogy.change_uv") -%}
             <td>{% trans %}Edit{% endtrans %}</td>
+          {%- endif -%}
+          {%- if user.has_perm("pedagogy.delete_uv") -%}
             <td>{% trans %}Delete{% endtrans %}</td>
           {% endif %}
         </tr>
@@ -109,8 +111,10 @@
             <td x-text="uv.credit_type"></td>
             <td><i :class="uv.semester.includes('AUTUMN') && 'fa fa-leaf'"></i></td>
             <td><i :class="uv.semester.includes('SPRING') && 'fa-regular fa-sun'"></i></td>
-            {% if can_create_uv -%}
+            {%- if user.has_perm("pedagogy.change_uv") -%}
               <td><a :href="`/pedagogy/uv/${uv.id}/edit`">{% trans %}Edit{% endtrans %}</a></td>
+            {%- endif -%}
+            {%- if user.has_perm("pedagogy.delete_uv") -%}
               <td><a :href="`/pedagogy/uv/${uv.id}/delete`">{% trans %}Delete{% endtrans %}</a></td>
             {%- endif -%}
           </tr>

pedagogy/templates/pedagogy/uv_detail.jinja

@@ -89,7 +89,7 @@
         <div id="leave_comment_not_allowed">
           <p>{% trans %}You already posted a comment on this UV. If you want to comment again, please modify or delete your previous comment.{% endtrans %}</p>
         </div>
-      {% else %}
+      {% elif user.has_perm("pedagogy.add_uvcomment") %}
         <div id="leave_comment">
           <h2>{% trans %}Leave comment{% endtrans %}</h2>
           <div>
@@ -146,9 +146,9 @@
       {% endif %}
       <br>
 
-      {% if object.comments.exists() %}
+      {% if comments %}
         <h2>{% trans %}Comments{% endtrans %}</h2>
-        {% for comment in object.comments.order_by("-publish_date").all() %}
+        {% for comment in comments %}
           <div id="{{ comment.id }}" class="comment-container">
 
             <div class="grade-block">
@@ -183,16 +183,28 @@
                 </p>
               {% endif %}
 
-              {% if user.is_owner(comment) %}
+              {% if comment.author_id == user.id or user.has_perm("pedagogy.change_comment") %}
                 <p class="actions">
-                  <a href="{{ url('pedagogy:comment_update', comment_id=comment.id) }}">{% trans %}Edit{% endtrans %}</a>
+                  <a href="{{ url('pedagogy:comment_update', comment_id=comment.id) }}">
-                  <a href="{{ url('pedagogy:comment_delete', comment_id=comment.id) }}">{% trans %}Delete{% endtrans %}</a>
+                    {% trans %}Edit{% endtrans %}
+                  </a>
+              {% endif %}
+              {% if comment.author_id == user.id or user.has_perm("pedagogy.delete_comment") %}
+                <a href="{{ url('pedagogy:comment_delete', comment_id=comment.id) }}">
+                  {% trans %}Delete{% endtrans %}
+                </a>
                 </p>
               {% endif %}
             </div>
 
             <div class="comment-end-bar">
-              <div class="report"><p><a href="{{ url('pedagogy:comment_report', comment_id=comment.id) }}">{% trans %}Report this comment{% endtrans %}</a></p></div>
+              <div class="report">
+                <p>
+                  <a href="{{ url('pedagogy:comment_report', comment_id=comment.id) }}">
+                    {% trans %}Report this comment{% endtrans %}
+                  </a>
+                </p>
+              </div>
 
               <div class="date"><p>{{ comment.publish_date.strftime('%d/%m/%Y') }}</p></div>
 
@@ -209,7 +221,7 @@
   <script type="text/javascript">
     $("#return_noscript").hide();
     $("#return_js").show();
-    var icons = {
+    const icons = {
       header: "fa fa-toggle-right",
       activeHeader: "fa fa-toggle-down"
     };

pedagogy/tests/tests.py

@@ -20,14 +20,18 @@
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
+from typing import Callable
 
 import pytest
 from django.conf import settings
+from django.contrib.auth.models import Permission
 from django.test import Client, TestCase
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
+from model_bakery import baker
 from pytest_django.asserts import assertRedirects
 
+from core.baker_recipes import old_subscriber_user, subscriber_user
 from core.models import Notification, User
 from pedagogy.models import UV, UVComment, UVCommentReport
 
@@ -144,17 +148,17 @@ class TestUVCreation(TestCase):
 
 @pytest.mark.django_db
 @pytest.mark.parametrize(
-    ("username", "expected_code"),
+    ("user_factory", "expected_code"),
     [
-        ("root", 200),
+        (subscriber_user.make, 200),
-        ("tutu", 200),
+        (old_subscriber_user.make, 200),
-        ("sli", 200),
+        (lambda: baker.make(User), 403),
-        ("old_subscriber", 200),
-        ("public", 403),
     ],
 )
-def test_guide_permissions(client: Client, username: str, expected_code: int):
+def test_guide_permissions(
-    client.force_login(User.objects.get(username=username))
+    client: Client, user_factory: Callable[[], User], expected_code: int
+):
+    client.force_login(user_factory())
     res = client.get(reverse("pedagogy:guide"))
     assert res.status_code == expected_code
 
@@ -190,17 +194,12 @@ class TestUVDelete(TestCase):
     def test_uv_delete_pedagogy_unauthorized_fail(self):
         # Anonymous user
         response = self.client.post(self.delete_uv_url)
-        assert response.status_code == 403
+        assertRedirects(response, reverse("core:login") + f"?next={self.delete_uv_url}")
         assert UV.objects.filter(pk=self.uv.pk).exists()
 
-        # Not subscribed user
+        for user in baker.make(User), subscriber_user.make():
-        self.client.force_login(self.guy)
+            with self.subTest():
-        response = self.client.post(self.delete_uv_url)
+                self.client.force_login(user)
-        assert response.status_code == 403
-        assert UV.objects.filter(pk=self.uv.pk).exists()
-
-        # Simply subscribed user
-        self.client.force_login(self.sli)
                 response = self.client.post(self.delete_uv_url)
                 assert response.status_code == 403
                 assert UV.objects.filter(pk=self.uv.pk).exists()
@@ -249,7 +248,7 @@ class TestUVUpdate(TestCase):
         response = self.client.post(
             self.update_uv_url, create_uv_template(self.bibou.id, code="PA00")
         )
-        assert response.status_code == 403
+        assertRedirects(response, reverse("core:login") + f"?next={self.update_uv_url}")
 
         # Not subscribed user
         self.client.force_login(self.guy)
@@ -312,7 +311,7 @@ class TestUVCommentCreationAndDisplay(TestCase):
         response = self.client.post(
             self.uv_url, create_uv_comment_template(self.bibou.id)
         )
-        self.assertRedirects(response, self.uv_url)
+        assertRedirects(response, self.uv_url)
         response = self.client.get(self.uv_url)
         self.assertContains(response, text="Superbe UV")
 
@@ -338,7 +337,7 @@ class TestUVCommentCreationAndDisplay(TestCase):
         nb_comments = self.uv.comments.count()
         # Test with anonymous user
         response = self.client.post(self.uv_url, create_uv_comment_template(0))
-        assert response.status_code == 403
+        assertRedirects(response, reverse("core:login") + f"?next={self.uv_url}")
 
         # Test with non subscribed user
         self.client.force_login(self.guy)
@@ -405,61 +404,34 @@ class TestUVCommentDelete(TestCase):
 
     @classmethod
     def setUpTestData(cls):
-        cls.bibou = User.objects.get(username="root")
+        cls.comment = baker.make(UVComment)
-        cls.tutu = User.objects.get(username="tutu")
-        cls.sli = User.objects.get(username="sli")
-        cls.guy = User.objects.get(username="guy")
-        cls.krophil = User.objects.get(username="krophil")
 
-    def setUp(self):
+    def test_uv_comment_delete_success(self):
-        comment_kwargs = create_uv_comment_template(
+        url = reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
-            User.objects.get(username="krophil").id
+        for user in (
-        )
+            baker.make(User, is_superuser=True),
-        comment_kwargs["author"] = User.objects.get(id=comment_kwargs["author"])
+            baker.make(
-        comment_kwargs["uv"] = UV.objects.get(id=comment_kwargs["uv"])
+                User, user_permissions=[Permission.objects.get(codename="view_uv")]
-        self.comment = UVComment(**comment_kwargs)
+            ),
-        self.comment.save()
+            self.comment.author,
-
+        ):
-    def test_uv_comment_delete_root_success(self):
+            with self.subTest():
-        self.client.force_login(self.bibou)
+                self.client.force_login(user)
-        self.client.post(
+                self.client.post(url)
-            reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
-        )
-        assert not UVComment.objects.filter(id=self.comment.id).exists()
-
-    def test_uv_comment_delete_pedagogy_admin_success(self):
-        self.client.force_login(self.tutu)
-        self.client.post(
-            reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
-        )
-        assert not UVComment.objects.filter(id=self.comment.id).exists()
-
-    def test_uv_comment_delete_author_success(self):
-        self.client.force_login(self.krophil)
-        self.client.post(
-            reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
-        )
                 assert not UVComment.objects.filter(id=self.comment.id).exists()
 
     def test_uv_comment_delete_unauthorized_fail(self):
+        url = reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
+
         # Anonymous user
-        response = self.client.post(
+        response = self.client.post(url)
-            reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
+        assertRedirects(response, reverse("core:login") + f"?next={url}")
-        )
-        assert response.status_code == 403
 
         # Unsbscribed user
-        self.client.force_login(self.guy)
+        for user in baker.make(User), subscriber_user.make():
-        response = self.client.post(
+            with self.subTest():
-            reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
+                self.client.force_login(user)
-        )
+                response = self.client.post(url)
-        assert response.status_code == 403
-
-        # Subscribed user (not author of the comment)
-        self.client.force_login(self.sli)
-        response = self.client.post(
-            reverse("pedagogy:comment_delete", kwargs={"comment_id": self.comment.id})
-        )
                 assert response.status_code == 403
 
         # Check that the comment still exists
@@ -499,16 +471,6 @@ class TestUVCommentUpdate(TestCase):
         self.comment.refresh_from_db()
         self.assertEqual(self.comment.comment, self.comment_edit["comment"])
 
-    def test_uv_comment_update_pedagogy_admin_success(self):
-        self.client.force_login(self.tutu)
-        response = self.client.post(
-            reverse("pedagogy:comment_update", kwargs={"comment_id": self.comment.id}),
-            self.comment_edit,
-        )
-        assert response.status_code == 302
-        self.comment.refresh_from_db()
-        self.assertEqual(self.comment.comment, self.comment_edit["comment"])
-
     def test_uv_comment_update_author_success(self):
         self.client.force_login(self.krophil)
         response = self.client.post(
@@ -520,25 +482,18 @@ class TestUVCommentUpdate(TestCase):
         self.assertEqual(self.comment.comment, self.comment_edit["comment"])
 
     def test_uv_comment_update_unauthorized_fail(self):
+        url = reverse("pedagogy:comment_update", kwargs={"comment_id": self.comment.id})
         # Anonymous user
-        response = self.client.post(
+        response = self.client.post(url, self.comment_edit)
-            reverse("pedagogy:comment_update", kwargs={"comment_id": self.comment.id}),
+        assertRedirects(response, reverse("core:login") + f"?next={url}")
-            self.comment_edit,
-        )
-        assert response.status_code == 403
 
         # Unsbscribed user
-        response = self.client.post(
+        self.client.force_login(baker.make(User))
-            reverse("pedagogy:comment_update", kwargs={"comment_id": self.comment.id}),
+        response = self.client.post(url, self.comment_edit)
-            self.comment_edit,
-        )
         assert response.status_code == 403
 
         # Subscribed user (not author of the comment)
-        response = self.client.post(
+        response = self.client.post(url, self.comment_edit)
-            reverse("pedagogy:comment_update", kwargs={"comment_id": self.comment.id}),
-            self.comment_edit,
-        )
         assert response.status_code == 403
 
         # Check that the comment hasn't change
@@ -611,18 +566,19 @@ class TestUVModerationForm(TestCase):
         assert response.status_code == 200
 
     def test_access_unauthorized_fail(self):
+        url = reverse("pedagogy:moderation")
         # Test with anonymous user
-        response = self.client.get(reverse("pedagogy:moderation"))
+        response = self.client.get(url)
-        assert response.status_code == 403
+        assertRedirects(response, reverse("core:login") + f"?next={url}")
 
         # Test with unsubscribed user
         self.client.force_login(self.guy)
-        response = self.client.get(reverse("pedagogy:moderation"))
+        response = self.client.get(url)
         assert response.status_code == 403
 
         # Test with subscribed user
         self.client.force_login(self.sli)
-        response = self.client.get(reverse("pedagogy:moderation"))
+        response = self.client.get(url)
         assert response.status_code == 403
 
     def test_do_nothing(self):

pedagogy/views.py

@@ -22,8 +22,7 @@
 #
 
 from django.conf import settings
-from django.contrib.auth.mixins import LoginRequiredMixin, PermissionRequiredMixin
+from django.contrib.auth.mixins import PermissionRequiredMixin
-from django.core.exceptions import PermissionDenied
 from django.db.models import Exists, OuterRef
 from django.shortcuts import get_object_or_404
 from django.urls import reverse, reverse_lazy
@@ -35,7 +34,7 @@ from django.views.generic import (
     UpdateView,
 )
 
-from core.auth.mixins import CanEditPropMixin, CanViewMixin, FormerSubscriberMixin
+from core.auth.mixins import PermissionOrAuthorRequiredMixin
 from core.models import Notification, User
 from core.views import DetailFormView
 from pedagogy.forms import (
@@ -47,7 +46,7 @@ from pedagogy.forms import (
 from pedagogy.models import UV, UVComment, UVCommentReport
 
 
-class UVDetailFormView(CanViewMixin, DetailFormView):
+class UVDetailFormView(PermissionRequiredMixin, DetailFormView):
     """Display every comment of an UV and detailed infos about it.
 
     Allow to comment the UV.
@@ -57,11 +56,21 @@ class UVDetailFormView(CanViewMixin, DetailFormView):
     pk_url_kwarg = "uv_id"
     template_name = "pedagogy/uv_detail.jinja"
     form_class = UVCommentForm
+    permission_required = "pedagogy.view_uv"
+
+    def has_permission(self):
+        if self.request.method == "POST" and not self.request.user.has_perm(
+            "pedagogy.add_uvcomment"
+        ):
+            # if it's a POST request, the user is trying to add a new UVComment
+            # thus he also needs the "add_uvcomment" permission
+            return False
+        return super().has_permission()
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         kwargs["author_id"] = self.request.user.id
-        kwargs["uv_id"] = self.get_object().id
+        kwargs["uv_id"] = self.object.id
         kwargs["is_creation"] = True
         return kwargs
 
@@ -69,66 +78,61 @@ class UVDetailFormView(CanViewMixin, DetailFormView):
         form.save()
         return super().form_valid(form)
 
-    def get_success_url(self):
-        return reverse_lazy(
-            "pedagogy:uv_detail", kwargs={"uv_id": self.get_object().id}
-        )
-
     def get_context_data(self, **kwargs):
-        user = self.request.user
         return super().get_context_data(**kwargs) | {
-            "can_create_uv": (
+            "comments": list(
-                user.is_root
+                self.object.comments.viewable_by(self.request.user)
-                or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)
+                .annotate_is_reported()
+                .select_related("author")
+                .order_by("-publish_date")
             )
         }
 
+    def get_success_url(self):
+        # once the new uv comment has been saved
+        # redirect to the same page we are currently
+        return self.request.path
 
-class UVCommentUpdateView(CanEditPropMixin, UpdateView):
+
+class UVCommentUpdateView(PermissionOrAuthorRequiredMixin, UpdateView):
     """Allow edit of a given comment."""
 
     model = UVComment
     form_class = UVCommentForm
     pk_url_kwarg = "comment_id"
     template_name = "core/edit.jinja"
+    permission_required = "pedagogy.change_uvcomment"
+    author_field = "author"
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
-        obj = self.get_object()
+        kwargs["author_id"] = self.object.author_id
-        kwargs["author_id"] = obj.author.id
+        kwargs["uv_id"] = self.object.uv_id
-        kwargs["uv_id"] = obj.uv.id
         kwargs["is_creation"] = False
-
         return kwargs
 
     def get_success_url(self):
-        return reverse_lazy("pedagogy:uv_detail", kwargs={"uv_id": self.object.uv.id})
+        return reverse("pedagogy:uv_detail", kwargs={"uv_id": self.object.uv_id})
 
 
-class UVCommentDeleteView(CanEditPropMixin, DeleteView):
+class UVCommentDeleteView(PermissionOrAuthorRequiredMixin, DeleteView):
     """Allow delete of a given comment."""
 
     model = UVComment
     pk_url_kwarg = "comment_id"
     template_name = "core/delete_confirm.jinja"
+    permission_required = "pedagogy.delete_uvcomment"
+    author_field = "author"
 
     def get_success_url(self):
-        return reverse_lazy("pedagogy:uv_detail", kwargs={"uv_id": self.object.uv.id})
+        return reverse("pedagogy:uv_detail", kwargs={"uv_id": self.object.uv_id})
 
 
-class UVGuideView(LoginRequiredMixin, FormerSubscriberMixin, TemplateView):
+class UVGuideView(PermissionRequiredMixin, TemplateView):
     """UV guide main page."""
 
     template_name = "pedagogy/guide.jinja"
-
+    permission_required = "pedagogy.view_uv"
-    def get_context_data(self, **kwargs):
-        user = self.request.user
-        return super().get_context_data(**kwargs) | {
-            "can_create_uv": (
-                user.is_root
-                or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID)
-            )
-        }
 
 
 class UVCommentReportCreateView(PermissionRequiredMixin, CreateView):
@@ -168,21 +172,16 @@ class UVCommentReportCreateView(PermissionRequiredMixin, CreateView):
         return resp
 
     def get_success_url(self):
-        return reverse_lazy(
+        return reverse("pedagogy:uv_detail", kwargs={"uv_id": self.uv_comment.uv_id})
-            "pedagogy:uv_detail", kwargs={"uv_id": self.uv_comment.uv.id}
-        )
 
 
-class UVModerationFormView(FormView):
+class UVModerationFormView(PermissionRequiredMixin, FormView):
     """Moderation interface (Privileged)."""
 
     form_class = UVCommentModerationForm
     template_name = "pedagogy/moderation.jinja"
-
+    permission_required = "pedagogy.delete_uvcomment"
-    def dispatch(self, request, *args, **kwargs):
+    success_url = reverse_lazy("pedagogy:moderation")
-        if not request.user.is_owner(UV()):
-            raise PermissionDenied
-        return super().dispatch(request, *args, **kwargs)
 
     def form_valid(self, form):
         form_clean = form.clean()
@@ -194,9 +193,6 @@ class UVModerationFormView(FormView):
             UVCommentReport.objects.filter(id__in={d.id for d in denied}).delete()
         return super().form_valid(form)
 
-    def get_success_url(self):
-        return reverse_lazy("pedagogy:moderation")
-
 
 class UVCreateView(PermissionRequiredMixin, CreateView):
     """Add a new UV (Privileged)."""
@@ -211,34 +207,28 @@ class UVCreateView(PermissionRequiredMixin, CreateView):
         kwargs["author_id"] = self.request.user.id
         return kwargs
 
-    def get_success_url(self):
-        return reverse_lazy("pedagogy:uv_detail", kwargs={"uv_id": self.object.id})
 
-
+class UVDeleteView(PermissionRequiredMixin, DeleteView):
-class UVDeleteView(CanEditPropMixin, DeleteView):
     """Allow to delete an UV (Privileged)."""
 
     model = UV
     pk_url_kwarg = "uv_id"
     template_name = "core/delete_confirm.jinja"
-
+    permission_required = "pedagogy.delete_uv"
-    def get_success_url(self):
+    success_url = reverse_lazy("pedagogy:guide")
-        return reverse_lazy("pedagogy:guide")
 
 
-class UVUpdateView(CanEditPropMixin, UpdateView):
+class UVUpdateView(PermissionRequiredMixin, UpdateView):
     """Allow to edit an UV (Privilegied)."""
 
     model = UV
     form_class = UVForm
     pk_url_kwarg = "uv_id"
     template_name = "pedagogy/uv_edit.jinja"
+    permission_required = "pedagogy.change_uv"
 
     def get_form_kwargs(self):
         kwargs = super().get_form_kwargs()
         obj = self.get_object()
-        kwargs["author_id"] = obj.author.id
+        kwargs["author_id"] = obj.author_id
         return kwargs
-
-    def get_success_url(self):
-        return reverse_lazy("pedagogy:uv_detail", kwargs={"uv_id": self.object.id})

sith/settings.py

@@ -517,14 +517,6 @@ SITH_PRODUCT_SUBSCRIPTION_ONE_SEMESTER = 1
 SITH_PRODUCT_SUBSCRIPTION_TWO_SEMESTERS = 2
 SITH_PRODUCTTYPE_SUBSCRIPTION = 2
 
-# Defines which club lets its member the ability to make subscriptions
-# Elements of this list are club's id
-SITH_CAN_CREATE_SUBSCRIPTIONS = [1]
-
-# Defines which clubs lets its members the ability to see users subscription history
-# Elements of this list are club's id
-SITH_CAN_READ_SUBSCRIPTION_HISTORY = []
-
 # Number of weeks before the end of a subscription when the subscriber can resubscribe
 SITH_SUBSCRIPTION_END = 10
 

subscription/tests/test_new_susbcription.py

@@ -5,6 +5,7 @@ from typing import Callable
 
 import pytest
 from dateutil.relativedelta import relativedelta
+from django.contrib.auth.models import Permission
 from django.test import Client
 from django.urls import reverse
 from django.utils.timezone import localdate
@@ -108,7 +109,12 @@ def test_page_access(
 
 @pytest.mark.django_db
 def test_submit_form_existing_user(client: Client, settings: SettingsWrapper):
-    client.force_login(board_user.make())
+    client.force_login(
+        baker.make(
+            User,
+            user_permissions=Permission.objects.filter(codename="add_subscription"),
+        )
+    )
     user = old_subscriber_user.make()
     response = client.post(
         reverse("subscription:fragment-existing-user"),
@@ -133,7 +139,12 @@ def test_submit_form_existing_user(client: Client, settings: SettingsWrapper):
 
 @pytest.mark.django_db
 def test_submit_form_new_user(client: Client, settings: SettingsWrapper):
-    client.force_login(board_user.make())
+    client.force_login(
+        baker.make(
+            User,
+            user_permissions=Permission.objects.filter(codename="add_subscription"),
+        )
+    )
     response = client.post(
         reverse("subscription:fragment-new-user"),
         {

subscription/tests/test_permissions.py

@@ -0,0 +1,43 @@
+from django.contrib.auth.models import Permission
+from django.test import TestCase
+from django.urls import reverse
+from model_bakery import baker
+from pytest_django.asserts import assertRedirects
+
+from club.models import Club, Membership
+from core.baker_recipes import subscriber_user
+from core.models import User
+
+
+class TestSubscriptionPermission(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.user: User = subscriber_user.make()
+        cls.admin = baker.make(User, is_superuser=True)
+        cls.club = baker.make(Club)
+        baker.make(Membership, user=cls.user, club=cls.club, role=7)
+
+    def test_give_permission(self):
+        self.client.force_login(self.admin)
+        response = self.client.post(
+            reverse("subscription:perms"), {"groups": [self.club.board_group_id]}
+        )
+        assertRedirects(response, reverse("subscription:perms"))
+        assert self.user.has_perm("subscription.add_subscription")
+
+    def test_remove_permission(self):
+        self.client.force_login(self.admin)
+        response = self.client.post(reverse("subscription:perms"), {"groups": []})
+        assertRedirects(response, reverse("subscription:perms"))
+        assert not self.user.has_perm("subscription.add_subscription")
+
+    def test_subscription_page_access(self):
+        self.client.force_login(self.user)
+        response = self.client.get(reverse("subscription:subscription"))
+        assert response.status_code == 403
+
+        self.club.board_group.permissions.add(
+            Permission.objects.get(codename="add_subscription")
+        )
+        response = self.client.get(reverse("subscription:subscription"))
+        assert response.status_code == 200

subscription/urls.py

@@ -20,6 +20,7 @@ from subscription.views import (
     CreateSubscriptionNewUserFragment,
     NewSubscription,
     SubscriptionCreatedFragment,
+    SubscriptionPermissionView,
     SubscriptionsStatsView,
 )
 
@@ -41,5 +42,10 @@ urlpatterns = [
         SubscriptionCreatedFragment.as_view(),
         name="creation-success",
     ),
+    path(
+        "perms/",
+        SubscriptionPermissionView.as_view(),
+        name="perms",
+    ),
     path("stats/", SubscriptionsStatsView.as_view(), name="stats"),
 ]

subscription/views.py

@@ -14,13 +14,15 @@
 #
 
 from django.conf import settings
-from django.contrib.auth.mixins import UserPassesTestMixin
+from django.contrib.auth.mixins import PermissionRequiredMixin
 from django.core.exceptions import PermissionDenied
 from django.urls import reverse, reverse_lazy
 from django.utils.timezone import localdate
+from django.utils.translation import gettext_lazy as _
 from django.views.generic import CreateView, DetailView, TemplateView
 from django.views.generic.edit import FormView
 
+from core.views.group import PermissionGroupsUpdateView
 from counter.apps import PAYMENT_METHOD
 from subscription.forms import (
     SelectionDateForm,
@@ -30,13 +32,9 @@ from subscription.forms import (
 from subscription.models import Subscription
 
 
-class CanCreateSubscriptionMixin(UserPassesTestMixin):
+class NewSubscription(PermissionRequiredMixin, TemplateView):
-    def test_func(self):
-        return self.request.user.can_create_subscription
-
-
-class NewSubscription(CanCreateSubscriptionMixin, TemplateView):
     template_name = "subscription/subscription.jinja"
+    permission_required = "subscription.add_subscription"
 
     def get_context_data(self, **kwargs):
         return super().get_context_data(**kwargs) | {
@@ -49,8 +47,9 @@ class NewSubscription(CanCreateSubscriptionMixin, TemplateView):
         }
 
 
-class CreateSubscriptionFragment(CanCreateSubscriptionMixin, CreateView):
+class CreateSubscriptionFragment(PermissionRequiredMixin, CreateView):
     template_name = "subscription/fragments/creation_form.jinja"
+    permission_required = "subscription.add_subscription"
 
     def get_success_url(self):
         return reverse(
@@ -72,13 +71,21 @@ class CreateSubscriptionNewUserFragment(CreateSubscriptionFragment):
     extra_context = {"post_url": reverse_lazy("subscription:fragment-new-user")}
 
 
-class SubscriptionCreatedFragment(CanCreateSubscriptionMixin, DetailView):
+class SubscriptionCreatedFragment(PermissionRequiredMixin, DetailView):
     template_name = "subscription/fragments/creation_success.jinja"
+    permission_required = "subscription.add_subscription"
     model = Subscription
     pk_url_kwarg = "subscription_id"
     context_object_name = "subscription"
 
 
+class SubscriptionPermissionView(PermissionGroupsUpdateView):
+    """Manage the groups that have access to the subscription creation page."""
+
+    permission = "subscription.add_subscription"
+    extra_context = {"object_name": _("the groups that can create subscriptions")}
+
+
 class SubscriptionsStatsView(FormView):
     template_name = "subscription/stats.jinja"
     form_class = SelectionDateForm