Skip to content

Commit

Permalink
fix performance issue
Browse files Browse the repository at this point in the history
  • Loading branch information
tehreem-sadat committed Dec 2, 2024
1 parent b2c0bce commit 98de523
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 34 deletions.
31 changes: 6 additions & 25 deletions futurex_openedx_extensions/dashboard/details/learners.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
"""Learners details collectors"""
from __future__ import annotations

import re
from datetime import timedelta

from common.djangoapps.student.models import CourseEnrollment
Expand All @@ -15,9 +14,8 @@
from lms.djangoapps.grades.models import PersistentCourseGrade
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers import constants as cs
from futurex_openedx_extensions.helpers.exceptions import FXCodedException, FXExceptionCodes
from futurex_openedx_extensions.helpers.extractors import get_partial_access_course_ids
from futurex_openedx_extensions.helpers.extractors import get_partial_access_course_ids, verify_course_ids
from futurex_openedx_extensions.helpers.querysets import (
check_staff_exist_queryset,
get_base_queryset_courses,
Expand Down Expand Up @@ -295,37 +293,20 @@ def get_learners_enrollments_queryset(
message=f'Invalid user ids: {invalid_user_ids}',
)
accessible_users = accessible_users.filter(id__in=user_ids)
invalid_user_ids = set(user_ids) - set(accessible_users.values_list('id', flat=True)) # type: ignore
if invalid_user_ids:
raise FXCodedException(
code=FXExceptionCodes.INVALID_INPUT,
message=f'Inaccessible user ids: {invalid_user_ids}',
)

accessible_courses = CourseOverview.objects.filter(
Q(id__in=get_partial_access_course_ids(fx_permission_info)) |
Q(org__in=fx_permission_info['view_allowed_full_access_orgs'])
)
if course_ids:
invalid_course_ids = [course_id for course_id in course_ids if not re.match(cs.COURSE_ID_REGX, course_id)]
if invalid_course_ids:
raise FXCodedException(
code=FXExceptionCodes.INVALID_INPUT,
message=f'Invalid course ids: {invalid_course_ids}',
)
verify_course_ids(course_ids)
accessible_courses = accessible_courses.filter(id__in=course_ids)
invalid_course_ids = set(course_ids) - {str(course.id) for course in accessible_courses} # type: ignore
if invalid_course_ids:
raise FXCodedException(
code=FXExceptionCodes.INVALID_INPUT,
message=f'Inaccessible course ids: {invalid_course_ids}',
)

queryset = CourseEnrollment.objects.filter(
Q(is_active=True) & Q(course__in=accessible_courses) & Q(user__in=accessible_users)
)

queryset = queryset.annotate(
is_active=True,
course__in=Subquery(accessible_courses.values('id')),
user__in=Subquery(accessible_users.values('id'))
).annotate(
certificate_available=Exists(
GeneratedCertificate.objects.filter(
user_id=OuterRef('user_id'),
Expand Down
2 changes: 1 addition & 1 deletion futurex_openedx_extensions/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ def get_queryset(self) -> QuerySet:
excluded_role_types=dummy_serializers.query_params['excluded_role_types'],
).values('user_id').distinct().order_by()
).select_related('profile').order_by('id')
except ValueError as exc:
except (ValueError, FXCodedException) as exc:
raise ParseError(f'Invalid parameter: {exc}') from exc

return q_set
Expand Down
16 changes: 13 additions & 3 deletions futurex_openedx_extensions/helpers/extractors.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from xmodule.modulestore.django import modulestore

from futurex_openedx_extensions.helpers import constants as cs
from futurex_openedx_extensions.helpers.exceptions import FXCodedException, FXExceptionCodes


@dataclass
Expand Down Expand Up @@ -118,13 +119,22 @@ def verify_course_ids(course_ids: List[str]) -> None:
:type course_ids: List[str]
"""
if course_ids is None:
raise ValueError('course_ids must be a list of course IDs, but got None')
raise FXCodedException(
code=FXExceptionCodes.INVALID_INPUT,
message='course_ids must be a list of course IDs, but got None',
)

for course_id in course_ids:
if not isinstance(course_id, str):
raise ValueError(f'course_id must be a string, but got {type(course_id).__name__}')
raise FXCodedException(
code=FXExceptionCodes.INVALID_INPUT,
message=f'course_id must be a string, but got {type(course_id).__name__}',
)
if not re.search(cs.COURSE_ID_REGX_EXACT, course_id) and not re.search(cs.LIBRARY_ID_REGX_EXACT, course_id):
raise ValueError(f'Invalid course ID format: {course_id}')
raise FXCodedException(
code=FXExceptionCodes.INVALID_INPUT,
message=f'Invalid course ID format: {course_id}',
)


def get_orgs_of_courses(course_ids: List[str]) -> Dict[str, Any]:
Expand Down
4 changes: 1 addition & 3 deletions tests/test_dashboard/test_details/test_details_learners.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,8 @@ def test_get_learners_enrollments_queryset_annotations(
([21], None, '', 3, 'only user_ids filter'),
([], ['course-v1:ORG1+5+5'], '', 2, 'only course_ids filter'),
([15, 29], ['course-v1:ORG1+5+5'], '', 1, 'both course_ids and user_ids filter'),
([15, 29, 2], [], 'Inaccessible user ids: {2}', 0, 'inaccessible user ids.'),
([15, 29, 'not-int', 0.2], [], 'Invalid user ids: [\'not-int\', 0.2]', 0, 'invalid user ids.'),
(None, ['course-v1:ORG3+1+1'], 'Inaccessible course ids: {\'course-v1:ORG3+1+1\'}', 0, ''),
([15, 29], ['course-v1:ORG1+5+5', 'invalid'], 'Invalid course ids: [\'invalid\']', 0, ''),
([15, 29], ['course-v1:ORG1+5+5', 'invalid'], 'Invalid course ID format: invalid', 0, ''),
]
)
@patch('futurex_openedx_extensions.dashboard.details.learners.get_permitted_learners_queryset')
Expand Down
3 changes: 2 additions & 1 deletion tests/test_helpers/test_extractors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview

from futurex_openedx_extensions.helpers import constants as cs
from futurex_openedx_extensions.helpers.exceptions import FXCodedException
from futurex_openedx_extensions.helpers.extractors import (
DictHashcode,
DictHashcodeSet,
Expand Down Expand Up @@ -68,7 +69,7 @@ def test_verify_course_ids_success(course_ids):
])
def test_verify_course_ids_fail(course_ids, error_message):
"""Verify that verify_course_ids raises an error for invalid course IDs."""
with pytest.raises(ValueError) as exc:
with pytest.raises(FXCodedException) as exc:
verify_course_ids(course_ids)

assert str(exc.value) == error_message
Expand Down
2 changes: 1 addition & 1 deletion tests/test_helpers/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ def test_get_roles_for_users_queryset_bad_exclude(bad_role_type, expected_error_

def test_get_roles_for_users_queryset_bad_course_id():
"""Verify that get_roles_for_users_queryset raises an error if the course_id parameter is invalid."""
with pytest.raises(ValueError) as exc_info:
with pytest.raises(FXCodedException) as exc_info:
get_course_access_roles_queryset(
orgs_filter=['org1', 'org2'],
remove_redundant=False,
Expand Down

0 comments on commit 98de523

Please sign in to comment.