From 365cbb0c90b1373d5fa1347d3699c8cc52397c49 Mon Sep 17 00:00:00 2001 From: Shadi Naif Date: Thu, 18 Apr 2024 17:47:04 +0300 Subject: [PATCH] feat: course details and course statuses APIs --- .../dashboard/details/courses.py | 100 ++++++++++++++ .../dashboard/serializers.py | 88 +++++++++++- .../dashboard/statistics/courses.py | 36 ++++- futurex_openedx_extensions/dashboard/urls.py | 8 +- futurex_openedx_extensions/dashboard/views.py | 76 +++++++++-- .../helpers/constants.py | 8 ++ futurex_openedx_extensions/helpers/filters.py | 6 + futurex_openedx_extensions/helpers/tenants.py | 29 ++++ .../eox_nelp/course_experience/models.py | 2 + .../edx_platform_mocks/fake_models/models.py | 37 +++++ test_utils/edx_platform_mocks/setup.py | 2 +- test_utils/eox_settings.py | 8 +- tests/__init__.py | 0 tests/base_test_data.py | 35 +++++ tests/conftest.py | 23 ++++ .../test_details/test_details_courses.py | 65 +++++++++ .../test_statistics/test_courses.py | 43 +++--- tests/test_dashboard/test_views.py | 127 ++++++++++++++++-- tests/test_helpers/test_filters.py | 10 ++ tests/test_helpers/test_pagination.py | 4 +- tests/test_helpers/test_tenants.py | 44 ++++-- tox.ini | 3 + 22 files changed, 681 insertions(+), 73 deletions(-) create mode 100644 futurex_openedx_extensions/dashboard/details/courses.py create mode 100644 futurex_openedx_extensions/helpers/constants.py create mode 100644 futurex_openedx_extensions/helpers/filters.py create mode 100644 test_utils/edx_platform_mocks/eox_nelp/course_experience/models.py delete mode 100644 tests/__init__.py create mode 100644 tests/test_dashboard/test_details/test_details_courses.py create mode 100644 tests/test_helpers/test_filters.py diff --git a/futurex_openedx_extensions/dashboard/details/courses.py b/futurex_openedx_extensions/dashboard/details/courses.py new file mode 100644 index 00000000..665d3b22 --- /dev/null +++ b/futurex_openedx_extensions/dashboard/details/courses.py @@ -0,0 +1,100 @@ +"""Courses details collectors""" +from __future__ import annotations + +from typing import List + +from common.djangoapps.student.models import CourseAccessRole +from django.db.models import Count, Exists, IntegerField, OuterRef, Q, Subquery, Sum +from django.db.models.functions import Coalesce +from django.db.models.query import QuerySet +from eox_nelp.course_experience.models import FeedbackCourse +from lms.djangoapps.certificates.models import GeneratedCertificate +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + +from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list, get_tenant_site + + +def get_courses_queryset(tenant_ids: List, search_text: str = None) -> QuerySet: + """ + Get the courses queryset for the given tenant IDs and search text. + + :param tenant_ids: List of tenant IDs to get the courses for + :type tenant_ids: List + :param search_text: Search text to filter the courses by + :type search_text: str + """ + course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list'] + tenant_sites = [] + for tenant_id in tenant_ids: + if site := get_tenant_site(tenant_id): + tenant_sites.append(site) + + queryset = CourseOverview.objects.filter( + org__in=course_org_filter_list, + ) + search_text = (search_text or '').strip() + if search_text: + queryset = queryset.filter( + Q(display_name__icontains=search_text) | + Q(id__icontains=search_text), + ) + queryset = queryset.annotate( + rating_count=Coalesce(Subquery( + FeedbackCourse.objects.filter( + course_id=OuterRef('id'), + rating_content__isnull=False, + rating_content__gt=0, + ).values('course_id').annotate(count=Count('id')).values('count'), + output_field=IntegerField(), + ), 0), + ).annotate( + rating_total=Coalesce(Subquery( + FeedbackCourse.objects.filter( + course_id=OuterRef('id'), + rating_content__isnull=False, + rating_content__gt=0, + ).values('course_id').annotate(total=Sum('rating_content')).values('total'), + ), 0), + ).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) & + ~Exists( + CourseAccessRole.objects.filter( + user_id=OuterRef('courseenrollment__user_id'), + org=OuterRef('org'), + ), + ) + ), + ) + ).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) & + ~Exists( + CourseAccessRole.objects.filter( + user_id=OuterRef('courseenrollment__user_id'), + org=OuterRef('org'), + ), + ) + ), + ) + ).annotate( + certificates_count=Coalesce(Subquery( + GeneratedCertificate.objects.filter( + course_id=OuterRef('id'), + status='downloadable' + ).values('course_id').annotate(count=Count('id')).values('count'), + output_field=IntegerField(), + ), 0), + ) + + return queryset diff --git a/futurex_openedx_extensions/dashboard/serializers.py b/futurex_openedx_extensions/dashboard/serializers.py index 99a4d1f5..b27c3d2c 100644 --- a/futurex_openedx_extensions/dashboard/serializers.py +++ b/futurex_openedx_extensions/dashboard/serializers.py @@ -1,8 +1,12 @@ """Serializers for the dashboard details API.""" - from django.contrib.auth import get_user_model +from django.utils.timezone import now +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from rest_framework import serializers +from futurex_openedx_extensions.helpers.constants import COURSE_STATUS_SELF_PREFIX, COURSE_STATUSES +from futurex_openedx_extensions.helpers.tenants import get_tenants_by_org + class LearnerDetailsSerializer(serializers.ModelSerializer): """Serializer for learner details.""" @@ -66,3 +70,85 @@ def get_certificates_count(self, obj): def get_enrolled_courses_count(self, obj): """Return enrolled courses count.""" return obj.courses_count + + +class CourseDetailsSerializer(serializers.ModelSerializer): + """Serializer for course details.""" + status = serializers.SerializerMethodField() + rating = serializers.SerializerMethodField() + enrolled_count = serializers.IntegerField() + active_count = serializers.IntegerField() + certificates_count = serializers.IntegerField() + start_date = serializers.SerializerMethodField() + end_date = serializers.SerializerMethodField() + start_enrollment_date = serializers.SerializerMethodField() + end_enrollment_date = serializers.SerializerMethodField() + display_name = serializers.CharField() + image_url = serializers.SerializerMethodField() + org = serializers.CharField() + tenant_ids = serializers.SerializerMethodField() + author_name = serializers.SerializerMethodField() + + class Meta: + model = CourseOverview + fields = [ + 'id', + 'status', + 'self_paced', + 'rating', + 'enrolled_count', + 'active_count', + 'certificates_count', + 'start_date', + 'end_date', + 'start_enrollment_date', + 'end_enrollment_date', + 'display_name', + 'image_url', + 'org', + 'tenant_ids', + 'author_name', + ] + + def get_status(self, obj): + """Return the course status.""" + if obj.end and obj.end < now(): + status = COURSE_STATUSES['archived'] + elif obj.start and obj.start > now(): + status = COURSE_STATUSES['upcoming'] + else: + status = COURSE_STATUSES['active'] + + return f'{COURSE_STATUS_SELF_PREFIX if obj.self_paced else ""}{status}' + + def get_rating(self, obj): + """Return the course rating.""" + return round(obj.rating_total / obj.rating_count if obj.rating_count else 0, 1) + + def get_start_enrollment_date(self, obj): + """Return the start enrollment date.""" + return obj.enrollment_start + + def get_end_enrollment_date(self, obj): + """Return the end enrollment date.""" + return obj.enrollment_end + + def get_image_url(self, obj): + """Return the course image URL.""" + return obj.course_image_url + + def get_tenant_ids(self, obj): + """Return the tenant IDs.""" + return get_tenants_by_org(obj.org) + + def get_start_date(self, obj): + """Return the start date.""" + return obj.start + + def get_end_date(self, obj): + """Return the end date.""" + return obj.end + + def get_author_name(self, obj): # pylint: disable=unused-argument + """Return the author name.""" + return None diff --git a/futurex_openedx_extensions/dashboard/statistics/courses.py b/futurex_openedx_extensions/dashboard/statistics/courses.py index 7008f165..3b8c80a9 100644 --- a/futurex_openedx_extensions/dashboard/statistics/courses.py +++ b/futurex_openedx_extensions/dashboard/statistics/courses.py @@ -3,11 +3,12 @@ from typing import List -from django.db.models import Count, Q +from django.db.models import Case, CharField, Count, Q, Value, When from django.db.models.query import QuerySet from django.utils.timezone import now from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from futurex_openedx_extensions.helpers.constants import COURSE_STATUSES from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list @@ -39,3 +40,36 @@ def get_courses_count(tenant_ids: List[int], only_active=False, only_visible=Fal return q_set.values('org').annotate( courses_count=Count('id') ).order_by('org') + + +def get_courses_count_by_status(tenant_ids: List[int]) -> QuerySet: + """ + Get the count of courses in the given tenants by status + + :param tenant_ids: List of tenant IDs to get the count for + :type tenant_ids: List[int] + :return: QuerySet of courses count per organization and status + :rtype: QuerySet + """ + course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list'] + + q_set = CourseOverview.objects.filter( + org__in=course_org_filter_list + ).annotate( + status=Case( + When( + Q(end__isnull=False) & Q(end__lt=now()), + then=Value(COURSE_STATUSES['archived']) + ), + When( + Q(start__isnull=False) & Q(start__gt=now()), + then=Value(COURSE_STATUSES['upcoming']) + ), + default=Value(COURSE_STATUSES['active']), + output_field=CharField() + ) + ).values('status', 'self_paced').annotate( + courses_count=Count('id') + ).values('status', 'self_paced', 'courses_count') + + return q_set diff --git a/futurex_openedx_extensions/dashboard/urls.py b/futurex_openedx_extensions/dashboard/urls.py index 1bb15397..e0fddddc 100644 --- a/futurex_openedx_extensions/dashboard/urls.py +++ b/futurex_openedx_extensions/dashboard/urls.py @@ -3,11 +3,13 @@ """ from django.urls import re_path -from futurex_openedx_extensions.dashboard.views import LearnersView, TotalCountsView +from futurex_openedx_extensions.dashboard import views app_name = 'fx_dashboard' urlpatterns = [ - re_path(r'^api/fx/statistics/v1/total_counts', TotalCountsView.as_view(), name='total-counts'), - re_path(r'^api/fx/learners/v1/learners', LearnersView.as_view(), name='learners'), + 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(r'^api/fx/statistics/v1/course_ratings', views.CourseRatingsView.as_view(), name='course-ratings'), + re_path(r'^api/fx/statistics/v1/total_counts', views.TotalCountsView.as_view(), name='total-counts'), ] diff --git a/futurex_openedx_extensions/dashboard/views.py b/futurex_openedx_extensions/dashboard/views.py index 05c2d74f..9cf70445 100644 --- a/futurex_openedx_extensions/dashboard/views.py +++ b/futurex_openedx_extensions/dashboard/views.py @@ -4,15 +4,18 @@ from rest_framework.response import Response from rest_framework.views import APIView +from futurex_openedx_extensions.dashboard.details.courses import get_courses_queryset from futurex_openedx_extensions.dashboard.details.learners import get_learners_queryset -from futurex_openedx_extensions.dashboard.serializers import LearnerDetailsSerializer +from futurex_openedx_extensions.dashboard.serializers import CourseDetailsSerializer, LearnerDetailsSerializer from futurex_openedx_extensions.dashboard.statistics.certificates import get_certificates_count -from futurex_openedx_extensions.dashboard.statistics.courses import get_courses_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 -from futurex_openedx_extensions.helpers.converters import error_details_to_dictionary, ids_string_to_list +from futurex_openedx_extensions.helpers.constants import COURSE_STATUS_SELF_PREFIX, COURSE_STATUSES +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 -from futurex_openedx_extensions.helpers.tenants import get_accessible_tenant_ids +from futurex_openedx_extensions.helpers.tenants import get_selected_tenants class TotalCountsView(APIView): @@ -76,11 +79,7 @@ def get(self, request, *args, **kwargs): if invalid_stats: return Response(error_details_to_dictionary(reason="Invalid stats type", invalid=invalid_stats), status=400) - tenant_ids = request.query_params.get('tenant_ids') - if tenant_ids is None: - tenant_ids = get_accessible_tenant_ids(request.user) - else: - tenant_ids = ids_string_to_list(tenant_ids) + tenant_ids = get_selected_tenants(request) result = dict({tenant_id: {} for tenant_id in tenant_ids}) result.update({ @@ -103,9 +102,64 @@ class LearnersView(ListAPIView): def get_queryset(self): """Get the list of learners""" - tenant_ids = self.request.query_params.get('tenant_ids') + tenant_ids = get_selected_tenants(self.request) search_text = self.request.query_params.get('search_text') return get_learners_queryset( - tenant_ids=ids_string_to_list(tenant_ids) if tenant_ids else get_accessible_tenant_ids(self.request.user), + tenant_ids=tenant_ids, search_text=search_text, ) + + +class CoursesView(ListAPIView): + """View to get the list of courses""" + serializer_class = CourseDetailsSerializer + permission_classes = [HasTenantAccess] + pagination_class = DefaultPagination + filter_backends = [DefaultOrderingFilter] + ordering_fields = [ + 'id', 'status', 'self_paced', 'rating', 'enrolled_count', 'active_count', + 'certificates_count', 'start_date', 'end_date', 'start_enrollment_date', + 'end_enrollment_date', 'display_name', 'image_url', 'org', 'tenant_ids', + ] + ordering = ['display_name'] + + def get_queryset(self): + """Get the list of learners""" + tenant_ids = get_selected_tenants(self.request) + search_text = self.request.query_params.get('search_text') + return get_courses_queryset( + tenant_ids=tenant_ids, + search_text=search_text, + ) + + +class CourseRatingsView(APIView): + """View to get the course ratings""" + permission_classes = [HasTenantAccess] + + @staticmethod + def to_json(result): + """Convert the result to JSON format""" + dict_result = { + f"{COURSE_STATUS_SELF_PREFIX if self_paced else ''}{status}": 0 + for status in COURSE_STATUSES + for self_paced in [False, True] + } + + for item in result: + status = f"{COURSE_STATUS_SELF_PREFIX if item['self_paced'] else ''}{item['status']}" + dict_result[status] = item['courses_count'] + return dict_result + + def get(self, request, *args, **kwargs): + """ + GET /api/fx/statistics/v1/course_ratings/?tenant_ids= + + (optional): a comma-separated list of the tenant IDs to get the information for. If not provided, + the API will assume the list of all accessible tenants by the user + """ + tenant_ids = get_selected_tenants(request) + + result = get_courses_count_by_status(tenant_ids=tenant_ids) + + return JsonResponse(self.to_json(result)) diff --git a/futurex_openedx_extensions/helpers/constants.py b/futurex_openedx_extensions/helpers/constants.py new file mode 100644 index 00000000..970fe557 --- /dev/null +++ b/futurex_openedx_extensions/helpers/constants.py @@ -0,0 +1,8 @@ +"""Constants for the FutureX Open edX Extensions app.""" +COURSE_STATUSES = { + 'active': 'active', + 'archived': 'archived', + 'upcoming': 'upcoming', +} + +COURSE_STATUS_SELF_PREFIX = 'self_' diff --git a/futurex_openedx_extensions/helpers/filters.py b/futurex_openedx_extensions/helpers/filters.py new file mode 100644 index 00000000..39d65910 --- /dev/null +++ b/futurex_openedx_extensions/helpers/filters.py @@ -0,0 +1,6 @@ +"""Filters helpers and classes for the API views.""" +from rest_framework.filters import OrderingFilter + + +class DefaultOrderingFilter(OrderingFilter): + ordering_param = 'sort' diff --git a/futurex_openedx_extensions/helpers/tenants.py b/futurex_openedx_extensions/helpers/tenants.py index 0989d93f..dbf0f6d5 100644 --- a/futurex_openedx_extensions/helpers/tenants.py +++ b/futurex_openedx_extensions/helpers/tenants.py @@ -8,6 +8,7 @@ from django.db.models import Exists, OuterRef from django.db.models.query import QuerySet from eox_tenant.models import Route, TenantConfig +from rest_framework.request import Request from futurex_openedx_extensions.helpers.converters import error_details_to_dictionary, ids_string_to_list @@ -225,3 +226,31 @@ def check_tenant_access(user: get_user_model(), tenant_ids_string: str) -> tuple ) return True, {} + + +def get_tenants_by_org(org: str) -> List[int]: + """ + Get the tenants that have in their course org filter + + :param org: The org to check + :type org: str + :return: List of tenant IDs + :rtype: List[int] + """ + tenant_configs = get_all_course_org_filter_list() + return [t_id for t_id, course_org_filter in tenant_configs.items() if org in course_org_filter] + + +def get_selected_tenants(request: Request) -> List[int]: + """ + Get the tenant IDs from the request + + :param request: The request + :type request: Request + :return: List of tenant IDs + :rtype: List[int] + """ + tenant_ids = request.query_params.get('tenant_ids') + if tenant_ids is None: + return get_accessible_tenant_ids(request.user) + return ids_string_to_list(tenant_ids) diff --git a/test_utils/edx_platform_mocks/eox_nelp/course_experience/models.py b/test_utils/edx_platform_mocks/eox_nelp/course_experience/models.py new file mode 100644 index 00000000..3d9c88a2 --- /dev/null +++ b/test_utils/edx_platform_mocks/eox_nelp/course_experience/models.py @@ -0,0 +1,2 @@ +"""edx-platform Mocks""" +from fake_models.models import FeedbackCourse # pylint: disable=unused-import diff --git a/test_utils/edx_platform_mocks/fake_models/models.py b/test_utils/edx_platform_mocks/fake_models/models.py index 9dca4e23..e6ad4c51 100644 --- a/test_utils/edx_platform_mocks/fake_models/models.py +++ b/test_utils/edx_platform_mocks/fake_models/models.py @@ -11,6 +11,11 @@ class CourseOverview(models.Model): visible_to_staff_only = models.BooleanField() start = models.DateTimeField(null=True) end = models.DateTimeField(null=True) + display_name = models.TextField(null=True) + enrollment_start = models.DateTimeField(null=True) + enrollment_end = models.DateTimeField(null=True) + self_paced = models.BooleanField(default=False) + course_image_url = models.TextField() class Meta: app_label = "fake_models" @@ -89,3 +94,35 @@ def has_profile_image(self): class Meta: app_label = "fake_models" db_table = "auth_userprofile" + + +class BaseFeedback(models.Model): + """Mock""" + RATING_OPTIONS = [ + (0, '0'), + (1, '1'), + (2, '2'), + (3, '3'), + (4, '4'), + (5, '5') + ] + author = models.ForeignKey(get_user_model(), null=True, on_delete=models.SET_NULL) + rating_content = models.IntegerField(blank=True, null=True, choices=RATING_OPTIONS) + feedback = models.CharField(max_length=500, blank=True, null=True) + public = models.BooleanField(null=True, default=False) + course_id = models.ForeignKey(CourseOverview, null=True, on_delete=models.SET_NULL) + + class Meta: + """Set model abstract""" + abstract = True + + +class FeedbackCourse(BaseFeedback): + """Mock""" + rating_instructors = models.IntegerField(blank=True, null=True, choices=BaseFeedback.RATING_OPTIONS) + recommended = models.BooleanField(null=True, default=True) + + class Meta: + """Set constrain for author an course id""" + unique_together = [["author", "course_id"]] + db_table = "eox_nelp_feedbackcourse" diff --git a/test_utils/edx_platform_mocks/setup.py b/test_utils/edx_platform_mocks/setup.py index 2353e348..b81a1482 100644 --- a/test_utils/edx_platform_mocks/setup.py +++ b/test_utils/edx_platform_mocks/setup.py @@ -4,5 +4,5 @@ setup( name='edx_platform_mocks', version='0.1.0', - packages=['common', 'fake_models', 'lms', 'openedx'], + packages=[], ) diff --git a/test_utils/eox_settings.py b/test_utils/eox_settings.py index 99a52e76..676c7971 100644 --- a/test_utils/eox_settings.py +++ b/test_utils/eox_settings.py @@ -1,5 +1,7 @@ -"""eox_tenant test settings.""" -GET_SITE_CONFIGURATION_MODULE = 'eox_tenant.edxapp_wrapper.backends.site_configuration_module_i_v1' +"""EOX test settings.""" + +# eox-tenant settings +EOX_TENANT_USERS_BACKEND = 'eox_tenant.edxapp_wrapper.backends.users_l_v1' GET_BRANDING_API = 'eox_tenant.edxapp_wrapper.backends.branding_api_l_v1' +GET_SITE_CONFIGURATION_MODULE = 'eox_tenant.edxapp_wrapper.backends.site_configuration_module_i_v1' GET_THEMING_HELPERS = 'eox_tenant.edxapp_wrapper.backends.theming_helpers_h_v1' -EOX_TENANT_USERS_BACKEND = 'eox_tenant.edxapp_wrapper.backends.users_l_v1' diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/base_test_data.py b/tests/base_test_data.py index ff878315..b3d4bdf2 100644 --- a/tests/base_test_data.py +++ b/tests/base_test_data.py @@ -106,6 +106,41 @@ "ORG6": 1, # This is an org with no tenant "ORG8": 2, }, + "course_attributes": { # org id, course id + "course-v1:ORG1+1+1": { + "start": "F", + }, + "course-v1:ORG1+2+2": { + "start": "F", + "end": "F", + }, + + "course-v1:ORG2+3+3": { + "end": "P", + }, + "course-v1:ORG2+4+4": { + "end": "P", + }, + "course-v1:ORG2+5+5": { + "start": "P", + "end": "P", + }, + + "course-v1:ORG2+1+1": { + "start": "P", + }, + "course-v1:ORG2+2+2": { + "start": "P", + "end": "F", + }, + "course-v1:ORG1+3+3": { + "end": "F", + }, + + "course-v1:ORG1+4+4": { + "self_paced": True, + }, + }, "course_enrollments": { # org id, course id, user ids "ORG1": { 1: [4, 5], diff --git a/tests/conftest.py b/tests/conftest.py index c53719a5..93eecc98 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ import pytest from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment, UserSignupSource from django.contrib.auth import get_user_model +from django.utils import timezone from eox_tenant.models import Route, TenantConfig from lms.djangoapps.certificates.models import GeneratedCertificate from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -84,8 +85,30 @@ def _create_course_overviews(): id=f"course-v1:{org}+{i}+{i}", org=org, visible_to_staff_only=False, + display_name=f"Course {i} of {org}", ) + now_time = timezone.now() + for course_id, data in _base_data["course_attributes"].items(): + course = CourseOverview.objects.get(id=course_id) + for field, value in data.items(): + if field in ("start", "end"): + assert value in ("F", "P"), f"Bad value for {field} in course_attributes testing data: {value}" + if field == "start": + if value == "F": + course.start = now_time + timezone.timedelta(days=1) + else: + course.start = now_time - timezone.timedelta(days=10) + continue + if field == "end": + if value == "F": + course.end = now_time + timezone.timedelta(days=10) + else: + course.end = now_time - timezone.timedelta(days=1) + continue + setattr(course, field, value) + course.save() + def _create_course_enrollments(): """Create course enrollments.""" for org, enrollments in _base_data["course_enrollments"].items(): diff --git a/tests/test_dashboard/test_details/test_details_courses.py b/tests/test_dashboard/test_details/test_details_courses.py new file mode 100644 index 00000000..15a76f79 --- /dev/null +++ b/tests/test_dashboard/test_details/test_details_courses.py @@ -0,0 +1,65 @@ +"""Tests for courses details collectors""" +import pytest +from eox_nelp.course_experience.models import FeedbackCourse +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + +from futurex_openedx_extensions.dashboard.details.courses import get_courses_queryset + + +@pytest.mark.django_db +@pytest.mark.parametrize('tenant_ids, search_text, expected_count', [ + ([7, 8], None, 5), + ([7], None, 3), + ([8], None, 2), + ([7], 'Course 1', 1), + ([7], 'Course 3', 1), + ([7], 'course 3', 1), + ([7], 'course 4', 0), + ([4], None, 0), +]) +def test_get_courses_queryset(base_data, tenant_ids, search_text, expected_count): # pylint: disable=unused-argument + """Verify that get_courses_queryset returns the correct QuerySet.""" + assert get_courses_queryset(tenant_ids, search_text).count() == expected_count + + +@pytest.mark.django_db +def test_get_courses_queryset_result_excludes_staff(base_data): # pylint: disable=unused-argument + """Verify that get_courses_queryset excludes staff users from enrollment, but not from certificates.""" + expected_results = { + 'course-v1:ORG1+1+1': [1, 0], + 'course-v1:ORG1+2+2': [0, 0], + 'course-v1:ORG1+3+3': [0, 0], + 'course-v1:ORG1+4+4': [0, 0], + 'course-v1:ORG1+5+5': [3, 4], + 'course-v1:ORG2+1+1': [0, 0], + 'course-v1:ORG2+2+2': [0, 0], + 'course-v1:ORG2+3+3': [1, 0], + 'course-v1:ORG2+4+4': [6, 4], + 'course-v1:ORG2+5+5': [5, 3], + 'course-v1:ORG2+6+6': [5, 0], + 'course-v1:ORG2+7+7': [5, 3], + } + queryset = get_courses_queryset([1]) + for record in queryset: + assert record.enrolled_count == expected_results[record.id][0] + assert record.certificates_count == expected_results[record.id][1] + + +@pytest.mark.django_db +def test_get_courses_queryset_result_rating(base_data): # pylint: disable=unused-argument + """Verify that get_courses_queryset returns the correct rating.""" + ratings = [3, 4, 5, 3, 4, 5, 3, 2, 5, 2, 4, 5] + no_ratings = [0, 0, 0, 0, 0, 0] + all_ratings = ratings + no_ratings + course = CourseOverview.objects.get(id='course-v1:ORG1+5+5') + for rating in all_ratings: + FeedbackCourse.objects.create( + course_id=course, + rating_content=rating, + ) + queryset = get_courses_queryset([1]) + for record in queryset: + if record.id != course.id: + continue + assert record.rating_count == len(ratings) + assert record.rating_total == sum(ratings) diff --git a/tests/test_dashboard/test_statistics/test_courses.py b/tests/test_dashboard/test_statistics/test_courses.py index 30aa9def..7b619a7a 100644 --- a/tests/test_dashboard/test_statistics/test_courses.py +++ b/tests/test_dashboard/test_statistics/test_courses.py @@ -1,15 +1,15 @@ """Tests for courses statistics.""" import pytest -from django.utils.timezone import now, timedelta from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from futurex_openedx_extensions.dashboard.statistics import courses +from futurex_openedx_extensions.helpers.constants import COURSE_STATUSES from futurex_openedx_extensions.helpers.tenants import get_course_org_filter_list from tests.base_test_data import _base_data @pytest.mark.django_db -def test_get_courses_count(base_data): +def test_get_courses_count(base_data): # pylint: disable=unused-argument """Verify get_courses_count function.""" all_tenants = _base_data["tenant_config"].keys() result = courses.get_courses_count(all_tenants) @@ -28,30 +28,11 @@ def test_get_courses_count(base_data): @pytest.mark.django_db -@pytest.mark.parametrize('start_diff, end_diff, expected_org1_count', [ - (None, None, 5), - (None, 1, 5), - (None, -1, 4), - (1, None, 4), - (-1, None, 5), - (1, 2, 4), - (-1, 1, 5), - (-2, -1, 4), -]) -def test_get_courses_count_only_active(base_data, start_diff, end_diff, expected_org1_count): +def test_get_courses_count_only_active(base_data): # pylint: disable=unused-argument """Verify get_courses_count function with only_active=True.""" - course = CourseOverview.objects.filter(org="ORG1").first() - assert course.start is None - assert course.end is None - - if start_diff is not None: - course.start = now() + timedelta(days=start_diff) - if end_diff is not None: - course.end = now() + timedelta(days=end_diff) - course.save() expected_result = [ - {'org': 'ORG1', 'courses_count': expected_org1_count}, - {'org': 'ORG2', 'courses_count': 7}, + {'org': 'ORG1', 'courses_count': 3}, + {'org': 'ORG2', 'courses_count': 4}, ] result = courses.get_courses_count([1], only_active=True) @@ -59,7 +40,7 @@ def test_get_courses_count_only_active(base_data, start_diff, end_diff, expected @pytest.mark.django_db -def test_get_courses_count_only_visible(base_data): +def test_get_courses_count_only_visible(base_data): # pylint: disable=unused-argument """Verify get_courses_count function with only_visible=True.""" course = CourseOverview.objects.filter(org="ORG1").first() assert course.visible_to_staff_only is False @@ -72,3 +53,15 @@ def test_get_courses_count_only_visible(base_data): result = courses.get_courses_count([1], only_visible=True) assert expected_result == list(result), f'Wrong result: {result}' + + +@pytest.mark.django_db +def test_get_courses_count_by_status(base_data): # pylint: disable=unused-argument + """Verify get_courses_count_by_status function.""" + result = courses.get_courses_count_by_status([1]) + assert list(result) == [ + {'self_paced': False, 'status': COURSE_STATUSES['active'], 'courses_count': 6}, + {'self_paced': False, 'status': COURSE_STATUSES['archived'], 'courses_count': 3}, + {'self_paced': False, 'status': COURSE_STATUSES['upcoming'], 'courses_count': 2}, + {'self_paced': True, 'status': COURSE_STATUSES['active'], 'courses_count': 1} + ] diff --git a/tests/test_dashboard/test_views.py b/tests/test_dashboard/test_views.py index b78c4da3..6da8f763 100644 --- a/tests/test_dashboard/test_views.py +++ b/tests/test_dashboard/test_views.py @@ -5,23 +5,34 @@ import pytest from django.contrib.auth import get_user_model from django.http import JsonResponse -from django.urls import reverse +from django.urls import resolve, reverse +from django.utils.timezone import now, timedelta +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from rest_framework.test import APITestCase +from futurex_openedx_extensions.helpers.constants import COURSE_STATUSES +from futurex_openedx_extensions.helpers.filters import DefaultOrderingFilter from tests.base_test_data import expected_statistics -@pytest.mark.usefixtures('base_data') -class TestTotalCountsView(APITestCase): - """Tests for TotalCountsView""" +class BaseTextViewMixin(APITestCase): + """Base test view mixin""" + VIEW_NAME = 'view name is not set!' + def setUp(self): - self.url = reverse('fx_dashboard:total-counts') + self.url = reverse(self.VIEW_NAME) self.staff_user = 2 def login_user(self, user_id): """Helper to login user""" self.client.force_login(get_user_model().objects.get(id=user_id)) + +@pytest.mark.usefixtures('base_data') +class TestTotalCountsView(BaseTextViewMixin): + """Tests for TotalCountsView""" + VIEW_NAME = 'fx_dashboard:total-counts' + def test_unauthorized(self): """Test unauthorized access""" response = self.client.get(self.url) @@ -59,15 +70,9 @@ def test_selected_tenants(self): @pytest.mark.usefixtures('base_data') -class TestLearnersView(APITestCase): +class TestLearnersView(BaseTextViewMixin): """Tests for LearnersView""" - def setUp(self): - self.url = reverse('fx_dashboard:learners') - self.staff_user = 2 - - def login_user(self, user_id): - """Helper to login user""" - self.client.force_login(get_user_model().objects.get(id=user_id)) + VIEW_NAME = 'fx_dashboard:learners' def test_unauthorized(self): """Verify that the view returns 403 when the user is not authenticated""" @@ -95,3 +100,99 @@ def test_success(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.data['count'], 46) self.assertGreater(len(response.data['results']), 0) + + +@pytest.mark.usefixtures('base_data') +class TesttCoursesView(BaseTextViewMixin): + """Tests for CoursesView""" + VIEW_NAME = 'fx_dashboard:courses' + + 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_no_tenants(self): + """Verify that the view returns the result for all accessible tenants when no tenant IDs are provided""" + 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) + + 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') + + def helper_test_success(self, response): + """Verify that the view returns the correct response""" + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['count'], 18) + self.assertGreater(len(response.data['results']), 0) + self.assertEqual(response.data['results'][0]['id'], 'course-v1:ORG1+1+1') + + 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.helper_test_success(response=response) + self.assertEqual(response.data['results'][0]['status'], COURSE_STATUSES['upcoming']) + + def test_status_archived(self): + """Verify that the view sets the correct status when the course is archived""" + CourseOverview.objects.filter(id='course-v1:ORG1+1+1').update(end=now() - timedelta(days=1)) + + self.login_user(self.staff_user) + response = self.client.get(self.url) + self.helper_test_success(response=response) + self.assertEqual(response.data['results'][0]['status'], 'archived') + + def test_status_upcoming(self): + """Verify that the view sets the correct status when the course is upcoming""" + CourseOverview.objects.filter(id='course-v1:ORG1+1+1').update(start=now() + timedelta(days=1)) + + self.login_user(self.staff_user) + response = self.client.get(self.url) + self.helper_test_success(response=response) + self.assertEqual(response.data['results'][0]['status'], 'upcoming') + + def test_sorting(self): + """Verify that the view soring filter is set correctly""" + view_func, _, _ = resolve(self.url) + view_class = view_func.view_class + self.assertEqual(view_class.filter_backends, [DefaultOrderingFilter]) + + +@pytest.mark.usefixtures('base_data') +class TesttCourseRatingsView(BaseTextViewMixin): + """Tests for CourseRatingsView""" + VIEW_NAME = 'fx_dashboard:course-ratings' + + 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_no_tenants(self): + """Verify that the view returns the result for all accessible tenants when no tenant IDs are provided""" + self.login_user(self.staff_user) + with patch('futurex_openedx_extensions.dashboard.views.get_courses_count_by_status') as mock_queryset: + self.client.get(self.url) + mock_queryset.assert_called_once_with(tenant_ids=[1, 2, 3, 7, 8]) + + 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) + data = json.loads(response.content) + self.assertDictEqual(data, { + "active": 12, + "archived": 3, + "upcoming": 2, + "self_active": 1, + "self_archived": 0, + "self_upcoming": 0, + }) diff --git a/tests/test_helpers/test_filters.py b/tests/test_helpers/test_filters.py new file mode 100644 index 00000000..c53d9f7b --- /dev/null +++ b/tests/test_helpers/test_filters.py @@ -0,0 +1,10 @@ +"""Tests for pagination helpers""" +from rest_framework.filters import OrderingFilter + +from futurex_openedx_extensions.helpers.filters import DefaultOrderingFilter + + +def test_default_sorting_filter(): + """Verify that the DefaultOrderingFilter class is correctly defined.""" + assert issubclass(DefaultOrderingFilter, OrderingFilter) + assert DefaultOrderingFilter.ordering_param == 'sort' diff --git a/tests/test_helpers/test_pagination.py b/tests/test_helpers/test_pagination.py index 880d6dd1..22f78c06 100644 --- a/tests/test_helpers/test_pagination.py +++ b/tests/test_helpers/test_pagination.py @@ -1,10 +1,12 @@ """Tests for pagination helpers""" +from rest_framework.pagination import PageNumberPagination + from futurex_openedx_extensions.helpers.pagination import DefaultPagination def test_default_pagination(): """Verify that the DefaultPagination class is correctly defined.""" - assert issubclass(DefaultPagination, DefaultPagination) + assert issubclass(DefaultPagination, PageNumberPagination) assert DefaultPagination.page_size == 20 assert DefaultPagination.page_size_query_param == 'page_size' assert DefaultPagination.max_page_size == 100 diff --git a/tests/test_helpers/test_tenants.py b/tests/test_helpers/test_tenants.py index 95f43b03..2d0bf374 100644 --- a/tests/test_helpers/test_tenants.py +++ b/tests/test_helpers/test_tenants.py @@ -10,14 +10,14 @@ @pytest.mark.django_db -def test_get_excluded_tenant_ids(base_data): +def test_get_excluded_tenant_ids(base_data): # pylint: disable=unused-argument """Verify get_excluded_tenant_ids function.""" result = tenants.get_excluded_tenant_ids() assert result == [4, 5, 6] @pytest.mark.django_db -def test_get_all_tenants(base_data): +def test_get_all_tenants(base_data): # pylint: disable=unused-argument """Verify get_all_tenants function.""" result = tenants.get_all_tenants() assert TenantConfig.objects.count() == 8 @@ -27,14 +27,14 @@ def test_get_all_tenants(base_data): @pytest.mark.django_db -def test_get_all_tenant_ids(base_data): +def test_get_all_tenant_ids(base_data): # pylint: disable=unused-argument """Verify get_all_tenant_ids function.""" result = tenants.get_all_tenant_ids() assert result == [1, 2, 3, 7, 8] @pytest.mark.django_db -def test_get_accessible_tenant_ids_none(base_data): +def test_get_accessible_tenant_ids_none(base_data): # pylint: disable=unused-argument """Verify that get_accessible_tenant_ids returns an empty list when user is None.""" result = tenants.get_accessible_tenant_ids(None) assert result == [] @@ -44,7 +44,7 @@ def test_get_accessible_tenant_ids_none(base_data): @pytest.mark.parametrize("user_id, expected", [ (1, [1, 2, 3, 7, 8]), ]) -def test_get_accessible_tenant_ids_super_users(base_data, user_id, expected): +def test_get_accessible_tenant_ids_super_users(base_data, user_id, expected): # pylint: disable=unused-argument """Verify get_accessible_tenant_ids function for super users.""" user = get_user_model().objects.get(id=user_id) assert user.is_superuser, 'only super users allowed in this test' @@ -56,7 +56,7 @@ def test_get_accessible_tenant_ids_super_users(base_data, user_id, expected): @pytest.mark.parametrize("user_id, expected", [ (2, [1, 2, 3, 7, 8]), ]) -def test_get_accessible_tenant_ids_staff(base_data, user_id, expected): +def test_get_accessible_tenant_ids_staff(base_data, user_id, expected): # pylint: disable=unused-argument """Verify get_accessible_tenant_ids function for staff users.""" user = get_user_model().objects.get(id=user_id) assert user.is_staff, 'only staff users allowed in this test' @@ -71,7 +71,9 @@ def test_get_accessible_tenant_ids_staff(base_data, user_id, expected): (9, [1]), (23, [2, 3, 8]), ]) -def test_get_accessible_tenant_ids_no_staff_no_sueperuser(base_data, user_id, expected): +def test_get_accessible_tenant_ids_no_staff_no_sueperuser( + base_data, user_id, expected +): # pylint: disable=unused-argument """Verify get_accessible_tenant_ids function for users with no staff and no superuser.""" user = get_user_model().objects.get(id=user_id) assert not user.is_staff and not user.is_superuser, 'only users with no staff and no superuser allowed in this test' @@ -80,7 +82,7 @@ def test_get_accessible_tenant_ids_no_staff_no_sueperuser(base_data, user_id, ex @pytest.mark.django_db -def test_get_accessible_tenant_ids_complex(base_data): +def test_get_accessible_tenant_ids_complex(base_data): # pylint: disable=unused-argument """Verify get_accessible_tenant_ids function for complex cases""" user = get_user_model().objects.get(id=10) user_access_role = 'org_course_creator_group' @@ -143,7 +145,7 @@ def test_get_accessible_tenant_ids_complex(base_data): } )), ]) -def test_check_tenant_access(base_data, user_id, ids_to_check, expected): +def test_check_tenant_access(base_data, user_id, ids_to_check, expected): # pylint: disable=unused-argument """Verify check_tenant_access function.""" user = get_user_model().objects.get(id=user_id) result = tenants.check_tenant_access(user, ids_to_check) @@ -151,7 +153,7 @@ def test_check_tenant_access(base_data, user_id, ids_to_check, expected): @pytest.mark.django_db -def test_get_all_course_org_filter_list(base_data): +def test_get_all_course_org_filter_list(base_data): # pylint: disable=unused-argument """Verify get_all_course_org_filter_list function.""" result = tenants.get_all_course_org_filter_list() assert result == { @@ -198,7 +200,7 @@ def test_get_all_course_org_filter_list(base_data): 'invalid': [], }), ]) -def test_get_course_org_filter_list(base_data, tenant_ids, expected): +def test_get_course_org_filter_list(base_data, tenant_ids, expected): # pylint: disable=unused-argument """Verify get_course_org_filter_list function.""" result = tenants.get_course_org_filter_list(tenant_ids) assert result == expected @@ -210,7 +212,7 @@ def test_get_course_org_filter_list(base_data, tenant_ids, expected): (2, [1, 2, 3, 7, 8]), (3, []), ]) -def test_get_accessible_tenant_ids(base_data, user_id, expected): +def test_get_accessible_tenant_ids(base_data, user_id, expected): # pylint: disable=unused-argument """Verify get_accessible_tenant_ids function.""" user = get_user_model().objects.get(id=user_id) result = tenants.get_accessible_tenant_ids(user) @@ -218,7 +220,7 @@ def test_get_accessible_tenant_ids(base_data, user_id, expected): @pytest.mark.django_db -def test_get_all_tenants_info(base_data): +def test_get_all_tenants_info(base_data): # pylint: disable=unused-argument """Verify get_all_tenants_info function.""" result = tenants.get_all_tenants_info() assert result['tenant_ids'] == [1, 2, 3, 7, 8] @@ -242,6 +244,20 @@ def test_get_all_tenants_info(base_data): (7, 's7.sample.com'), (8, 's8.sample.com'), ]) -def test_get_tenant_site(base_data, tenant_id, expected): +def test_get_tenant_site(base_data, tenant_id, expected): # pylint: disable=unused-argument """Verify get_tenant_site function.""" assert expected == tenants.get_tenant_site(tenant_id) + + +@pytest.mark.django_db +@pytest.mark.parametrize("org, expected", [ + ('ORG1', [1]), + ('ORG2', [1]), + ('ORG3', [2, 7]), + ('ORG4', [3]), + ('ORG5', [3]), + ('ORG8', [2, 8]), +]) +def test_get_tenants_by_org(base_data, org, expected): # pylint: disable=unused-argument + """Verify get_tenants_by_org function.""" + assert expected == tenants.get_tenants_by_org(org) diff --git a/tox.ini b/tox.ini index ca36bfcf..8398eb4b 100644 --- a/tox.ini +++ b/tox.ini @@ -35,12 +35,15 @@ addopts = --cov futurex_openedx_extensions --cov tests --cov-report term-missing norecursedirs = .* docs requirements site-packages [testenv] +allowlist_externals = + rm deps = django32: Django>=3.2,<4.0 django40: Django>=4.0,<4.1 -r{toxinidir}/requirements/test.txt -e{toxinidir}/test_utils/edx_platform_mocks commands = + rm -Rf {toxinidir}/test_utils/edx_platform_mocks/fake_models/migrations python manage.py makemigrations fake_models python manage.py check pytest {posargs}