diff --git a/course/api/full_serializers.py b/course/api/full_serializers.py index cc613f0bd..11e2b0c0d 100644 --- a/course/api/full_serializers.py +++ b/course/api/full_serializers.py @@ -46,10 +46,13 @@ class Meta(AplusModelSerializer.Meta): 'exercises', ) - def get_exercises(self, obj): + def get_exercises(self, obj: CourseModule) -> OrderedDict: # List only exercises derived from BaseExercise, thus no Chapter texts (/exercise/ returns 404 for those) - exercises = BaseExercise.objects.filter(course_module=obj).all() - # FIXME: populating learning_object.parent_list() creates subqueries -> get exercise list and data from cache + # Check if the exercises were prefetched in the view + if hasattr(obj, 'exercises'): + exercises = obj.exercises + else: + exercises = BaseExercise.objects.filter(course_module=obj).all() serializer = ExerciseBriefSerializer(instance=exercises, many=True, context=self.context) return serializer.data diff --git a/course/api/views.py b/course/api/views.py index 9aa247d37..18bea82bf 100644 --- a/course/api/views.py +++ b/course/api/views.py @@ -6,9 +6,10 @@ from rest_framework.reverse import reverse from rest_framework_extensions.mixins import NestedViewSetMixin from rest_framework.permissions import IsAdminUser -from django.db.models import Q +from django.db.models import Prefetch, Q, QuerySet from django.utils.text import format_lazy from django.utils.translation import gettext_lazy as _ +from exercise.exercise_models import BaseExercise from lib.email_messages import email_course_instance from lib.viewbase import BaseMixin @@ -195,10 +196,15 @@ class CourseExercisesViewSet(NestedViewSetMixin, parent_lookup_map = {'course_id': 'course_instance.id'} serializer_class = CourseModuleSerializer - def get_queryset(self): - return ( CourseModule.objects - .get_visible(self.request.user) - .all() ) + def get_queryset(self) -> QuerySet[CourseModule]: + return ( + CourseModule.objects + .get_visible(self.request.user) + .prefetch_related( + Prefetch('learning_objects', BaseExercise.objects.all(), 'exercises') + ) + .all() + ) def get_object(self): return self.get_member_object('module', 'Exercise module') @@ -423,13 +429,17 @@ class CourseUsertaggingsViewSet(NestedViewSetMixin, lookup_field = 'id' lookup_url_kwarg = 'usertag_id' serializer_class = CourseUsertaggingsSerializer - queryset = ( UserTagging.objects - .select_related('tag', 'user', 'user__user') - .only('tag__id', 'tag__course_instance', 'tag__name', 'tag__slug', - 'user__user__id', 'user__user__email', 'user__user__username', 'user__student_id', - 'course_instance__id') - .order_by('user__user__id') - .all() ) + queryset = ( + UserTagging.objects + .select_related('tag', 'user', 'user__user') + .only('tag__id', 'tag__course_instance', 'tag__name', 'tag__slug', + 'user__user__id', 'user__user__email', 'user__user__username', 'user__student_id', + 'user__user__first_name', 'user__user__last_name', 'user__organization', + 'course_instance__id' + ) + .order_by('user__user__id') + .all() + ) parent_lookup_map = {'course_id': 'course_instance_id'} def get_serializer_context(self): @@ -494,10 +504,13 @@ class CourseOwnStudentGroupsViewSet(NestedViewSetMixin, serializer_class = CourseStudentGroupBriefSerializer parent_lookup_map = {'course_id': 'course_instance.id'} - def get_queryset(self): - return StudentGroup.objects.filter( - course_instance=self.instance, - members=self.request.user.userprofile, + def get_queryset(self) -> QuerySet[StudentGroup]: + return ( + StudentGroup.objects.filter( + course_instance=self.instance, + members=self.request.user.userprofile, + ) + .select_related('course_instance') ) @@ -521,7 +534,10 @@ class CourseStudentGroupsViewSet(NestedViewSetMixin, OnlyCourseTeacherPermission, ] serializer_class = CourseStudentGroupSerializer - queryset = StudentGroup.objects.all() + queryset = ( + StudentGroup.objects + .select_related('course_instance') + ) parent_lookup_map = {'course_id': 'course_instance.id'} class CourseNewsViewSet(NestedViewSetMixin, @@ -545,10 +561,10 @@ class CourseNewsViewSet(NestedViewSetMixin, serializer_class = CourseNewsSerializer parent_lookup_map = {'course_id': 'course_instance.id'} - def get_queryset(self): + def get_queryset(self) -> QuerySet[News]: user = self.request.user AUDIENCE = CourseInstance.ENROLLMENT_AUDIENCE - queryset = News.objects.all() + queryset = News.objects.select_related('course_instance') if not user.is_superuser and not self.instance.is_course_staff(user): if user.userprofile.is_external: return queryset.filter( diff --git a/course/models.py b/course/models.py index c6aeaa373..4b4a4f561 100644 --- a/course/models.py +++ b/course/models.py @@ -2,7 +2,7 @@ import json import logging import string -from typing import Any, Dict +from typing import Any, Dict, List import urllib.request, urllib.parse from random import randint, choice @@ -103,6 +103,11 @@ def get_url_kwargs(self): return dict(course_slug=self.url) +class StudentGroupManager(models.Manager): + def get_queryset(self) -> models.QuerySet['StudentGroup']: + return super().get_queryset().prefetch_related('members') + + class StudentGroup(models.Model): """ Stores a user group for a course instance. @@ -121,6 +126,8 @@ class StudentGroup(models.Model): auto_now_add=True, ) + objects = StudentGroupManager() + class Meta: verbose_name = _('MODEL_NAME_STUDENT_GROUP') verbose_name_plural = _('MODEL_NAME_STUDENT_GROUP_PLURAL') @@ -354,12 +361,20 @@ def tags_for_instance(self, course_instance): .select_related('tag') return [t.tag for t in ts] - def get_all(self, profile, course_instance): - qs = (self.filter(user=profile, - course_instance=course_instance) - .select_related('tag')) + def get_all(self, profile: UserProfile, course_instance: 'CourseInstance') -> List[UserTag]: + # Check if taggings for the course instance were prefetched + if hasattr(profile, 'instance_taggings'): + taggings = profile.instance_taggings + else: + taggings = ( + self.filter( + user=profile, + course_instance=course_instance + ) + .select_related('tag') + ) tags = [USERTAG_EXTERNAL if profile.is_external else USERTAG_INTERNAL] - tags.extend(t.tag for t in qs.all()) + tags.extend(t.tag for t in taggings) return tags def set(self, profile, tag): diff --git a/course/staff_views.py b/course/staff_views.py index 60ae95ae3..524697a64 100644 --- a/course/staff_views.py +++ b/course/staff_views.py @@ -1,5 +1,8 @@ import json +from typing import Any, Dict, List + from django.contrib import messages +from django.db import models from django.shortcuts import get_object_or_404 from django.urls import reverse from django.utils.text import format_lazy @@ -9,6 +12,7 @@ from authorization.permissions import ACCESS from lib.helpers import settings_text from lib.viewbase import BaseFormView, BaseTemplateView, BaseRedirectMixin +from userprofile.models import UserProfile from .cache.students import CachedStudent from .forms import EnrollStudentsForm, GroupEditForm from .models import ( @@ -43,7 +47,7 @@ def get_common_objects(self): 'internal_user_label', 'external_user_label', ) - def _get_students_with_tags(self): + def _get_students_with_tags(self) -> List[Dict[str, Any]]: ci = self.instance fake = '10' * 32 link = (reverse('user-results', @@ -56,11 +60,11 @@ def _get_students_with_tags(self): for k in Enrollment.ENROLLMENT_STATUS.keys() } - participants = ci.all_students.all() + participants = ci.all_students.prefetch_tags(ci) data = [] for participant in participants: user_id = participant.user.id - user_tags = CachedStudent(ci, user_id).data + user_tags = CachedStudent(ci, participant.user).data user_tags_html = ' '.join(tags[slug].html_label for slug in user_tags['tag_slugs'] if slug in tags) data.append({ 'id': participant.student_id or '', @@ -82,9 +86,13 @@ class GroupsView(CourseInstanceBaseView): access_mode = ACCESS.ASSISTANT template_name = "course/staff/groups.html" - def get_common_objects(self): + def get_common_objects(self) -> None: super().get_common_objects() - self.groups = list(self.instance.groups.all()) + self.groups = list( + self.instance.groups.prefetch_related(None).prefetch_related( + models.Prefetch('members', UserProfile.objects.prefetch_tags(self.instance)), + ) + ) self.note('groups') @@ -128,9 +136,12 @@ class GroupsDeleteView(CourseInstanceMixin, BaseRedirectMixin, BaseTemplateView) group_kw = "group_id" template_name = "course/staff/group_delete.html" - def get_resource_objects(self): + def get_resource_objects(self) -> None: super().get_resource_objects() - self.group = get_object_or_404(StudentGroup, + self.group = get_object_or_404( + StudentGroup.objects.prefetch_related(None).prefetch_related( + models.Prefetch('members', UserProfile.objects.prefetch_tags(self.instance)) + ), course_instance=self.instance, id=self._get_kwarg(self.group_kw), ) diff --git a/course/templates/course/_course_menu.html b/course/templates/course/_course_menu.html index d85792bee..ad22179a8 100644 --- a/course/templates/course/_course_menu.html +++ b/course/templates/course/_course_menu.html @@ -118,11 +118,12 @@

{{ group.label|parse_localization }}

{% endfor %} {% endfor %} -{% if instance.tabs.exists %} +{% for tab in instance.tabs.all %} +{% if forloop.first %}
  • {% translate "APPS" %}

  • -{% for tab in instance.tabs.all %} +{% endif %} {% endfor %} -{% endif %} {% if is_course_staff %}
  • diff --git a/course/templatetags/course.py b/course/templatetags/course.py index 1ac3dc914..290bdd15f 100644 --- a/course/templatetags/course.py +++ b/course/templatetags/course.py @@ -1,6 +1,9 @@ from datetime import timedelta +from typing import Any, Dict, List, Union + from django import template from django.conf import settings +from django.db import models from django.utils import timezone from django.utils.safestring import mark_safe from django.utils.translation import get_language @@ -112,9 +115,16 @@ def avatars(profiles): @register.inclusion_tag("course/_profiles.html") -def profiles(profiles, instance, is_teacher): +def profiles( + profiles: Union[UserProfile, List[UserProfile], models.QuerySet[UserProfile]], + instance: CourseInstance, + is_teacher: bool + ) -> Dict[str, Any]: if isinstance(profiles, UserProfile): profiles = [profiles] + elif isinstance(profiles, models.QuerySet): + # Avoid re-fetching the queryset + profiles = list(profiles) return { 'instance': instance, 'profiles': profiles, diff --git a/course/viewbase.py b/course/viewbase.py index f40868485..0257d6c09 100644 --- a/course/viewbase.py +++ b/course/viewbase.py @@ -140,9 +140,9 @@ def handle_exception(self, exc): return super().handle_exception(exc) class CourseInstanceMixin(CourseInstanceBaseMixin, UserProfileMixin): - def get_course_instance_object(self): + def get_course_instance_object(self) -> CourseInstance: return get_object_or_404( - CourseInstance, + CourseInstance.objects.prefetch_related('tabs'), url=self.kwargs[self.instance_kw], course__url=self.kwargs[self.course_kw], ) diff --git a/deviations/viewbase.py b/deviations/viewbase.py index 35822d08a..ae9ce1cd8 100644 --- a/deviations/viewbase.py +++ b/deviations/viewbase.py @@ -238,7 +238,11 @@ def get_deviation_groups( ordered_deviations = ( all_deviations - .select_related() + .select_related( + 'submitter', 'submitter__user', + 'granter', 'granter__user', + 'exercise', 'exercise__course_module', + ) # parent is prefetched because there may be multiple ancestors, and # they are needed for building the deviation's URL. .prefetch_related('exercise__parent') diff --git a/edit_course/templates/edit_course/edit_content.html b/edit_course/templates/edit_course/edit_content.html index 6b5ab0c82..ac8f0464c 100644 --- a/edit_course/templates/edit_course/edit_content.html +++ b/edit_course/templates/edit_course/edit_content.html @@ -38,7 +38,7 @@

    {% translate "EXERCISE_CATEGORIES" %}

    - {% for category in instance.categories.all %} + {% for category in categories %}
    {{ category.name|parse_localization }} @@ -48,7 +48,7 @@

    {% translate "EXERCISE_CATEGORIES" %}

    {% translate "EDIT_CATEGORY" %} - {% if category.learning_objects.count == 0 %} + {% if category.count_lobjects == 0 %} {% translate "REMOVE" %} diff --git a/edit_course/views.py b/edit_course/views.py index 1c69d52cb..ac1ced483 100644 --- a/edit_course/views.py +++ b/edit_course/views.py @@ -3,7 +3,7 @@ from django.contrib import messages from django.contrib.auth import login as auth_login from django.contrib.auth.models import User -from django.db import IntegrityError +from django.db import models, IntegrityError from django.http.response import Http404, HttpResponse from django.urls import reverse from django.utils.text import format_lazy, capfirst @@ -73,21 +73,27 @@ class EditContentView(EditInstanceView): template_name = "edit_course/edit_content.html" form_class = CourseContentForm - def get_common_objects(self): - self.modules = list(self.instance.course_modules.all()) + def get_common_objects(self) -> None: + self.modules = list( + self.instance.course_modules.prefetch_related( + models.Prefetch('learning_objects', LearningObject.objects.all()), + ), + ) for module in self.modules: + learning_objects = {lobject.id: lobject.as_leaf_class() for lobject in module.learning_objects.all()} module.flat_objects = [] try: for entry in self.content.flat_module(module, enclosed=False): if entry['type'] != 'level': - try: - module.flat_objects.append( - LearningObject.objects.get(id=entry['id']).as_leaf_class()) - except LearningObject.DoesNotExist: - continue + learning_object = learning_objects.get(entry['id']) + if learning_object: + module.flat_objects.append(learning_object) except NoSuchContent: continue - self.note('modules') + self.categories = self.instance.categories.annotate( + count_lobjects=models.Count('learning_objects') + ) + self.note('modules', 'categories') def get_success_url(self): return self.instance.get_url('course-edit') diff --git a/exercise/api/csv/views.py b/exercise/api/csv/views.py index 1ea1bf6e2..9a62dba2f 100644 --- a/exercise/api/csv/views.py +++ b/exercise/api/csv/views.py @@ -1,7 +1,7 @@ from typing import Any, Dict, Optional, Set, Union from django.db.models.aggregates import Count -from django.db.models.query import QuerySet +from django.db.models.query import Prefetch, QuerySet from rest_framework import viewsets from rest_framework.request import Request from rest_framework.response import Response @@ -17,7 +17,7 @@ from userprofile.models import UserProfile from ...cache.points import CachedPoints -from ...models import Submission +from ...models import BaseExercise, Submission from .submission_sheet import filter_best_submissions, submissions_sheet from .aggregate_sheet import aggregate_sheet from .aggregate_points import aggregate_points @@ -101,7 +101,7 @@ def list( queryset = Submission.objects.filter( exercise_id__in=ids, submitters__in=profiles - ) + ).prefetch_related(Prefetch('exercise', BaseExercise.objects.all()), 'notifications', 'files') return self.serialize_submissions(request, queryset, revealed_ids, best=search_args['best']) def retrieve( @@ -116,7 +116,9 @@ def retrieve( points = CachedPoints(self.instance, profile.user, self.content, self.is_course_staff) ids = points.submission_ids(**search_args) revealed_ids = get_revealed_exercise_ids(search_args, points) - queryset = Submission.objects.filter(id__in=ids) + queryset = Submission.objects.filter( + id__in=ids + ).prefetch_related(Prefetch('exercise', BaseExercise.objects.all()), 'notifications', 'files') return self.serialize_submissions(request, queryset, revealed_ids) def serialize_submissions( diff --git a/exercise/cache/content.py b/exercise/cache/content.py index ee4a3b080..78e3973c8 100644 --- a/exercise/cache/content.py +++ b/exercise/cache/content.py @@ -1,5 +1,6 @@ from typing import Any, Dict, List, Optional, Type, Union +from django.db.models import Prefetch from django.db.models.base import Model from django.db.models.signals import post_save, post_delete from django.utils import timezone @@ -98,7 +99,15 @@ def recursion( # Collect each module. i = 0 - for module in instance.course_modules.all(): + for module in instance.course_modules.prefetch_related( + 'requirements', + 'requirements__threshold__passed_modules', + 'requirements__threshold__passed_categories', + 'requirements__threshold__passed_exercises', + 'requirements__threshold__passed_exercises__parent', + 'requirements__threshold__points', + Prefetch('learning_objects', LearningObject.objects.all()), + ): entry = { 'type': 'module', 'id': module.id, diff --git a/exercise/staff_views.py b/exercise/staff_views.py index 76d97c646..297cdceb8 100644 --- a/exercise/staff_views.py +++ b/exercise/staff_views.py @@ -7,7 +7,7 @@ from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied, ValidationError from django.core.validators import URLValidator -from django.db.models import Count, F, Max, Q +from django.db.models import Count, F, Max, Prefetch, Q from django.http.request import HttpRequest from django.http.response import HttpResponse, JsonResponse, Http404 from django.shortcuts import get_object_or_404 @@ -58,13 +58,18 @@ class ListSubmissionsView(ExerciseListBaseView): ajax_template_name = "exercise/staff/_submissions_table.html" default_limit = 50 - def get_common_objects(self): + def get_common_objects(self) -> None: super().get_common_objects() if not self.exercise.is_submittable: raise Http404() - qs = self.exercise.submissions\ - .defer("feedback", "submission_data", "grading_data")\ - .prefetch_related('submitters').all() + qs = ( + self.exercise.submissions + .defer("feedback", "submission_data", "grading_data") + .prefetch_related(None) + .prefetch_related( + Prefetch('submitters', UserProfile.objects.prefetch_tags(self.instance)), + ) + ) self.all = self.request.GET.get('all', None) self.all_url = self.exercise.get_submission_list_url() + "?all=yes" self.submissions = qs if self.all else qs[:self.default_limit] @@ -115,7 +120,12 @@ def get_common_objects(self) -> None: ) # Get a dict of submitters, accessed by their id. - profiles = UserProfile.objects.filter(submissions__exercise=self.exercise).in_bulk() + profiles = ( + UserProfile.objects + .filter(submissions__exercise=self.exercise) + .prefetch_tags(self.instance) + .in_bulk() + ) # Add UserProfile instances to the dicts in submitter_summaries, so we can # use the 'profiles' template tag. for submitter_summary in submitter_summaries: @@ -499,8 +509,10 @@ class EditSubmittersView(SubmissionMixin, BaseFormView): template_name = "exercise/staff/edit_submitters.html" form_class = EditSubmittersForm - def get_common_objects(self): - self.groups = self.instance.groups.all() + def get_common_objects(self) -> None: + self.groups = self.instance.groups.prefetch_related(None).prefetch_related( + Prefetch('members', UserProfile.objects.prefetch_tags(self.instance)), + ) self.note('groups') def get_form_kwargs(self): diff --git a/exercise/templatetags/exercise.py b/exercise/templatetags/exercise.py index 22a4e1ee0..dc79ea852 100644 --- a/exercise/templatetags/exercise.py +++ b/exercise/templatetags/exercise.py @@ -4,7 +4,7 @@ from django import template from django.contrib.auth.models import User -from django.db.models import Max, Min +from django.db import models from django.template.context import Context from django.template.loader import render_to_string from django.utils import timezone @@ -75,6 +75,16 @@ def user_results(context: Context, student: Optional[User] = None) -> Dict[str, values['total_json'] = json.dumps(values['total']) if student: values['is_course_staff'] = False + if values['is_course_staff']: + instance = context['instance'] + values['student_count'] = instance.students.count() + counts = (instance.students + .values('submissions__exercise_id') + .annotate(count=models.Count('submissions__submitters', distinct=True)) + .filter(submissions__exercise__course_module__course_instance=instance) + .order_by() + ) + values['exercise_submitter_counts'] = {row['submissions__exercise_id']: row['count'] for row in counts} return values @@ -314,7 +324,7 @@ def get_grading_errors(submission): @register.inclusion_tag("exercise/_text_stats.html", takes_context=True) -def exercise_text_stats(context, exercise): +def exercise_text_stats(context: Context, exercise: Union[int, BaseExercise]) -> Dict[str, Any]: if not 'instance' in context: raise TagUsageError() instance = context['instance'] @@ -324,7 +334,10 @@ def exercise_text_stats(context, exercise): total = context['student_count'] if isinstance(exercise, int): - num = instance.students.filter(submissions__exercise_id=exercise).distinct().count() + if 'exercise_submitter_counts' in context: + num = context['exercise_submitter_counts'].get(exercise, 0) + else: + num = instance.students.filter(submissions__exercise_id=exercise).distinct().count() else: num = exercise.number_of_submitters() if exercise else 0 return { diff --git a/exercise/viewbase.py b/exercise/viewbase.py index b21b74806..dac8f4f1a 100644 --- a/exercise/viewbase.py +++ b/exercise/viewbase.py @@ -3,6 +3,7 @@ from django.contrib import messages from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied +from django.db import models from django.http import Http404 from django.shortcuts import get_object_or_404 from django.utils import timezone @@ -13,6 +14,8 @@ from authorization.permissions import ACCESS from course.viewbase import CourseModuleMixin from lib.viewbase import BaseTemplateView, BaseView +from userprofile.models import UserProfile + from .cache.hierarchy import NoSuchContent from .cache.points import CachedPoints from .exercise_summary import UserExerciseSummary @@ -188,9 +191,11 @@ def get_resource_objects(self) -> None: class SubmissionMixin(SubmissionBaseMixin, ExerciseMixin): - def get_submission_object(self): + def get_submission_object(self) -> Submission: return get_object_or_404( - Submission, + Submission.objects.prefetch_related(None).prefetch_related( + models.Prefetch('submitters', UserProfile.objects.prefetch_tags(self.instance)) + ), id=self.kwargs[self.submission_kw], exercise=self.exercise ) diff --git a/userprofile/models.py b/userprofile/models.py index 01298ed46..4190f6355 100644 --- a/userprofile/models.py +++ b/userprofile/models.py @@ -15,14 +15,31 @@ if TYPE_CHECKING: from django.db.models.manager import RelatedManager + from course.models import CourseInstance from exercise.models import BaseExercise, Submission, SubmissionDraft from external_services.models import LTIService + +class UserProfileQuerySet(models.QuerySet['UserProfile']): + def prefetch_tags(self, instance: 'CourseInstance', to_attr: str = 'instance_taggings') -> 'UserProfileQuerySet': + return self.prefetch_related( + models.Prefetch( + 'taggings', + instance.taggings.select_related('tag'), + to_attr, + ), + ) + + class UserProfileManager(models.Manager): + _queryset_class = UserProfileQuerySet def get_queryset(self): return super().get_queryset().select_related("user") + def prefetch_tags(self, instance: 'CourseInstance', to_attr: str = 'instance_taggings') -> UserProfileQuerySet: + return self.all().prefetch_tags(instance, to_attr) + class UserProfile(models.Model): """