Merge branch 'master' into feature/performance-optimizations

aleksis/apps/alsijil/forms.py

@@ -88,7 +88,10 @@ class SelectForm(forms.Form):
 
         group_qs = Group.get_groups_with_lessons()
 
+        # Filter selectable groups by permissions
         if not check_global_permission(self.request.user, "alsijil.view_week"):
+            # 1) All groups the user is allowed to see the week view by object permissions
+            # 2) All groups the user is a member of an owner of
             group_qs = (
                 group_qs.filter(
                     pk__in=get_objects_for_user(
@@ -96,6 +99,8 @@ class SelectForm(forms.Form):
                     ).values_list("pk", flat=True)
                 )
             ).union(group_qs.filter(Q(members=person) | Q(owners=person)))
+
+        # Flatten query by filtering groups by pk
         self.fields["group"].queryset = Group.objects.filter(
             pk__in=list(group_qs.values_list("pk", flat=True))
         )
@@ -103,7 +108,10 @@ class SelectForm(forms.Form):
         teacher_qs = Person.objects.annotate(
             lessons_count=Count("lessons_as_teacher")
         ).filter(lessons_count__gt=0)
+
+        # Filter selectable teachers by permissions
         if not check_global_permission(self.request.user, "alsijil.view_week"):
+            # If the user hasn't the global permission, the user is only allowed to see his own person
             teacher_qs = teacher_qs.filter(pk=person.pk)
 
         self.fields["teacher"].queryset = teacher_qs
@@ -138,12 +146,18 @@ class RegisterAbsenceForm(forms.Form):
     remarks = forms.CharField(label=_("Remarks"), max_length=30, required=False)
 
     def __init__(self, *args, **kwargs):
-        self.request = kwargs.pop("request")
+        self.request = get_request()
         super().__init__(*args, **kwargs)
         period_choices = TimePeriod.period_choices
+
+        # Filter selectable persons by permissions
         if check_global_permission(self.request.user, "alsijil.register_absence"):
+            # Global permission, user can register absences for all persons
             self.fields["person"].queryset = Person.objects.all()
         else:
+            # 1) All persons the user is allowed to register an absence for by object permissions
+            # 2) All persons the user is the primary group owner
+            # 3) All persons the user is allowed to register an absence for by object permissions of the person's group
             persons_qs = (
                 get_objects_for_user(
                     self.request.user, "core.register_absence_person", Person
@@ -161,6 +175,8 @@ class RegisterAbsenceForm(forms.Form):
                     )
                 )
             )
+
+            # Flatten query by getting all pks and filter persons
             self.fields["person"].queryset = Person.objects.filter(
                 pk__in=list(persons_qs.values_list("pk", flat=True))
             )

aleksis/apps/alsijil/preferences.py

@@ -24,6 +24,16 @@ class ViewOwnPersonalNotes(BooleanPreference):
     verbose_name = _("Allow users to view their own personal notes")
 
 
+@site_preferences_registry.register
+class RegisterAbsenceAsPrimaryGroupOwner(BooleanPreference):
+    section = alsijil
+    name = "register_absence_as_primary_group_owner"
+    default = True
+    verbose_name = _(
+        "Allow primary group owners to register future absences for students in their groups"
+    )
+
+
 @site_preferences_registry.register
 class CarryOverDataToNextPeriods(BooleanPreference):
     section = alsijil

aleksis/apps/alsijil/rules.py

@@ -5,6 +5,7 @@ from aleksis.core.util.predicates import (
     has_object_perm,
     has_person,
     is_current_person,
+    is_site_preference_set,
 )
 
 from .util.predicates import (
@@ -17,6 +18,7 @@ from .util.predicates import (
     is_lesson_parent_group_owner,
     is_lesson_participant,
     is_lesson_teacher,
+    is_none,
     is_own_personal_note,
     is_person_group_owner,
     is_person_primary_group_owner,
@@ -27,7 +29,8 @@ from .util.predicates import (
 
 # View lesson
 view_lesson_predicate = has_person & (
-    is_lesson_teacher
+    is_none  # View is opened as "Current lesson"
+    | is_lesson_teacher
     | is_lesson_participant
     | is_lesson_parent_group_owner
     | has_global_perm("alsijil.view_lesson")
@@ -39,26 +42,27 @@ add_perm("alsijil.view_lesson", view_lesson_predicate)
 add_perm("alsijil.view_lesson_menu", has_person)
 
 # View lesson personal notes
-view_lesson_personal_notes_predicate = has_person & (
-    is_lesson_teacher
-    | is_lesson_parent_group_owner
+view_lesson_personal_notes_predicate = view_lesson_predicate & (
+    ~is_lesson_participant
     | has_global_perm("alsijil.view_personalnote")
     | has_lesson_group_object_perm("core.view_personalnote_group")
 )
 add_perm("alsijil.view_lesson_personalnote", view_lesson_personal_notes_predicate)
 
 # Edit personal note
-edit_lesson_personal_note_predicate = has_person & (
-    is_lesson_teacher
+edit_lesson_personal_note_predicate = view_lesson_personal_notes_predicate & (
+    ~is_lesson_parent_group_owner
     | has_global_perm("alsijil.change_personalnote")
     | has_lesson_group_object_perm("core.edit_personalnote_group")
 )
 add_perm("alsijil.edit_lesson_personalnote", edit_lesson_personal_note_predicate)
 
-
 # View personal note
 view_personal_note_predicate = has_person & (
-    is_own_personal_note
+    (
+        is_own_personal_note
+        & is_site_preference_set("alsijil", "view_own_personal_notes")
+    )
     | is_personal_note_lesson_teacher
     | is_personal_note_lesson_parent_group_owner
     | has_global_perm("alsijil.view_personalnote")
@@ -67,26 +71,19 @@ view_personal_note_predicate = has_person & (
 add_perm("alsijil.view_personalnote", view_personal_note_predicate)
 
 # Edit personal note
-edit_personal_note_predicate = has_person & (
-    is_personal_note_lesson_teacher
-    | is_personal_note_lesson_parent_group_owner
+edit_personal_note_predicate = view_personal_note_predicate & (
+    ~is_own_personal_note
     | has_global_perm("alsijil.view_personalnote")
     | has_personal_note_group_perm("core.edit_personalnote_group")
 )
 add_perm("alsijil.edit_personalnote", edit_personal_note_predicate)
 
 # View lesson documentation
-view_lesson_documentation_predicate = has_person & (
-    is_lesson_teacher
-    | is_lesson_participant
-    | is_lesson_parent_group_owner
-    | has_global_perm("alsijil.view_lessondocumentation")
-    | has_lesson_group_object_perm("core.view_lessondocumentation_group")
-)
+view_lesson_documentation_predicate = view_lesson_predicate
 add_perm("alsijil.view_lessondocumentation", view_lesson_documentation_predicate)
 
 # Edit lesson documentation
-edit_lesson_documentation_predicate = has_person & (
+edit_lesson_documentation_predicate = view_lesson_predicate & (
     is_lesson_teacher
     | has_global_perm("alsijil.change_lessondocumentation")
     | has_lesson_group_object_perm("core.edit_lessondocumentation_group")
@@ -123,7 +120,10 @@ add_perm("alsijil.view_register_absence", view_register_absence_predicate)
 
 # Register absence
 register_absence_predicate = has_person & (
-    is_person_primary_group_owner
+    (
+        is_person_primary_group_owner
+        & is_site_preference_set("alsijil", "register_absence_as_primary_group_owner")
+    )
     | has_global_perm("alsijil.register_absence")
     | has_object_perm("core.register_absence_person")
     | has_person_group_object_perm("core.register_absence_group")
@@ -148,7 +148,8 @@ add_perm("alsijil.view_my_groups", view_my_groups_predicate)
 
 # View person overview
 view_person_overview_predicate = has_person & (
-    is_current_person | is_person_group_owner
+    (is_current_person & is_site_preference_set("alsijil", "view_own_personal_notes"))
+    | is_person_group_owner
 )
 add_perm("alsijil.view_person_overview", view_person_overview_predicate)
 
@@ -157,8 +158,8 @@ view_person_overview_menu_predicate = has_person
 add_perm("alsijil.view_person_overview_menu", view_person_overview_menu_predicate)
 
 # View person overview personal notes
-view_person_overview_personal_notes_predicate = has_person & (
-    is_current_person
+view_person_overview_personal_notes_predicate = view_person_overview_predicate & (
+    (is_current_person & is_site_preference_set("alsijil", "view_own_personal_notes"))
     | is_person_primary_group_owner
     | has_global_perm("alsijil.view_personalnote")
     | has_person_group_object_perm("core.view_personalnote_group")
@@ -169,10 +170,13 @@ add_perm(
 )
 
 # Edit person overview personal notes
-edit_person_overview_personal_notes_predicate = has_person & (
-    is_person_primary_group_owner
-    | has_global_perm("alsijil.edit_personalnote")
-    | has_person_group_object_perm("core.edit_personalnote_group")
+edit_person_overview_personal_notes_predicate = (
+    view_person_overview_personal_notes_predicate
+    & (
+        ~is_current_person
+        | has_global_perm("alsijil.edit_personalnote")
+        | has_person_group_object_perm("core.edit_personalnote_group")
+    )
 )
 add_perm(
     "alsijil.edit_person_overview_personalnote",
@@ -180,11 +184,8 @@ add_perm(
 )
 
 # View person statistics on personal notes
-view_person_statistics_personal_notes_predicate = has_person & (
-    is_current_person
-    | is_person_primary_group_owner
-    | has_global_perm("alsijil.view_personalnote")
-    | has_person_group_object_perm("core.view_personalnote_group")
+view_person_statistics_personal_notes_predicate = (
+    view_person_overview_personal_notes_predicate
 )
 add_perm(
     "alsijil.view_person_statistics_personalnote",
@@ -196,15 +197,21 @@ view_excusetypes_predicate = has_person & has_global_perm("alsijil.view_excusety
 add_perm("alsijil.view_excusetypes", view_excusetypes_predicate)
 
 # Add excuse type
-add_excusetype_predicate = has_person & has_global_perm("alsijil.add_excusetype")
+add_excusetype_predicate = view_excusetypes_predicate & has_global_perm(
+    "alsijil.add_excusetype"
+)
 add_perm("alsijil.add_excusetype", add_excusetype_predicate)
 
 # Edit excuse type
-edit_excusetype_predicate = has_person & has_global_perm("alsijil.change_excusetype")
+edit_excusetype_predicate = view_excusetypes_predicate & has_global_perm(
+    "alsijil.change_excusetype"
+)
 add_perm("alsijil.edit_excusetype", edit_excusetype_predicate)
 
 # Delete excuse type
-delete_excusetype_predicate = has_person & has_global_perm("alsijil.delete_excusetype")
+delete_excusetype_predicate = view_excusetypes_predicate & has_global_perm(
+    "alsijil.delete_excusetype"
+)
 add_perm("alsijil.delete_excusetype", delete_excusetype_predicate)
 
 # View extra mark list
@@ -212,13 +219,19 @@ view_extramarks_predicate = has_person & has_global_perm("alsijil.view_extramark
 add_perm("alsijil.view_extramarks", view_extramarks_predicate)
 
 # Add extra mark
-add_extramark_predicate = has_person & has_global_perm("alsijil.add_extramark")
+add_extramark_predicate = view_extramarks_predicate & has_global_perm(
+    "alsijil.add_extramark"
+)
 add_perm("alsijil.add_extramark", add_extramark_predicate)
 
 # Edit extra mark
-edit_extramark_predicate = has_person & has_global_perm("alsijil.change_extramark")
+edit_extramark_predicate = view_extramarks_predicate & has_global_perm(
+    "alsijil.change_extramark"
+)
 add_perm("alsijil.edit_extramark", edit_extramark_predicate)
 
 # Delete extra mark
-delete_extramark_predicate = has_person & has_global_perm("alsijil.delete_extramark")
+delete_extramark_predicate = view_extramarks_predicate & has_global_perm(
+    "alsijil.delete_extramark"
+)
 add_perm("alsijil.delete_extramark", delete_extramark_predicate)

aleksis/apps/alsijil/util/alsijil_helpers.py

@@ -38,14 +38,14 @@ def get_lesson_period_by_pk(
     return lesson_period
 
 
-def get_instance_by_pk(
+def get_timetable_instance_by_pk(
     request: HttpRequest,
     year: Optional[int] = None,
     week: Optional[int] = None,
     type_: Optional[str] = None,
     id_: Optional[int] = None,
 ):
-    """Get Instance object by given type and id or the current person."""
+    """Get timetable object (teacher, room or group) by given type and id or the current person."""
     if type_ and id_:
         return get_el_by_pk(request, type_, id_)
     elif hasattr(request, "user") and hasattr(request.user, "person"):

aleksis/apps/alsijil/util/predicates.py

-from typing import Union
+from typing import Any, Union
 
 from django.contrib.auth.models import Permission, User
 
@@ -12,10 +12,17 @@ from aleksis.core.util.core_helpers import (
     get_content_type_by_perm,
     get_site_preferences,
 )
+from aleksis.core.util.predicates import check_object_permission
 
 from ..models import PersonalNote
 
 
+@predicate
+def is_none(user: User, obj: Any) -> bool:
+    """Predicate that checks if the provided object is None-like."""
+    return bool(obj)
+
+
 @predicate
 def is_lesson_teacher(user: User, obj: LessonPeriod) -> bool:
     """Predicate for teachers of a lesson.
@@ -28,7 +35,7 @@ def is_lesson_teacher(user: User, obj: LessonPeriod) -> bool:
         if sub and sub in user.person.lesson_substitutions.all():
             return True
         return user.person in obj.lesson.teachers.all()
-    return True
+    return False
 
 
 @predicate
@@ -42,8 +49,7 @@ def is_lesson_participant(user: User, obj: LessonPeriod) -> bool:
         for group in obj.lesson.groups.all():
             if user.person in list(group.members.all()):
                 return True
-        return False
-    return True
+    return False
 
 
 @predicate
@@ -59,8 +65,7 @@ def is_lesson_parent_group_owner(user: User, obj: LessonPeriod) -> bool:
             for parent_group in group.parent_groups.all():
                 if user.person in list(parent_group.owners.all()):
                     return True
-        return False
-    return True
+    return False
 
 
 @predicate
@@ -201,15 +206,10 @@ def is_own_personal_note(user: User, obj: PersonalNote) -> bool:
     """Predicate for users referred to in a personal note
 
     Checks whether the user referred to in a PersonalNote is the active user.
-    Is configurable via dynamic preferences.
     """
-    if hasattr(obj, "person"):
-        if (
-            get_site_preferences()["alsijil__view_own_personal_notes"]
-            and obj.person is user.person
-        ):
-            return True
-        return False
+    if hasattr(obj, "person") and obj.person is user.person:
+        return True
+    return False
 
 
 @predicate

aleksis/apps/alsijil/views.py

@@ -34,7 +34,7 @@ from .forms import (
 )
 from .models import ExcuseType, ExtraMark, LessonDocumentation, PersonalNote
 from .tables import ExcuseTypeTable, ExtraMarkTable
-from .util.alsijil_helpers import get_instance_by_pk, get_lesson_period_by_pk
+from .util.alsijil_helpers import get_lesson_period_by_pk, get_timetable_instance_by_pk
 
 
 @permission_required("alsijil.view_lesson", fn=get_lesson_period_by_pk)
@@ -55,7 +55,7 @@ def lesson(
     else:
         wanted_week = None
 
-    if not (year and week and period_id):
+    if not all((year, week, period_id)):
         if lesson_period:
             return redirect(
                 "lesson_by_week_and_period",
@@ -109,9 +109,11 @@ def lesson(
     )
 
     # Create a formset that holds all personal notes for all persons in this lesson
-    persons = Person.objects.all()
     if not request.user.has_perm("alsijil.view_lesson_personalnote", lesson_period):
-        persons = persons.filter(pk=request.user.person.pk)
+        persons = Person.objects.filter(pk=request.user.person.pk)
+    else:
+        persons = Person.objects.all()
+
     persons_qs = lesson_period.get_personal_notes(persons, wanted_week)
     personal_note_formset = PersonalNoteFormSet(
         request.POST or None, queryset=persons_qs, prefix="personal_notes"
@@ -162,7 +164,7 @@ def lesson(
     return render(request, "alsijil/class_register/lesson.html", context)
 
 
-@permission_required("alsijil.view_week", fn=get_instance_by_pk)
+@permission_required("alsijil.view_week", fn=get_timetable_instance_by_pk)
 def week_view(
     request: HttpRequest,
     year: Optional[int] = None,
@@ -177,7 +179,7 @@ def week_view(
     else:
         wanted_week = CalendarWeek()
 
-    instance = get_instance_by_pk(request, year, week, type_, id_)
+    instance = get_timetable_instance_by_pk(request, year, week, type_, id_)
 
     lesson_periods = LessonPeriod.objects.in_week(wanted_week).prefetch_related(
         "lesson__groups__members",
@@ -282,7 +284,7 @@ def week_view(
                         lesson_period__in=lesson_periods_pk,
                     ),
                 ),
-                "member_of__owners"
+                "member_of__owners",
             )
             .annotate(
                 absences_count=Count(
@@ -625,20 +627,19 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
 
                 person.refresh_from_db()
 
-    allowed_personal_notes = person.personal_notes.all().prefetch_related(
+    person_personal_notes = person.personal_notes.all().prefetch_related(
         "lesson_period__lesson__groups",
         "lesson_period__lesson__teachers",
         "lesson_period__substitutions",
     )
 
-    if not request.user.has_perm("alsijil.view_person_overview_personalnote", person):
-        print("has")
-        allowed_personal_notes = allowed_personal_notes.filter(
+    if request.user.has_perm("alsijil.view_person_overview_personalnote", person):
+        allowed_personal_notes = person_personal_notes.all()
+    else:
+        allowed_personal_notes = person_personal_notes.filter(
             lesson_period__lesson__groups__owners=request.user.person
         )
 
-    print(allowed_personal_notes)
-
     unexcused_absences = allowed_personal_notes.filter(absent=True, excused=False)
     context["unexcused_absences"] = unexcused_absences
 
@@ -711,7 +712,7 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
 def register_absence(request: HttpRequest) -> HttpResponse:
     context = {}
 
-    register_absence_form = RegisterAbsenceForm(request.POST or None, request=request)
+    register_absence_form = RegisterAbsenceForm(request.POST or None)
 
     if request.method == "POST":
         if register_absence_form.is_valid() and request.user.has_perm(