diff --git a/aleksis/apps/alsijil/model_extensions.py b/aleksis/apps/alsijil/model_extensions.py index 9eeec915822ad4d49babb50614fba8f27de40202..e0bdb6c2ab21c80cd7932768d95d3a769e1a6f8e 100644 --- a/aleksis/apps/alsijil/model_extensions.py +++ b/aleksis/apps/alsijil/model_extensions.py @@ -1,9 +1,8 @@ from datetime import date from typing import Dict, Iterable, Iterator, Optional, Union -from django.db.models import Exists, OuterRef, Q, QuerySet +from django.db.models import Exists, FilteredRelation, OuterRef, Q, QuerySet from django.db.models.aggregates import Count, Sum -from django.db.models.expressions import Subquery from django.urls import reverse from django.utils.translation import gettext as _ @@ -386,59 +385,45 @@ def generate_person_list_with_class_register_statistics( if persons is None: persons = self.members.all() - # Build reusable Q objects for filtering by school term and by groups - # Necessary for the following annotations - school_term_q = ( - Q(personal_notes__lesson_period__lesson__validity__school_term=self.school_term) - | Q(personal_notes__extra_lesson__school_term=self.school_term) - | Q(personal_notes__event__school_term=self.school_term) + lesson_periods = LessonPeriod.objects.filter( + lesson__validity__school_term=self.school_term + ).filter(Q(lesson__groups=self) | Q(lesson__groups__parent_groups=self)) + extra_lessons = ExtraLesson.objects.filter(school_term=self.school_term).filter( + Q(groups=self) | Q(groups__parent_groups=self) ) - groups_q = ( - Q(personal_notes__lesson_period__lesson__groups=self) - | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self) - | Q(personal_notes__extra_lesson__groups=self) - | Q(personal_notes__extra_lesson__groups__parent_groups=self) - | Q(personal_notes__event__groups=self) - | Q(personal_notes__event__groups__parent_groups=self) + events = Event.objects.filter(school_term=self.school_term).filter( + Q(groups=self) | Q(groups__parent_groups=self) ) - persons = persons.filter(personal_notes__groups_of_person=self).filter(school_term_q).distinct() - + persons = persons.select_related("primary_group", "primary_group__school_term") persons = persons.annotate( - absences_count=Count( + filtered_personal_notes=FilteredRelation( "personal_notes", - filter=Q(personal_notes__absent=True) & school_term_q & groups_q, - distinct=True, + condition=( + Q(personal_notes__event__in=events) + | Q(personal_notes__lesson_period__in=lesson_periods) + | Q(personal_notes__extra_lesson__in=extra_lessons) + ), + ) + ).annotate( + absences_count=Count( + "filtered_personal_notes", filter=Q(filtered_personal_notes__absent=True), ), excused=Count( - "personal_notes", + "filtered_personal_notes", filter=Q( - personal_notes__absent=True, - personal_notes__excused=True, - personal_notes__excuse_type__isnull=True, - ) - & school_term_q - & groups_q, - distinct=True, + filtered_personal_notes__absent=True, + filtered_personal_notes__excused=True, + filtered_personal_notes__excuse_type__isnull=True, + ), ), unexcused=Count( - "personal_notes", - filter=Q(personal_notes__absent=True, personal_notes__excused=False) - & school_term_q - & groups_q, - distinct=True, - ), - tardiness=Subquery( - Person.objects.filter(school_term_q & groups_q) - .filter(pk=OuterRef("pk"),) - .distinct() - .annotate(tardiness=Sum("personal_notes__late")) - .values("tardiness") + "filtered_personal_notes", + filter=Q(filtered_personal_notes__absent=True, filtered_personal_notes__excused=False), ), + tardiness=Sum("filtered_personal_notes__late"), tardiness_count=Count( - "personal_notes", - filter=~Q(personal_notes__late=0) & school_term_q & groups_q, - distinct=True, + "filtered_personal_notes", filter=Q(filtered_personal_notes__late__gt=0), ), ) @@ -446,9 +431,8 @@ def generate_person_list_with_class_register_statistics( persons = persons.annotate( **{ extra_mark.count_label: Count( - "personal_notes", - filter=Q(personal_notes__extra_marks=extra_mark) & school_term_q & groups_q, - distinct=True, + "filtered_personal_notes", + filter=Q(filtered_personal_notes__extra_marks=extra_mark), ) } ) @@ -457,11 +441,11 @@ def generate_person_list_with_class_register_statistics( persons = persons.annotate( **{ excuse_type.count_label: Count( - "personal_notes__absent", - filter=Q(personal_notes__absent=True, personal_notes__excuse_type=excuse_type,) - & school_term_q - & groups_q, - distinct=True, + "filtered_personal_notes__absent", + filter=Q( + filtered_personal_notes__absent=True, + filtered_personal_notes__excuse_type=excuse_type, + ), ) } ) 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 31ad878230fef101e954d1a1683dc0e9d1a62377..39f31d2e7411d84cac881e46f63e49667e7db7e6 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 @@ -194,6 +195,9 @@ def register_object( ) # Create a formset that holds all personal notes for all persons in this lesson + checker = ObjectPermissionChecker(request.user) + checker.prefetch_perms(register_object.get_groups().all()) + register_object.annotate_object_permission_checker(checker) if not request.user.has_perm("alsijil.view_register_object_personalnote", register_object): persons = Person.objects.filter(pk=request.user.person.pk) else: @@ -396,6 +400,10 @@ def week_view( | Q(member_of__extra_lessons__in=extra_lessons_pk) ) + checker = ObjectPermissionChecker(request.user) + checker.prefetch_perms(persons_qs) + checker.prefetch_perms(Group.objects.filter(members__in=persons_qs)) + personal_notes_q = ( Q( personal_notes__week=wanted_week.week, @@ -490,6 +498,7 @@ def week_view( if note.lesson_period: note.lesson_period.annotate_week(wanted_week) personal_notes.append(note) + person.annotate_object_permission_checker(checker) persons.append({"person": person, "personal_notes": personal_notes}) else: persons = None @@ -692,7 +701,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 @@ -725,13 +736,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": @@ -806,6 +822,10 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp "lesson_period__substitutions", ) + checker = ObjectPermissionChecker(request.user) + checker.prefetch_perms(Group.objects.filter(members=person)) + person.annotate_object_permission_checker(checker) + if request.user.has_perm("alsijil.view_person_overview_personalnote", person): allowed_personal_notes = person_personal_notes.all() else: @@ -855,7 +875,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.annotate_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()