From 6c29d76d737bae7a58706c034077d482bc16ff3b Mon Sep 17 00:00:00 2001
From: Jonathan Weth <git@jonathanweth.de>
Date: Mon, 7 Sep 2020 14:26:43 +0200
Subject: [PATCH] Use select_related and prefetch_related everywhere and
optimize query count
---
aleksis/apps/alsijil/model_extensions.py | 13 +-
aleksis/apps/alsijil/views.py | 161 +++++++++++++++--------
2 files changed, 110 insertions(+), 64 deletions(-)
diff --git a/aleksis/apps/alsijil/model_extensions.py b/aleksis/apps/alsijil/model_extensions.py
index 4c5872aa8..b4c2ac1ca 100644
--- a/aleksis/apps/alsijil/model_extensions.py
+++ b/aleksis/apps/alsijil/model_extensions.py
@@ -119,7 +119,7 @@ def get_personal_notes(self, persons: QuerySet, wanted_week: CalendarWeek):
for personal_note in new_personal_notes:
personal_note.groups_of_person.set(personal_note.person.member_of.all())
- return PersonalNote.objects.select_related("person").filter(
+ return PersonalNote.objects.filter(
lesson_period=self,
week=wanted_week.week,
year=wanted_week.year,
@@ -165,12 +165,11 @@ def get_lesson_documentation(
"""Get lesson documentation object for this lesson."""
if not week:
week = self.week
- try:
- return LessonDocumentation.objects.get(
- lesson_period=self, week=week.week, year=week.year
- )
- except LessonDocumentation.DoesNotExist:
- return None
+ # Use all to make effect of prefetched data
+ for documentation in self.documentations.all():
+ if documentation.week == week.week and documentation.year == week.year:
+ return documentation
+ return None
@LessonPeriod.method
diff --git a/aleksis/apps/alsijil/views.py b/aleksis/apps/alsijil/views.py
index 24c73d2e6..6585929aa 100644
--- a/aleksis/apps/alsijil/views.py
+++ b/aleksis/apps/alsijil/views.py
@@ -2,7 +2,7 @@ from datetime import date, datetime, timedelta
from typing import Optional
from django.core.exceptions import PermissionDenied
-from django.db.models import Count, Exists, OuterRef, Q, Subquery, Sum
+from django.db.models import Count, Exists, OuterRef, Prefetch, Q, Subquery, Sum
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
@@ -16,7 +16,7 @@ from reversion.views import RevisionMixin
from rules.contrib.views import PermissionRequiredMixin, permission_required
from aleksis.apps.chronos.managers import TimetableType
-from aleksis.apps.chronos.models import LessonPeriod, TimePeriod
+from aleksis.apps.chronos.models import LessonPeriod, LessonSubstitution, TimePeriod
from aleksis.apps.chronos.util.chronos_helpers import get_el_by_pk
from aleksis.apps.chronos.util.date import get_weeks_for_year, week_weekday_to_date
from aleksis.core.mixins import AdvancedCreateView, AdvancedDeleteView, AdvancedEditView
@@ -179,17 +179,9 @@ def week_view(
instance = get_instance_by_pk(request, year, week, type_, id_)
- lesson_periods = LessonPeriod.objects.in_week(wanted_week).annotate(
- has_documentation=Exists(
- LessonDocumentation.objects.filter(
- ~Q(topic__exact=""),
- lesson_period=OuterRef("pk"),
- week=wanted_week.week,
- year=wanted_week.year,
- )
- )
- )
+ lesson_periods = LessonPeriod.objects.in_week(wanted_week)
+ lesson_periods_query_exists = True
if type_ and id_:
if isinstance(instance, HttpResponseNotFound):
return HttpResponseNotFound()
@@ -204,6 +196,7 @@ def week_view(
else:
lesson_periods = lesson_periods.filter_participant(request.user.person)
else:
+ lesson_periods_query_exists = False
lesson_periods = None
# Add a form to filter the view
@@ -231,10 +224,38 @@ def week_view(
else:
group = None
- if lesson_periods:
- # Aggregate all personal notes for this group and week
+ extra_marks = ExtraMark.objects.all()
+
+ if lesson_periods_query_exists:
lesson_periods_pk = list(lesson_periods.values_list("pk", flat=True))
+ lesson_periods = (
+ LessonPeriod.objects.prefetch_related(
+ Prefetch(
+ "documentations",
+ queryset=LessonDocumentation.objects.filter(
+ week=wanted_week.week, year=wanted_week.year
+ ),
+ )
+ )
+ .filter(pk__in=lesson_periods_pk)
+ .annotate_week(wanted_week)
+ .annotate(
+ has_documentation=Exists(
+ LessonDocumentation.objects.filter(
+ ~Q(topic__exact=""),
+ lesson_period=OuterRef("pk"),
+ week=wanted_week.week,
+ year=wanted_week.year,
+ )
+ )
+ )
+ .order_by("period__weekday", "period__period")
+ )
+ else:
+ lesson_periods_pk = []
+ if lesson_periods_pk:
+ # Aggregate all personal notes for this group and week
persons_qs = Person.objects.filter(is_active=True)
if not request.user.has_perm("alsijil.view_week_personalnote", instance):
@@ -285,7 +306,7 @@ def week_view(
)
)
- for extra_mark in ExtraMark.objects.all():
+ for extra_mark in extra_marks:
persons_qs = persons_qs.annotate(
**{
extra_mark.count_label: Count(
@@ -306,22 +327,19 @@ def week_view(
persons.append(
{
"person": person,
- "personal_notes": person.personal_notes.filter(
- week=wanted_week.week,
- year=wanted_week.year,
- lesson_period__in=lesson_periods_pk,
+ "personal_notes": list(
+ person.personal_notes.filter(
+ week=wanted_week.week,
+ year=wanted_week.year,
+ lesson_period__in=lesson_periods_pk,
+ ).prefetch_related("lesson_period__substitutions__subject")
),
}
)
else:
persons = None
- # Resort lesson periods manually because an union queryset doesn't support order_by
- lesson_periods = sorted(
- lesson_periods, key=lambda x: (x.period.weekday, x.period.period)
- )
-
- context["extra_marks"] = ExtraMark.objects.all()
+ context["extra_marks"] = extra_marks
context["week"] = wanted_week
context["weeks"] = get_weeks_for_year(year=wanted_week.year)
context["lesson_periods"] = lesson_periods
@@ -369,7 +387,14 @@ def full_register_group(request: HttpRequest, id_: int) -> HttpResponse:
LessonPeriod.objects.filter_group(group)
.filter(lesson__validity__school_term=current_school_term)
.distinct()
- .prefetch_related("documentations", "personal_notes")
+ .prefetch_related(
+ "documentations",
+ "personal_notes",
+ "personal_notes__excuse_type",
+ "personal_notes__extra_marks",
+ "personal_notes__person",
+ "personal_notes__groups_of_person",
+ )
)
weeks = CalendarWeek.weeks_within(
@@ -404,35 +429,49 @@ def full_register_group(request: HttpRequest, id_: int) -> HttpResponse:
(lesson_period, documentations, notes, substitution)
)
- persons = Person.objects.filter(
- personal_notes__groups_of_person=group,
- personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
- ).annotate(
- absences_count=Count(
- "personal_notes__absent",
- filter=Q(
- personal_notes__absent=True,
- personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ persons = (
+ Person.objects.prefetch_related(
+ "personal_notes",
+ "personal_notes__excuse_type",
+ "personal_notes__extra_marks",
+ "personal_notes__lesson_period__lesson__subject",
+ "personal_notes__lesson_period__substitutions",
+ "personal_notes__lesson_period__substitutions__subject",
+ "personal_notes__lesson_period__substitutions__teachers",
+ "personal_notes__lesson_period__lesson__teachers",
+ "personal_notes__lesson_period__period",
+ )
+ .filter(
+ personal_notes__groups_of_person=group,
+ personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ )
+ .annotate(
+ absences_count=Count(
+ "personal_notes__absent",
+ filter=Q(
+ personal_notes__absent=True,
+ personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ ),
),
- ),
- excused=Count(
- "personal_notes__absent",
- filter=Q(
- personal_notes__absent=True,
- personal_notes__excused=True,
- personal_notes__excuse_type__isnull=True,
- personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ excused=Count(
+ "personal_notes__absent",
+ filter=Q(
+ personal_notes__absent=True,
+ personal_notes__excused=True,
+ personal_notes__excuse_type__isnull=True,
+ personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ ),
),
- ),
- unexcused=Count(
- "personal_notes__absent",
- filter=Q(
- personal_notes__absent=True,
- personal_notes__excused=False,
- personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ unexcused=Count(
+ "personal_notes__absent",
+ filter=Q(
+ personal_notes__absent=True,
+ personal_notes__excused=False,
+ personal_notes__lesson_period__lesson__validity__school_term=current_school_term,
+ ),
),
- ),
- tardiness=Sum("personal_notes__late"),
+ tardiness=Sum("personal_notes__late"),
+ )
)
for extra_mark in ExtraMark.objects.all():
@@ -578,7 +617,11 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
person.refresh_from_db()
- allowed_personal_notes = person.personal_notes.all()
+ allowed_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")
@@ -602,6 +645,8 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
context["personal_notes"] = personal_notes
context["excuse_types"] = ExcuseType.objects.all()
+ extra_marks = ExtraMark.objects.all()
+ excuse_types = ExcuseType.objects.all()
if request.user.has_perm("alsijil.view_person_statistics_personalnote", person):
school_terms = SchoolTerm.objects.all().order_by("-date_start")
stats = []
@@ -631,14 +676,14 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
)
stat.update(personal_notes.aggregate(tardiness=Sum("late")))
- for extra_mark in ExtraMark.objects.all():
+ for extra_mark in extra_marks:
stat.update(
personal_notes.filter(extra_marks=extra_mark).aggregate(
**{extra_mark.count_label: Count("pk")}
)
)
- for excuse_type in ExcuseType.objects.all():
+ for excuse_type in excuse_types:
stat.update(
personal_notes.filter(
absent=True, excuse_type=excuse_type
@@ -647,8 +692,10 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
stats.append((school_term, stat))
context["stats"] = stats
- context["excuse_types"] = ExcuseType.objects.all()
- context["extra_marks"] = ExtraMark.objects.all()
+
+ context["excuse_types"] = excuse_types
+ context["extra_marks"] = extra_marks
+
return render(request, "alsijil/class_register/person.html", context)
--
GitLab