From 514889ac967d07bb772777adbf7b077d32c40ab6 Mon Sep 17 00:00:00 2001 From: Shadi Naif Date: Fri, 8 Mar 2024 11:52:22 +0300 Subject: [PATCH] feat: learners-count statistics --- .github/workflows/ci.yml | 26 +- .gitignore | 3 + .../dashboard/__init__.py | 0 futurex_openedx_extensions/dashboard/apps.py | 4 +- .../dashboard/models.py | 3 - .../dashboard/settings/__init__.py | 0 .../dashboard/settings/common_production.py | 2 +- .../dashboard/statistics/__init__.py | 0 .../dashboard/statistics/certificates.py | 36 +++ .../dashboard/statistics/courses.py | 41 +++ .../dashboard/statistics/learners.py | 194 ++++++++++++++ futurex_openedx_extensions/dashboard/urls.py | 10 +- futurex_openedx_extensions/dashboard/views.py | 95 +++++++ .../helpers/__init__.py | 0 futurex_openedx_extensions/helpers/apps.py | 9 + .../helpers/converters.py | 19 ++ .../helpers/permissions.py | 23 ++ futurex_openedx_extensions/helpers/tenants.py | 227 ++++++++++++++++ requirements/base.txt | 2 +- requirements/ci.txt | 2 +- requirements/dev.txt | 35 ++- requirements/doc.txt | 40 ++- requirements/pip-tools.txt | 4 +- requirements/pip.txt | 4 +- requirements/quality.txt | 37 ++- requirements/test.in | 9 + requirements/test.txt | 36 ++- setup.py | 4 +- test_settings.py | 15 +- test_utils/__init__.py | 10 - test_utils/edx_platform_mocks/__init__.py | 0 .../common/djangoapps/student/models.py | 2 + .../fake_models/__init__.py | 0 .../edx_platform_mocks/fake_models/models.py | 61 +++++ .../lms/djangoapps/certificates/models.py | 2 + .../content/course_overviews/models.py | 2 + .../djangoapps/site_configuration/helpers.py | 1 + .../djangoapps/site_configuration/models.py | 1 + .../core/djangoapps/theming/helpers.py | 1 + test_utils/edx_platform_mocks/setup.py | 8 + test_utils/eox_settings.py | 5 + tests/base_test_data.py | 189 +++++++++++++ tests/conftest.py | 124 +++++++++ tests/test_base_test_data.py | 20 ++ tests/test_dashboard/__init__.py | 0 tests/test_dashboard/test_apps.py | 36 +++ .../test_statistics/test_certificates.py | 41 +++ .../test_statistics/test_courses.py | 74 ++++++ .../test_statistics/test_learners.py | 82 ++++++ tests/test_dashboard/test_views.py | 65 +++++ tests/test_helpers/test_apps.py | 7 + tests/test_helpers/test_converters.py | 48 ++++ tests/test_helpers/test_permissions.py | 72 +++++ tests/test_helpers/test_tenants.py | 248 ++++++++++++++++++ tests/test_models.py | 18 -- tests/test_urls.py | 8 + tox.ini | 3 + 57 files changed, 1915 insertions(+), 93 deletions(-) create mode 100644 futurex_openedx_extensions/dashboard/__init__.py delete mode 100644 futurex_openedx_extensions/dashboard/models.py create mode 100644 futurex_openedx_extensions/dashboard/settings/__init__.py create mode 100644 futurex_openedx_extensions/dashboard/statistics/__init__.py create mode 100644 futurex_openedx_extensions/dashboard/statistics/certificates.py create mode 100644 futurex_openedx_extensions/dashboard/statistics/courses.py create mode 100644 futurex_openedx_extensions/dashboard/statistics/learners.py create mode 100644 futurex_openedx_extensions/dashboard/views.py create mode 100644 futurex_openedx_extensions/helpers/__init__.py create mode 100644 futurex_openedx_extensions/helpers/apps.py create mode 100644 futurex_openedx_extensions/helpers/converters.py create mode 100644 futurex_openedx_extensions/helpers/permissions.py create mode 100644 futurex_openedx_extensions/helpers/tenants.py delete mode 100644 test_utils/__init__.py create mode 100644 test_utils/edx_platform_mocks/__init__.py create mode 100644 test_utils/edx_platform_mocks/common/djangoapps/student/models.py create mode 100644 test_utils/edx_platform_mocks/fake_models/__init__.py create mode 100644 test_utils/edx_platform_mocks/fake_models/models.py create mode 100644 test_utils/edx_platform_mocks/lms/djangoapps/certificates/models.py create mode 100644 test_utils/edx_platform_mocks/openedx/core/djangoapps/content/course_overviews/models.py create mode 100644 test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/helpers.py create mode 100644 test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/models.py create mode 100644 test_utils/edx_platform_mocks/openedx/core/djangoapps/theming/helpers.py create mode 100644 test_utils/edx_platform_mocks/setup.py create mode 100644 test_utils/eox_settings.py create mode 100644 tests/base_test_data.py create mode 100644 tests/conftest.py create mode 100644 tests/test_base_test_data.py create mode 100644 tests/test_dashboard/__init__.py create mode 100644 tests/test_dashboard/test_apps.py create mode 100644 tests/test_dashboard/test_statistics/test_certificates.py create mode 100644 tests/test_dashboard/test_statistics/test_courses.py create mode 100644 tests/test_dashboard/test_statistics/test_learners.py create mode 100644 tests/test_dashboard/test_views.py create mode 100644 tests/test_helpers/test_apps.py create mode 100644 tests/test_helpers/test_converters.py create mode 100644 tests/test_helpers/test_permissions.py create mode 100644 tests/test_helpers/test_tenants.py delete mode 100644 tests/test_models.py create mode 100644 tests/test_urls.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 890bc069..fafbf91d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,27 +1,11 @@ name: Python CI on: - push: - branches: [main] - issue_comment: - types: [created] + pull_request: + branches: + - '*' jobs: - check_comment: - runs-on: ubuntu-latest - if: github.event.issue.pull_request != '' # Only runs if the comment is on a PR - outputs: - should_run: ${{ steps.comment_check.outputs.should_run }} - steps: - - name: Check for 'run tests' comment - id: comment_check - uses: actions/github-script@v5 - with: - github-token: ${{secrets.GITHUB_TOKEN}} - script: | - const isRunTestsComment = '${{ github.event.comment.body }}'.trim() === 'run tests'; - core.setOutput('should_run', isRunTestsComment ? 'true' : 'false'); - run_tests: name: tests runs-on: ${{ matrix.os }} @@ -32,9 +16,9 @@ jobs: toxenv: [quality, django32] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: setup python - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} diff --git a/.gitignore b/.gitignore index d7b2acb3..927f88d1 100644 --- a/.gitignore +++ b/.gitignore @@ -63,3 +63,6 @@ docs/futurex_openedx_extensions.*.rst # Private requirements requirements/private.in requirements/private.txt + +# temporary tests migration files +/test_utils/edx_platform_mocks/fake_models/migrations/ diff --git a/futurex_openedx_extensions/dashboard/__init__.py b/futurex_openedx_extensions/dashboard/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/futurex_openedx_extensions/dashboard/apps.py b/futurex_openedx_extensions/dashboard/apps.py index 61fc37be..255e79a8 100644 --- a/futurex_openedx_extensions/dashboard/apps.py +++ b/futurex_openedx_extensions/dashboard/apps.py @@ -6,7 +6,7 @@ class DashboardConfig(AppConfig): """Configuration for the dashboard Django application""" - name = 'dashboard' + name = 'futurex_openedx_extensions.dashboard' plugin_app = { 'settings_config': { @@ -23,7 +23,7 @@ class DashboardConfig(AppConfig): }, 'url_config': { 'lms.djangoapp': { - 'namespace': 'dashboard', + 'namespace': 'fx_dashboard', }, }, } diff --git a/futurex_openedx_extensions/dashboard/models.py b/futurex_openedx_extensions/dashboard/models.py deleted file mode 100644 index 6dc63ca1..00000000 --- a/futurex_openedx_extensions/dashboard/models.py +++ /dev/null @@ -1,3 +0,0 @@ -""" -Database models for dashboard. -""" diff --git a/futurex_openedx_extensions/dashboard/settings/__init__.py b/futurex_openedx_extensions/dashboard/settings/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/futurex_openedx_extensions/dashboard/settings/common_production.py b/futurex_openedx_extensions/dashboard/settings/common_production.py index ac562f21..f53f51c0 100644 --- a/futurex_openedx_extensions/dashboard/settings/common_production.py +++ b/futurex_openedx_extensions/dashboard/settings/common_production.py @@ -5,4 +5,4 @@ def plugin_settings(settings): # pylint: disable=unused-argument """ plugin settings """ - # Nothing to do here yet. + # Nothing to do here yet diff --git a/futurex_openedx_extensions/dashboard/statistics/__init__.py b/futurex_openedx_extensions/dashboard/statistics/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/futurex_openedx_extensions/dashboard/statistics/certificates.py b/futurex_openedx_extensions/dashboard/statistics/certificates.py new file mode 100644 index 00000000..a0e30bcf --- /dev/null +++ b/futurex_openedx_extensions/dashboard/statistics/certificates.py @@ -0,0 +1,36 @@ +"""functions for getting statistics about certificates""" +from __future__ import annotations + +from typing import Dict, List + +from django.db.models import Count, OuterRef, Subquery +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 + + +def get_certificates_count(tenant_ids: List[int]) -> Dict[str, int]: + """ + Get the count of issued certificates in the given tenants. The count is grouped by organization. Certificates + for admins, staff, and superusers are also included. + + :param tenant_ids: List of tenant IDs to get the count for + :type tenant_ids: List[int] + :return: Count of certificates per organization + :rtype: Dict[str, int] + """ + course_org_filter_list = get_course_org_filter_list(tenant_ids)['course_org_filter_list'] + + result = list(GeneratedCertificate.objects.filter( + status='downloadable', + course_id__in=CourseOverview.objects.filter( + org__in=course_org_filter_list + ), + ).annotate(course_org=Subquery( + CourseOverview.objects.filter( + id=OuterRef('course_id') + ).values('org') + )).values('course_org').annotate(certificates_count=Count('id')).values_list('course_org', 'certificates_count')) + + return dict(result) diff --git a/futurex_openedx_extensions/dashboard/statistics/courses.py b/futurex_openedx_extensions/dashboard/statistics/courses.py new file mode 100644 index 00000000..7008f165 --- /dev/null +++ b/futurex_openedx_extensions/dashboard/statistics/courses.py @@ -0,0 +1,41 @@ +"""functions for getting statistics about courses""" +from __future__ import annotations + +from typing import List + +from django.db.models import Count, Q +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.tenants import get_course_org_filter_list + + +def get_courses_count(tenant_ids: List[int], only_active=False, only_visible=False) -> QuerySet: + """ + Get the count of courses in the given tenants + + :param tenant_ids: List of tenant IDs to get the count for + :type tenant_ids: List[int] + :param only_active: Whether to only count active courses (according to dates) + :type only_active: bool + :param only_visible: Whether to only count visible courses (according to staff-only visibility) + :type only_visible: bool + :return: QuerySet of courses count per organization + :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) + if only_active: + q_set = q_set.filter( + Q(start__isnull=True) | Q(start__lte=now()), + ).filter( + Q(end__isnull=True) | Q(end__gte=now()), + ) + if only_visible: + q_set = q_set.filter(visible_to_staff_only=False) + + return q_set.values('org').annotate( + courses_count=Count('id') + ).order_by('org') diff --git a/futurex_openedx_extensions/dashboard/statistics/learners.py b/futurex_openedx_extensions/dashboard/statistics/learners.py new file mode 100644 index 00000000..823ff142 --- /dev/null +++ b/futurex_openedx_extensions/dashboard/statistics/learners.py @@ -0,0 +1,194 @@ +"""functions for getting statistics about learners""" +from __future__ import annotations + +from typing import Dict, List + +from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment, UserSignupSource +from django.contrib.auth import get_user_model +from django.db.models import Count, Exists, OuterRef, Q, Subquery +from django.db.models.query import QuerySet +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_learners_count_having_enrollment_per_org(tenant_id) -> QuerySet: + """ + TODO: Cache the result of this function + Get the count of learners with enrollments per organization. Admins and staff are excluded from the count. This + function takes one tenant ID for performance reasons. + + + SELECT coc.org, COUNT(DISTINCT au.id) + FROM edxapp.auth_user au + INNER JOIN edxapp.student_courseenrollment sc ON + au.id = sc.user_id + INNER JOIN edxapp.course_overviews_courseoverview coc ON + sc.course_id = coc.id AND + coc.org IN ('ORG1', 'ORG2') -- course_org_filter_list + WHERE au.id NOT IN ( + SELECT DISTINCT cr.user_id + FROM edxapp.student_courseaccessrole cr + WHERE cr.org = coc.org + ) AND + au.is_superuser = 0 AND + au.is_staff = 0 AND + au.is_active = 1 + GROUP BY coc.org + + + :return: QuerySet of learners count per organization + :rtype: QuerySet + """ + course_org_filter_list = get_course_org_filter_list([tenant_id])['course_org_filter_list'] + + return CourseOverview.objects.filter( + org__in=course_org_filter_list + ).values('org').annotate( + learners_count=Count( + 'courseenrollment__user_id', + filter=~Exists( + CourseAccessRole.objects.filter( + user_id=OuterRef('courseenrollment__user_id'), + org=OuterRef('org') + ) + ) & + Q(courseenrollment__user__is_superuser=False) & + Q(courseenrollment__user__is_staff=False) & + Q(courseenrollment__user__is_active=True), + distinct=True + ) + ) + + +def get_learners_count_having_enrollment_for_tenant(tenant_id) -> QuerySet: + """ + TODO: Cache the result of this function + Get the count of learners with enrollments per organization. Admins and staff are excluded from the count + + + SELECT COUNT(DISTINCT au.id) + FROM edxapp.auth_user au + INNER JOIN edxapp.student_courseenrollment sc ON + au.id = sc.user_id + INNER JOIN edxapp.course_overviews_courseoverview coc ON + sc.course_id = coc.id AND + coc.org IN ('ORG1', 'ORG2') -- course_org_filter_list + WHERE au.id NOT IN ( + SELECT DISTINCT cr.user_id + FROM edxapp.student_courseaccessrole cr + WHERE cr.org = coc.org + ) AND + au.is_superuser = 0 AND + au.is_staff = 0 AND + au.is_active = 1 + + + :return: QuerySet of learners count per organization + :rtype: QuerySet + """ + course_org_filter_list = get_course_org_filter_list([tenant_id])['course_org_filter_list'] + + return get_user_model().objects.filter( + is_superuser=False, + is_staff=False, + is_active=True, + courseenrollment__course__org__in=course_org_filter_list, + ).exclude( + Exists( + CourseAccessRole.objects.filter( + user_id=OuterRef('id'), + org=OuterRef('courseenrollment__course__org'), + ).values('user_id') + ) + ).values('id').distinct().count() + + +def get_learners_count_having_no_enrollment(tenant_id) -> QuerySet: + """ + TODO: Cache the result of this function + Get the count of learners with no enrollments per organization. Admins and staff are excluded from the count. + Since there is no enrollment, we'll use UserSignupSource + + The function returns the count for one tenant for performance reasons. + + SELECT COUNT(distinct su.id) + FROM edxapp.student_usersignupsource su + WHERE su.site = 'demo.example.com' -- tenant_site + AND su.user_id not in ( + SELECT distinct au.id + FROM edxapp.auth_user au + INNER JOIN edxapp.student_courseenrollment sc ON + au.id = sc.user_id + INNER JOIN edxapp.course_overviews_courseoverview coc ON + sc.course_id = coc.id AND + coc.org IN ('ORG1', 'ORG2') -- course_org_filter_list + WHERE au.id NOT IN ( + SELECT DISTINCT cr.user_id + FROM edxapp.student_courseaccessrole cr + WHERE cr.org = coc.org + ) AND + au.is_superuser = 0 AND + au.is_staff = 0 AND + au.is_active = 1 AND + ) AND su.user_id NOT IN ( + SELECT DISTINCT cr.user_id + FROM edxapp.student_courseaccessrole cr + WHERE cr.org IN ('ORG1', 'ORG2') -- course_org_filter_list + ) + + """ + course_org_filter_list = get_course_org_filter_list([tenant_id])['course_org_filter_list'] + tenant_site = get_tenant_site(tenant_id) + + return UserSignupSource.objects.filter( + site=tenant_site + ).exclude( + user_id__in=Subquery( + CourseEnrollment.objects.filter( + user_id=OuterRef('user_id'), + course__org__in=course_org_filter_list, + user__is_superuser=False, + user__is_staff=False, + user__is_active=True, + ).exclude( + Exists( + CourseAccessRole.objects.filter( + user_id=OuterRef('user_id'), + org=OuterRef('course__org'), + ).values('user_id').distinct() + ), + ).values('user_id') + ) + ).exclude( + user_id__in=Subquery( + CourseAccessRole.objects.filter( + org__in=course_org_filter_list, + ).values('user_id').distinct() + ) + ).values('user_id').distinct().count() + + +def get_learners_count(tenant_ids: List[int]) -> Dict[int, Dict[str, int]]: + """ + Get the count of learners in the given list of tenants. Admins and staff are excluded from the count. + + :param tenant_ids: List of tenant IDs to get the count for + :type tenant_ids: List[int] + :return: Dictionary of tenant ID and the count of learners + :rtype: Dict[int, Dict[str, int]] + """ + result = { + tenant_id: { + 'learners_count': get_learners_count_having_enrollment_for_tenant(tenant_id), + 'learners_count_no_enrollment': get_learners_count_having_no_enrollment(tenant_id), + 'learners_count_per_org': {}, + } + for tenant_id in tenant_ids + } + for tenant_id in tenant_ids: + result[tenant_id]['learners_count_per_org'] = { + item['org']: item['learners_count'] + for item in get_learners_count_having_enrollment_per_org(tenant_id) + } + return result diff --git a/futurex_openedx_extensions/dashboard/urls.py b/futurex_openedx_extensions/dashboard/urls.py index 50d97ba7..a2379f3c 100644 --- a/futurex_openedx_extensions/dashboard/urls.py +++ b/futurex_openedx_extensions/dashboard/urls.py @@ -1,10 +1,12 @@ """ URLs for dashboard. """ -from django.urls import re_path # pylint: disable=unused-import -from django.views.generic import TemplateView # pylint: disable=unused-import +from django.urls import re_path + +from futurex_openedx_extensions.dashboard.views import TotalCountsView + +app_name = 'fx_dashboard' urlpatterns = [ - # TODO: Fill in URL patterns and views here. - # re_path(r'', TemplateView.as_view(template_name="dashboard/base.html")), + re_path(r'^api/fx/statistics/v1/total_counts', TotalCountsView.as_view(), name='total-counts'), ] diff --git a/futurex_openedx_extensions/dashboard/views.py b/futurex_openedx_extensions/dashboard/views.py new file mode 100644 index 00000000..4e8b6243 --- /dev/null +++ b/futurex_openedx_extensions/dashboard/views.py @@ -0,0 +1,95 @@ +"""Views for the dashboard app""" +from django.http import JsonResponse +from rest_framework.response import Response +from rest_framework.views import APIView + +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.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.permissions import HasTenantAccess +from futurex_openedx_extensions.helpers.tenants import get_accessible_tenant_ids + + +class TotalCountsView(APIView): + """View to get the total count statistics""" + STAT_CERTIFICATES = 'certificates' + STAT_COURSES = 'courses' + STAT_LEARNERS = 'learners' + + valid_stats = [STAT_CERTIFICATES, STAT_COURSES, STAT_LEARNERS] + STAT_RESULT_KEYS = { + STAT_CERTIFICATES: 'certificates_count', + STAT_COURSES: 'courses_count', + STAT_LEARNERS: 'learners_count' + } + + permission_classes = [HasTenantAccess] + + @staticmethod + def _get_certificates_count_data(tenant_id): + """Get the count of certificates for the given tenant""" + collector_result = get_certificates_count([tenant_id]) + return sum(certificate_count for certificate_count in collector_result.values()) + + @staticmethod + def _get_courses_count_data(tenant_id): + """Get the count of courses for the given tenant""" + collector_result = get_courses_count([tenant_id]) + return sum(org_count['courses_count'] for org_count in collector_result) + + @staticmethod + def _get_learners_count_data(tenant_id): + """Get the count of learners for the given tenant""" + collector_result = get_learners_count([tenant_id]) + print('*' * 100) + print(type(collector_result)) + print(collector_result) + print('-' * 100) + return collector_result[tenant_id]['learners_count'] + \ + collector_result[tenant_id]['learners_count_no_enrollment'] + + def _get_stat_count(self, stat, tenant_id): + """Get the count of the given stat for the given tenant""" + if stat == self.STAT_CERTIFICATES: + return self._get_certificates_count_data(tenant_id) + + if stat == self.STAT_COURSES: + return self._get_courses_count_data(tenant_id) + + return self._get_learners_count_data(tenant_id) + + def get(self, request, *args, **kwargs): + """ + GET /api/fx/statistics/v1/total_counts/?stats=&tenant_ids= + + (required): a comma-separated list of the types of count statistics to include in the + response. Available count statistics are: + certificates: total number of issued certificates in the selected tenants + courses: total number of courses in the selected tenants + learners: total number of learners in the selected tenants + (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 + """ + stats = request.query_params.get('stats', '').split(',') + invalid_stats = list(set(stats) - set(self.valid_stats)) + 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) + + result = {tenant_id: {} for tenant_id in tenant_ids} + result.update({ + f'total_{self.STAT_RESULT_KEYS[stat]}': 0 for stat in stats + }) + for tenant_id in tenant_ids: + for stat in stats: + count = self._get_stat_count(stat, tenant_id) + result[tenant_id][self.STAT_RESULT_KEYS[stat]] = count + result[f'total_{self.STAT_RESULT_KEYS[stat]}'] += count + + return JsonResponse(result) diff --git a/futurex_openedx_extensions/helpers/__init__.py b/futurex_openedx_extensions/helpers/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/futurex_openedx_extensions/helpers/apps.py b/futurex_openedx_extensions/helpers/apps.py new file mode 100644 index 00000000..5cfdc3ef --- /dev/null +++ b/futurex_openedx_extensions/helpers/apps.py @@ -0,0 +1,9 @@ +"""helpers Django application initialization""" + +from django.apps import AppConfig + + +class HelpersConfig(AppConfig): + """Configuration for the helpers Django application""" + + name = 'futurex_openedx_extensions.helpers' diff --git a/futurex_openedx_extensions/helpers/converters.py b/futurex_openedx_extensions/helpers/converters.py new file mode 100644 index 00000000..15f9f30b --- /dev/null +++ b/futurex_openedx_extensions/helpers/converters.py @@ -0,0 +1,19 @@ +"""Type conversion helpers""" +from __future__ import annotations + +from typing import Any, List + + +def ids_string_to_list(ids_string: str) -> List[int]: + """Convert a comma-separated string of ids to a list of integers. Duplicate ids are not removed.""" + if not ids_string: + return [] + return [int(id_value.strip()) for id_value in ids_string.split(",") if id_value.strip()] + + +def error_details_to_dictionary(reason: str, **details: Any) -> dict: + """Constructing the dictionary for error details""" + return { + "reason": reason, + "details": details, + } diff --git a/futurex_openedx_extensions/helpers/permissions.py b/futurex_openedx_extensions/helpers/permissions.py new file mode 100644 index 00000000..bbef29e1 --- /dev/null +++ b/futurex_openedx_extensions/helpers/permissions.py @@ -0,0 +1,23 @@ +"""Permission classes for FutureX Open edX Extensions.""" +import json + +from rest_framework.exceptions import NotAuthenticated, PermissionDenied +from rest_framework.permissions import IsAuthenticated + +from futurex_openedx_extensions.helpers.tenants import check_tenant_access + + +class HasTenantAccess(IsAuthenticated): + """Permission class to check if the user is a tenant admin.""" + def has_permission(self, request, view): + """Check if the user is a permission to the tenant""" + if not super().has_permission(request, view): + raise NotAuthenticated() + + tenant_ids = request.GET.get('tenant_ids') + if tenant_ids: + has_access, details = check_tenant_access(request.user, tenant_ids) + if not has_access: + raise PermissionDenied(detail=json.dumps(details)) + + return True diff --git a/futurex_openedx_extensions/helpers/tenants.py b/futurex_openedx_extensions/helpers/tenants.py new file mode 100644 index 00000000..0989d93f --- /dev/null +++ b/futurex_openedx_extensions/helpers/tenants.py @@ -0,0 +1,227 @@ +"""Tenant management helpers""" +from __future__ import annotations + +from typing import Any, Dict, List + +from common.djangoapps.student.models import CourseAccessRole +from django.contrib.auth import get_user_model +from django.db.models import Exists, OuterRef +from django.db.models.query import QuerySet +from eox_tenant.models import Route, TenantConfig + +from futurex_openedx_extensions.helpers.converters import error_details_to_dictionary, ids_string_to_list + +TENANT_LIMITED_ADMIN_ROLES = ['org_course_creator_group'] + + +def get_excluded_tenant_ids() -> List[int]: + """ + Get list of IDs of tenants excluded because they are not exposed in the route table, or have empty configs + + :return: List of tenant IDs to exclude + :rtype: List[int] + """ + def bad_config(tenant): + """Check if the tenant has a bad config""" + return ( + tenant.no_route or + not tenant.lms_configs.get('course_org_filter') or ( + not tenant.lms_configs.get('SITE_NAME') and + not tenant.lms_configs.get('LMS_BASE') + ) + ) + tenants = TenantConfig.objects.annotate( + no_route=~Exists(Route.objects.filter(config_id=OuterRef('pk'))) + ) + return [tenant.id for tenant in tenants if bad_config(tenant)] + + +def get_all_tenants() -> QuerySet: + """ + Get all tenants in the system that are exposed in the route table, and with a valid config + + Note: a tenant is a TenantConfig object + + :return: QuerySet of all tenants + :rtype: QuerySet + """ + return TenantConfig.objects.exclude(id__in=get_excluded_tenant_ids()) + + +def get_all_tenants_info() -> Dict[str, Any]: + """ + TODO: Cache the result of this function + Get all tenants in the system that are exposed in the route table, and with a valid config + + Note: a tenant is a TenantConfig object + + :return: Dictionary of tenant IDs and Sites + :rtype: Dict[str, Any] + """ + tenant_ids = list(get_all_tenants().values_list('id', flat=True)) + info = TenantConfig.objects.filter(id__in=tenant_ids).values('id', 'route__domain') + return { + 'tenant_ids': tenant_ids, + 'sites': { + tenant['id']: tenant['route__domain'] for tenant in info + } + } + + +def get_all_tenant_ids() -> List[int]: + """ + Get list of IDs of all tenants in the system + + :return: List of all tenant IDs + :rtype: List[int] + """ + return get_all_tenants_info()['tenant_ids'] + + +def get_tenant_site(tenant_id: int) -> str: + """ + Get the site for a tenant + + :param tenant_id: The tenant ID + :type tenant_id: int + :return: The site for the tenant + :rtype: str + """ + return get_all_tenants_info()['sites'].get(tenant_id) + + +def get_all_course_org_filter_list() -> Dict[int, List[str]]: + """ + TODO: Cache the result of this function + Get all course org filters for all tenants. + + :return: Dictionary of tenant IDs and their course org filters + :rtype: Dict[int, List[str]] + """ + tenant_configs = get_all_tenants().values_list('id', 'lms_configs') + + result = {} + for t_id, config in tenant_configs: + course_org_filter = config.get('course_org_filter', []) + if isinstance(course_org_filter, str): + course_org_filter = [course_org_filter] + result[t_id] = course_org_filter + + return result + + +def get_course_org_filter_list(tenant_ids: List[int]) -> Dict[str, List | Dict[str, List[int]]]: + """ + Get the filters to use for course orgs. + + returns two information: + { + 'course_org_filter_list': List of course org filters, + 'duplicates': Dictionary of tenant IDs and their duplicates, + 'invalid': List of invalid tenant IDs, + } + + duplicates looks like this: + { + 1: [2, 3], + 2: [1, 3], + 3: [1, 2], + 4: [], + 5: [], + } + + :param tenant_ids: List of tenant IDs to get the filters for + :type tenant_ids: List[int] + :return: Dictionary of course org filters and duplicates + :rtype: Dict[str, List | Dict[str, List[int]]] + """ + tenant_configs = get_all_course_org_filter_list() + + orgs_list = [] + duplicate_trace = {} + duplicates = {} + invalid = [] + for tenant_id in tenant_ids: + course_org_filter = tenant_configs.get(tenant_id, []) + if not course_org_filter: + invalid.append(tenant_id) + continue + + for org in course_org_filter: + if org not in orgs_list: + orgs_list.append(org) + duplicate_trace[org] = [tenant_id] + else: + for other_id in duplicate_trace[org]: + if other_id not in duplicates: + duplicates[other_id] = [tenant_id] + else: + duplicates[other_id].append(tenant_id) + duplicates[tenant_id] = list(duplicate_trace[org]) + duplicate_trace[org].append(tenant_id) + + return { + 'course_org_filter_list': orgs_list, + 'duplicates': duplicates, + 'invalid': invalid, + } + + +def get_accessible_tenant_ids(user: get_user_model()) -> List[int]: + """ + Get the tenants that the user has access to. + + :param user: The user to check. + :type user: get_user_model() + :return: List of accessible tenant IDs + :rtype: List[int] + """ + if not user: + return [] + if user.is_superuser or user.is_staff: + return get_all_tenant_ids() + + 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, + ).values_list('org', flat=True).distinct() + + return [t_id for t_id, course_org_filter in course_org_filter_list.items() if any( + org in course_org_filter for org in accessible_orgs + )] + + +def check_tenant_access(user: get_user_model(), tenant_ids_string: str) -> tuple[bool, dict]: + """ + Check if the user has access to the provided tenant IDs + + :param user: The user to check. + :type user: get_user_model() + :param tenant_ids_string: Comma-separated string of tenant IDs + :type tenant_ids_string: str + :return: Tuple of a boolean indicating if the user has access, and a dictionary of error details if any + """ + try: + tenant_ids = set(ids_string_to_list(tenant_ids_string)) + except ValueError as e: + return False, error_details_to_dictionary( + reason="Invalid tenant IDs provided. It must be a comma-separated list of integers", + error=str(e) + ) + + wrong_tenant_ids = tenant_ids - set(get_all_tenant_ids()) + if wrong_tenant_ids: + return False, error_details_to_dictionary( + reason="Invalid tenant IDs provided", + tenant_ids=list(wrong_tenant_ids) + ) + + inaccessible_tenants = tenant_ids - set(get_accessible_tenant_ids(user)) + if inaccessible_tenants: + return False, error_details_to_dictionary( + reason="User does not have access to these tenants", + tenant_ids=list(inaccessible_tenants), + ) + + return True, {} diff --git a/requirements/base.txt b/requirements/base.txt index 08c7935e..7c425700 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -13,7 +13,7 @@ django==4.2.11 # django-model-utils django-model-utils==4.4.0 # via -r requirements/base.in -eox-tenant==11.0.1 +eox-tenant==11.0.2 # via -r requirements/base.in sqlparse==0.4.4 # via django diff --git a/requirements/ci.txt b/requirements/ci.txt index baae04fa..cafc7d05 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -16,7 +16,7 @@ filelock==3.13.1 # via # tox # virtualenv -packaging==23.2 +packaging==24.0 # via # pyproject-api # tox diff --git a/requirements/dev.txt b/requirements/dev.txt index 4fdef7f2..bf415792 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -38,7 +38,7 @@ click-log==0.4.0 # via # -r requirements/quality.txt # edx-lint -code-annotations==1.6.0 +code-annotations==1.7.0 # via # -r requirements/quality.txt # edx-lint @@ -46,7 +46,7 @@ colorama==0.4.6 # via # -r requirements/ci.txt # tox -coverage[toml]==7.4.3 +coverage[toml]==7.4.4 # via # -r requirements/quality.txt # pytest-cov @@ -64,15 +64,28 @@ django==4.2.11 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/quality.txt + # django-crum # django-model-utils + # django-mysql + # djangorestframework # edx-i18n-tools + # jsonfield + # openedx-filters +django-crum==0.7.9 + # via -r requirements/quality.txt django-model-utils==4.4.0 # via -r requirements/quality.txt +django-mysql==4.12.0 + # via -r requirements/quality.txt +djangorestframework==3.15.0 + # via -r requirements/quality.txt edx-i18n-tools==1.3.0 # via -r requirements/dev.in edx-lint==5.3.6 # via -r requirements/quality.txt -eox-tenant==11.0.1 +edx-opaque-keys[django]==2.5.1 + # via -r requirements/quality.txt +eox-tenant==11.0.2 # via -r requirements/quality.txt exceptiongroup==1.2.0 # via @@ -96,6 +109,8 @@ jinja2==3.1.3 # -r requirements/quality.txt # code-annotations # diff-cover +jsonfield==3.1.0 + # via -r requirements/quality.txt lxml==5.1.0 # via edx-i18n-tools markupsafe==2.1.5 @@ -106,7 +121,9 @@ mccabe==0.7.0 # via # -r requirements/quality.txt # pylint -packaging==23.2 +openedx-filters==1.6.0 + # via -r requirements/quality.txt +packaging==24.0 # via # -r requirements/ci.txt # -r requirements/pip-tools.txt @@ -165,6 +182,10 @@ pylint-plugin-utils==0.8.2 # -r requirements/quality.txt # pylint-celery # pylint-django +pymongo==3.13.0 + # via + # -r requirements/quality.txt + # edx-opaque-keys pyproject-api==1.6.1 # via # -r requirements/ci.txt @@ -174,7 +195,7 @@ pyproject-hooks==1.0.0 # -r requirements/pip-tools.txt # build # pip-tools -pytest==8.0.2 +pytest==8.1.1 # via # -r requirements/quality.txt # pytest-cov @@ -208,6 +229,7 @@ stevedore==5.2.0 # via # -r requirements/quality.txt # code-annotations + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/quality.txt @@ -236,11 +258,12 @@ typing-extensions==4.10.0 # -r requirements/quality.txt # asgiref # astroid + # edx-opaque-keys virtualenv==20.25.1 # via # -r requirements/ci.txt # tox -wheel==0.42.0 +wheel==0.43.0 # via # -r requirements/pip-tools.txt # pip-tools diff --git a/requirements/doc.txt b/requirements/doc.txt index 90c53264..c3cfd5b4 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -30,9 +30,9 @@ click==8.1.7 # via # -r requirements/test.txt # code-annotations -code-annotations==1.6.0 +code-annotations==1.7.0 # via -r requirements/test.txt -coverage[toml]==7.4.3 +coverage[toml]==7.4.4 # via # -r requirements/test.txt # pytest-cov @@ -42,9 +42,20 @@ django==4.2.11 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-crum # django-model-utils + # django-mysql + # djangorestframework + # jsonfield + # openedx-filters +django-crum==0.7.9 + # via -r requirements/test.txt django-model-utils==4.4.0 # via -r requirements/test.txt +django-mysql==4.12.0 + # via -r requirements/test.txt +djangorestframework==3.15.0 + # via -r requirements/test.txt doc8==1.1.1 # via -r requirements/doc.in docutils==0.20.1 @@ -54,7 +65,9 @@ docutils==0.20.1 # readme-renderer # restructuredtext-lint # sphinx -eox-tenant==11.0.1 +edx-opaque-keys[django]==2.5.1 + # via -r requirements/test.txt +eox-tenant==11.0.2 # via -r requirements/test.txt exceptiongroup==1.2.0 # via @@ -64,8 +77,9 @@ idna==3.6 # via requests imagesize==1.4.1 # via sphinx -importlib-metadata==7.0.2 +importlib-metadata==6.11.0 # via + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # keyring # twine iniconfig==2.0.0 @@ -83,6 +97,8 @@ jinja2==3.1.3 # -r requirements/test.txt # code-annotations # sphinx +jsonfield==3.1.0 + # via -r requirements/test.txt keyring==24.3.1 # via twine markdown-it-py==3.0.0 @@ -97,7 +113,9 @@ more-itertools==10.2.0 # via jaraco-classes nh3==0.2.15 # via readme-renderer -packaging==23.2 +openedx-filters==1.6.0 + # via -r requirements/test.txt +packaging==24.0 # via # -r requirements/test.txt # build @@ -126,9 +144,13 @@ pygments==2.17.2 # readme-renderer # rich # sphinx +pymongo==3.13.0 + # via + # -r requirements/test.txt + # edx-opaque-keys pyproject-hooks==1.0.0 # via build -pytest==8.0.2 +pytest==8.1.1 # via # -r requirements/test.txt # pytest-cov @@ -162,6 +184,8 @@ rich==13.7.1 # via twine secretstorage==3.3.3 # via keyring +six==1.16.0 + # via -r requirements/test.txt snowballstemmer==2.2.0 # via sphinx soupsieve==2.5 @@ -194,6 +218,7 @@ stevedore==5.2.0 # -r requirements/test.txt # code-annotations # doc8 + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/test.txt @@ -212,10 +237,11 @@ typing-extensions==4.10.0 # via # -r requirements/test.txt # asgiref + # edx-opaque-keys # pydata-sphinx-theme urllib3==2.2.1 # via # requests # twine -zipp==3.17.0 +zipp==3.18.1 # via importlib-metadata diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index 67f898be..080b3921 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -8,7 +8,7 @@ build==1.1.1 # via pip-tools click==8.1.7 # via pip-tools -packaging==23.2 +packaging==24.0 # via build pip-tools==7.4.1 # via -r requirements/pip-tools.in @@ -21,7 +21,7 @@ tomli==2.0.1 # build # pip-tools # pyproject-hooks -wheel==0.42.0 +wheel==0.43.0 # via pip-tools # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/pip.txt b/requirements/pip.txt index 2c7f181e..9c6ea0da 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -4,11 +4,11 @@ # # make upgrade # -wheel==0.42.0 +wheel==0.43.0 # via -r requirements/pip.in # The following packages are considered to be unsafe in a requirements file: pip==24.0 # via -r requirements/pip.in -setuptools==69.1.1 +setuptools==69.2.0 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index 12b1f6e2..1744ec19 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -20,11 +20,11 @@ click==8.1.7 # edx-lint click-log==0.4.0 # via edx-lint -code-annotations==1.6.0 +code-annotations==1.7.0 # via # -r requirements/test.txt # edx-lint -coverage[toml]==7.4.3 +coverage[toml]==7.4.4 # via # -r requirements/test.txt # pytest-cov @@ -34,12 +34,25 @@ django==4.2.11 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-crum # django-model-utils + # django-mysql + # djangorestframework + # jsonfield + # openedx-filters +django-crum==0.7.9 + # via -r requirements/test.txt django-model-utils==4.4.0 # via -r requirements/test.txt +django-mysql==4.12.0 + # via -r requirements/test.txt +djangorestframework==3.15.0 + # via -r requirements/test.txt edx-lint==5.3.6 # via -r requirements/quality.in -eox-tenant==11.0.1 +edx-opaque-keys[django]==2.5.1 + # via -r requirements/test.txt +eox-tenant==11.0.2 # via -r requirements/test.txt exceptiongroup==1.2.0 # via @@ -57,13 +70,17 @@ jinja2==3.1.3 # via # -r requirements/test.txt # code-annotations +jsonfield==3.1.0 + # via -r requirements/test.txt markupsafe==2.1.5 # via # -r requirements/test.txt # jinja2 mccabe==0.7.0 # via pylint -packaging==23.2 +openedx-filters==1.6.0 + # via -r requirements/test.txt +packaging==24.0 # via # -r requirements/test.txt # pytest @@ -95,7 +112,11 @@ pylint-plugin-utils==0.8.2 # via # pylint-celery # pylint-django -pytest==8.0.2 +pymongo==3.13.0 + # via + # -r requirements/test.txt + # edx-opaque-keys +pytest==8.1.1 # via # -r requirements/test.txt # pytest-cov @@ -113,7 +134,9 @@ pyyaml==6.0.1 # -r requirements/test.txt # code-annotations six==1.16.0 - # via edx-lint + # via + # -r requirements/test.txt + # edx-lint snowballstemmer==2.2.0 # via pydocstyle sqlparse==0.4.4 @@ -124,6 +147,7 @@ stevedore==5.2.0 # via # -r requirements/test.txt # code-annotations + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/test.txt @@ -141,3 +165,4 @@ typing-extensions==4.10.0 # -r requirements/test.txt # asgiref # astroid + # edx-opaque-keys diff --git a/requirements/test.in b/requirements/test.in index 6797160b..33048682 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -6,3 +6,12 @@ pytest-cov # pytest extension for code coverage statistics pytest-django # pytest extension for better Django support code-annotations # provides commands used by the pii_check make target. + +# eox-tenant requirements +six +djangorestframework +django-crum +django-mysql +jsonfield +edx-opaque-keys[django] +openedx_filters diff --git a/requirements/test.txt b/requirements/test.txt index a1bd7ff2..c3767af2 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -10,17 +10,30 @@ asgiref==3.7.2 # django click==8.1.7 # via code-annotations -code-annotations==1.6.0 +code-annotations==1.7.0 # via -r requirements/test.in -coverage[toml]==7.4.3 +coverage[toml]==7.4.4 # via pytest-cov # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt + # django-crum # django-model-utils + # django-mysql + # djangorestframework + # jsonfield + # openedx-filters +django-crum==0.7.9 + # via -r requirements/test.in django-model-utils==4.4.0 # via -r requirements/base.txt -eox-tenant==11.0.1 +django-mysql==4.12.0 + # via -r requirements/test.in +djangorestframework==3.15.0 + # via -r requirements/test.in +edx-opaque-keys[django]==2.5.1 + # via -r requirements/test.in +eox-tenant==11.0.2 # via -r requirements/base.txt exceptiongroup==1.2.0 # via pytest @@ -28,15 +41,21 @@ iniconfig==2.0.0 # via pytest jinja2==3.1.3 # via code-annotations +jsonfield==3.1.0 + # via -r requirements/test.in markupsafe==2.1.5 # via jinja2 -packaging==23.2 +openedx-filters==1.6.0 + # via -r requirements/test.in +packaging==24.0 # via pytest pbr==6.0.0 # via stevedore pluggy==1.4.0 # via pytest -pytest==8.0.2 +pymongo==3.13.0 + # via edx-opaque-keys +pytest==8.1.1 # via # pytest-cov # pytest-django @@ -48,12 +67,16 @@ python-slugify==8.0.4 # via code-annotations pyyaml==6.0.1 # via code-annotations +six==1.16.0 + # via -r requirements/test.in sqlparse==0.4.4 # via # -r requirements/base.txt # django stevedore==5.2.0 - # via code-annotations + # via + # code-annotations + # edx-opaque-keys text-unidecode==1.3 # via python-slugify tomli==2.0.1 @@ -64,3 +87,4 @@ typing-extensions==4.10.0 # via # -r requirements/base.txt # asgiref + # edx-opaque-keys diff --git a/setup.py b/setup.py index 25bb3c47..1404f25f 100755 --- a/setup.py +++ b/setup.py @@ -138,6 +138,7 @@ def is_requirement(line): include=[ 'futurex_openedx_extensions', 'futurex_openedx_extensions.*', 'futurex_openedx_extensions.dashboard', 'futurex_openedx_extensions.dashboard.*', + 'futurex_openedx_extensions.helpers', 'futurex_openedx_extensions.helpers.*', ], exclude=["*tests"], ), @@ -159,7 +160,8 @@ def is_requirement(line): ], entry_points={ 'lms.djangoapp': [ - 'dashboard = futurex_openedx_extensions.dashboard.apps:DashboardConfig', + 'fx_dashboard = futurex_openedx_extensions.dashboard.apps:DashboardConfig', + 'fx_helpers = futurex_openedx_extensions.helpers.apps:HelpersConfig', ], }, ) diff --git a/test_settings.py b/test_settings.py index 6949b540..baa8800c 100644 --- a/test_settings.py +++ b/test_settings.py @@ -7,6 +7,8 @@ from os.path import abspath, dirname, join +from test_utils.eox_settings import * + def root(*args): """ @@ -32,7 +34,10 @@ def root(*args): 'django.contrib.contenttypes', 'django.contrib.messages', 'django.contrib.sessions', - 'futurex_openedx_extensions', + 'eox_tenant', + 'common', + 'fake_models', + 'openedx', ) LOCALE_PATHS = [ @@ -41,10 +46,12 @@ def root(*args): SECRET_KEY = 'insecure-secret-key' +ROOT_URLCONF = 'tests.test_urls' + MIDDLEWARE = ( + 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', ) TEMPLATES = [{ @@ -53,7 +60,11 @@ def root(*args): 'OPTIONS': { 'context_processors': [ 'django.contrib.auth.context_processors.auth', # this is required for admin + 'django.template.context_processors.request', 'django.contrib.messages.context_processors.messages', # this is required for admin ], }, }] + +# Avoid warnings about migrations +DEFAULT_AUTO_FIELD = 'django.db.models.AutoField' diff --git a/test_utils/__init__.py b/test_utils/__init__.py deleted file mode 100644 index 7961e470..00000000 --- a/test_utils/__init__.py +++ /dev/null @@ -1,10 +0,0 @@ -""" -Test utilities. - -Since pytest discourages putting __init__.py into testdirectory -(i.e. making tests a package) one cannot import from anywhere -under tests folder. However, some utility classes/methods might be useful -in multiple test modules (i.e. factoryboy factories, base test classes). - -So this package is the place to put them. -""" diff --git a/test_utils/edx_platform_mocks/__init__.py b/test_utils/edx_platform_mocks/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test_utils/edx_platform_mocks/common/djangoapps/student/models.py b/test_utils/edx_platform_mocks/common/djangoapps/student/models.py new file mode 100644 index 00000000..d3e87219 --- /dev/null +++ b/test_utils/edx_platform_mocks/common/djangoapps/student/models.py @@ -0,0 +1,2 @@ +"""edx-platform Mocks""" +from fake_models.models import CourseAccessRole, CourseEnrollment, UserSignupSource # pylint: disable=unused-import diff --git a/test_utils/edx_platform_mocks/fake_models/__init__.py b/test_utils/edx_platform_mocks/fake_models/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/test_utils/edx_platform_mocks/fake_models/models.py b/test_utils/edx_platform_mocks/fake_models/models.py new file mode 100644 index 00000000..e7ae25b0 --- /dev/null +++ b/test_utils/edx_platform_mocks/fake_models/models.py @@ -0,0 +1,61 @@ +"""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 + + +class CourseOverview(models.Model): + """Mock""" + id = models.CharField(max_length=255, primary_key=True) + org = models.CharField(max_length=255) + visible_to_staff_only = models.BooleanField() + start = models.DateTimeField(null=True) + end = models.DateTimeField(null=True) + + class Meta: + app_label = "fake_models" + db_table = "course_overviews_courseoverview" + + +class CourseAccessRole(models.Model): + """Mock""" + user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE) + role = models.CharField(max_length=255) + org = models.CharField(blank=True, max_length=255) + + class Meta: + app_label = "fake_models" + db_table = "student_courseaccessrole" + + +class CourseEnrollment(models.Model): + """Mock""" + user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE) + course = models.ForeignKey(CourseOverview, on_delete=models.CASCADE) + is_active = models.BooleanField() + + class Meta: + app_label = "fake_models" + db_table = "student_courseenrollment" + + +class UserSignupSource(models.Model): + """Mock""" + site = models.CharField(max_length=255) + user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE) + + class Meta: + app_label = "fake_models" + db_table = "student_usersignupsource" + + +class GeneratedCertificate(models.Model): + """Mock""" + user = models.ForeignKey(get_user_model(), on_delete=models.CASCADE) + course_id = CourseKeyField(max_length=255, blank=True, default=None) + status = models.CharField(max_length=32, default='unavailable') + + class Meta: + unique_together = (('user', 'course_id'),) + app_label = "fake_models" + db_table = "certificates_generatedcertificate" diff --git a/test_utils/edx_platform_mocks/lms/djangoapps/certificates/models.py b/test_utils/edx_platform_mocks/lms/djangoapps/certificates/models.py new file mode 100644 index 00000000..fde33e74 --- /dev/null +++ b/test_utils/edx_platform_mocks/lms/djangoapps/certificates/models.py @@ -0,0 +1,2 @@ +"""edx-platform Mocks""" +from fake_models.models import GeneratedCertificate # pylint: disable=unused-import diff --git a/test_utils/edx_platform_mocks/openedx/core/djangoapps/content/course_overviews/models.py b/test_utils/edx_platform_mocks/openedx/core/djangoapps/content/course_overviews/models.py new file mode 100644 index 00000000..6a091eb8 --- /dev/null +++ b/test_utils/edx_platform_mocks/openedx/core/djangoapps/content/course_overviews/models.py @@ -0,0 +1,2 @@ +"""edx-platform Mocks""" +from fake_models.models import CourseOverview # pylint: disable=unused-import diff --git a/test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/helpers.py b/test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/helpers.py new file mode 100644 index 00000000..26e073a4 --- /dev/null +++ b/test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/helpers.py @@ -0,0 +1 @@ +"""Needed to mock eox_tenant imports""" diff --git a/test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/models.py b/test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/models.py new file mode 100644 index 00000000..26e073a4 --- /dev/null +++ b/test_utils/edx_platform_mocks/openedx/core/djangoapps/site_configuration/models.py @@ -0,0 +1 @@ +"""Needed to mock eox_tenant imports""" diff --git a/test_utils/edx_platform_mocks/openedx/core/djangoapps/theming/helpers.py b/test_utils/edx_platform_mocks/openedx/core/djangoapps/theming/helpers.py new file mode 100644 index 00000000..26e073a4 --- /dev/null +++ b/test_utils/edx_platform_mocks/openedx/core/djangoapps/theming/helpers.py @@ -0,0 +1 @@ +"""Needed to mock eox_tenant imports""" diff --git a/test_utils/edx_platform_mocks/setup.py b/test_utils/edx_platform_mocks/setup.py new file mode 100644 index 00000000..2353e348 --- /dev/null +++ b/test_utils/edx_platform_mocks/setup.py @@ -0,0 +1,8 @@ +"""Setup for edx-platform mocks.""" +from setuptools import setup + +setup( + name='edx_platform_mocks', + version='0.1.0', + packages=['common', 'fake_models', 'lms', 'openedx'], +) diff --git a/test_utils/eox_settings.py b/test_utils/eox_settings.py new file mode 100644 index 00000000..99a52e76 --- /dev/null +++ b/test_utils/eox_settings.py @@ -0,0 +1,5 @@ +"""eox_tenant test settings.""" +GET_SITE_CONFIGURATION_MODULE = 'eox_tenant.edxapp_wrapper.backends.site_configuration_module_i_v1' +GET_BRANDING_API = 'eox_tenant.edxapp_wrapper.backends.branding_api_l_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/base_test_data.py b/tests/base_test_data.py new file mode 100644 index 00000000..41de6b02 --- /dev/null +++ b/tests/base_test_data.py @@ -0,0 +1,189 @@ +"""Base test data for the tests. dictionary instead of json file.""" + +_base_data = { + "tenant_config": { + 1: { # Organisations are duplicated with tenant 4 and 6 + "lms_configs": { + "LMS_BASE": "s1.sample.com", + "SITE_NAME": "s1.sample.com", + "course_org_filter": ["ORG1", "ORG2"], + }, + }, + 2: { # Organisation is duplicated with tenant 7 + "lms_configs": { + "LMS_BASE": "s2.sample.com", + "course_org_filter": ["ORG3", "ORG8"], + }, + }, + 3: { + "lms_configs": { + "SITE_NAME": "s3.sample.com", + "course_org_filter": ["ORG4", "ORG5"], + }, + }, + 4: { # This is a tenant with no SITE_NAME nor LMS_BASE + "lms_configs": { + "LMS_BASE": None, + "SITE_NAME": None, + "course_org_filter": ["ORG1", "ORG2"], + }, + }, + 5: { # This is a tenant with no course_org_filter + "lms_configs": { + "LMS_BASE": "s5.sample.com", + "SITE_NAME": "s5.sample.com", + "course_org_filter": [], + }, + }, + 6: { # This is a tenant with no route + "lms_configs": { + "LMS_BASE": "s6.sample.com", + "course_org_filter": ["ORG2"], + }, + }, + 7: { + "lms_configs": { + "LMS_BASE": "s7.sample.com", + "course_org_filter": "ORG3", # This a string, not a list, it should be fine too + }, + }, + 8: { + "lms_configs": { + "LMS_BASE": "s8.sample.com", + "SITE_NAME": "s8.sample.com", + "course_org_filter": ["ORG8"], + }, + }, + }, + "routes": { + 1: { + "domain": "s1.sample.com", + "config_id": 1, + }, + 2: { + "domain": "s2.sample.com", + "config_id": 2, + }, + 3: { + "domain": "s3.sample.com", + "config_id": 3, + }, + 4: { + "domain": "s4.sample.com", + "config_id": 4, + }, + 5: { + "domain": "s5.sample.com", + "config_id": 5, + }, + 7: { + "domain": "s7.sample.com", + "config_id": 7, + }, + 8: { + "domain": "s8.sample.com", + "config_id": 8, + }, + }, + "users_count": 55, + "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], + "s3.sample.com": [15, 16, 17, 18, 19, 20], + "s4.sample.com": [21, 22, 23, 24, 25, 26, 27], + "s5.sample.com": [21, 22, 23, 24, 25, 26, 27], + "s6.sample.com": [28, 29, 30, 31, 32, 33, 34, 35, 36, 37], + "s7.sample.com": [15, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50], + "s8.sample.com": [47, 48, 49, 50, 51, 52, 53, 54, 55], + "dah.sample.com": [1, 2, 3, 4, 5], # This is not a valid domain + }, + "course_overviews": { # count of courses per org + "ORG1": 5, + "ORG2": 7, + "ORG3": 3, + "ORG4": 1, + "ORG5": 0, + "ORG6": 1, # This is an org with no tenant + "ORG8": 2, + }, + "course_enrollments": { # org id, course id, user ids + "ORG1": { + 1: [4, 5], + 2: [3], + 3: [], + 4: [4], + 5: [1, 2, 3, 4, 15, 21, 40], + }, + "ORG2": { + 1: [2], + 3: [1, 2, 3], + 4: [4, 5, 21, 22, 23, 24, 25], + 5: [21, 22, 23, 24, 25], + 6: [28, 29, 30, 31, 32], + 7: [15, 38, 39, 40, 41], + }, + "ORG3": { + 1: [10, 40, 41, 42, 43, 44], + 2: [45, 46, 47, 48, 49], + 3: [7, 8, 9, 10, 11, 12], + }, + "ORG4": { + 1: [1, 2, 15, 16, 17, 18], + }, + "ORG6": { + 1: [30, 31, 32, 41, 42], + }, + "ORG8": { + 1: [47, 48, 49, 52], + 2: [23, 52, 53, 54], + }, + }, + "super_users": [1], + "staff_users": [2], + "course_access_roles": { # roles, user ids per org + "staff": { # at least one role that is not in TENANT_LIMITED_ADMIN_ROLES + "ORG1": [3], + "ORG2": [8, 9], + "ORG3": [9, 18], + "ORG4": [10, 23], + "ORG5": [23], + }, + "org_course_creator_group": { + "ORG1": [4], + "ORG2": [1, 2, 4, 9], + "ORG3": [4, 10, 11], + "ORG4": [23, 48], + "ORG5": [23], + "ORG8": [23], + }, + }, + "ignored_course_access_roles": { + "no_org": { + "staff": [10, 30, 40, 41], + "org_course_creator_group": [24, 40, 42], + }, + }, + "certificates": { # org id, course id, user ids + "ORG1": { + 5: [2, 3, 4, 40], + }, + "ORG2": { + 4: [4, 5, 24, 25], + 5: [21, 24, 25], + 7: [15, 40, 41], + }, + "ORG3": { + 1: [42, 43, 44], + 2: [48, 49], + 3: [8, 9], + }, + "ORG4": { + }, + "ORG6": { + 1: [30, 42], + }, + "ORG8": { + 1: [49, 52], + }, + }, +} diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..c53719a5 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,124 @@ +"""PyTest fixtures for tests.""" +import pytest +from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment, UserSignupSource +from django.contrib.auth import get_user_model +from eox_tenant.models import Route, TenantConfig +from lms.djangoapps.certificates.models import GeneratedCertificate +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + +from tests.base_test_data import _base_data + + +@pytest.fixture(scope="session") +def base_data(django_db_setup, django_db_blocker): # pylint: disable=unused-argument + """Create base data for tests.""" + def _create_users(): + """Create users.""" + user = get_user_model() + for i in range(1, _base_data["users_count"] + 1): + user.objects.create( + id=i, + username=f"user{i}", + email=f"user{i}@example.com", + ) + for user_id in _base_data["super_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) + + def _create_tenants(): + """Create tenants.""" + for tenant_id, tenant_config in _base_data["tenant_config"].items(): + TenantConfig.objects.create( + id=tenant_id, + lms_configs=tenant_config["lms_configs"], + ) + + def _create_routes(): + """Create routes.""" + for route_id in _base_data["routes"]: + Route.objects.create( + id=route_id, + domain=_base_data["routes"][route_id]["domain"], + config_id=_base_data["routes"][route_id]["config_id"], + ) + + def _create_user_signup_sources(): + """Create user signup sources.""" + for site, users in _base_data["user_signup_source__users"].items(): + for user_id in users: + UserSignupSource.objects.create( + site=site, + user_id=user_id, + ) + + def _create_course_access_roles(): + """Create course access roles.""" + for role, orgs in _base_data["course_access_roles"].items(): + for org, users in orgs.items(): + for user_id in users: + CourseAccessRole.objects.create( + user_id=user_id, + role=role, + org=org, + ) + + def _create_ignored_course_access_roles(): + """Create course access roles for records that will be ignored by our APIs.""" + for reason, orgs in _base_data["ignored_course_access_roles"].items(): + for role, users in orgs.items(): + for user_id in users: + assert reason in ['no_org'], f"Unknown reason for (ignored_course_access_roles) test data: {reason}" + params = { + 'user_id': user_id, + 'role': role, + 'org': '', + } + CourseAccessRole.objects.create(**params) + + def _create_course_overviews(): + """Create course overviews.""" + for org, count in _base_data["course_overviews"].items(): + for i in range(1, count + 1): + CourseOverview.objects.create( + id=f"course-v1:{org}+{i}+{i}", + org=org, + visible_to_staff_only=False, + ) + + def _create_course_enrollments(): + """Create course enrollments.""" + for org, enrollments in _base_data["course_enrollments"].items(): + for course_id, users in enrollments.items(): + for user_id in users: + assert 0 < course_id <= _base_data["course_overviews"][org], \ + f"Bad course_id in enrollment testing data for org: {org}, course: {course_id}" + assert 0 < user_id <= _base_data["users_count"], \ + f"Bad user_id in enrollment testing data for org: {org}, user: {user_id}, course: {course_id}" + CourseEnrollment.objects.create( + user_id=user_id, + course_id=f"course-v1:{org}+{course_id}+{course_id}", + is_active=True, + ) + + def _create_certificates(): + """Create certificates.""" + for org, courses in _base_data["certificates"].items(): + for course_id, user_ids in courses.items(): + for user_id in user_ids: + GeneratedCertificate.objects.create( + user_id=user_id, + course_id=f"course-v1:{org}+{course_id}+{course_id}", + status='downloadable', + ) + + with django_db_blocker.unblock(): + _create_users() + _create_tenants() + _create_routes() + _create_user_signup_sources() + _create_course_access_roles() + _create_ignored_course_access_roles() + _create_course_overviews() + _create_course_enrollments() + _create_certificates() diff --git a/tests/test_base_test_data.py b/tests/test_base_test_data.py new file mode 100644 index 00000000..23a9945b --- /dev/null +++ b/tests/test_base_test_data.py @@ -0,0 +1,20 @@ +"""Test integrity of base test data.""" +from futurex_openedx_extensions.helpers.tenants import TENANT_LIMITED_ADMIN_ROLES +from tests.base_test_data import _base_data + + +def test_at_least_one_no_admin_role(): + """Verify that at least one tenant has no admin role.""" + assert len([role for role in _base_data['course_access_roles'] if role in TENANT_LIMITED_ADMIN_ROLES]) > 0, ( + 'At least one tenant should have an admin role' + ) + + +def test_certificate_must_be_enrolled(): + """Verify that certificates are issued only to enrolled users.""" + for org, courses in _base_data['certificates'].items(): + for course_id, user_ids in courses.items(): + for user_id in user_ids: + assert user_id in _base_data['course_enrollments'].get(org, {}).get(course_id, []), ( + f'User {user_id} is not enrolled in course {course_id} of org {org}' + ) diff --git a/tests/test_dashboard/__init__.py b/tests/test_dashboard/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_dashboard/test_apps.py b/tests/test_dashboard/test_apps.py new file mode 100644 index 00000000..cd1ab5e4 --- /dev/null +++ b/tests/test_dashboard/test_apps.py @@ -0,0 +1,36 @@ +"""Tests for the apps module of the helpers app""" +from futurex_openedx_extensions.dashboard.apps import DashboardConfig +from futurex_openedx_extensions.dashboard.settings import common_production + + +def test_app_name(): + """Test that the app name is correct""" + assert DashboardConfig.name == 'futurex_openedx_extensions.dashboard' + + +def test_app_config(): + """Verify that the app is compatible with edx-platform plugins""" + assert DashboardConfig.plugin_app == { + 'settings_config': { + 'lms.djangoapp': { + 'production': { + 'relative_path': 'settings.common_production', + }, + }, + 'cms.djangoapp': { + 'production': { + 'relative_path': 'settings.common_production', + }, + }, + }, + 'url_config': { + 'lms.djangoapp': { + 'namespace': 'fx_dashboard', + }, + }, + }, 'The app is not compatible with edx-platform plugins!' + + +def test_common_production_plugin_settings(): + """Verify that used settings contain the method plugin_settings""" + assert hasattr(common_production, 'plugin_settings'), 'settings is missing the method plugin_settings!' diff --git a/tests/test_dashboard/test_statistics/test_certificates.py b/tests/test_dashboard/test_statistics/test_certificates.py new file mode 100644 index 00000000..67a68b41 --- /dev/null +++ b/tests/test_dashboard/test_statistics/test_certificates.py @@ -0,0 +1,41 @@ +"""Tests for certificates statistics.""" +import pytest +from lms.djangoapps.certificates.models import GeneratedCertificate + +from futurex_openedx_extensions.dashboard.statistics import certificates + + +@pytest.mark.django_db +@pytest.mark.parametrize('tenant_ids, expected_result', [ + ([1], {'ORG1': 4, 'ORG2': 10}), + ([2], {'ORG3': 7, 'ORG8': 2}), + ([3], {}), + ([4], {}), + ([5], {}), + ([6], {}), + ([7], {'ORG3': 7}), + ([8], {'ORG8': 2}), + ([1, 2], {'ORG1': 4, 'ORG2': 10, 'ORG3': 7, 'ORG8': 2}), + ([2, 7], {'ORG3': 7, 'ORG8': 2}), + ([7, 8], {'ORG3': 7, 'ORG8': 2}), +]) +def test_get_certificates_count(base_data, tenant_ids, expected_result): + """Verify get_certificates_count function.""" + result = certificates.get_certificates_count(tenant_ids) + assert result == expected_result, \ + f'Wrong certificates result for tenant(s) {tenant_ids}. expected: {expected_result}, got: {result}' + + +@pytest.mark.django_db +def test_get_certificates_count_not_downloadable(base_data): + """Verify get_certificates_count function with empty tenant_ids.""" + result = certificates.get_certificates_count([1]) + assert result == {'ORG1': 4, 'ORG2': 10}, f'Wrong certificates result. expected: {result}' + + some_status_not_downloadable = 'whatever' + GeneratedCertificate.objects.filter( + course_id='course-v1:ORG1+5+5', + user_id=40, + ).update(status=some_status_not_downloadable) + result = certificates.get_certificates_count([1]) + assert result == {'ORG1': 3, 'ORG2': 10}, f'Wrong certificates result. expected: {result}' diff --git a/tests/test_dashboard/test_statistics/test_courses.py b/tests/test_dashboard/test_statistics/test_courses.py new file mode 100644 index 00000000..30aa9def --- /dev/null +++ b/tests/test_dashboard/test_statistics/test_courses.py @@ -0,0 +1,74 @@ +"""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.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): + """Verify get_courses_count function.""" + all_tenants = _base_data["tenant_config"].keys() + result = courses.get_courses_count(all_tenants) + orgs_in_result = [org["org"] for org in result] + + for tenant_id in all_tenants: + course_org_filter_list = get_course_org_filter_list([tenant_id])["course_org_filter_list"] + for org in course_org_filter_list: + expected_count = _base_data["course_overviews"].get(org, 0) + assert ( + expected_count != 0 and { + "org": org, "courses_count": _base_data["course_overviews"].get(org, 0) + } in result or + expected_count == 0 and org not in orgs_in_result + ), f'Missing org: {org} in tenant: {tenant_id} results' + + +@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): + """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}, + ] + + result = courses.get_courses_count([1], only_active=True) + assert expected_result == list(result), f'Wrong result: {result}' + + +@pytest.mark.django_db +def test_get_courses_count_only_visible(base_data): + """Verify get_courses_count function with only_visible=True.""" + course = CourseOverview.objects.filter(org="ORG1").first() + assert course.visible_to_staff_only is False + course.visible_to_staff_only = True + course.save() + expected_result = [ + {'org': 'ORG1', 'courses_count': 4}, + {'org': 'ORG2', 'courses_count': 7}, + ] + + result = courses.get_courses_count([1], only_visible=True) + assert expected_result == list(result), f'Wrong result: {result}' diff --git a/tests/test_dashboard/test_statistics/test_learners.py b/tests/test_dashboard/test_statistics/test_learners.py new file mode 100644 index 00000000..3c1a6474 --- /dev/null +++ b/tests/test_dashboard/test_statistics/test_learners.py @@ -0,0 +1,82 @@ +"""Tests for learners statistics.""" +import pytest + +from futurex_openedx_extensions.dashboard.statistics import learners + + +@pytest.mark.django_db +@pytest.mark.parametrize('tenant_id, expected_result', [ + (1, {'ORG1': 4, 'ORG2': 17}), + (2, {'ORG3': 13, 'ORG8': 6}), + (3, {'ORG4': 4}), + (4, {}), + (5, {}), + (6, {}), + (7, {'ORG3': 13}), + (8, {'ORG8': 6}), +]) +def test_get_learners_count_having_enrollment_per_org(base_data, tenant_id, expected_result): + """Test get_learners_count_having_enrollment_per_org function.""" + result = learners.get_learners_count_having_enrollment_per_org(tenant_id) + assert result.count() == len(expected_result), 'Wrong number of organizations returned' + + for result_tenant_id in result: + assert result_tenant_id['org'] in expected_result, f'Unexpected org: {result_tenant_id["org"]}' + assert result_tenant_id['learners_count'] == expected_result[result_tenant_id['org']], \ + f'Wrong learners count: {result_tenant_id["learners_count"]}, org: {result_tenant_id["org"]}' + + +@pytest.mark.django_db +@pytest.mark.parametrize('tenant_id, expected_result', [ + (1, 17), + (2, 16), + (3, 4), + (4, 0), + (5, 0), + (6, 0), + (7, 13), + (8, 6), +]) +def test_get_learners_count_having_enrollment_for_tenant(base_data, tenant_id, expected_result): + """Test get_learners_count_having_enrollment_for_tenant function.""" + result = learners.get_learners_count_having_enrollment_for_tenant(tenant_id) + assert result == expected_result, f'Wrong learners count: {result} for tenant: {tenant_id}' + + +@pytest.mark.django_db +@pytest.mark.parametrize('tenant_id, expected_result', [ + (1, 0), + (2, 5), + (3, 2), + (4, 0), + (5, 0), + (6, 0), + (7, 4), + (8, 3), +]) +def test_get_learners_count_having_no_enrollment(base_data, tenant_id, expected_result): + """Test get_learners_count_having_no_enrollment function.""" + result = learners.get_learners_count_having_no_enrollment(tenant_id) + assert result == expected_result, f'Wrong learners count: {result} for tenant: {tenant_id}' + + +@pytest.mark.django_db +def test_get_learners_count(base_data): + """Test get_learners_count function.""" + result = learners.get_learners_count([1, 2, 4]) + assert result == { + 1: { + 'learners_count': 17, + 'learners_count_no_enrollment': 0, + 'learners_count_per_org': {'ORG1': 4, 'ORG2': 17}, + }, + 2: {'learners_count': 16, + 'learners_count_no_enrollment': 5, + 'learners_count_per_org': {'ORG3': 13, 'ORG8': 6}, + }, + 4: { + 'learners_count': 0, + 'learners_count_no_enrollment': 0, + 'learners_count_per_org': {}, + }, + }, f'Wrong learners count: {result}' diff --git a/tests/test_dashboard/test_views.py b/tests/test_dashboard/test_views.py new file mode 100644 index 00000000..9314d8d6 --- /dev/null +++ b/tests/test_dashboard/test_views.py @@ -0,0 +1,65 @@ +"""Test views for the dashboard app""" +import json + +import pytest +from django.contrib.auth import get_user_model +from django.http import JsonResponse +from django.urls import reverse +from rest_framework.test import APITestCase + + +@pytest.mark.usefixtures('base_data') +class TestTotalCountsView(APITestCase): + """Tests for TotalCountsView""" + def setUp(self): + self.url = reverse('fx_dashboard:total-counts') + 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)) + + def test_unauthorized(self): + """Test unauthorized access""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, 403) + + def test_invalid_stats(self): + """Test invalid stats""" + self.login_user(self.staff_user) + response = self.client.get(self.url + '?stats=invalid') + self.assertEqual(response.status_code, 400) + self.assertEqual(response.data, {'reason': 'Invalid stats type', 'details': {'invalid': ['invalid']}}) + + def test_all_stats(self): + """Test get method""" + self.login_user(self.staff_user) + response = self.client.get(self.url + '?stats=certificates,courses,learners') + self.assertTrue(isinstance(response, JsonResponse)) + self.assertEqual(response.status_code, 200) + expected_response = { + '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}, + 'total_certificates_count': 32, + 'total_courses_count': 23, + 'total_learners_count': 70 + } + self.assertDictEqual(json.loads(response.content), expected_response) + + def test_selected_tenants(self): + """Test get method with selected tenants""" + self.login_user(self.staff_user) + response = self.client.get(self.url + '?stats=certificates,courses,learners&tenant_ids=1,2') + self.assertTrue(isinstance(response, JsonResponse)) + self.assertEqual(response.status_code, 200) + expected_response = { + '1': {'certificates_count': 14, 'courses_count': 12, 'learners_count': 17}, + '2': {'certificates_count': 9, 'courses_count': 5, 'learners_count': 21}, + 'total_certificates_count': 23, + 'total_courses_count': 17, + 'total_learners_count': 38 + } + self.assertDictEqual(json.loads(response.content), expected_response) diff --git a/tests/test_helpers/test_apps.py b/tests/test_helpers/test_apps.py new file mode 100644 index 00000000..035f398b --- /dev/null +++ b/tests/test_helpers/test_apps.py @@ -0,0 +1,7 @@ +"""Tests for the apps module of the helpers app""" +from futurex_openedx_extensions.helpers.apps import HelpersConfig + + +def test_app_name(): + """Test that the app name is correct""" + assert HelpersConfig.name == 'futurex_openedx_extensions.helpers' diff --git a/tests/test_helpers/test_converters.py b/tests/test_helpers/test_converters.py new file mode 100644 index 00000000..6d0d6072 --- /dev/null +++ b/tests/test_helpers/test_converters.py @@ -0,0 +1,48 @@ +"""Tests for converters helpers.""" +import pytest + +from futurex_openedx_extensions.helpers import converters + + +@pytest.mark.parametrize("ids_string, expected", [ + ('1,2,3,7', [1, 2, 3, 7]), + (', 1, 2, 3, 7, 8, ', [1, 2, 3, 7, 8]), + (None, []), + ('', []), + (' ', []), + (', , ', []), +]) +def test_ids_string_to_list(ids_string, expected): + """Verify ids_string_to_list function.""" + result = converters.ids_string_to_list(ids_string) + assert result == expected + + +@pytest.mark.parametrize("ids_string", [ + '1.1, 2', + '1, 2, 3, 7, 8, a', +]) +def test_ids_string_to_list_bad_string(ids_string): + """Verify ids_string_to_list function.""" + with pytest.raises(ValueError): + converters.ids_string_to_list(ids_string) + + +def test_error_details_to_dictionary(): + """Verify error_details_to_dictionary function.""" + result = converters.error_details_to_dictionary( + 'This is an error message', + code=123, + key1='value1', + key2='value2', + anything={'key': 'value'}, + ) + assert result == { + 'reason': 'This is an error message', + 'details': { + 'code': 123, + 'key1': 'value1', + 'key2': 'value2', + 'anything': {'key': 'value'}, + }, + } diff --git a/tests/test_helpers/test_permissions.py b/tests/test_helpers/test_permissions.py new file mode 100644 index 00000000..c9c7c48a --- /dev/null +++ b/tests/test_helpers/test_permissions.py @@ -0,0 +1,72 @@ +"""Test permissions helper classes""" +import json + +import pytest +from django.contrib.auth import get_user_model +from django.contrib.auth.models import AnonymousUser +from rest_framework.exceptions import NotAuthenticated, PermissionDenied +from rest_framework.test import APIRequestFactory + +from futurex_openedx_extensions.helpers.permissions import HasTenantAccess + + +def set_user(request, user_id): + """Set user""" + if user_id is None: + request.user = None + elif user_id == 0: + request.user = AnonymousUser() + else: + request.user = get_user_model().objects.get(id=user_id) + + +@pytest.mark.django_db +@pytest.mark.parametrize('method, query_params, user_id', [ + ('GET', '?tenant_ids=1,2,3', 1), + ('GET', '?tenant_ids=1,2,3', 2), + ('GET', '', 1), +]) +def test_has_tenant_access(base_data, method, query_params, user_id): + """Verify that HasTenantAccess returns True when user has access to all tenants.""" + permission = HasTenantAccess() + request = APIRequestFactory().generic(method, f'/dummy/{query_params}') + request.user = get_user_model().objects.get(id=user_id) + assert permission.has_permission(request, None) is True + + +@pytest.mark.django_db +@pytest.mark.parametrize('method, query_params, user_id, reason, bad_tenant_ids', [ + ('GET', '?tenant_ids=1,2,3', 5, 'denied', [1, 2, 3]), + ('GET', '?tenant_ids=1,2,3', 6, 'denied', [1, 2, 3]), + ('GET', '?tenant_ids=1,2,3,4,9', 1, 'invalid', [9, 4]), +]) +def test_has_tenant_access_no_access(base_data, method, query_params, user_id, reason, bad_tenant_ids): + """Verify that PermissionDenied is raised when user does not have access to one of the tenants.""" + permission = HasTenantAccess() + request = APIRequestFactory().generic(method, f'/dummy/{query_params}') + set_user(request, user_id) + + expected_error = { + 'reason': 'User does not have access to these tenants', + 'details': {'tenant_ids': bad_tenant_ids} + } + if reason == 'denied': + expected_error['reason'] = 'User does not have access to these tenants' + else: + expected_error['reason'] = 'Invalid tenant IDs provided' + expected_error = json.dumps(expected_error) + + with pytest.raises(PermissionDenied) as exc_info: + permission.has_permission(request, None) + assert str(exc_info.value) == expected_error + + +@pytest.mark.django_db +@pytest.mark.parametrize('user_id', [0, None]) +def test_has_tenant_access_not_authenticated(base_data, user_id): + """Verify that NotAuthenticated is raised when user is not authenticated.""" + permission = HasTenantAccess() + request = APIRequestFactory().generic('GET', '/dummy/') + set_user(request, user_id) + with pytest.raises(NotAuthenticated): + permission.has_permission(request, None) diff --git a/tests/test_helpers/test_tenants.py b/tests/test_helpers/test_tenants.py new file mode 100644 index 00000000..fe582d5a --- /dev/null +++ b/tests/test_helpers/test_tenants.py @@ -0,0 +1,248 @@ +"""Tests for tenants helpers.""" + +import pytest +from django.contrib.auth import get_user_model +from eox_tenant.models import TenantConfig + +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 + + +@pytest.mark.django_db +def test_get_excluded_tenant_ids(base_data): + """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): + """Verify get_all_tenants function.""" + result = tenants.get_all_tenants() + assert TenantConfig.objects.count() == 8 + print('test_get_all_tenants.count:', TenantConfig.objects.count()) + assert result.count() == 5 + assert result.exclude(id__in=[4, 5, 6]).count() == result.count() + assert result.exclude(id__in=[4, 5, 6]).count() == TenantConfig.objects.exclude(id__in=[4, 5, 6]).count() + + +@pytest.mark.django_db +def test_get_all_tenant_ids(base_data): + """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): + """Verify that get_accessible_tenant_ids returns an empty list when user is None.""" + result = tenants.get_accessible_tenant_ids(None) + assert result == [] + + +@pytest.mark.django_db +@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): + """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' + result = tenants.get_accessible_tenant_ids(user) + assert result == expected + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_id, expected", [ + (2, [1, 2, 3, 7, 8]), +]) +def test_get_accessible_tenant_ids_staff(base_data, user_id, expected): + """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' + result = tenants.get_accessible_tenant_ids(user) + assert result == expected + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_id, expected", [ + (3, []), + (4, [1, 2, 7]), + (9, [1]), + (23, [2, 3, 8]), +]) +def test_get_accessible_tenant_ids_no_staff_no_sueperuser(base_data, user_id, expected): + """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' + result = tenants.get_accessible_tenant_ids(user) + assert result == expected + + +@pytest.mark.django_db +def test_get_accessible_tenant_ids_complex(base_data): + """Verify get_accessible_tenant_ids function for complex cases""" + user = get_user_model().objects.get(id=10) + user_access_role = 'org_course_creator_group' + user_access = 'ORG3' + + assert not user.is_staff and not user.is_superuser, 'only users with no staff and no superuser allowed in this test' + assert _base_data['tenant_config'][2]['lms_configs']['course_org_filter'] == [ + 'ORG3', 'ORG8'], 'test data is not as expected' + assert _base_data['tenant_config'][7]['lms_configs']['course_org_filter'] == 'ORG3', 'test data is not as expected' + assert _base_data['tenant_config'][8]['lms_configs']['course_org_filter'] == [ + 'ORG8'], 'test data is not as expected' + + for role, orgs in _base_data["course_access_roles"].items(): + for org, users in orgs.items(): + if role not in TENANT_LIMITED_ADMIN_ROLES: + continue + if role != user_access_role or org != user_access: + assert user.id not in users, ( + f'test data is not as expected, user {user.id} should be only in ' + f'{user_access_role} for {user_access}. Found in {role} for {org}' + ) + else: + assert user.id in users, ( + f'test data is not as expected, user {user.id} was not found in ' + f'{user_access_role} for {user_access}' + ) + assert ( + (role not in 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) + ), (f'test data is not as expected, user {user.id} should be only in {user_access_role} for {user_access}. ' + f'Found in {role} for {org}' if user.id in users else f'Found in {role} for {org}') + + expected_to_not_include_tenant_8 = [2, 7] + result = tenants.get_accessible_tenant_ids(user) + assert result == expected_to_not_include_tenant_8 + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_id, ids_to_check, expected", [ + (1, '1,2,3,7', (True, {})), + (2, '1,2,3,7', (True, {})), + (3, '1,2,3,7', ( + False, { + 'details': {'tenant_ids': [1, 2, 3, 7]}, + 'reason': 'User does not have access to these tenants' + } + )), + (1, '1,7,9', ( + False, { + 'details': {'tenant_ids': [9]}, + 'reason': 'Invalid tenant IDs provided' + } + )), + (1, '1,2,E,7', ( + False, { + 'details': {'error': "invalid literal for int() with base 10: 'E'"}, + 'reason': 'Invalid tenant IDs provided. It must be a comma-separated list of integers' + } + )), +]) +def test_check_tenant_access(base_data, user_id, ids_to_check, expected): + """Verify check_tenant_access function.""" + user = get_user_model().objects.get(id=user_id) + result = tenants.check_tenant_access(user, ids_to_check) + assert result == expected + + +@pytest.mark.django_db +def test_get_all_course_org_filter_list(base_data): + """Verify get_all_course_org_filter_list function.""" + result = tenants.get_all_course_org_filter_list() + assert result == { + 1: ['ORG1', 'ORG2'], + 2: ['ORG3', 'ORG8'], + 3: ['ORG4', 'ORG5'], + 7: ['ORG3'], + 8: ['ORG8'], + } + + +@pytest.mark.django_db +@pytest.mark.parametrize("tenant_ids, expected", [ + ([1, 2, 3, 7], { + 'course_org_filter_list': ['ORG1', 'ORG2', 'ORG3', 'ORG8', 'ORG4', 'ORG5'], + 'duplicates': { + 2: [7], + 7: [2], + }, + 'invalid': [], + }), + ([2, 3], { + 'course_org_filter_list': ['ORG3', 'ORG8', 'ORG4', 'ORG5'], + 'duplicates': {}, + 'invalid': [], + }), + ([2, 3, 4], { + 'course_org_filter_list': ['ORG3', 'ORG8', 'ORG4', 'ORG5'], + 'duplicates': {}, + 'invalid': [4], + }), + ([2, 3, 7, 8], { + 'course_org_filter_list': ['ORG3', 'ORG8', 'ORG4', 'ORG5'], + 'duplicates': { + 2: [7, 8], + 7: [2], + 8: [2], + }, + 'invalid': [], + }), + ([], { + 'course_org_filter_list': [], + 'duplicates': {}, + 'invalid': [], + }), +]) +def test_get_course_org_filter_list(base_data, tenant_ids, expected): + """Verify get_course_org_filter_list function.""" + result = tenants.get_course_org_filter_list(tenant_ids) + assert result == expected + + +@pytest.mark.django_db +@pytest.mark.parametrize("user_id, expected", [ + (1, [1, 2, 3, 7, 8]), + (2, [1, 2, 3, 7, 8]), + (3, []), +]) +def test_get_accessible_tenant_ids(base_data, user_id, expected): + """Verify get_accessible_tenant_ids function.""" + user = get_user_model().objects.get(id=user_id) + result = tenants.get_accessible_tenant_ids(user) + assert result == expected + + +@pytest.mark.django_db +def test_get_all_tenants_info(base_data): + """Verify get_all_tenants_info function.""" + result = tenants.get_all_tenants_info() + assert result['tenant_ids'] == [1, 2, 3, 7, 8] + assert result['sites'] == { + 1: 's1.sample.com', + 2: 's2.sample.com', + 3: 's3.sample.com', + 7: 's7.sample.com', + 8: 's8.sample.com', + } + + +@pytest.mark.django_db +@pytest.mark.parametrize("tenant_id, expected", [ + (1, 's1.sample.com'), + (2, 's2.sample.com'), + (3, 's3.sample.com'), + (4, None), + (5, None), + (6, None), + (7, 's7.sample.com'), + (8, 's8.sample.com'), +]) +def test_get_tenant_site(base_data, tenant_id, expected): + """Verify get_tenant_site function.""" + assert expected == tenants.get_tenant_site(tenant_id) diff --git a/tests/test_models.py b/tests/test_models.py deleted file mode 100644 index 85f58c88..00000000 --- a/tests/test_models.py +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env python -""" -Tests for the `futurex-openedx-extensions` models module. -""" - -import pytest - - -class Testdashboard: - """ - Tests of the dashboard model. - """ - - @pytest.mark.skip(reason="Placeholder to allow pytest to succeed before real tests are in place.") - def test_placeholder(self): - """ - TODO: Delete this test once there are real tests. - """ diff --git a/tests/test_urls.py b/tests/test_urls.py new file mode 100644 index 00000000..bfc12aea --- /dev/null +++ b/tests/test_urls.py @@ -0,0 +1,8 @@ +"""URLs configuration for testing purposes.""" +from django.urls import include +from django.urls import path as rpath + +urlpatterns = [ + # include the urls from the dashboard app using include + rpath('', include('futurex_openedx_extensions.dashboard.urls'), name='fx_dashboard'), +] diff --git a/tox.ini b/tox.ini index 65fbf99f..ca36bfcf 100644 --- a/tox.ini +++ b/tox.ini @@ -39,7 +39,9 @@ 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 = + python manage.py makemigrations fake_models python manage.py check pytest {posargs} @@ -70,6 +72,7 @@ allowlist_externals = touch deps = -r{toxinidir}/requirements/quality.txt + -e{toxinidir}/test_utils/edx_platform_mocks commands = touch tests/__init__.py pylint futurex_openedx_extensions tests test_utils manage.py setup.py