Merge pull request #145 from nelc/shadinaif/removable-annotations

feat: removable annotations on pagination

futurex_openedx_extensions/__init__.py

@@ -1,3 +1,3 @@
 """One-line description for README and other doc files."""
 
-__version__ = '0.9.13'
+__version__ = '0.9.14'

futurex_openedx_extensions/dashboard/details/courses.py

@@ -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 (
@@ -29,6 +30,7 @@
     check_staff_exist_queryset,
     get_base_queryset_courses,
     get_one_user_queryset,
+    update_removable_annotations,
 )
 
 
@@ -62,6 +64,8 @@ def annotate_courses_rating_queryset(
         ), 0),
     )
 
+    update_removable_annotations(queryset, removable=['rating_count', 'rating_total'])
+
     return queryset
 
 
@@ -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(
@@ -144,6 +154,10 @@ def get_courses_queryset(
         )
     )
 
+    update_removable_annotations(queryset, removable=[
+        'enrolled_count', 'active_count', 'certificates_count', 'completion_rate',
+    ])
+
     return queryset
 
 
@@ -216,4 +230,6 @@ def get_learner_courses_info_queryset(
         )
     )
 
+    update_removable_annotations(queryset, removable=['related_user_id', 'enrollment_date', 'last_activity'])
+
     return queryset

futurex_openedx_extensions/dashboard/details/learners.py

@@ -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
@@ -17,6 +19,7 @@
     get_learners_search_queryset,
     get_one_user_queryset,
     get_permitted_learners_queryset,
+    update_removable_annotations,
 )
 
 
@@ -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.
 
@@ -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.
 
@@ -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(
@@ -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
 
 
@@ -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
 
 
@@ -249,4 +257,6 @@ def get_learner_info_queryset(
         )
     ).select_related('profile')
 
+    update_removable_annotations(queryset, removable=['courses_count', 'certificates_count'])
+
     return queryset

futurex_openedx_extensions/helpers/apps.py

@@ -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

futurex_openedx_extensions/helpers/exceptions.py

@@ -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."""

futurex_openedx_extensions/helpers/monkey_patches.py

@@ -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

futurex_openedx_extensions/helpers/pagination.py

@@ -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