diff --git a/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html b/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html index 1f2531a89d5279286e47bd88230e5ec1763b342f..8c83accaf9b7cfc9e07a2b05c16961d3f959fd75 100644 --- a/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html +++ b/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html @@ -333,11 +333,13 @@ </a> {% endif %} </h5> - <p> - {% for assignment in person.person.group_roles.all %} - {% include "alsijil/group_role/chip.html" with role=assignment.role small=assignment.date_range %} - {% endfor %} - </p> + {% if group_roles %} + <p> + {% for assignment in person.person.group_roles.all %} + {% include "alsijil/group_role/chip.html" with role=assignment.role small=assignment.date_range %} + {% endfor %} + </p> + {% endif %} <p class="card-text"> {% trans "Absent" %}: {{ person.person.absences_count }} ({{ person.person.unexcused_count }} {% trans "unexcused" %}) diff --git a/aleksis/apps/alsijil/util/predicates.py b/aleksis/apps/alsijil/util/predicates.py index 8ef58e204d474227c281a5b15ef4a2faba2b2eb1..628ae7ae5e0079fe402df9e44c84ef9c07a36877 100644 --- a/aleksis/apps/alsijil/util/predicates.py +++ b/aleksis/apps/alsijil/util/predicates.py @@ -1,13 +1,12 @@ from typing import Any, Union -from django.contrib.auth.models import Permission, User +from django.contrib.auth.models import User -from guardian.models import UserObjectPermission from rules import predicate from aleksis.apps.chronos.models import Event, ExtraLesson, LessonPeriod from aleksis.core.models import Group, Person -from aleksis.core.util.core_helpers import get_content_type_by_perm +from aleksis.core.util.predicates import check_object_permission from ..models import PersonalNote @@ -128,16 +127,11 @@ def has_person_group_object_perm(perm: str): @predicate(name) def fn(user: User, obj: Person) -> bool: - ct = get_content_type_by_perm(perm) - permissions = Permission.objects.filter(content_type=ct, codename=perm) groups = obj.member_of.all() - qs = UserObjectPermission.objects.filter( - object_pk__in=list(groups.values_list("pk", flat=True)), - content_type=ct, - user=user, - permission__in=permissions, - ) - return qs.exists() + for group in groups: + if check_object_permission(user, perm, group, checker_obj=obj): + return True + return False return fn @@ -167,15 +161,9 @@ def has_lesson_group_object_perm(perm: str): def fn(user: User, obj: LessonPeriod) -> bool: if hasattr(obj, "lesson"): groups = obj.lesson.groups.all() - ct = get_content_type_by_perm(perm) - permissions = Permission.objects.filter(content_type=ct, codename=perm) - qs = UserObjectPermission.objects.filter( - object_pk__in=list(groups.values_list("pk", flat=True)), - content_type=ct, - user=user, - permission__in=permissions, - ) - return qs.exists() + for group in groups: + if check_object_permission(user, perm, group, checker_obj=obj): + return True return False return fn @@ -191,16 +179,10 @@ def has_personal_note_group_perm(perm: str): @predicate(name) def fn(user: User, obj: PersonalNote) -> bool: if hasattr(obj, "person"): - ct = get_content_type_by_perm(perm) - permissions = Permission.objects.filter(content_type=ct, codename=perm) groups = obj.person.member_of.all() - qs = UserObjectPermission.objects.filter( - object_pk__in=list(groups.values_list("pk", flat=True)), - content_type=ct, - user=user, - permission__in=permissions, - ) - return qs.exists() + for group in groups: + if check_object_permission(user, perm, group, checker_obj=obj): + return True return False return fn diff --git a/aleksis/apps/alsijil/views.py b/aleksis/apps/alsijil/views.py index 503fff099f0f88618bb74cc55748f91ce3724076..9e8ba6807db53b3b85665d2d99b8c55535d52499 100644 --- a/aleksis/apps/alsijil/views.py +++ b/aleksis/apps/alsijil/views.py @@ -20,6 +20,7 @@ from django.views.generic import DetailView import reversion from calendarweek import CalendarWeek from django_tables2 import RequestConfig, SingleTableView +from guardian.core import ObjectPermissionChecker from guardian.shortcuts import get_objects_for_user from reversion.views import RevisionMixin from rules.contrib.views import PermissionRequiredMixin, permission_required @@ -193,6 +194,14 @@ def register_object( request.POST or None, instance=lesson_documentation, prefix="lesson_documentation", ) + # Prefetch object permissions for all related groups of the register object + # because the object permissions are checked for all groups of the register object + # That has to be set as an attribute of the register object, + # so that the permission system can use the prefetched data. + checker = ObjectPermissionChecker(request.user) + checker.prefetch_perms(register_object.get_groups().all()) + register_object.set_object_permission_checker(checker) + # Create a formset that holds all personal notes for all persons in this lesson if not request.user.has_perm("alsijil.view_register_object_personalnote", register_object): persons = Person.objects.filter(pk=request.user.person.pk) @@ -396,6 +405,12 @@ def week_view( | Q(member_of__extra_lessons__in=extra_lessons_pk) ) + # Prefetch object permissions for persons and groups the persons are members of + # because the object permissions are checked for both persons and groups + checker = ObjectPermissionChecker(request.user) + checker.prefetch_perms(persons_qs) + checker.prefetch_perms(Group.objects.filter(members__in=persons_qs)) + persons_qs = ( Person.objects.filter(pk__in=persons_qs) .select_related("primary_group") @@ -472,6 +487,7 @@ def week_view( if note.lesson_period: note.lesson_period.annotate_week(wanted_week) personal_notes.append(note) + person.set_object_permission_checker(checker) persons.append({"person": person, "personal_notes": personal_notes}) else: persons = None @@ -674,7 +690,9 @@ def my_students(request: HttpRequest) -> HttpResponse: new_groups = [] for group in relevant_groups: - persons = group.generate_person_list_with_class_register_statistics() + persons = group.generate_person_list_with_class_register_statistics( + group.members.prefetch_related("primary_group__owners") + ) new_groups.append((group, persons)) context["groups"] = new_groups @@ -707,13 +725,18 @@ class StudentsList(PermissionRequiredMixin, DetailView): @permission_required( - "alsijil.view_person_overview", fn=objectgetter_optional(Person, "request.user.person", True), + "alsijil.view_person_overview", + fn=objectgetter_optional( + Person.objects.prefetch_related("member_of__owners"), "request.user.person", True + ), ) def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse: context = {} - person = objectgetter_optional(Person, default="request.user.person", default_eval=True)( - request, id_ - ) + person = objectgetter_optional( + Person.objects.prefetch_related("member_of__owners"), + default="request.user.person", + default_eval=True, + )(request, id_) context["person"] = person if request.method == "POST": @@ -788,6 +811,14 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp "lesson_period__substitutions", ) + # Prefetch object permissions for groups the person is a member of + # because the object permissions are checked for all groups the person is a member of + # That has to be set as an attribute of the register object, + # so that the permission system can use the prefetched data. + checker = ObjectPermissionChecker(request.user) + checker.prefetch_perms(Group.objects.filter(members=person)) + person.set_object_permission_checker(checker) + if request.user.has_perm("alsijil.view_person_overview_personalnote", person): allowed_personal_notes = person_personal_notes.all() else: @@ -837,7 +868,11 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp "-school_term_start", "-order_year", "-order_week", "-order_weekday", "order_period", ) ) - context["personal_notes"] = personal_notes + personal_notes_list = [] + for note in personal_notes: + note.set_object_permission_checker(checker) + personal_notes_list.append(note) + context["personal_notes"] = personal_notes_list context["excuse_types"] = ExcuseType.objects.all() extra_marks = ExtraMark.objects.all()