diff --git a/.github/actions/setup_project/action.yml b/.github/actions/setup_project/action.yml index 909bf757..2d2aae89 100644 --- a/.github/actions/setup_project/action.yml +++ b/.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 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a436879..57f36d6f 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/club/views.py b/club/views.py index de5ccaee..767f5788 100644 --- a/club/views.py +++ b/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( - "club:club_members", kwargs={"club_id": self.get_object().id} - ) + return reverse_lazy("club:club_members", kwargs={"club_id": self.object.id}) class ClubOldMembersView(ClubTabsMixin, CanViewMixin, DetailView): diff --git a/com/static/bundled/com/components/ics-calendar-index.ts b/com/static/bundled/com/components/ics-calendar-index.ts index 11a15a32..3c78f98f 100644 --- a/com/static/bundled/com/components/ics-calendar-index.ts +++ b/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", }, ], diff --git a/com/static/com/components/ics-calendar.scss b/com/static/com/components/ics-calendar.scss index 21aa55d7..6c86cce0 100644 --- a/com/static/com/components/ics-calendar.scss +++ b/com/static/com/components/ics-calendar.scss @@ -75,10 +75,10 @@ ics-calendar { } td { - overflow-x: visible; // Show events on multiple days + overflow: visible; // Show events on multiple days } - //Reset from style.scss + //Reset from style.scss table { box-shadow: none; border-radius: 0px; @@ -86,13 +86,13 @@ ics-calendar { margin: 0px; } - // Reset from style.scss + // Reset from style.scss thead { background-color: white; color: black; } - // Reset from style.scss + // Reset from style.scss tbody>tr { &:nth-child(even):not(.highlight) { background: white; diff --git a/com/static/com/css/news-list.scss b/com/static/com/css/news-list.scss index bcbf8273..d073c4ac 100644 --- a/com/static/com/css/news-list.scss +++ b/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) { diff --git a/com/templates/com/news_list.jinja b/com/templates/com/news_list.jinja index 168a95b4..0f1f4301 100644 --- a/com/templates/com/news_list.jinja +++ b/com/templates/com/news_list.jinja @@ -8,6 +8,9 @@ {% block additional_css %} + + {# Atom feed discovery, not really css but also goes there #} + {% endblock %} {% block additional_js %} @@ -19,7 +22,10 @@
{% 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') %} -

{% trans %}Events today and the next few days{% endtrans %}

+

+ {% trans %}Events today and the next few days{% endtrans %} + +

{% if user.is_authenticated and (user.is_com_admin or user.memberships.board().ongoing().exists()) %} @@ -73,7 +79,10 @@
{% endif %} -

{% trans %}All coming events{% endtrans %}

+

+ {% trans %}All coming events{% endtrans %} + +

diff --git a/com/tests/test_views.py b/com/tests/test_views.py index 100a83ef..f80839ab 100644 --- a/com/tests/test_views.py +++ b/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" diff --git a/com/urls.py b/com/urls.py index 592e653b..8afbfd12 100644 --- a/com/urls.py +++ b/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//edit/", NewsUpdateView.as_view(), name="news_edit"), diff --git a/com/views.py b/com/views.py index a6faf214..0ab8fc1c 100644 --- a/com/views.py +++ b/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 diff --git a/core/auth/api_permissions.py b/core/auth/api_permissions.py index 4d83143e..6a28f13c 100644 --- a/core/auth/api_permissions.py +++ b/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.""" diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 5e0f099d..53638699 100644 --- a/core/management/commands/populate.py +++ b/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( - "pk", flat=True - ) + perms.filter(content_type__app_label="pedagogy") + .exclude(codename__in=["change_uvcomment"]) + .values_list("pk", flat=True) ) ) self.reset_index("core", "auth") diff --git a/core/models.py b/core/models.py index b1caa912..4748f311 100644 --- a/core/models.py +++ b/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 diff --git a/core/templates/core/edit.jinja b/core/templates/core/edit.jinja index 25c6bd74..82c7a035 100644 --- a/core/templates/core/edit.jinja +++ b/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 %} - {% trans obj=object %}Edit {{ obj }}{% endtrans %} + {% if object_name %} + {% trans name=object_name %}Edit {{ name }}{% endtrans %} {% else %} {% trans %}Save{% endtrans %} {% endif %} {% endblock %} {% block content %} - {% if object %} -

{% trans obj=object %}Edit {{ obj }}{% endtrans %}

+ {% if object_name %} +

{% trans name=object_name %}Edit {{ name }}{% endtrans %}

{% else %}

{% trans %}Save{% endtrans %}

{% endif %} + {% if messages %} +
+ + {% for message in messages %} + {% if message.level_tag == "success" %} + {{ message }} + {% endif %} + {% endfor %} + + + + +
+ {% endif %}
{% csrf_token %} {{ form.as_p() }} diff --git a/core/templates/core/user_detail.jinja b/core/templates/core/user_detail.jinja index cb93b1cd..5fceb126 100644 --- a/core/templates/core/user_detail.jinja +++ b/core/templates/core/user_detail.jinja @@ -166,7 +166,7 @@ {% endif %}
-{% 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")) %}
@@ -197,9 +197,9 @@
+
{% endif %} -
{% if user.is_root or user.is_board_member %} diff --git a/core/templates/core/user_tools.jinja b/core/templates/core/user_tools.jinja index 177b4199..4c9b9462 100644 --- a/core/templates/core/user_tools.jinja +++ b/core/templates/core/user_tools.jinja @@ -13,7 +13,7 @@

{% trans %}User Tools{% endtrans %}

- {% 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 %}

{% trans %}Sith management{% endtrans %}

@@ -42,152 +42,202 @@ {% set is_admin_on_a_counter = true %} {% endfor %} - {% if - is_admin_on_a_counter - or user.is_root - or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) - %} + {% if is_admin_on_a_counter or user.is_root or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) %} +
+

{% trans %}Counters{% endtrans %}

+ + +
+ {% endif %} + + {% if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} +
+

{% trans %}Accounting{% endtrans %}

+ +
+ {% endif %} + + {% if user.is_root or user.is_com_admin or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) %} + + {% endif %} + + {% if user.has_perm("subscription.add_subscription") or user.has_perm("auth.change_perm") or user.is_root or user.is_board_member %} +
+

{% trans %}Subscriptions{% endtrans %}

+ +
+ {% endif %} + + {% if user.memberships.filter(end_date=None).all().count() > 0 %} +
+

{% trans %}Club tools{% endtrans %}

+
    + {% for m in user.memberships.filter(end_date=None).all() %} +
  • {{ m.club }}
    • + {% endfor %} +
+
+ {% endif %} + + {% if user.has_perm("pedagogy.add_uv") or user.has_perm("pedagogy.delete_uvcomment") %} +
+

{% trans %}Pedagogy{% endtrans %}

+ +
+ {% endif %} +
-

{% trans %}Counters{% endtrans %}

+

{% trans %}Elections{% endtrans %}

- +
-
  • - {{ b[1] }} - - - - {% trans %}Edit{% endtrans %} - {% trans %}Stats{% endtrans %} - - -
    • - {% endif %} - {% endfor %} - -
    -{% endif %} - -{% if -user.is_root -or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) -or user.memberships.ongoing().filter(role__gte=7).count() > 10 -%} -
    -

    {% trans %}Accounting{% endtrans %}

    - -
    -{% endif %} - -{% if -user.is_root -or user.is_com_admin -or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) -%} -
    -

    {% trans %}Communication{% endtrans %}

    - -
    -{% endif %} - -{% if user.memberships.filter(end_date=None).all().count() > 0 %} -
    -

    {% trans %}Club tools{% endtrans %}

    -
      - {% for m in user.memberships.filter(end_date=None).all() %} -
    • {{ m.club }}
      • - {% endfor %} -
    -
    -{% endif %} - -{% if -user.is_root -or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) -%} -
    -

    {% trans %}Pedagogy{% endtrans %}

    - -
    -{% endif %} - -
    -

    {% trans %}Elections{% endtrans %}

    - -
    - -
    -

    {% trans %}Other tools{% endtrans %}

    - -
    -
    - +
    +

    {% trans %}Other tools{% endtrans %}

    + +
    +
    + {% endblock %} \ No newline at end of file diff --git a/core/tests/test_user.py b/core/tests/test_user.py index 0e14bad8..9b2209b3 100644 --- a/core/tests/test_user.py +++ b/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], + } diff --git a/core/views/__init__.py b/core/views/__init__.py index c8152f78..a53671d5 100644 --- a/core/views/__init__.py +++ b/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 SingleObjectMixin +from django.views.generic.detail import BaseDetailView 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): - """Get current group from id in url.""" - return self.cached_object - - @cached_property - def cached_object(self): - """Optimisation on group retrieval.""" - return super().get_object() + def post(self, request, *args, **kwargs): + self.object = self.get_object() + return super().post(request, *args, **kwargs) # F403: those star-imports would be hellish to refactor diff --git a/core/views/forms.py b/core/views/forms.py index a0cdfb6b..381fc8a3 100644 --- a/core/views/forms.py +++ b/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 diff --git a/core/views/group.py b/core/views/group.py index e17db138..ba6b406d 100644 --- a/core/views/group.py +++ b/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 diff --git a/core/views/user.py b/core/views/user.py index 5647d720..d742a6f5 100644 --- a/core/views/user.py +++ b/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) diff --git a/counter/models.py b/counter/models.py index 9fade666..2ac691e0 100644 --- a/counter/models.py +++ b/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") diff --git a/counter/tests/test_counter.py b/counter/tests/test_counter.py index fb64759c..27ce62bc 100644 --- a/counter/tests/test_counter.py +++ b/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 diff --git a/counter/tests/test_customer.py b/counter/tests/test_customer.py index 7153142d..bc3f4fb4 100644 --- a/counter/tests/test_customer.py +++ b/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) diff --git a/counter/views/click.py b/counter/views/click.py index 4a1e1c88..eb6f8e28 100644 --- a/counter/views/click.py +++ b/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() - or not obj.club.has_rights_in_club(request.user) + request.user.is_anonymous + 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 ( diff --git a/docs/tutorial/groups.md b/docs/tutorial/groups.md index bccd713f..2c67b3f0 100644 --- a/docs/tutorial/groups.md +++ b/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). diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index 07eede14..0cfef902 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2025-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 \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 -#: counter/templates/counter/cash_register_summary.jinja +#: core/templates/core/edit.jinja #, python-format -msgid "Edit %(obj)s" -msgstr "Éditer %(obj)s" +msgid "Edit %(name)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" diff --git a/pedagogy/api.py b/pedagogy/api.py index 68e3d5e2..e8d34351 100644 --- a/pedagogy/api.py +++ b/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()) diff --git a/pedagogy/models.py b/pedagogy/models.py index 956e6791..892f7028 100644 --- a/pedagogy/models.py +++ b/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) diff --git a/pedagogy/templates/pedagogy/guide.jinja b/pedagogy/templates/pedagogy/guide.jinja index 112ff1a5..79b66c24 100644 --- a/pedagogy/templates/pedagogy/guide.jinja +++ b/pedagogy/templates/pedagogy/guide.jinja @@ -19,7 +19,7 @@ {% endblock head %} {% block content %} - {% if can_create_uv %} + {% if user.has_perm("pedagogy.add_uv") %}

    {% trans %}Create UV{% endtrans %} @@ -94,8 +94,10 @@ {% trans %}Credit type{% endtrans %} - {% if can_create_uv %} + {%- if user.has_perm("pedagogy.change_uv") -%} {% trans %}Edit{% endtrans %} + {%- endif -%} + {%- if user.has_perm("pedagogy.delete_uv") -%} {% trans %}Delete{% endtrans %} {% endif %} @@ -109,8 +111,10 @@ - {% if can_create_uv -%} + {%- if user.has_perm("pedagogy.change_uv") -%} {% trans %}Edit{% endtrans %} + {%- endif -%} + {%- if user.has_perm("pedagogy.delete_uv") -%} {% trans %}Delete{% endtrans %} {%- endif -%} diff --git a/pedagogy/templates/pedagogy/uv_detail.jinja b/pedagogy/templates/pedagogy/uv_detail.jinja index c2133eae..a5b07f68 100644 --- a/pedagogy/templates/pedagogy/uv_detail.jinja +++ b/pedagogy/templates/pedagogy/uv_detail.jinja @@ -89,7 +89,7 @@

    {% trans %}You already posted a comment on this UV. If you want to comment again, please modify or delete your previous comment.{% endtrans %}

    - {% else %} + {% elif user.has_perm("pedagogy.add_uvcomment") %}

    {% trans %}Leave comment{% endtrans %}

    @@ -146,9 +146,9 @@ {% endif %}
    - {% if object.comments.exists() %} + {% if comments %}

    {% trans %}Comments{% endtrans %}

    - {% for comment in object.comments.order_by("-publish_date").all() %} + {% for comment in comments %}
    @@ -183,16 +183,28 @@

    {% endif %} - {% if user.is_owner(comment) %} + {% if comment.author_id == user.id or user.has_perm("pedagogy.change_comment") %}

    - {% trans %}Edit{% endtrans %} - {% trans %}Delete{% endtrans %} + + {% trans %}Edit{% endtrans %} + + {% endif %} + {% if comment.author_id == user.id or user.has_perm("pedagogy.delete_comment") %} + + {% trans %}Delete{% endtrans %} +

    {% endif %}
    -

    {% trans %}Report this comment{% endtrans %}

    +
    +

    + + {% trans %}Report this comment{% endtrans %} + +

    +

    {{ comment.publish_date.strftime('%d/%m/%Y') }}

    @@ -209,7 +221,7 @@