From b0572970a7c2869912806d4d76ebc950c95d68f6 Mon Sep 17 00:00:00 2001
From: Jonathan Weth <git@jonathanweth.de>
Date: Sat, 13 Feb 2021 16:55:40 +0100
Subject: [PATCH] Add support for events and extra lessons in statistic views

---
 aleksis/apps/alsijil/model_extensions.py      | 110 ++++++++----------
 aleksis/apps/alsijil/models.py                |  10 +-
 .../alsijil/class_register/person.html        |  43 ++++---
 aleksis/apps/alsijil/util/predicates.py       |  17 ++-
 aleksis/apps/alsijil/views.py                 |  74 +++++++++---
 5 files changed, 150 insertions(+), 104 deletions(-)

diff --git a/aleksis/apps/alsijil/model_extensions.py b/aleksis/apps/alsijil/model_extensions.py
index 6be0957fb..0946e8241 100644
--- a/aleksis/apps/alsijil/model_extensions.py
+++ b/aleksis/apps/alsijil/model_extensions.py
@@ -3,6 +3,7 @@ from typing import Dict, Iterable, Iterator, Optional, Union
 
 from django.db.models import Exists, 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 _
 
@@ -17,16 +18,20 @@ from aleksis.core.models import Group, Person
 from .models import ExcuseType, ExtraMark, LessonDocumentation, PersonalNote
 
 
-def alsijil_url(self):
+def alsijil_url(self, week: Optional[CalendarWeek] = None):
     if isinstance(self, LessonPeriod):
-        return reverse("lesson_period", args=[self.week.year, self.week.week, self.pk])
+        week = week or self.week
+        return reverse("lesson_period", args=[week.year, week.week, self.pk])
     else:
         return reverse(self.label_, args=[self.pk])
 
 
 LessonPeriod.property_(alsijil_url)
+LessonPeriod.method(alsijil_url, "get_alsijil_url")
 Event.property_(alsijil_url)
+Event.method(alsijil_url, "get_alsijil_url")
 ExtraLesson.property_(alsijil_url)
+ExtraLesson.method(alsijil_url, "get_alsijil_url")
 
 
 @Person.method
@@ -237,7 +242,7 @@ def get_or_create_lesson_documentation_simple(
     self, week: Optional[CalendarWeek] = None
 ) -> LessonDocumentation:
     """Get or create lesson documentation object for this event/extra lesson."""
-    lesson_documentation, created = LessonDocumentation.objects.get_or_create({self.label_: self})
+    lesson_documentation, created = LessonDocumentation.objects.get_or_create(**{self.label_: self})
     return lesson_documentation
 
 
@@ -376,61 +381,57 @@ def generate_person_list_with_class_register_statistics(
 ) -> QuerySet:
     """Get with class register statistics annotated list of all members."""
     persons = persons or self.members.all()
-    persons = persons.filter(
-        personal_notes__groups_of_person=self,
-        personal_notes__lesson_period__lesson__validity__school_term=self.school_term,
-    ).annotate(
+    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)
+    )
+    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)
+    )
+
+    persons = persons.filter(personal_notes__groups_of_person=self).filter(school_term_q).distinct()
+
+    persons = persons.annotate(
         absences_count=Count(
-            "personal_notes__absent",
-            filter=Q(
-                personal_notes__absent=True,
-                personal_notes__lesson_period__lesson__validity__school_term=self.school_term,
-            )
-            & (
-                Q(personal_notes__lesson_period__lesson__groups=self)
-                | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-            ),
+            "personal_notes",
+            filter=Q(personal_notes__absent=True) & school_term_q & groups_q,
+            distinct=True,
         ),
         excused=Count(
-            "personal_notes__absent",
+            "personal_notes",
             filter=Q(
                 personal_notes__absent=True,
                 personal_notes__excused=True,
                 personal_notes__excuse_type__isnull=True,
-                personal_notes__lesson_period__lesson__validity__school_term=self.school_term,
             )
-            & (
-                Q(personal_notes__lesson_period__lesson__groups=self)
-                | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-            ),
+            & school_term_q
+            & groups_q,
+            distinct=True,
         ),
         unexcused=Count(
-            "personal_notes__absent",
-            filter=Q(
-                personal_notes__absent=True,
-                personal_notes__excused=False,
-                personal_notes__lesson_period__lesson__validity__school_term=self.school_term,
-            )
-            & (
-                Q(personal_notes__lesson_period__lesson__groups=self)
-                | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-            ),
+            "personal_notes",
+            filter=Q(personal_notes__absent=True, personal_notes__excused=False)
+            & school_term_q
+            & groups_q,
+            distinct=True,
         ),
-        tardiness=Sum(
-            "personal_notes__late",
-            filter=(
-                Q(personal_notes__lesson_period__lesson__groups=self)
-                | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-            ),
+        tardiness=Subquery(
+            Person.objects.filter(school_term_q & groups_q)
+            .filter(pk=OuterRef("pk"),)
+            .distinct()
+            .annotate(tardiness=Sum("personal_notes__late"))
+            .values("tardiness")
         ),
         tardiness_count=Count(
             "personal_notes",
-            filter=~Q(personal_notes__late=0)
-            & Q(personal_notes__lesson_period__lesson__validity__school_term=self.school_term,)
-            & (
-                Q(personal_notes__lesson_period__lesson__groups=self)
-                | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-            ),
+            filter=~Q(personal_notes__late=0) & school_term_q & groups_q,
+            distinct=True,
         ),
     )
 
@@ -439,14 +440,7 @@ def generate_person_list_with_class_register_statistics(
             **{
                 extra_mark.count_label: Count(
                     "personal_notes",
-                    filter=Q(
-                        personal_notes__extra_marks=extra_mark,
-                        personal_notes__lesson_period__lesson__validity__school_term=self.school_term,  # noqa
-                    )
-                    & (
-                        Q(personal_notes__lesson_period__lesson__groups=self)
-                        | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-                    ),
+                    filter=Q(personal_notes__extra_marks=extra_mark) & school_term_q & groups_q,
                 )
             }
         )
@@ -456,15 +450,9 @@ def generate_person_list_with_class_register_statistics(
             **{
                 excuse_type.count_label: Count(
                     "personal_notes__absent",
-                    filter=Q(
-                        personal_notes__absent=True,
-                        personal_notes__excuse_type=excuse_type,
-                        personal_notes__lesson_period__lesson__validity__school_term=self.school_term,  # noqa
-                    )
-                    & (
-                        Q(personal_notes__lesson_period__lesson__groups=self)
-                        | Q(personal_notes__lesson_period__lesson__groups__parent_groups=self)
-                    ),
+                    filter=Q(personal_notes__absent=True, personal_notes__excuse_type=excuse_type,)
+                    & school_term_q
+                    & groups_q,
                 )
             }
         )
diff --git a/aleksis/apps/alsijil/models.py b/aleksis/apps/alsijil/models.py
index 71f8d2b3e..c319bf34d 100644
--- a/aleksis/apps/alsijil/models.py
+++ b/aleksis/apps/alsijil/models.py
@@ -20,6 +20,7 @@ from aleksis.apps.alsijil.managers import PersonalNoteManager
 from aleksis.apps.chronos.mixins import WeekRelatedMixin
 from aleksis.apps.chronos.models import Event, ExtraLesson, LessonPeriod
 from aleksis.core.mixins import ExtensibleModel
+from aleksis.core.models import SchoolTerm
 from aleksis.core.util.core_helpers import get_site_preferences
 
 
@@ -93,6 +94,13 @@ class RegisterObjectRelatedMixin(WeekRelatedMixin):
         else:
             return CalendarWeek.from_date(self.register_object.date_start)
 
+    @property
+    def school_term(self) -> SchoolTerm:
+        if self.lesson_period:
+            return self.lesson_period.lesson.validity.school_term
+        else:
+            return self.register_object.school_term
+
     @property
     def date(self) -> Optional[date]:
         if self.lesson_period:
@@ -110,7 +118,7 @@ class RegisterObjectRelatedMixin(WeekRelatedMixin):
         )
 
     def get_absolute_url(self) -> str:
-        return self.register_object.alsijil_url
+        return self.register_object.get_alsijil_url(self.calendar_week)
 
 
 class PersonalNote(RegisterObjectRelatedMixin, ExtensibleModel):
diff --git a/aleksis/apps/alsijil/templates/alsijil/class_register/person.html b/aleksis/apps/alsijil/templates/alsijil/class_register/person.html
index 259b7b347..7254e0e24 100644
--- a/aleksis/apps/alsijil/templates/alsijil/class_register/person.html
+++ b/aleksis/apps/alsijil/templates/alsijil/class_register/person.html
@@ -37,7 +37,6 @@
 
     <ul class="collection">
       {% for note in unexcused_absences %}
-        {% weekday_to_date note.calendar_week note.lesson_period.period.weekday as note_date %}
         <li class="collection-item">
           {% has_perm "alsijil.edit_personalnote" user note as can_edit_personal_note %}
           {% if can_edit_personal_note %}
@@ -54,7 +53,7 @@
           {% endif %}
           <i class="material-icons left red-text">warning</i>
           <p class="no-margin">
-            <a href="{% url "lesson_by_week_and_period" note.year note.week note.lesson_period.pk %}">{{ note_date }}, {{ note.lesson_period }}</a>
+            <a href="{{ note.get_absolute_url }}">{{ note.date }}, {{ note.lesson_period }}</a>
           </p>
           {% if note.remarks %}
             <p class="no-margin"><em>{{ note.remarks }}</em></p>
@@ -130,38 +129,44 @@
         <div>
           <ul>
             {% for note in personal_notes %}
-              {% ifchanged note.lesson_period.lesson.validity.school_term %}</ul></div></li>
+              {% ifchanged note.school_term %}</ul></div></li>
                 <li {% if forloop.first %}class="active"{% endif %}>
                 <div class="collapsible-header"><i
-                        class="material-icons">date_range</i>{{ note.lesson_period.lesson.validity.school_term }}</div>
+                    class="material-icons">date_range</i>{{ note.school_term }}</div>
                 <div class="collapsible-body">
                 <ul class="collection">
               {% endifchanged %}
 
               {% ifchanged note.week %}
                 <li class="collection-item">
-                  <strong>{% blocktrans with week=note.week %}Week {{ week }}{% endblocktrans %}</strong>
+                  <strong>{% blocktrans with week=note.calendar_week.week %}Week {{ week }}{% endblocktrans %}</strong>
                 </li>
               {% endifchanged %}
-              {% weekday_to_date note.calendar_week note.lesson_period.period.weekday as note_date %}
-              {% ifchanged note_date %}
+              {% ifchanged note.date %}
                 <li class="collection-item">
-                  {% if can_mark_all_as_excused %}
+                  {% if can_mark_all_as_excused and note.date %}
                     <form action="" method="post" class="right hide-on-small-only" style="margin-top: -7px;">
                       {% csrf_token %}
                       {% trans "Mark all as" %}
-                      <input type="hidden" value="{{ note_date|date:"Y-m-d" }}" name="date">
+                      <input type="hidden" value="{{ note.date|date:"Y-m-d" }}" name="date">
                       {% include "alsijil/partials/mark_as_buttons.html" %}
                     </form>
                   {% endif %}
                   <i class="material-icons left">schedule</i>
-                  {{ note_date }}
 
-                  {% if can_mark_all_as_excused %}
+                  {% if note.date %}
+                    {{ note.date }}
+                  {% else %}
+                    {{ note.register_object.date_start }}
+                    {{ note.register_object.period_from.period }}.–{{ note.register_object.date_end }}
+                    {{ note.register_object.period_to.period }}.
+                  {% endif %}
+
+                  {% if can_mark_all_as_excused and note.date %}
                     <form action="" method="post" class="hide-on-med-and-up">
                       {% csrf_token %}
                       {% trans "Mark all as" %}
-                      <input type="hidden" value="{{ note_date|date:"Y-m-d" }}" name="date">
+                      <input type="hidden" value="{{ note.date|date:"Y-m-d" }}" name="date">
                       {% include "alsijil/partials/mark_as_buttons.html" %}
                     </form>
                   {% endif %}
@@ -171,14 +176,20 @@
               <li class="collection-item">
                 <div class="row no-margin">
                   <div class="col s2 m1">
-                    {{ note.lesson_period.period.period }}.
+                    {% if note.register_object.period %}
+                      {{ note.register_object.period.period }}.
+                    {% endif %}
                   </div>
 
                   <div class="col s10 m4">
                     <i class="material-icons left">event_note</i>
-                    <a href="{% url "lesson_by_week_and_period" note.year note.week note.lesson_period.pk %}">
-                      {{ note.lesson_period.get_subject.name }}<br/>
-                      {{ note.lesson_period.get_teacher_names }}
+                    <a href="{{ note.get_absolute_url }}">
+                      {% if note.register_object.get_subject %}
+                        {{ note.register_object.get_subject.name }}
+                      {% else %}
+                        {% trans "Event" %} ({{ note.register_object.title }})
+                      {% endif %}<br/>
+                      {{ note.register_object.teacher_names }}
                     </a>
                   </div>
 
diff --git a/aleksis/apps/alsijil/util/predicates.py b/aleksis/apps/alsijil/util/predicates.py
index a831cf0fd..2faca64dd 100644
--- a/aleksis/apps/alsijil/util/predicates.py
+++ b/aleksis/apps/alsijil/util/predicates.py
@@ -211,15 +211,15 @@ def is_personal_note_lesson_teacher(user: User, obj: PersonalNote) -> bool:
     Checks whether the person linked to the user is a teacher
     in the lesson or the substitution linked to the LessonPeriod of the given PersonalNote.
     """
-    if hasattr(obj, "lesson_period"):
-        if hasattr(obj.lesson_period, "lesson"):
+    if hasattr(obj, "register_object"):
+        if getattr(obj, "lesson_period", None):
             sub = obj.lesson_period.get_substitution()
             if sub and user.person in Person.objects.filter(
                 lesson_substitutions=obj.lesson_period.get_substitution()
             ):
                 return True
 
-            return user.person in obj.lesson_period.lesson.teachers.all()
+        return user.person in obj.register_object.get_teachers().all()
 
         return False
     return False
@@ -233,12 +233,11 @@ def is_personal_note_lesson_parent_group_owner(user: User, obj: PersonalNote) ->
     Checks whether the person linked to the user is the owner of
     any parent groups of any groups of the given LessonPeriod lesson of the given PersonalNote.
     """
-    if hasattr(obj, "lesson_period"):
-        if hasattr(obj.lesson_period, "lesson"):
-            for group in obj.lesson_period.lesson.groups.all():
-                for parent_group in group.parent_groups.all():
-                    if user.person in list(parent_group.owners.all()):
-                        return True
+    if hasattr(obj, "register_object"):
+        for group in obj.register_object.get_groups().all():
+            for parent_group in group.parent_groups.all():
+                if user.person in list(parent_group.owners.all()):
+                    return True
     return False
 
 
diff --git a/aleksis/apps/alsijil/views.py b/aleksis/apps/alsijil/views.py
index 5076124f7..447025ea8 100644
--- a/aleksis/apps/alsijil/views.py
+++ b/aleksis/apps/alsijil/views.py
@@ -4,6 +4,8 @@ from typing import Optional
 
 from django.core.exceptions import PermissionDenied
 from django.db.models import Count, Exists, OuterRef, Prefetch, Q, Subquery, Sum
+from django.db.models.expressions import Case, When
+from django.db.models.functions import Extract
 from django.http import Http404, HttpRequest, HttpResponse, HttpResponseNotFound
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse, reverse_lazy
@@ -641,13 +643,17 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
                         ):
                             raise PermissionDenied()
 
-                        notes = person.personal_notes.filter(
-                            week=date.isocalendar()[1],
-                            lesson_period__period__weekday=date.weekday(),
-                            lesson_period__lesson__validity__date_start__lte=date,
-                            lesson_period__lesson__validity__date_end__gte=date,
-                            absent=True,
-                            excused=False,
+                        notes = person.personal_notes.filter(absent=True, excused=False,).filter(
+                            Q(
+                                week=date.isocalendar()[1],
+                                lesson_period__period__weekday=date.weekday(),
+                                lesson_period__lesson__validity__date_start__lte=date,
+                                lesson_period__lesson__validity__date_end__gte=date,
+                            )
+                            | Q(
+                                extra_lesson__week=date.isocalendar()[1],
+                                extra_lesson__period__weekday=date.weekday(),
+                            )
                         )
                         for note in notes:
                             note.excused = True
@@ -687,19 +693,51 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
         allowed_personal_notes = person_personal_notes.all()
     else:
         allowed_personal_notes = person_personal_notes.filter(
-            lesson_period__lesson__groups__owners=request.user.person
+            Q(lesson_period__lesson__groups__owners=request.user.person)
+            | Q(extra_lesson__groups__owners=request.user.person)
+            | Q(event__groups__owners=request.user.person)
         )
 
     unexcused_absences = allowed_personal_notes.filter(absent=True, excused=False)
     context["unexcused_absences"] = unexcused_absences
 
-    personal_notes = allowed_personal_notes.filter(
-        Q(absent=True) | Q(late__gt=0) | ~Q(remarks="") | Q(extra_marks__isnull=False)
-    ).order_by(
-        "-lesson_period__lesson__validity__date_start",
-        "-week",
-        "lesson_period__period__weekday",
-        "lesson_period__period__period",
+    personal_notes = (
+        allowed_personal_notes.filter(
+            Q(absent=True) | Q(late__gt=0) | ~Q(remarks="") | Q(extra_marks__isnull=False)
+        )
+        .annotate(
+            school_term_start=Case(
+                When(event__isnull=False, then="event__school_term__date_start"),
+                When(extra_lesson__isnull=False, then="extra_lesson__school_term__date_start"),
+                When(
+                    lesson_period__isnull=False,
+                    then="lesson_period__lesson__validity__school_term__date_start",
+                ),
+            ),
+            order_year=Case(
+                When(event__isnull=False, then=Extract("event__date_start", "year")),
+                When(extra_lesson__isnull=False, then="extra_lesson__year"),
+                When(lesson_period__isnull=False, then="year"),
+            ),
+            order_week=Case(
+                When(event__isnull=False, then=Extract("event__date_start", "week")),
+                When(extra_lesson__isnull=False, then="extra_lesson__week"),
+                When(lesson_period__isnull=False, then="week"),
+            ),
+            order_weekday=Case(
+                When(event__isnull=False, then="event__period_from__weekday"),
+                When(extra_lesson__isnull=False, then="extra_lesson__period__weekday"),
+                When(lesson_period__isnull=False, then="lesson_period__period__weekday"),
+            ),
+            order_period=Case(
+                When(event__isnull=False, then="event__period_from__period"),
+                When(extra_lesson__isnull=False, then="extra_lesson__period__period"),
+                When(lesson_period__isnull=False, then="lesson_period__period__period"),
+            ),
+        )
+        .order_by(
+            "-school_term_start", "-order_year", "-order_week", "-order_weekday", "order_period",
+        )
     )
     context["personal_notes"] = personal_notes
     context["excuse_types"] = ExcuseType.objects.all()
@@ -711,8 +749,10 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
         stats = []
         for school_term in school_terms:
             stat = {}
-            personal_notes = PersonalNote.objects.filter(
-                person=person, lesson_period__lesson__validity__school_term=school_term
+            personal_notes = PersonalNote.objects.filter(person=person,).filter(
+                Q(lesson_period__lesson__validity__school_term=school_term)
+                | Q(extra_lesson__school_term=school_term)
+                | Q(event__school_term=school_term)
             )
 
             if not personal_notes.exists():
-- 
GitLab