Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: removable annotations on pagination #145

Merged
merged 2 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion futurex_openedx_extensions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""One-line description for README and other doc files."""

__version__ = '0.9.13'
__version__ = '0.9.14'
58 changes: 37 additions & 21 deletions futurex_openedx_extensions/dashboard/details/courses.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Courses details collectors"""
from __future__ import annotations

from common.djangoapps.student.models import CourseEnrollment
from completion.models import BlockCompletion
from django.contrib.auth import get_user_model
from django.db.models import (
Expand Down Expand Up @@ -29,6 +30,7 @@
check_staff_exist_queryset,
get_base_queryset_courses,
get_one_user_queryset,
update_removable_annotations,
)


Expand Down Expand Up @@ -62,6 +64,8 @@ def annotate_courses_rating_queryset(
), 0),
)

update_removable_annotations(queryset, removable=['rating_count', 'rating_total'])

return queryset


Expand Down Expand Up @@ -104,30 +108,36 @@ def get_courses_queryset(
if include_staff:
is_staff_queryset = Q(Value(False, output_field=BooleanField()))
else:
is_staff_queryset = check_staff_exist_queryset('courseenrollment__user_id', 'org', 'id')
is_staff_queryset = check_staff_exist_queryset(
ref_user_id='user_id', ref_org='course__org', ref_course_id='course_id',
)

queryset = queryset.annotate(
enrolled_count=Count(
'courseenrollment',
filter=(
Q(courseenrollment__is_active=True) &
Q(courseenrollment__user__is_active=True) &
Q(courseenrollment__user__is_staff=False) &
Q(courseenrollment__user__is_superuser=False) &
~is_staff_queryset
),
)
enrolled_count=Coalesce(Subquery(
CourseEnrollment.objects.filter(
course_id=OuterRef('id'),
is_active=True,
user__is_active=True,
user__is_staff=False,
user__is_superuser=False,
).filter(
~is_staff_queryset,
).values('course_id').annotate(count=Count('id')).values('count'),
output_field=IntegerField(),
), 0)
).annotate(
active_count=Count(
'courseenrollment',
filter=(
Q(courseenrollment__is_active=True) &
Q(courseenrollment__user__is_active=True) &
Q(courseenrollment__user__is_staff=False) &
Q(courseenrollment__user__is_superuser=False) &
~is_staff_queryset
),
)
active_count=Coalesce(Subquery(
CourseEnrollment.objects.filter(
course_id=OuterRef('id'),
is_active=True,
user__is_active=True,
user__is_staff=False,
user__is_superuser=False,
).filter(
~is_staff_queryset,
).values('course_id').annotate(count=Count('id')).values('count'),
output_field=IntegerField(),
), 0)
).annotate(
certificates_count=Coalesce(Subquery(
GeneratedCertificate.objects.filter(
Expand All @@ -144,6 +154,10 @@ def get_courses_queryset(
)
)

update_removable_annotations(queryset, removable=[
'enrolled_count', 'active_count', 'certificates_count', 'completion_rate',
])

return queryset


Expand Down Expand Up @@ -216,4 +230,6 @@ def get_learner_courses_info_queryset(
)
)

update_removable_annotations(queryset, removable=['related_user_id', 'enrollment_date', 'last_activity'])

return queryset
64 changes: 37 additions & 27 deletions futurex_openedx_extensions/dashboard/details/learners.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

from datetime import timedelta

from common.djangoapps.student.models import CourseEnrollment
from django.contrib.auth import get_user_model
from django.db.models import BooleanField, Case, Count, Exists, OuterRef, Q, Subquery, Value, When
from django.db.models import BooleanField, Case, Count, Exists, IntegerField, OuterRef, Q, Subquery, Value, When
from django.db.models.functions import Coalesce
from django.db.models.query import QuerySet
from django.utils import timezone
from lms.djangoapps.certificates.models import GeneratedCertificate
Expand All @@ -17,6 +19,7 @@
get_learners_search_queryset,
get_one_user_queryset,
get_permitted_learners_queryset,
update_removable_annotations,
)


Expand All @@ -25,7 +28,7 @@ def get_courses_count_for_learner_queryset(
visible_courses_filter: bool | None = True,
active_courses_filter: bool | None = None,
include_staff: bool = False,
) -> Count:
) -> Coalesce:
"""
Annotate the given queryset with the courses count for the learner.

Expand All @@ -37,36 +40,37 @@ def get_courses_count_for_learner_queryset(
:type active_courses_filter: bool | None
:param include_staff: flag to include staff users
:type include_staff: bool
:return: Count of learners
:rtype: Count
:return: Count of enrolled courses
:rtype: Coalesce
"""
if not include_staff:
is_staff_queryset = check_staff_exist_queryset(
ref_user_id='id', ref_org='courseenrollment__course__org', ref_course_id='courseenrollment__course_id',
ref_user_id='user_id', ref_org='course__org', ref_course_id='course_id',
)
else:
is_staff_queryset = Q(Value(False, output_field=BooleanField()))

return Count(
'courseenrollment',
filter=(
Q(courseenrollment__course_id__in=get_base_queryset_courses(
return Coalesce(Subquery(
CourseEnrollment.objects.filter(
user_id=OuterRef('id'),
course_id__in=get_base_queryset_courses(
fx_permission_info,
visible_filter=visible_courses_filter,
active_filter=active_courses_filter,
)) &
Q(courseenrollment__is_active=True) &
~is_staff_queryset
),
distinct=True
)
),
is_active=True,
).filter(
~is_staff_queryset,
).values('user_id').annotate(count=Count('id')).values('count'),
output_field=IntegerField(),
), 0)


def get_certificates_count_for_learner_queryset(
fx_permission_info: dict,
visible_courses_filter: bool | None = True,
active_courses_filter: bool | None = None,
) -> Count:
) -> Coalesce:
"""
Annotate the given queryset with the certificate counts.

Expand All @@ -76,23 +80,23 @@ def get_certificates_count_for_learner_queryset(
:type visible_courses_filter: bool | None
:param active_courses_filter: Value to filter courses on active status. None means no filter.
:type active_courses_filter: bool | None
:return: QuerySet of learners
:rtype: QuerySet
:return: Count of certificates
:rtype: Coalesce
"""
return Count(
'generatedcertificate',
filter=(
Q(generatedcertificate__course_id__in=Subquery(
return Coalesce(Subquery(
GeneratedCertificate.objects.filter(
user_id=OuterRef('id'),
course_id__in=Subquery(
get_base_queryset_courses(
fx_permission_info,
visible_filter=visible_courses_filter,
active_filter=active_courses_filter
).values_list('id', flat=True)
)) &
Q(generatedcertificate__status='downloadable')
),
distinct=True
)
),
status='downloadable',
).values('user_id').annotate(count=Count('id')).values('count'),
output_field=IntegerField(),
), 0)


def get_learners_queryset(
Expand Down Expand Up @@ -141,6 +145,8 @@ def get_learners_queryset(
)
).select_related('profile', 'extrainfo').order_by('id')

update_removable_annotations(queryset, removable=['courses_count', 'certificates_count'])

return queryset


Expand Down Expand Up @@ -202,6 +208,8 @@ def get_learners_by_course_queryset(
)
).select_related('profile').order_by('id')

update_removable_annotations(queryset, removable=['certificate_available', 'course_score', 'active_in_course'])

return queryset


Expand Down Expand Up @@ -249,4 +257,6 @@ def get_learner_info_queryset(
)
).select_related('profile')

update_removable_annotations(queryset, removable=['courses_count', 'certificates_count'])

return queryset
2 changes: 2 additions & 0 deletions futurex_openedx_extensions/helpers/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ class HelpersConfig(AppConfig):

def ready(self) -> None:
"""Connect handlers to send notifications about discussions."""
from futurex_openedx_extensions.helpers import \
monkey_patches # pylint: disable=unused-import, import-outside-toplevel
from futurex_openedx_extensions.helpers import signals # pylint: disable=unused-import, import-outside-toplevel
2 changes: 2 additions & 0 deletions futurex_openedx_extensions/helpers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class FXExceptionCodes(Enum):

SERIALIZER_FILED_NAME_DOES_NOT_EXIST = 7001

QUERY_SET_BAD_OPERATION = 8001


class FXCodedException(Exception):
"""Exception with a code."""
Expand Down
21 changes: 21 additions & 0 deletions futurex_openedx_extensions/helpers/monkey_patches.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
"""Monkey patches defined here."""
from __future__ import annotations

from typing import Any

from django.db.models.query import QuerySet

original_queryset_chain = QuerySet._chain # pylint: disable=protected-access


def customized_queryset_chain(self: Any, **kwargs: Any) -> QuerySet:
"""Customized queryset chain method for the QuerySet class."""
result = original_queryset_chain(self, **kwargs)

if hasattr(self, 'removable_annotations'):
result.removable_annotations = self.removable_annotations.copy()

return result


QuerySet._chain = customized_queryset_chain # pylint: disable=protected-access
24 changes: 24 additions & 0 deletions futurex_openedx_extensions/helpers/pagination.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,33 @@
"""Pagination helpers and classes for the API views."""
from django.core.paginator import Paginator
from django.db.models.query import QuerySet
from django.utils.functional import cached_property
from rest_framework.pagination import PageNumberPagination

from futurex_openedx_extensions.helpers.querysets import verify_queryset_removable_annotations


class DefaultPaginator(Paginator):
"""Default paginator settings for the API views."""
@cached_property
def count(self) -> int:
"""Return the total number of objects, across all pages."""
if isinstance(self.object_list, QuerySet) and hasattr(self.object_list, 'removable_annotations'):
verify_queryset_removable_annotations(self.object_list)

clone = self.object_list._chain() # pylint: disable=protected-access
for key in self.object_list.removable_annotations:
clone.query.annotations.pop(key, None)

return clone.count()

return super().count


class DefaultPagination(PageNumberPagination):
"""Default pagination settings for the API views."""
page_size: int = 20
page_size_query_param: str = 'page_size'
max_page_size: int = 100

django_paginator_class = DefaultPaginator
Loading