diff --git a/futurex_openedx_extensions/__init__.py b/futurex_openedx_extensions/__init__.py index c801816d..22e5c00d 100644 --- a/futurex_openedx_extensions/__init__.py +++ b/futurex_openedx_extensions/__init__.py @@ -1,3 +1,3 @@ """One-line description for README and other doc files.""" -__version__ = "0.3.8" +__version__ = "0.4.5" diff --git a/futurex_openedx_extensions/dashboard/details/courses.py b/futurex_openedx_extensions/dashboard/details/courses.py index 3528a37c..ab3bcc7e 100644 --- a/futurex_openedx_extensions/dashboard/details/courses.py +++ b/futurex_openedx_extensions/dashboard/details/courses.py @@ -30,7 +30,7 @@ def get_courses_queryset( - tenant_ids: List, search_text: str = None, visible_filter: bool = True, active_filter: bool = None + tenant_ids: List, search_text: str = None, visible_filter: bool | None = True, active_filter: bool | None = None ) -> QuerySet: """ Get the courses queryset for the given tenant IDs and search text. @@ -39,10 +39,10 @@ def get_courses_queryset( :type tenant_ids: List :param search_text: Search text to filter the courses by :type search_text: str - :param visible_filter: Whether to only include courses that are visible in the catalog - :type visible_filter: bool - :param active_filter: Whether to only include active courses - :type active_filter: bool + :param visible_filter: Value to filter courses on catalog visibility. None means no filter + :type visible_filter: bool | None + :param active_filter: Value to filter courses on active status. None means no filter + :type active_filter: bool | None :return: QuerySet of courses :rtype: QuerySet """ @@ -121,7 +121,7 @@ def get_courses_queryset( def get_learner_courses_info_queryset( - tenant_ids: List, user_id: int, visible_filter: bool = True, active_filter: bool = None + tenant_ids: List, user_id: int, visible_filter: bool | None = True, active_filter: bool | None = None ) -> QuerySet: """ Get the learner's courses queryset for the given user ID. This method assumes a valid user ID. @@ -130,10 +130,10 @@ def get_learner_courses_info_queryset( :type tenant_ids: List :param user_id: The user ID to get the learner for :type user_id: int - :param visible_filter: Whether to only count courses that are visible in the catalog - :type visible_filter: bool - :param active_filter: Whether to only count active courses - :type active_filter: bool + :param visible_filter: Value to filter courses on catalog visibility. None means no filter + :type visible_filter: bool | None + :param active_filter: Value to filter courses on active status. None means no filter + :type active_filter: bool | None :return: QuerySet of learners :rtype: QuerySet """ diff --git a/futurex_openedx_extensions/dashboard/details/learners.py b/futurex_openedx_extensions/dashboard/details/learners.py index 924feec3..46c2733c 100644 --- a/futurex_openedx_extensions/dashboard/details/learners.py +++ b/futurex_openedx_extensions/dashboard/details/learners.py @@ -1,12 +1,17 @@ """Learners details collectors""" from __future__ import annotations +from datetime import timedelta from typing import List from common.djangoapps.student.models import CourseAccessRole from django.contrib.auth import get_user_model -from django.db.models import Count, Exists, OuterRef, Q, Subquery +from django.db.models import BooleanField, Case, Count, Exists, OuterRef, Q, Subquery, Value, When from django.db.models.query import QuerySet +from django.utils import timezone +from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.courseware.models import StudentModule +from lms.djangoapps.grades.models import PersistentCourseGrade from futurex_openedx_extensions.helpers.querysets import get_base_queryset_courses, get_has_site_login_queryset from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list, get_tenants_sites @@ -81,6 +86,46 @@ def get_certificates_count_for_learner_queryset( ) +def get_learners_search_queryset( + search_text: str = None, + superuser_filter: bool | None = False, + staff_filter: bool | None = False, + active_filter: bool | None = True +) -> QuerySet: + """ + Get the learners queryset for the given search text. + + :param search_text: Search text to filter the learners by + :type search_text: str + :param superuser_filter: Value to filter superusers. None means no filter + :type superuser_filter: bool + :param staff_filter: Value to filter staff users. None means no filter + :type staff_filter: bool + :param active_filter: Value to filter active users. None means no filter + :type active_filter: bool + :return: QuerySet of learners + :rtype: QuerySet + """ + queryset = get_user_model().objects.all() + + if superuser_filter is not None: + queryset = queryset.filter(is_superuser=superuser_filter) + if staff_filter is not None: + queryset = queryset.filter(is_staff=staff_filter) + if active_filter is not None: + queryset = queryset.filter(is_active=active_filter) + + search_text = (search_text or '').strip() + if search_text: + queryset = queryset.filter( + Q(username__icontains=search_text) | + Q(email__icontains=search_text) | + Q(profile__name__icontains=search_text) + ) + + return queryset + + def get_learners_queryset( tenant_ids: List, search_text: str = None, visible_courses_filter: bool = True, active_courses_filter: bool = None ) -> QuerySet: @@ -91,9 +136,9 @@ def get_learners_queryset( :type tenant_ids: List :param search_text: Search text to filter the learners by :type search_text: str - :param visible_courses_filter: Whether to only count courses that are visible in the catalog + :param visible_courses_filter: Value to filter courses on catalog visibility. None means no filter :type visible_courses_filter: bool - :param active_courses_filter: Whether to only count active courses + :param active_courses_filter: Value to filter courses on active status. None means no filter :type active_courses_filter: bool :return: QuerySet of learners :rtype: QuerySet @@ -101,18 +146,7 @@ def get_learners_queryset( course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list'] tenant_sites = get_tenants_sites(tenant_ids) - queryset = get_user_model().objects.filter( - is_superuser=False, - is_staff=False, - is_active=True, - ) - search_text = (search_text or '').strip() - if search_text: - queryset = queryset.filter( - Q(username__icontains=search_text) | - Q(email__icontains=search_text) | - Q(profile__name__icontains=search_text) - ) + queryset = get_learners_search_queryset(search_text) queryset = queryset.annotate( courses_count=get_courses_count_for_learner_queryset( @@ -135,6 +169,62 @@ def get_learners_queryset( return queryset +def get_learners_by_course_queryset(course_id: str, search_text: str = None) -> QuerySet: + """ + Get the learners queryset for the given course ID. + + :param course_id: The course ID to get the learners for + :type course_id: str + :param search_text: Search text to filter the learners by + :type search_text: str + :return: QuerySet of learners + :rtype: QuerySet + """ + queryset = get_learners_search_queryset(search_text) + queryset = queryset.filter( + courseenrollment__course_id=course_id + ).filter( + ~Exists( + CourseAccessRole.objects.filter( + user_id=OuterRef('id'), + org=OuterRef('courseenrollment__course__org') + ) + ) + ).annotate( + certificate_available=Exists( + GeneratedCertificate.objects.filter( + user_id=OuterRef('id'), + course_id=course_id, + status='downloadable' + ) + ) + ).annotate( + course_score=Subquery( + PersistentCourseGrade.objects.filter( + user_id=OuterRef('id'), + course_id=course_id + ).values('percent_grade')[:1] + ) + ).annotate( + active_in_course=Case( + When( + Exists( + StudentModule.objects.filter( + student_id=OuterRef('id'), + course_id=course_id, + modified__gte=timezone.now() - timedelta(days=30) + ) + ), + then=Value(True), + ), + default=Value(False), + output_field=BooleanField(), + ) + ).select_related('profile').order_by('id') + + return queryset + + def get_learner_info_queryset( tenant_ids: List, user_id: int, visible_courses_filter: bool = True, active_courses_filter: bool = None ) -> QuerySet: @@ -145,9 +235,9 @@ def get_learner_info_queryset( :type tenant_ids: List :param user_id: The user ID to get the learner for :type user_id: int - :param visible_courses_filter: Whether to only count courses that are visible in the catalog + :param visible_courses_filter: Value to filter courses on catalog visibility. None means no filter :type visible_courses_filter: bool - :param active_courses_filter: Whether to only count active courses + :param active_courses_filter: Value to filter courses on active status. None means no filter :type active_courses_filter: bool :return: QuerySet of learners :rtype: QuerySet diff --git a/futurex_openedx_extensions/dashboard/serializers.py b/futurex_openedx_extensions/dashboard/serializers.py index 496a8cc6..c5e0296c 100644 --- a/futurex_openedx_extensions/dashboard/serializers.py +++ b/futurex_openedx_extensions/dashboard/serializers.py @@ -14,8 +14,8 @@ from futurex_openedx_extensions.helpers.tenants import get_tenants_by_org -class LearnerDetailsSerializer(serializers.ModelSerializer): - """Serializer for learner details.""" +class LearnerBasicDetailsSerializer(serializers.ModelSerializer): + """Serializer for learner's basic details.""" user_id = serializers.SerializerMethodField() full_name = serializers.SerializerMethodField() alternative_full_name = serializers.SerializerMethodField() @@ -27,8 +27,6 @@ class LearnerDetailsSerializer(serializers.ModelSerializer): gender_display = serializers.SerializerMethodField() date_joined = serializers.DateTimeField() last_login = serializers.DateTimeField() - enrolled_courses_count = serializers.SerializerMethodField() - certificates_count = serializers.SerializerMethodField() class Meta: model = get_user_model() @@ -44,8 +42,6 @@ class Meta: "gender_display", "date_joined", "last_login", - "enrolled_courses_count", - "certificates_count", ] @staticmethod @@ -110,6 +106,23 @@ def get_gender_display(self, obj): """Return readable text for gender""" return self._get_profile_field(obj, "gender_display") + def get_year_of_birth(self, obj): + """Return year of birth.""" + return self._get_profile_field(obj, "year_of_birth") + + +class LearnerDetailsSerializer(LearnerBasicDetailsSerializer): + """Serializer for learner details.""" + enrolled_courses_count = serializers.SerializerMethodField() + certificates_count = serializers.SerializerMethodField() + + class Meta: + model = get_user_model() + fields = LearnerBasicDetailsSerializer.Meta.fields + [ + "enrolled_courses_count", + "certificates_count", + ] + def get_certificates_count(self, obj): # pylint: disable=no-self-use """Return certificates count.""" return obj.certificates_count @@ -118,9 +131,20 @@ def get_enrolled_courses_count(self, obj): # pylint: disable=no-self-use """Return enrolled courses count.""" return obj.courses_count - def get_year_of_birth(self, obj): - """Return year of birth.""" - return self._get_profile_field(obj, "year_of_birth") + +class LearnerDetailsForCourseSerializer(LearnerBasicDetailsSerializer): + """Serializer for learner details for a course.""" + certificate_available = serializers.BooleanField() + course_score = serializers.DecimalField(max_digits=5, decimal_places=2) + active_in_course = serializers.BooleanField() + + class Meta: + model = get_user_model() + fields = LearnerBasicDetailsSerializer.Meta.fields + [ + "certificate_available", + "course_score", + "active_in_course", + ] class LearnerDetailsExtendedSerializer(LearnerDetailsSerializer): @@ -190,7 +214,6 @@ class CourseDetailsBaseSerializer(serializers.ModelSerializer): image_url = serializers.SerializerMethodField() org = serializers.CharField() tenant_ids = serializers.SerializerMethodField() - author_name = serializers.SerializerMethodField() class Meta: model = CourseOverview @@ -206,7 +229,6 @@ class Meta: "image_url", "org", "tenant_ids", - "author_name", ] def get_status(self, obj): # pylint: disable=no-self-use @@ -245,10 +267,6 @@ def get_end_date(self, obj): # pylint: disable=no-self-use """Return the end date.""" return obj.end - def get_author_name(self, obj): # pylint: disable=unused-argument,no-self-use - """Return the author name.""" - return None - class CourseDetailsSerializer(CourseDetailsBaseSerializer): """Serializer for course details.""" diff --git a/futurex_openedx_extensions/dashboard/settings/common_production.py b/futurex_openedx_extensions/dashboard/settings/common_production.py index 9063ce4b..15d7d9f0 100644 --- a/futurex_openedx_extensions/dashboard/settings/common_production.py +++ b/futurex_openedx_extensions/dashboard/settings/common_production.py @@ -11,3 +11,12 @@ def plugin_settings(settings): "FX_CACHE_TIMEOUT_TENANTS_INFO", 60 * 60 * 2, # 2 hours ) + + settings.FX_RATE_LIMIT_ANONYMOUS_DATA_RETRIEVE = getattr( + settings, + "FX_RATE_LIMIT_ANONYMOUS_DATA_RETRIEVE", + "1/minute", + ) + + if settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"].get("fx_anonymous_data_retrieve") is None: + settings.REST_FRAMEWORK["DEFAULT_THROTTLE_RATES"]["fx_anonymous_data_retrieve"] = "5/hour" diff --git a/futurex_openedx_extensions/dashboard/statistics/courses.py b/futurex_openedx_extensions/dashboard/statistics/courses.py index 4d12b538..7f6f0b18 100644 --- a/futurex_openedx_extensions/dashboard/statistics/courses.py +++ b/futurex_openedx_extensions/dashboard/statistics/courses.py @@ -44,9 +44,9 @@ def get_courses_count_by_status( :param tenant_ids: List of tenant IDs to get the count for :type tenant_ids: List[int] - :param visible_filter: Whether to only count courses that are visible in the catalog + :param visible_filter: Value to filter courses on catalog visibility. None means no filter :type visible_filter: bool - :param active_filter: Whether to only count active courses (according to dates) + :param active_filter: Value to filter courses on active status. None means no filter (according to dates) :type active_filter: bool :return: QuerySet of courses count per organization and status :rtype: QuerySet diff --git a/futurex_openedx_extensions/dashboard/urls.py b/futurex_openedx_extensions/dashboard/urls.py index 56d72b97..3de3e4eb 100644 --- a/futurex_openedx_extensions/dashboard/urls.py +++ b/futurex_openedx_extensions/dashboard/urls.py @@ -5,12 +5,17 @@ from django.urls import re_path from futurex_openedx_extensions.dashboard import views +from futurex_openedx_extensions.helpers.constants import COURSE_ID_REGX app_name = 'fx_dashboard' urlpatterns = [ + re_path(r'^api/fx/accessible/v1/info/$', views.AccessibleTenantsInfoView.as_view(), name='accessible-info'), re_path(r'^api/fx/courses/v1/courses/$', views.CoursesView.as_view(), name='courses'), re_path(r'^api/fx/learners/v1/learners/$', views.LearnersView.as_view(), name='learners'), + re_path( + fr'^api/fx/learners/v1/learners/{COURSE_ID_REGX}/$', + views.LearnersDetailsForCourseView.as_view(), name='learners-course'), re_path( r'^api/fx/learners/v1/learner/' + settings.USERNAME_PATTERN + '/$', views.LearnerInfoView.as_view(), diff --git a/futurex_openedx_extensions/dashboard/views.py b/futurex_openedx_extensions/dashboard/views.py index 0ea6ed5e..5f627ccf 100644 --- a/futurex_openedx_extensions/dashboard/views.py +++ b/futurex_openedx_extensions/dashboard/views.py @@ -1,4 +1,6 @@ """Views for the dashboard app""" +from common.djangoapps.student.models import get_user_by_username_or_email +from django.core.exceptions import ObjectDoesNotExist from django.http import JsonResponse from rest_framework.generics import ListAPIView from rest_framework.response import Response @@ -6,7 +8,11 @@ from futurex_openedx_extensions.dashboard import serializers from futurex_openedx_extensions.dashboard.details.courses import get_courses_queryset, get_learner_courses_info_queryset -from futurex_openedx_extensions.dashboard.details.learners import get_learner_info_queryset, get_learners_queryset +from futurex_openedx_extensions.dashboard.details.learners import ( + get_learner_info_queryset, + get_learners_by_course_queryset, + get_learners_queryset, +) from futurex_openedx_extensions.dashboard.statistics.certificates import get_certificates_count from futurex_openedx_extensions.dashboard.statistics.courses import get_courses_count, get_courses_count_by_status from futurex_openedx_extensions.dashboard.statistics.learners import get_learners_count @@ -14,20 +20,33 @@ from futurex_openedx_extensions.helpers.converters import error_details_to_dictionary from futurex_openedx_extensions.helpers.filters import DefaultOrderingFilter from futurex_openedx_extensions.helpers.pagination import DefaultPagination -from futurex_openedx_extensions.helpers.permissions import HasTenantAccess, IsSystemStaff -from futurex_openedx_extensions.helpers.tenants import get_selected_tenants, get_user_id_from_username_tenants +from futurex_openedx_extensions.helpers.permissions import ( + HasCourseAccess, + HasTenantAccess, + IsAnonymousOrSystemStaff, + IsSystemStaff, +) +from futurex_openedx_extensions.helpers.tenants import ( + get_accessible_tenant_ids, + get_selected_tenants, + get_tenants_info, + get_user_id_from_username_tenants, +) +from futurex_openedx_extensions.helpers.throttles import AnonymousDataRetrieveRateThrottle class TotalCountsView(APIView): """View to get the total count statistics""" STAT_CERTIFICATES = 'certificates' STAT_COURSES = 'courses' + STAT_HIDDEN_COURSES = 'hidden_courses' STAT_LEARNERS = 'learners' - valid_stats = [STAT_CERTIFICATES, STAT_COURSES, STAT_LEARNERS] + valid_stats = [STAT_CERTIFICATES, STAT_COURSES, STAT_HIDDEN_COURSES, STAT_LEARNERS] STAT_RESULT_KEYS = { STAT_CERTIFICATES: 'certificates_count', STAT_COURSES: 'courses_count', + STAT_HIDDEN_COURSES: 'hidden_courses_count', STAT_LEARNERS: 'learners_count' } @@ -40,9 +59,9 @@ def _get_certificates_count_data(tenant_id): return sum(certificate_count for certificate_count in collector_result.values()) @staticmethod - def _get_courses_count_data(tenant_id): + def _get_courses_count_data(tenant_id, visible_filter): """Get the count of courses for the given tenant""" - collector_result = get_courses_count([tenant_id]) + collector_result = get_courses_count([tenant_id], visible_filter=visible_filter) return sum(org_count['courses_count'] for org_count in collector_result) @staticmethod @@ -58,7 +77,10 @@ def _get_stat_count(self, stat, tenant_id): return self._get_certificates_count_data(tenant_id) if stat == self.STAT_COURSES: - return self._get_courses_count_data(tenant_id) + return self._get_courses_count_data(tenant_id, visible_filter=True) + + if stat == self.STAT_HIDDEN_COURSES: + return self._get_courses_count_data(tenant_id, visible_filter=False) return self._get_learners_count_data(tenant_id) @@ -129,6 +151,7 @@ def get_queryset(self): return get_courses_queryset( tenant_ids=tenant_ids, search_text=search_text, + visible_filter=None, ) @@ -200,7 +223,7 @@ def get(self, request, username, *args, **kwargs): # pylint: disable=no-self-us if not user_id: return Response(error_details_to_dictionary(reason=f"User not found {username}"), status=404) - courses = get_learner_courses_info_queryset(tenant_ids, user_id) + courses = get_learner_courses_info_queryset(tenant_ids, user_id, visible_filter=None) return Response(serializers.LearnerCoursesDetailsSerializer( courses, context={'request': request}, many=True @@ -219,3 +242,42 @@ def get(self, request, *args, **kwargs): # pylint: disable=no-self-use return JsonResponse({ 'version': futurex_openedx_extensions.__version__, }) + + +class AccessibleTenantsInfoView(APIView): + """View to get the list of accessible tenants""" + permission_classes = [IsAnonymousOrSystemStaff] + throttle_classes = [AnonymousDataRetrieveRateThrottle] + + def get(self, request, *args, **kwargs): # pylint: disable=no-self-use + """ + GET /api/fx/tenants/v1/accessible_tenants/?username_or_email= + """ + username_or_email = request.query_params.get("username_or_email") + try: + user = get_user_by_username_or_email(username_or_email) + except ObjectDoesNotExist: + user = None + + if not user: + return JsonResponse({}) + + tenant_ids = get_accessible_tenant_ids(user) + return JsonResponse(get_tenants_info(tenant_ids)) + + +class LearnersDetailsForCourseView(ListAPIView): + """View to get the list of learners for a course""" + serializer_class = serializers.LearnerDetailsForCourseSerializer + permission_classes = [HasCourseAccess] + pagination_class = DefaultPagination + + def get_queryset(self, *args, **kwargs): + """Get the list of learners for a course""" + search_text = self.request.query_params.get('search_text') + course_id = self.kwargs.get('course_id') + + return get_learners_by_course_queryset( + course_id=course_id, + search_text=search_text, + ) diff --git a/futurex_openedx_extensions/helpers/constants.py b/futurex_openedx_extensions/helpers/constants.py index 970fe557..10de633f 100644 --- a/futurex_openedx_extensions/helpers/constants.py +++ b/futurex_openedx_extensions/helpers/constants.py @@ -1,8 +1,16 @@ """Constants for the FutureX Open edX Extensions app.""" +CACHE_NAME_ALL_TENANTS_INFO = "all_tenants_info_v2" +CACHE_NAME_ALL_COURSE_ORG_FILTER_LIST = "all_course_org_filter_list_v2" + +COURSE_ID_REGX = r"(?Pcourse-v1:(?P[a-zA-Z0-9_]+)\+(?P[a-zA-Z0-9_]+)\+(?P[a-zA-Z0-9_]+))" + + COURSE_STATUSES = { - 'active': 'active', - 'archived': 'archived', - 'upcoming': 'upcoming', + "active": "active", + "archived": "archived", + "upcoming": "upcoming", } -COURSE_STATUS_SELF_PREFIX = 'self_' +COURSE_STATUS_SELF_PREFIX = "self_" + +TENANT_LIMITED_ADMIN_ROLES = ["org_course_creator_group"] diff --git a/futurex_openedx_extensions/helpers/extractors.py b/futurex_openedx_extensions/helpers/extractors.py new file mode 100644 index 00000000..20aee744 --- /dev/null +++ b/futurex_openedx_extensions/helpers/extractors.py @@ -0,0 +1,25 @@ +"""Helper functions for FutureX Open edX Extensions.""" +from __future__ import annotations + +import re +from typing import List +from urllib.parse import urlparse + +from futurex_openedx_extensions.helpers.constants import COURSE_ID_REGX + + +def get_course_id_from_uri(uri: str) -> str | None: + """Extract the course_id from the URI.""" + path_parts = urlparse(uri).path.split("/") + + for part in path_parts: + result = re.search(r"^" + COURSE_ID_REGX, part) + if result: + return result.groupdict().get('course_id') + + return None + + +def get_first_not_empty_item(items: List, default=None) -> any: + """Return the first item in the list that is not empty.""" + return next((item for item in items if item), default) diff --git a/futurex_openedx_extensions/helpers/permissions.py b/futurex_openedx_extensions/helpers/permissions.py index 3a689d47..99e10212 100644 --- a/futurex_openedx_extensions/helpers/permissions.py +++ b/futurex_openedx_extensions/helpers/permissions.py @@ -1,12 +1,43 @@ """Permission classes for FutureX Open edX Extensions.""" import json +from common.djangoapps.student.models import CourseAccessRole +from django.db.models import Subquery +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from rest_framework.exceptions import NotAuthenticated, PermissionDenied -from rest_framework.permissions import IsAuthenticated +from rest_framework.permissions import BasePermission, IsAuthenticated +from futurex_openedx_extensions.helpers.constants import TENANT_LIMITED_ADMIN_ROLES +from futurex_openedx_extensions.helpers.extractors import get_course_id_from_uri from futurex_openedx_extensions.helpers.tenants import check_tenant_access +class HasCourseAccess(IsAuthenticated): + """Permission class to check if the user has access to the course.""" + def has_permission(self, request, view): + """Check if the user has access to the course.""" + if not super().has_permission(request, view): + raise NotAuthenticated() + + course_id = get_course_id_from_uri(request.build_absolute_uri()) + if not course_id or not CourseOverview.objects.filter(id=course_id).exists(): + raise PermissionDenied(detail=json.dumps({"reason": "Invalid course_id"})) + + if request.user.is_staff or request.user.is_superuser: + return True + + if not CourseAccessRole.objects.filter( + user=request.user, + org=Subquery( + CourseOverview.objects.filter(id=course_id).values('org') + ), + role__in=TENANT_LIMITED_ADMIN_ROLES, + ).exists(): + raise PermissionDenied(detail=json.dumps({"reason": "User does not have access to the course"})) + + return True + + class HasTenantAccess(IsAuthenticated): """Permission class to check if the user is a tenant admin.""" def has_permission(self, request, view): @@ -34,3 +65,12 @@ def has_permission(self, request, view): raise PermissionDenied(detail=json.dumps({"reason": "User is not a system staff member"})) return True + + +class IsAnonymousOrSystemStaff(BasePermission): + """Permission class to check if the user is anonymous or system staff.""" + def has_permission(self, request, view): + """Check if the user is anonymous""" + if not hasattr(request, "user") or not request.user or not request.user.is_authenticated: + return True + return request.user.is_staff or request.user.is_superuser diff --git a/futurex_openedx_extensions/helpers/querysets.py b/futurex_openedx_extensions/helpers/querysets.py index ba0bc508..4549dfcd 100644 --- a/futurex_openedx_extensions/helpers/querysets.py +++ b/futurex_openedx_extensions/helpers/querysets.py @@ -21,9 +21,9 @@ def get_base_queryset_courses( :param course_org_filter_list: List of course organizations to filter by :type course_org_filter_list: List[str] :param visible_filter: Value to filter courses on catalog visibility. None means no filter. - :type visible_filter: bool + :type visible_filter: bool | None :param active_filter: Value to filter courses on active status. None means no filter. - :type active_filter: bool + :type active_filter: bool | None :return: QuerySet of courses :rtype: QuerySet """ diff --git a/futurex_openedx_extensions/helpers/tenants.py b/futurex_openedx_extensions/helpers/tenants.py index 66b89738..b7b81e8f 100644 --- a/futurex_openedx_extensions/helpers/tenants.py +++ b/futurex_openedx_extensions/helpers/tenants.py @@ -11,12 +11,12 @@ from eox_tenant.models import Route, TenantConfig from rest_framework.request import Request +from futurex_openedx_extensions.helpers import constants as cs from futurex_openedx_extensions.helpers.caching import cache_dict from futurex_openedx_extensions.helpers.converters import error_details_to_dictionary, ids_string_to_list +from futurex_openedx_extensions.helpers.extractors import get_first_not_empty_item from futurex_openedx_extensions.helpers.querysets import get_has_site_login_queryset -TENANT_LIMITED_ADMIN_ROLES = ['org_course_creator_group'] - def get_excluded_tenant_ids() -> List[int]: """ @@ -52,7 +52,7 @@ def get_all_tenants() -> QuerySet: return TenantConfig.objects.exclude(id__in=get_excluded_tenant_ids()) -@cache_dict(timeout=settings.FX_CACHE_TIMEOUT_TENANTS_INFO, key_generator_or_name='all_tenants_info') +@cache_dict(timeout=settings.FX_CACHE_TIMEOUT_TENANTS_INFO, key_generator_or_name=cs.CACHE_NAME_ALL_TENANTS_INFO) def get_all_tenants_info() -> Dict[str, Any]: """ Get all tenants in the system that are exposed in the route table, and with a valid config @@ -62,13 +62,33 @@ def get_all_tenants_info() -> Dict[str, Any]: :return: Dictionary of tenant IDs and Sites :rtype: Dict[str, Any] """ + def _fix_lms_base(domain_name: str) -> str: + """Fix the LMS base URL""" + if not domain_name: + return '' + return f'https://{domain_name}' if not domain_name.startswith('http') else domain_name + tenant_ids = list(get_all_tenants().values_list('id', flat=True)) - info = TenantConfig.objects.filter(id__in=tenant_ids).values('id', 'route__domain') + info = TenantConfig.objects.filter(id__in=tenant_ids).values('id', 'route__domain', 'lms_configs') return { 'tenant_ids': tenant_ids, 'sites': { tenant['id']: tenant['route__domain'] for tenant in info - } + }, + 'info': { + tenant['id']: { + 'lms_root_url': get_first_not_empty_item([ + (tenant['lms_configs'].get('LMS_ROOT_URL') or '').strip(), + _fix_lms_base((tenant['lms_configs'].get('LMS_BASE') or '').strip()), + _fix_lms_base((tenant['lms_configs'].get('SITE_NAME') or '').strip()), + ], default=''), + 'platform_name': get_first_not_empty_item([ + (tenant['lms_configs'].get('PLATFORM_NAME') or '').strip(), + (tenant['lms_configs'].get('platform_name') or '').strip(), + ], default=''), + 'logo_image_url': (tenant['lms_configs'].get('logo_image_url') or '').strip(), + } for tenant in info + }, } @@ -82,6 +102,19 @@ def get_all_tenant_ids() -> List[int]: return get_all_tenants_info()['tenant_ids'] +def get_tenants_info(tenant_ids: List[int]) -> Dict[int, Any]: + """ + Get the information for the given tenant IDs + + :param tenant_ids: List of tenant IDs to get the information for + :type tenant_ids: List[int] + :return: Dictionary of tenant information + :rtype: Dict[str, Any] + """ + all_tenants_info = get_all_tenants_info() + return {t_id: all_tenants_info['info'].get(t_id) for t_id in tenant_ids} + + def get_tenant_site(tenant_id: int) -> str: """ Get the site for a tenant @@ -94,7 +127,9 @@ def get_tenant_site(tenant_id: int) -> str: return get_all_tenants_info()['sites'].get(tenant_id) -@cache_dict(timeout=settings.FX_CACHE_TIMEOUT_TENANTS_INFO, key_generator_or_name='all_course_org_filter_list') +@cache_dict( + timeout=settings.FX_CACHE_TIMEOUT_TENANTS_INFO, key_generator_or_name=cs.CACHE_NAME_ALL_COURSE_ORG_FILTER_LIST +) def get_all_course_org_filter_list() -> Dict[int, List[str]]: """ Get all course org filters for all tenants. @@ -188,7 +223,7 @@ def get_accessible_tenant_ids(user: get_user_model()) -> List[int]: course_org_filter_list = get_all_course_org_filter_list() accessible_orgs = CourseAccessRole.objects.filter( user_id=user.id, - role__in=TENANT_LIMITED_ADMIN_ROLES, + role__in=cs.TENANT_LIMITED_ADMIN_ROLES, ).values_list('org', flat=True).distinct() return [t_id for t_id, course_org_filter in course_org_filter_list.items() if any( diff --git a/futurex_openedx_extensions/helpers/throttles.py b/futurex_openedx_extensions/helpers/throttles.py new file mode 100644 index 00000000..3170ec14 --- /dev/null +++ b/futurex_openedx_extensions/helpers/throttles.py @@ -0,0 +1,7 @@ +"""Custom throttles for the API views.""" +from rest_framework.throttling import AnonRateThrottle + + +class AnonymousDataRetrieveRateThrottle(AnonRateThrottle): + """Throttle for anonymous users for data retrieval views.""" + scope = 'fx_anonymous_data_retrieve' diff --git a/test_settings.py b/test_settings.py index 10df1747..ff427657 100644 --- a/test_settings.py +++ b/test_settings.py @@ -82,3 +82,9 @@ def root(*args): # Non-default dashboard settings FX_CACHE_TIMEOUT_TENANTS_INFO = 60 * 60 * 3 # 3 hours + +REST_FRAMEWORK = { + 'DEFAULT_THROTTLE_RATES': { + 'fx_anonymous_data_retrieve': '5/hour', + }, +} diff --git a/test_utils/edx_platform_mocks/common/djangoapps/student/models.py b/test_utils/edx_platform_mocks/common/djangoapps/student/models.py index b0edf594..99cc004f 100644 --- a/test_utils/edx_platform_mocks/common/djangoapps/student/models.py +++ b/test_utils/edx_platform_mocks/common/djangoapps/student/models.py @@ -1,4 +1,5 @@ """edx-platform Mocks""" +from fake_models.functions import get_user_by_username_or_email # pylint: disable=unused-import from fake_models.models import ( # pylint: disable=unused-import CourseAccessRole, CourseEnrollment, diff --git a/test_utils/edx_platform_mocks/fake_models/functions.py b/test_utils/edx_platform_mocks/fake_models/functions.py index 261a74e8..892c330b 100644 --- a/test_utils/edx_platform_mocks/fake_models/functions.py +++ b/test_utils/edx_platform_mocks/fake_models/functions.py @@ -29,3 +29,8 @@ def get_certificates_for_user_by_course_keys(user, course_keys): # pylint: disa if not isinstance(user, get_user_model()): raise TypeError(f'Expects a user object but got "{user}" of type "{type(user)}"') return {} + + +def get_user_by_username_or_email(username_or_email): + """get_user_by_username_or_email Mock""" + raise get_user_model().DoesNotExist('Dummy function always returns DoesNotExist, mock it you need it') diff --git a/test_utils/edx_platform_mocks/fake_models/models.py b/test_utils/edx_platform_mocks/fake_models/models.py index be433951..d25e6333 100644 --- a/test_utils/edx_platform_mocks/fake_models/models.py +++ b/test_utils/edx_platform_mocks/fake_models/models.py @@ -1,7 +1,8 @@ """edx-platform models mocks for testing purposes.""" from django.contrib.auth import get_user_model from django.db import models -from opaque_keys.edx.django.models import CourseKeyField +from django.db.models.fields import AutoField +from opaque_keys.edx.django.models import CourseKeyField, LearningContextKeyField, UsageKeyField class CourseOverview(models.Model): @@ -28,6 +29,7 @@ class CourseAccessRole(models.Model): user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE) role = models.CharField(max_length=255) org = models.CharField(blank=True, max_length=255) + course_id = CourseKeyField(max_length=255, db_index=True, blank=True) class Meta: app_label = "fake_models" @@ -180,3 +182,32 @@ def update(self, *args, **kwargs): # pylint: disable=no-self-use return None return DummyGrade() + + +class PersistentCourseGrade(models.Model): + """Mock""" + id = AutoField(primary_key=True) # pylint: disable=invalid-name + user_id = models.IntegerField(blank=False, db_index=True) + course_id = CourseKeyField(blank=False, max_length=255) + + percent_grade = models.FloatField(blank=False) + + class Meta: + app_label = "fake_models" + unique_together = [ + ('course_id', 'user_id'), + ] + db_table = "persistentcoursegrade" + + +class StudentModule(models.Model): + """Mock""" + id = AutoField(primary_key=True) # pylint: disable=invalid-name + student = models.ForeignKey(get_user_model(), db_index=True, db_constraint=False, on_delete=models.CASCADE) + course_id = LearningContextKeyField(max_length=255, db_index=True) + modified = models.DateTimeField(auto_now=True, db_index=True) + module_state_key = UsageKeyField(max_length=255, db_column='module_id') + + class Meta: + app_label = "fake_models" + unique_together = (('student', 'module_state_key', 'course_id'),) diff --git a/test_utils/edx_platform_mocks/lms/djangoapps/courseware/models.py b/test_utils/edx_platform_mocks/lms/djangoapps/courseware/models.py new file mode 100644 index 00000000..75ca826e --- /dev/null +++ b/test_utils/edx_platform_mocks/lms/djangoapps/courseware/models.py @@ -0,0 +1,2 @@ +"""edx-platform Mock""" +from fake_models.models import StudentModule # pylint: disable=unused-import diff --git a/test_utils/edx_platform_mocks/lms/djangoapps/grades/models.py b/test_utils/edx_platform_mocks/lms/djangoapps/grades/models.py new file mode 100644 index 00000000..d42c6afa --- /dev/null +++ b/test_utils/edx_platform_mocks/lms/djangoapps/grades/models.py @@ -0,0 +1,2 @@ +"""ecx-platform Mocks""" +from fake_models.models import PersistentCourseGrade # pylint: disable=unused-import diff --git a/tests/base_test_data.py b/tests/base_test_data.py index 5a634ff8..6515f234 100644 --- a/tests/base_test_data.py +++ b/tests/base_test_data.py @@ -85,7 +85,7 @@ "config_id": 8, }, }, - "users_count": 60, + "users_count": 70, "user_signup_source__users": { "s1.sample.com": [1, 2, 3, 4, 5], "s2.sample.com": [4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15], @@ -175,6 +175,7 @@ }, "super_users": [1, 60], "staff_users": [2, 60], + "inactive_users": [61, 62, 63], "course_access_roles": { # roles, user ids per org "staff": { # at least one role that is not in TENANT_LIMITED_ADMIN_ROLES "ORG1": [3], @@ -223,13 +224,15 @@ }, } + expected_statistics = { - '1': {'certificates_count': 14, 'courses_count': 12, 'learners_count': 17}, - '2': {'certificates_count': 9, 'courses_count': 5, 'learners_count': 21}, - '3': {'certificates_count': 0, 'courses_count': 1, 'learners_count': 6}, - '7': {'certificates_count': 7, 'courses_count': 3, 'learners_count': 17}, - '8': {'certificates_count': 2, 'courses_count': 2, 'learners_count': 9}, + '1': {'certificates_count': 14, 'courses_count': 12, 'hidden_courses_count': 0, 'learners_count': 17}, + '2': {'certificates_count': 9, 'courses_count': 5, 'hidden_courses_count': 0, 'learners_count': 21}, + '3': {'certificates_count': 0, 'courses_count': 1, 'hidden_courses_count': 0, 'learners_count': 6}, + '7': {'certificates_count': 7, 'courses_count': 3, 'hidden_courses_count': 0, 'learners_count': 17}, + '8': {'certificates_count': 2, 'courses_count': 2, 'hidden_courses_count': 0, 'learners_count': 9}, 'total_certificates_count': 32, 'total_courses_count': 23, + 'total_hidden_courses_count': 0, 'total_learners_count': 70 } diff --git a/tests/conftest.py b/tests/conftest.py index 8fc9166a..0608a66b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -26,6 +26,8 @@ def _create_users(): user.objects.filter(id=user_id).update(is_superuser=True) for user_id in _base_data["staff_users"]: user.objects.filter(id=user_id).update(is_staff=True) + for user_id in _base_data["inactive_users"]: + user.objects.filter(id=user_id).update(is_active=False) def _create_tenants(): """Create tenants.""" diff --git a/tests/test_base_test_data.py b/tests/test_base_test_data.py index d6c00a4f..50caef81 100644 --- a/tests/test_base_test_data.py +++ b/tests/test_base_test_data.py @@ -1,5 +1,5 @@ """Test integrity of base test data.""" -from futurex_openedx_extensions.helpers.tenants import TENANT_LIMITED_ADMIN_ROLES +from futurex_openedx_extensions.helpers.constants import TENANT_LIMITED_ADMIN_ROLES from tests.base_test_data import _base_data diff --git a/tests/test_dashboard/test_details/test_details_learners.py b/tests/test_dashboard/test_details/test_details_learners.py index dbf1f1b3..cabb8194 100644 --- a/tests/test_dashboard/test_details/test_details_learners.py +++ b/tests/test_dashboard/test_details/test_details_learners.py @@ -2,13 +2,17 @@ from unittest.mock import patch import pytest +from common.djangoapps.student.models import UserProfile from django.contrib.auth import get_user_model +from lms.djangoapps.grades.models import PersistentCourseGrade from futurex_openedx_extensions.dashboard.details.learners import ( get_certificates_count_for_learner_queryset, get_courses_count_for_learner_queryset, get_learner_info_queryset, + get_learners_by_course_queryset, get_learners_queryset, + get_learners_search_queryset, ) @@ -44,7 +48,7 @@ def test_count_for_learner_queryset( @pytest.mark.django_db def test_get_learner_info_queryset(base_data): # pylint: disable=unused-argument - """Verify that get_learners_queryset returns the correct QuerySet.""" + """Verify that get_learner_info_queryset returns the correct QuerySet.""" queryset = get_learner_info_queryset([1], 3) assert queryset.count() == 1, "bad test data, user id (3) should be in the queryset" @@ -56,22 +60,78 @@ def test_get_learner_info_queryset(base_data): # pylint: disable=unused-argumen @pytest.mark.django_db def test_get_learner_info_queryset_selecting_profile(base_data): # pylint: disable=unused-argument - """Verify that get_learners_queryset returns the correct QuerySet along with the related profile record.""" + """Verify that get_learner_info_queryset returns the correct QuerySet along with the related profile record.""" with patch('django.db.models.query.QuerySet.select_related') as mocked_select_related: get_learner_info_queryset([1], 3) mocked_select_related.assert_called_once_with('profile') @pytest.mark.django_db -@pytest.mark.parametrize("tenant_ids, search_text, expected_count", [ - ([7, 8], None, 22), - ([7], None, 17), - ([7], "user", 17), - ([7], "user4", 10), - ([7], "user5", 1), - ([7], "user6", 0), - ([4], None, 0), +@pytest.mark.parametrize("search_text, expected_count", [ + (None, 64), + ("user", 64), + ("user4", 11), + ("example", 64), +]) +def test_get_learners_search_queryset(base_data, search_text, expected_count): # pylint: disable=unused-argument + """Verify that get_learners_search_queryset returns the correct QuerySet.""" + assert get_learners_search_queryset(search_text=search_text).count() == expected_count + + +@pytest.mark.django_db +def test_get_learners_search_queryset_name(base_data): # pylint: disable=unused-argument + """Verify that get_learners_search_queryset returns the correct QuerySet when searching in profile name.""" + assert get_learners_search_queryset(search_text="hn D").count() == 0 + UserProfile.objects.create(user_id=10, name="John Doe") + assert get_learners_search_queryset(search_text="hn D").count() == 1 + + +@pytest.mark.django_db +@pytest.mark.parametrize("filter_name, true_count", [ + ("superuser_filter", 2), + ("staff_filter", 2), + ("active_filter", 67), ]) -def test_get_learners_queryset(base_data, tenant_ids, search_text, expected_count): # pylint: disable=unused-argument +def test_get_learners_search_queryset_active_filter( + base_data, filter_name, true_count +): # pylint: disable=unused-argument + """Verify that get_learners_search_queryset returns the correct QuerySet when active_filters is used""" + kwargs = { + "superuser_filter": None, + "staff_filter": None, + "active_filter": None, + } + assert get_learners_search_queryset(**kwargs).count() == 70, "unexpected test data" + kwargs[filter_name] = True + assert get_learners_search_queryset(**kwargs).count() == true_count + kwargs[filter_name] = False + assert get_learners_search_queryset(**kwargs).count() == 70 - true_count + + +@pytest.mark.django_db +@pytest.mark.parametrize("tenant_ids, expected_count", [ + ([7, 8], 22), + ([7], 17), + ([4], 0), +]) +def test_get_learners_queryset(base_data, tenant_ids, expected_count): # pylint: disable=unused-argument """Verify that get_learners_queryset returns the correct QuerySet.""" - assert get_learners_queryset(tenant_ids, search_text).count() == expected_count + result = get_learners_queryset(tenant_ids) + assert result.count() == expected_count + if expected_count > 0: + assert result.first().courses_count is not None, "courses_count should be in the queryset" + assert result.first().certificates_count is not None, "certificates_count should be in the queryset" + assert result.first().has_site_login is not None, "has_site_login should be in the queryset" + + +@pytest.mark.django_db +def test_get_learners_by_course_queryset(base_data): # pylint: disable=unused-argument + """Verify that get_learners_by_course_queryset returns the correct QuerySet.""" + queryset = get_learners_by_course_queryset("course-v1:ORG1+5+5") + assert queryset.count() == 3, "unexpected test data" + PersistentCourseGrade.objects.create(user_id=15, course_id="course-v1:ORG1+5+5", percent_grade=0.67) + assert queryset.first().certificate_available is not None, "certificate_available should be in the queryset" + assert queryset.first().course_score == 0.67, \ + "course_score should be in the queryset with value 0.67 for the first record (user15)" + assert queryset.first().active_in_course is False, \ + "active_in_course should be in the queryset with value True for the first record (user15)" diff --git a/tests/test_dashboard/test_serializers.py b/tests/test_dashboard/test_serializers.py index d85de52a..e99c420b 100644 --- a/tests/test_dashboard/test_serializers.py +++ b/tests/test_dashboard/test_serializers.py @@ -4,7 +4,7 @@ import pytest from common.djangoapps.student.models import SocialLink, UserProfile from django.contrib.auth import get_user_model -from django.db.models import Count +from django.db.models import Value from django.utils.timezone import get_current_timezone, now, timedelta from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer @@ -12,8 +12,10 @@ from futurex_openedx_extensions.dashboard.serializers import ( CourseDetailsBaseSerializer, CourseDetailsSerializer, + LearnerBasicDetailsSerializer, LearnerCoursesDetailsSerializer, LearnerDetailsExtendedSerializer, + LearnerDetailsForCourseSerializer, LearnerDetailsSerializer, ) from futurex_openedx_extensions.helpers.constants import COURSE_STATUS_SELF_PREFIX, COURSE_STATUSES @@ -24,16 +26,19 @@ def get_dummy_queryset(users_list=None): if users_list is None: users_list = [10] return get_user_model().objects.filter(id__in=users_list).annotate( - courses_count=Count('id'), - certificates_count=Count('id'), + courses_count=Value(6), + certificates_count=Value(2), + certificate_available=Value(True), + course_score=Value(0.67), + active_in_course=Value(True), ).select_related('profile') @pytest.mark.django_db -def test_learner_details_serializer_no_profile(base_data): # pylint: disable=unused-argument - """Verify that the LearnerDetailsSerializer is correctly defined.""" +def test_learner_basic_details_serializer_no_profile(base_data): # pylint: disable=unused-argument + """Verify that the LearnerBasicDetailsSerializer is correctly defined.""" queryset = get_dummy_queryset() - data = LearnerDetailsSerializer(queryset, many=True).data + data = LearnerBasicDetailsSerializer(queryset, many=True).data assert len(data) == 1 assert data[0]['user_id'] == 10 assert data[0]['full_name'] == "" @@ -43,8 +48,8 @@ def test_learner_details_serializer_no_profile(base_data): # pylint: disable=un @pytest.mark.django_db -def test_learner_details_serializer_with_profile(base_data): # pylint: disable=unused-argument - """Verify that the LearnerDetailsSerializer processes the profile fields.""" +def test_learner_basic_details_serializer_with_profile(base_data): # pylint: disable=unused-argument + """Verify that the LearnerBasicDetailsSerializer processes the profile fields.""" UserProfile.objects.create( user_id=10, name='Test User', @@ -53,7 +58,7 @@ def test_learner_details_serializer_with_profile(base_data): # pylint: disable= year_of_birth=1988, ) queryset = get_dummy_queryset() - data = LearnerDetailsSerializer(queryset, many=True).data + data = LearnerBasicDetailsSerializer(queryset, many=True).data assert len(data) == 1 assert data[0]['user_id'] == 10 assert data[0]['full_name'] == 'Test User' @@ -74,10 +79,10 @@ def test_learner_details_serializer_with_profile(base_data): # pylint: disable= ("عربي", "Doe", "Alt John", "عربي Doe", "Alt John", "Arabic name"), ("John", "Doe", "عربي", "عربي", "John Doe", "Arabic alternative name"), ]) -def test_learner_details_serializer_full_name_alt_name( +def test_learner_basic_details_serializer_full_name_alt_name( base_data, first_name, last_name, profile_name, expected_full_name, expected_alt_name, use_case ): # pylint: disable=unused-argument, too-many-arguments - """Verify that the LearnerDetailsSerializer processes names as expected.""" + """Verify that the LearnerBasicDetailsSerializer processes names as expected.""" queryset = get_dummy_queryset() UserProfile.objects.create( user_id=10, @@ -88,7 +93,7 @@ def test_learner_details_serializer_full_name_alt_name( user.last_name = last_name user.save() - serializer = LearnerDetailsSerializer(queryset, many=True) + serializer = LearnerBasicDetailsSerializer(queryset, many=True) data = serializer.data assert len(data) == 1 assert data[0]['user_id'] == 10 @@ -96,6 +101,27 @@ def test_learner_details_serializer_full_name_alt_name( assert data[0]['alternative_full_name'] == expected_alt_name, f"checking ({use_case}) failed" +@pytest.mark.django_db +def test_learner_details_serializer(base_data): # pylint: disable=unused-argument + """Verify that the LearnerDetailsSerializer returns the needed fields""" + queryset = get_dummy_queryset() + data = LearnerDetailsSerializer(queryset, many=True).data + assert len(data) == 1 + assert data[0]['enrolled_courses_count'] == 6 + assert data[0]['certificates_count'] == 2 + + +@pytest.mark.django_db +def test_learner_details_for_course_serializer(base_data): # pylint: disable=unused-argument + """Verify that the LearnerDetailsForCourseSerializer returns the needed fields.""" + queryset = get_dummy_queryset() + data = LearnerDetailsForCourseSerializer(queryset, many=True).data + assert len(data) == 1 + assert data[0]['certificate_available'] is True + assert data[0]['course_score'] == '0.67' + assert data[0]['active_in_course'] is True + + @pytest.mark.django_db def test_learner_details_extended_serializer(base_data): # pylint: disable=unused-argument """Verify that the LearnerDetailsExtendedSerializer returns the correct data.""" diff --git a/tests/test_dashboard/test_views.py b/tests/test_dashboard/test_views.py index 4f735e7b..b746fe3f 100644 --- a/tests/test_dashboard/test_views.py +++ b/tests/test_dashboard/test_views.py @@ -14,7 +14,12 @@ from futurex_openedx_extensions.dashboard import serializers from futurex_openedx_extensions.helpers.constants import COURSE_STATUSES from futurex_openedx_extensions.helpers.filters import DefaultOrderingFilter -from futurex_openedx_extensions.helpers.permissions import HasTenantAccess, IsSystemStaff +from futurex_openedx_extensions.helpers.permissions import ( + HasCourseAccess, + HasTenantAccess, + IsAnonymousOrSystemStaff, + IsSystemStaff, +) from tests.base_test_data import expected_statistics @@ -68,7 +73,7 @@ def test_invalid_stats(self): def test_all_stats(self): """Test get method""" self.login_user(self.staff_user) - response = self.client.get(self.url + '?stats=certificates,courses,learners') + response = self.client.get(self.url + '?stats=certificates,courses,hidden_courses,learners') self.assertTrue(isinstance(response, JsonResponse)) self.assertEqual(response.status_code, 200) self.assertDictEqual(json.loads(response.content), expected_statistics) @@ -137,14 +142,14 @@ def test_no_tenants(self): self.login_user(self.staff_user) with patch('futurex_openedx_extensions.dashboard.views.get_courses_queryset') as mock_queryset: self.client.get(self.url) - mock_queryset.assert_called_once_with(tenant_ids=[1, 2, 3, 7, 8], search_text=None) + mock_queryset.assert_called_once_with(tenant_ids=[1, 2, 3, 7, 8], search_text=None, visible_filter=None) def test_search(self): """Verify that the view filters the courses by search text""" self.login_user(self.staff_user) with patch('futurex_openedx_extensions.dashboard.views.get_courses_queryset') as mock_queryset: self.client.get(self.url + '?tenant_ids=1&search_text=course') - mock_queryset.assert_called_once_with(tenant_ids=[1], search_text='course') + mock_queryset.assert_called_once_with(tenant_ids=[1], search_text='course', visible_filter=None) def helper_test_success(self, response): """Verify that the view returns the correct response""" @@ -318,7 +323,7 @@ def test_request_in_context(self, mock_serializer): ) @pytest.mark.usefixtures('base_data') class TestLearnerCoursesDetailsView(PermissionsTestOfLearnerInfoViewMixin, BaseTestViewMixin): - """Tests for CourseStatusesView""" + """Tests for LearnerCoursesView""" VIEW_NAME = 'fx_dashboard:learner-courses' def test_success(self): @@ -338,6 +343,7 @@ def test_success(self): mock_get_info.return_value = courses response = self.client.get(self.url) + mock_get_info.assert_called_once_with([1, 2, 3, 7, 8], 10, visible_filter=None) self.assertEqual(response.status_code, 200) data = json.loads(response.content) self.assertEqual(len(data), 2) @@ -381,3 +387,72 @@ def test_success(self): response = self.client.get(self.url) self.assertEqual(response.status_code, 200) self.assertEqual(json.loads(response.content), {'version': '0.1.dummy'}) + + +@pytest.mark.usefixtures("base_data") +class TestAccessibleTenantsInfoView(BaseTestViewMixin): + """Tests for AccessibleTenantsInfoView""" + VIEW_NAME = "fx_dashboard:accessible-info" + + def test_permission_classes(self): + """Verify that the view has the correct permission classes""" + view_func, _, _ = resolve(self.url) + view_class = view_func.view_class + self.assertEqual(view_class.permission_classes, [IsAnonymousOrSystemStaff]) + + @patch("futurex_openedx_extensions.dashboard.views.get_user_by_username_or_email") + def test_success(self, mock_get_user): + """Verify that the view returns the correct response""" + mock_get_user.return_value = get_user_model().objects.get(username="user4") + response = self.client.get(self.url, data={"username_or_email": "dummy, the user loader function is mocked"}) + self.assertEqual(response.status_code, 200) + self.assertDictEqual(json.loads(response.content), { + '1': {'lms_root_url': 'https://s1.sample.com', 'platform_name': '', 'logo_image_url': ''}, + '2': {'lms_root_url': 'https://s2.sample.com', 'platform_name': '', 'logo_image_url': ''}, + '7': {'lms_root_url': 'https://s7.sample.com', 'platform_name': '', 'logo_image_url': ''} + }) + + @patch("futurex_openedx_extensions.dashboard.views.get_user_by_username_or_email") + def test_no_username_or_email(self, mock_get_user): + """Verify that the view returns the correct response""" + mock_get_user.side_effect = get_user_model().DoesNotExist() + response = self.client.get(self.url) + mock_get_user.assert_called_once_with(None) + self.assertEqual(response.status_code, 200) + self.assertDictEqual(json.loads(response.content), {}) + + def test_not_existing_username_or_email(self): + """Verify that the view returns the correct response""" + response = self.client.get(self.url, data={"username_or_email": "dummy"}) + self.assertEqual(response.status_code, 200) + self.assertDictEqual(json.loads(response.content), {}) + + +@pytest.mark.usefixtures('base_data') +class TestLearnersDetailsForCourseView(BaseTestViewMixin): + """Tests for LearnersDetailsForCourseView""" + VIEW_NAME = 'fx_dashboard:learners-course' + + def setUp(self): + """Setup""" + super().setUp() + self.url_args = ['course-v1:ORG1+5+5'] + + def test_unauthorized(self): + """Verify that the view returns 403 when the user is not authenticated""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, 403) + + def test_permission_classes(self): + """Verify that the view has the correct permission classes""" + view_func, _, _ = resolve(self.url) + view_class = view_func.view_class + self.assertEqual(view_class.permission_classes, [HasCourseAccess]) + + def test_success(self): + """Verify that the view returns the correct response""" + self.login_user(self.staff_user) + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['count'], 3) + self.assertGreater(len(response.data['results']), 0) diff --git a/tests/test_helpers/test_apps.py b/tests/test_helpers/test_apps.py index 6b8c161e..c181f396 100644 --- a/tests/test_helpers/test_apps.py +++ b/tests/test_helpers/test_apps.py @@ -43,3 +43,16 @@ def test_common_production_plugin_settings_explicit(settings, setting_key, defau common_production.plugin_settings(settings) assert hasattr(settings, setting_key), f'Missing settings ({setting_key})!' assert getattr(settings, setting_key) == new_value, f'settings ({setting_key}) did not read from env correctly!' + + +def test_set_default_throttle_rates(settings): + """Verify that the plugin's settings set the default throttle rates""" + settings = copy.deepcopy(settings) + settings.REST_FRAMEWORK['DEFAULT_THROTTLE_RATES'] = {} + throttle_rates = settings.REST_FRAMEWORK['DEFAULT_THROTTLE_RATES'] + assert 'fx_anonymous_data_retrieve' not in throttle_rates, 'fx_anonymous_data_retrieve already exists!' + + common_production.plugin_settings(settings) + throttle_rates = settings.REST_FRAMEWORK['DEFAULT_THROTTLE_RATES'] + assert 'fx_anonymous_data_retrieve' in throttle_rates, 'fx_anonymous_data_retrieve was not set!' + assert throttle_rates['fx_anonymous_data_retrieve'] == '5/hour', 'fx_anonymous_data_retrieve was not set!' diff --git a/tests/test_helpers/test_constants.py b/tests/test_helpers/test_constants.py new file mode 100644 index 00000000..9e20d9d6 --- /dev/null +++ b/tests/test_helpers/test_constants.py @@ -0,0 +1,23 @@ +"""Tests for the constants module.""" +import re + +import pytest + +from futurex_openedx_extensions.helpers.constants import COURSE_ID_REGX + + +@pytest.mark.parametrize('match_string', [ + "course-v1:edX+DemoX+Demo_Course", + "/course-v1:edX+DemoX+Demo_Course", + "course-v1:edX+DemoX+Demo_Course/", + "/course-v1:edX+DemoX+Demo_Course/", +]) +def test_course_id_regx_success(match_string): + """Test the course_id pattern.""" + assert re.search(COURSE_ID_REGX, match_string).groupdict().get('course_id') == "course-v1:edX+DemoX+Demo_Course" + + +@pytest.mark.parametrize('match_string', ["course-v2:edX+DemoX+Demo_Course", "course-v1:edX+DemoX-Demo_Course"]) +def test_course_id_regx_fail(match_string): + """Test the course_id pattern.""" + assert re.search(COURSE_ID_REGX, match_string) is None diff --git a/tests/test_helpers/test_extractors.py b/tests/test_helpers/test_extractors.py new file mode 100644 index 00000000..8189a2b0 --- /dev/null +++ b/tests/test_helpers/test_extractors.py @@ -0,0 +1,37 @@ +"""Tests for the helper functions in the helpers module.""" +import pytest + +from futurex_openedx_extensions.helpers.extractors import get_course_id_from_uri, get_first_not_empty_item + + +@pytest.mark.parametrize("items, expected, error_message", [ + ([0, None, False, "", 3, "hello"], 3, "Test with a list containing truthy and falsy values"), + ([0, None, False, ""], None, "Test with a list containing only falsy values"), + ([1, "a", [1], {1: 1}], 1, "Test with a list containing only truthy values"), + ([], None, "Test with an empty list"), + ([0, [], {}, (), "non-empty"], "non-empty", "Test with a list containing mixed types"), + ([[], {}, (), 5], 5, "Test with a list containing different truthy types"), + ([None, "test"], "test", "Test with None as an element"), + ([[None, []], [], [1, 2, 3]], [None, []], "Test with nested lists"), + (["first", 0, None, False, "", 3, "hello"], "first", "Test with first element truthy") +]) +def test_get_first_not_empty_item(items, expected, error_message): + """Verify that the get_first_not_empty_item function returns the first non-empty item in the list.""" + assert get_first_not_empty_item(items) == expected, f"Failed: {error_message}" + + +@pytest.mark.parametrize("uri, expected_course_id", [ + ("http://domain/path/course-v1:ORG1+1+1/?page=1&page_size=%2F", "course-v1:ORG1+1+1"), + ("http://domain/path/course-v1:ORG2+2+2/?page=2&page_size=10", "course-v1:ORG2+2+2"), + ("http://domain/path/course-v1:ORG1+1+1", "course-v1:ORG1+1+1"), + ("http://domain/path/course-v1:ORG3+3+3/", "course-v1:ORG3+3+3"), + ("http://domain/path", None), + ("http://domain/path?course_id=course-v1:ORG2+2+2", None), + ("http://domain/path/some-other-path", None), + ("http://domain/path/course-v1:ORG4+4+4/morepath", "course-v1:ORG4+4+4"), + ("http://domain/path/bad-start-course-v1:ORG4+4+4/morepath", None), + ("http://domain/path/bad-start-course-v1:ORG4+4/morepath", None), +]) +def test_get_course_id_from_uri(uri, expected_course_id): + """Verify that the get_course_id_from_uri function returns the course ID from the URI.""" + assert get_course_id_from_uri(uri) == expected_course_id diff --git a/tests/test_helpers/test_permissions.py b/tests/test_helpers/test_permissions.py index 2e9c3813..ca2922d5 100644 --- a/tests/test_helpers/test_permissions.py +++ b/tests/test_helpers/test_permissions.py @@ -7,7 +7,12 @@ from rest_framework.exceptions import NotAuthenticated, PermissionDenied from rest_framework.test import APIRequestFactory -from futurex_openedx_extensions.helpers.permissions import HasTenantAccess, IsSystemStaff +from futurex_openedx_extensions.helpers.permissions import ( + HasCourseAccess, + HasTenantAccess, + IsAnonymousOrSystemStaff, + IsSystemStaff, +) def set_user(request, user_id): @@ -104,3 +109,73 @@ def test_is_system_staff_not_staff(base_data, user_id): # pylint: disable=unuse set_user(request, user_id) with pytest.raises(PermissionDenied): permission.has_permission(request, None) + + +@pytest.mark.django_db +@pytest.mark.parametrize('user_id', [1, 2, 60, 4]) +def test_has_course_access_true(base_data, user_id): # pylint: disable=unused-argument + """Verify that HasCourseAccess returns True when user has access to the course.""" + permission = HasCourseAccess() + request = APIRequestFactory().generic('GET', '/dummy/course-v1:ORG1+1+1/') + set_user(request, user_id) + assert permission.has_permission(request, None) is True + + +@pytest.mark.django_db +@pytest.mark.parametrize('user_id', [15, 21]) +def test_has_course_access_false(base_data, user_id): # pylint: disable=unused-argument + """Verify that HasCourseAccess raises PermissionDenied when user does not have access to the course.""" + permission = HasCourseAccess() + request = APIRequestFactory().generic('GET', '/dummy/course-v1:ORG1+1+1/') + set_user(request, user_id) + with pytest.raises(PermissionDenied): + permission.has_permission(request, None) + + +@pytest.mark.django_db +@pytest.mark.parametrize('user_id', [None, 0]) +def test_has_course_access_not_authenticated(base_data, user_id): # pylint: disable=unused-argument + """Verify that HasCourseAccess raises NotAuthenticated when user is not authenticated.""" + permission = HasCourseAccess() + request = APIRequestFactory().generic('GET', '/dummy/course-v1:ORG1+1+1/') + set_user(request, user_id) + with pytest.raises(NotAuthenticated): + permission.has_permission(request, None) + + +@pytest.mark.django_db +def test_has_course_access_no_course(base_data): # pylint: disable=unused-argument + """Verify that HasCourseAccess raises NotAuthenticated when there is no course in the request.""" + permission = HasCourseAccess() + request = APIRequestFactory().generic('GET', '/dummy/') + set_user(request, 1) + with pytest.raises(PermissionDenied): + permission.has_permission(request, None) + + +@pytest.mark.django_db +def test_has_course_access_bad_course(base_data): # pylint: disable=unused-argument + """Verify that HasCourseAccess raises PermissionDenied when the course dose not exist.""" + permission = HasCourseAccess() + request = APIRequestFactory().generic('GET', '/dummy/course-v1:ORG9+9+9/') + set_user(request, 1) + with pytest.raises(PermissionDenied): + permission.has_permission(request, None) + + +@pytest.mark.django_db +@pytest.mark.parametrize('user_id, expected_result, error_msg', [ + (None, True, 'anonymous users should be allowed!'), + (0, True, 'anonymous users should be allowed!'), + (1, True, 'superusers should be allowed!'), + (2, True, 'system staff should be allowed!'), + (15, False, 'non-staff users should not be allowed!'), +]) +def test_is_anonymous_or_system_staff( + base_data, user_id, expected_result, error_msg +): # pylint: disable=unused-argument + """Verify that IsAnonymousOrSystemStaff returns True when user is anonymous.""" + permission = IsAnonymousOrSystemStaff() + request = APIRequestFactory().generic('GET', '/dummy/') + set_user(request, user_id) + assert permission.has_permission(request, None) is expected_result, error_msg diff --git a/tests/test_helpers/test_tenants.py b/tests/test_helpers/test_tenants.py index 887ddadf..176f91c9 100644 --- a/tests/test_helpers/test_tenants.py +++ b/tests/test_helpers/test_tenants.py @@ -1,4 +1,5 @@ """Tests for tenants helpers.""" +from unittest.mock import patch import pytest from common.djangoapps.student.models import CourseEnrollment, UserSignupSource @@ -7,8 +8,8 @@ from django.test import override_settings from eox_tenant.models import TenantConfig +from futurex_openedx_extensions.helpers import constants as cs from futurex_openedx_extensions.helpers import tenants -from futurex_openedx_extensions.helpers.tenants import TENANT_LIMITED_ADMIN_ROLES from tests.base_test_data import _base_data @@ -100,7 +101,7 @@ def test_get_accessible_tenant_ids_complex(base_data): # pylint: disable=unused for role, orgs in _base_data["course_access_roles"].items(): for org, users in orgs.items(): - if role not in TENANT_LIMITED_ADMIN_ROLES: + if role not in cs.TENANT_LIMITED_ADMIN_ROLES: continue if role != user_access_role or org != user_access: assert user.id not in users, ( @@ -113,7 +114,7 @@ def test_get_accessible_tenant_ids_complex(base_data): # pylint: disable=unused f'{user_access_role} for {user_access}' ) assert ( - (role not in TENANT_LIMITED_ADMIN_ROLES) or + (role not in cs.TENANT_LIMITED_ADMIN_ROLES) or (role != user_access_role and user.id not in users) or (org != user_access and user.id not in users) or (role == user_access_role and org == user_access and user.id in users) @@ -172,9 +173,9 @@ def test_get_all_course_org_filter_list(base_data): # pylint: disable=unused-ar @pytest.mark.django_db def test_get_all_course_org_filter_list_is_being_cached(): """Verify that get_all_course_org_filter_list is being cached.""" - assert cache.get('all_course_org_filter_list') is None + assert cache.get(cs.CACHE_NAME_ALL_COURSE_ORG_FILTER_LIST) is None result = tenants.get_all_course_org_filter_list() - assert cache.get('all_course_org_filter_list') == result + assert cache.get(cs.CACHE_NAME_ALL_COURSE_ORG_FILTER_LIST) == result @pytest.mark.django_db @@ -245,13 +246,65 @@ def test_get_all_tenants_info(base_data): # pylint: disable=unused-argument } +@pytest.mark.django_db +@pytest.mark.parametrize("config_key, info_key, test_value, expected_result", [ + ("LMS_BASE", "lms_root_url", "lms.example.com", "https://lms.example.com"), + ("LMS_ROOT_URL", "lms_root_url", "https://lms.example.com", "https://lms.example.com"), + ("SITE_NAME", "lms_root_url", "lms.example.com", "https://lms.example.com"), + ("PLATFORM_NAME", "platform_name", "Test Platform", "Test Platform"), + ("platform_name", "platform_name", "Test Platform", "Test Platform"), + ("logo_image_url", "logo_image_url", "https://img.example.com/dummy.jpg", "https://img.example.com/dummy.jpg"), +]) +@patch('futurex_openedx_extensions.helpers.tenants.get_excluded_tenant_ids', return_value=[]) +def test_get_all_tenants_info_configs( + base_data, config_key, info_key, test_value, expected_result +): # pylint: disable=unused-argument + """Verify get_all_tenants_info function returning the correct logo_url.""" + tenant_config = TenantConfig.objects.create() + assert tenant_config.lms_configs.get(config_key) is None + + result = tenants.get_all_tenants_info() + assert result["info"][tenant_config.id][info_key] == "" + + tenant_config.lms_configs[config_key] = test_value + tenant_config.save() + result = tenants.get_all_tenants_info() + assert result["info"][tenant_config.id][info_key] == expected_result + + +@pytest.mark.django_db +@pytest.mark.parametrize("config_keys, data_prefix, call_index", [ + (["LMS_ROOT_URL", "LMS_BASE", "SITE_NAME"], "https://", 0), + (["PLATFORM_NAME", "platform_name"], "", 1), +]) +@patch( + 'futurex_openedx_extensions.helpers.tenants.get_excluded_tenant_ids', + return_value=[1, 2, 3, 4, 5, 6, 7, 8] +) +@patch('futurex_openedx_extensions.helpers.tenants.get_first_not_empty_item') +def test_get_all_tenants_info_config_priorities( + mock_get_first_not_empty_item, base_data, config_keys, data_prefix, call_index +): # pylint: disable=unused-argument + """Verify get_all_tenants_info is respecting the priority of the config keys.""" + assert not tenants.get_all_tenants_info()["tenant_ids"] + tenant_config = TenantConfig.objects.create() + for config_key in config_keys: + tenant_config.lms_configs[config_key] = f"{data_prefix}{config_key}_value" + tenant_config.save() + + _ = tenants.get_all_tenants_info() + assert mock_get_first_not_empty_item.call_args_list[call_index][0][0] == [ + f"{data_prefix}{config_key}_value" for config_key in config_keys + ] + + @override_settings(CACHES={'default': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache'}}) @pytest.mark.django_db def test_get_all_tenants_info_is_being_cached(): """Verify that get_all_tenants_info is being cached.""" - assert cache.get('all_tenants_info') is None + assert cache.get(cs.CACHE_NAME_ALL_TENANTS_INFO) is None result = tenants.get_all_tenants_info() - assert cache.get('all_tenants_info') == result + assert cache.get(cs.CACHE_NAME_ALL_TENANTS_INFO) == result @pytest.mark.django_db diff --git a/tests/test_helpers/test_throttles.py b/tests/test_helpers/test_throttles.py new file mode 100644 index 00000000..b7855023 --- /dev/null +++ b/tests/test_helpers/test_throttles.py @@ -0,0 +1,10 @@ +"""Tests for the thorttles module.""" +from rest_framework.throttling import AnonRateThrottle + +from futurex_openedx_extensions.helpers.throttles import AnonymousDataRetrieveRateThrottle + + +def test_anonymous_data_retrieve_rate_throttle(): + """Test the AnonymousDataRetrieveRateThrottle.""" + assert issubclass(AnonymousDataRetrieveRateThrottle, AnonRateThrottle) + assert AnonymousDataRetrieveRateThrottle.scope == 'fx_anonymous_data_retrieve'