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/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..4d63bfb9 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -895,13 +895,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 +976,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/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/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/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 @@