diff --git a/figures/compat.py b/figures/compat.py index 50edafcf0..30fef7914 100644 --- a/figures/compat.py +++ b/figures/compat.py @@ -1,7 +1,9 @@ """Figures compatability module -This module serves to provide a common access point to functionality that -differs from different named Open edX releases +This module serves to provide a common access point to edx-platform objects. + +It was originally intended to encapsulate functionality that differs from +different named Open edX releases We can identify the Open edX named release for Ginkgo and later by getting the value from openedx.core.release.RELEASE_LINE. This will be the release name as @@ -74,7 +76,8 @@ class CourseNotFound(Exception): # preemptive addition. Added it here to avoid adding to figures.models # In fact, we should probably do a refactoring that makes all Figures import it # from here -from student.models import CourseEnrollment # noqa pylint: disable=unused-import,import-error +from student.models import CourseAccessRole, CourseEnrollment # noqa pylint: disable=unused-import,import-error +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=unused-import,import-error def course_grade(learner, course): diff --git a/figures/filters.py b/figures/filters.py index 3263898fc..d9ddfdd8b 100644 --- a/figures/filters.py +++ b/figures/filters.py @@ -27,10 +27,7 @@ from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=import-error - -from student.models import CourseEnrollment # pylint: disable=import-error - +from figures.compat import CourseEnrollment, CourseOverview from figures.pipeline.course_daily_metrics import get_enrolled_in_exclude_admins from figures.models import ( CourseDailyMetrics, diff --git a/figures/models.py b/figures/models.py index 6012aeb6a..8ae1482eb 100644 --- a/figures/models.py +++ b/figures/models.py @@ -116,6 +116,8 @@ class SiteDailyMetrics(TimeStampedModel): # Should change this to default value of 0 mau = models.IntegerField(blank=True, null=True) + # TODO: Add field for number of CDMs reported + class Meta: """ SiteDailyMetrics view and serializer tests fail when we include 'site' diff --git a/figures/pipeline/course_daily_metrics.py b/figures/pipeline/course_daily_metrics.py index 4842dfe95..448ca5fcd 100644 --- a/figures/pipeline/course_daily_metrics.py +++ b/figures/pipeline/course_daily_metrics.py @@ -19,11 +19,12 @@ from django.db import transaction from django.utils.timezone import utc -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=import-error -from student.models import CourseEnrollment # pylint: disable=import-error from student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole # noqa pylint: disable=import-error -from figures.compat import GeneratedCertificate, StudentModule +from figures.compat import (CourseEnrollment, + CourseOverview, + GeneratedCertificate, + StudentModule) from figures.helpers import as_course_key, as_datetime, next_day, prev_day import figures.metrics from figures.models import CourseDailyMetrics, PipelineError diff --git a/figures/serializers.py b/figures/serializers.py index c4f5dded5..0b415a4c9 100644 --- a/figures/serializers.py +++ b/figures/serializers.py @@ -27,12 +27,13 @@ from rest_framework import serializers from rest_framework.fields import empty -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=import-error from openedx.core.djangoapps.user_api.accounts.serializers import AccountLegacyProfileSerializer # noqa pylint: disable=import-error -from student.models import CourseAccessRole, CourseEnrollment # pylint: disable=import-error - -from figures.compat import RELEASE_LINE, GeneratedCertificate +from figures.compat import (RELEASE_LINE, + CourseAccessRole, + CourseEnrollment, + CourseOverview, + GeneratedCertificate) from figures.helpers import as_course_key from figures.metrics import ( get_course_enrolled_users_for_time_period, diff --git a/figures/sites.py b/figures/sites.py index a8221a7d7..208b191a1 100644 --- a/figures/sites.py +++ b/figures/sites.py @@ -18,13 +18,13 @@ # TODO: Add exception handling import organizations -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=import-error - -from student.models import CourseEnrollment # pylint: disable=import-error - -from figures.compat import StudentModule -from figures.helpers import as_course_key -import figures.helpers +from figures.compat import ( + CourseEnrollment, + CourseOverview, + GeneratedCertificate, + StudentModule, +) +from figures.helpers import as_course_key, is_multisite class CrossSiteResourceError(Exception): @@ -106,7 +106,7 @@ def get_site_for_course(course_id): TODO: Figure out how we want to handle ``DoesNotExist`` whether to let it raise back up raw or handle with a custom exception """ - if figures.helpers.is_multisite(): + if is_multisite(): org_courses = organizations.models.OrganizationCourse.objects.filter( course_id=str(course_id)) if org_courses: @@ -152,7 +152,7 @@ def get_course_keys_for_site(site): We may also be able to reduce the queries here to also improve performance """ - if figures.helpers.is_multisite(): + if is_multisite(): course_ids = site_course_ids(site) else: course_ids = CourseOverview.objects.all().values_list('id', flat=True) @@ -164,7 +164,7 @@ def get_courses_for_site(site): This function relies on Appsembler's fork of edx-organizations """ - if figures.helpers.is_multisite(): + if is_multisite(): course_keys = get_course_keys_for_site(site) courses = CourseOverview.objects.filter(id__in=course_keys) else: @@ -173,7 +173,7 @@ def get_courses_for_site(site): def get_user_ids_for_site(site): - if figures.helpers.is_multisite(): + if is_multisite(): return get_users_for_site(site).values_list('id', flat=True) else: user_ids = get_user_model().objects.all().values_list('id', flat=True) @@ -181,7 +181,7 @@ def get_user_ids_for_site(site): def get_users_for_site(site): - if figures.helpers.is_multisite(): + if is_multisite(): return get_user_model().objects.filter(organizations__sites__in=[site]) else: users = get_user_model().objects.all() @@ -189,14 +189,14 @@ def get_users_for_site(site): def get_course_enrollments_for_site(site): - if figures.helpers.is_multisite(): + if is_multisite(): return CourseEnrollment.objects.filter(user__organizations__sites__in=[site]) else: return CourseEnrollment.objects.all() def get_student_modules_for_course_in_site(site, course_id): - if figures.helpers.is_multisite(): + if is_multisite(): site_id = site.id check_site = get_site_for_course(course_id) if not check_site or site_id != check_site.id: @@ -241,3 +241,25 @@ def student_modules_for_course_enrollment(ce): Relies on the fact that course_ids are globally unique """ return StudentModule.objects.filter(student=ce.user, course_id=ce.course_id) + + +def site_certificates(site): + """ + If we want to be clever, we can abstract a function: + ``` + def site_user_related(site, model_class): + if is_multisite(): + return model_class.objects.filter(user__organizations__sites__in=[site]) + else: + return model_class.objects.all() + + def site_certificates(site): + return site_user_related(GeneratedCertificate) + ``` + Then: + """ + if is_multisite(): + return GeneratedCertificate.objects.filter( + user__organizations__sites__in=[site]) + else: + return GeneratedCertificate.objects.all() diff --git a/figures/tasks.py b/figures/tasks.py index 94f051871..2fd13afec 100644 --- a/figures/tasks.py +++ b/figures/tasks.py @@ -6,6 +6,8 @@ import datetime import time +import six + from django.contrib.sites.models import Site from django.utils.timezone import utc @@ -13,11 +15,8 @@ from celery.app import shared_task from celery.utils.log import get_task_logger -# TODO: import CourseOverview from figures.compat -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=import-error -from student.models import CourseEnrollment # pylint: disable=import-error - from figures.backfill import backfill_enrollment_data_for_site +from figures.compat import CourseEnrollment, CourseOverview from figures.helpers import as_course_key, as_date from figures.log import log_exec_time from figures.models import PipelineError @@ -27,7 +26,6 @@ from figures.pipeline.mau_pipeline import collect_course_mau from figures.pipeline.site_monthly_metrics import fill_last_month as fill_last_smm_month from figures.pipeline.logger import log_error_to_db -import six logger = get_task_logger(__name__) diff --git a/figures/views.py b/figures/views.py index 6cac0524d..4244fa344 100644 --- a/figures/views.py +++ b/figures/views.py @@ -39,11 +39,7 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -# Directly including edx-platform objects for early development -# Follow-on, we'll likely consolidate edx-platform model imports to an adapter -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview # noqa pylint: disable=import-error - -from figures.compat import CourseEnrollment +from figures.compat import CourseEnrollment, CourseOverview from figures.filters import ( CourseDailyMetricsFilter, CourseEnrollmentFilter, diff --git a/setup.py b/setup.py index 6b620dc3a..61188562f 100644 --- a/setup.py +++ b/setup.py @@ -26,7 +26,7 @@ setup( name='Figures', - version='0.4.dev6', + version='0.4.dev7', packages=find_packages(), include_package_data=True, license='MIT', diff --git a/tests/pipeline/test_course_daily_metrics.py b/tests/pipeline/test_course_daily_metrics.py index 100750d03..e5b74e48b 100644 --- a/tests/pipeline/test_course_daily_metrics.py +++ b/tests/pipeline/test_course_daily_metrics.py @@ -10,10 +10,8 @@ from django.core.exceptions import PermissionDenied, ValidationError -from student.models import CourseEnrollment, CourseAccessRole - +from figures.compat import CourseAccessRole, CourseEnrollment from figures.helpers import as_datetime, next_day, prev_day - from figures.models import CourseDailyMetrics, PipelineError from figures.pipeline import course_daily_metrics as pipeline_cdm import figures.sites diff --git a/tests/test_filters.py b/tests/test_filters.py index 54994e3ed..c256f91c0 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -34,9 +34,7 @@ from django.contrib.sites.models import Site from django.test import TestCase -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from student.models import CourseEnrollment - +from figures.compat import CourseEnrollment, CourseOverview from figures.filters import ( CourseDailyMetricsFilter, CourseEnrollmentFilter, diff --git a/tests/test_serializers.py b/tests/test_serializers.py index 383918fe2..5b03bb8e1 100644 --- a/tests/test_serializers.py +++ b/tests/test_serializers.py @@ -15,8 +15,7 @@ from django.utils.timezone import utc from rest_framework.exceptions import ValidationError -from student.models import CourseEnrollment - +from figures.compat import CourseEnrollment from figures.models import ( CourseDailyMetrics, CourseMauMetrics, diff --git a/tests/test_sites.py b/tests/test_sites.py index e9c755634..e14feceae 100644 --- a/tests/test_sites.py +++ b/tests/test_sites.py @@ -31,10 +31,11 @@ import organizations -from openedx.core.djangoapps.content.course_overviews.models import ( - CourseOverview, -) +# from openedx.core.djangoapps.content.course_overviews.models import ( +# CourseOverview, +# ) +from figures.compat import CourseOverview import figures.helpers import figures.sites diff --git a/tests/views/test_course_enrollment_view.py b/tests/views/test_course_enrollment_view.py index f67e078f5..49d788811 100644 --- a/tests/views/test_course_enrollment_view.py +++ b/tests/views/test_course_enrollment_view.py @@ -14,8 +14,7 @@ from opaque_keys.edx.keys import CourseKey -from student.models import CourseEnrollment - +from figures.compat import CourseEnrollment from figures.helpers import is_multisite from figures.views import CourseEnrollmentViewSet diff --git a/tests/views/test_general_course_data_view.py b/tests/views/test_general_course_data_view.py index da2ccfbff..cb0a0172a 100644 --- a/tests/views/test_general_course_data_view.py +++ b/tests/views/test_general_course_data_view.py @@ -7,19 +7,13 @@ from django.contrib.sites.models import Site from django.db.models import F - - from rest_framework.test import ( APIRequestFactory, #RequestsClient, Not supported in older rest_framework versions force_authenticate, ) -from openedx.core.djangoapps.content.course_overviews.models import ( - CourseOverview, -) -from student.models import CourseEnrollment - +from figures.compat import CourseEnrollment, CourseOverview import figures.helpers from figures.helpers import as_course_key, is_multisite from figures.views import GeneralCourseDataViewSet diff --git a/tests/views/test_general_user_data_view.py b/tests/views/test_general_user_data_view.py index 6526b322b..4a3793151 100644 --- a/tests/views/test_general_user_data_view.py +++ b/tests/views/test_general_user_data_view.py @@ -41,8 +41,7 @@ force_authenticate, ) -from student.models import CourseEnrollment - +from figures.compat import CourseEnrollment from figures.helpers import is_multisite from figures.views import GeneralUserDataViewSet diff --git a/tests/views/test_learner_details_view.py b/tests/views/test_learner_details_view.py index 6f8ad2ec2..27ab285fa 100644 --- a/tests/views/test_learner_details_view.py +++ b/tests/views/test_learner_details_view.py @@ -1,4 +1,4 @@ -'''Tests Figures UserIndexView class +"""Tests Figures UserIndexView class TODO: make reasonable endpoint @@ -27,7 +27,7 @@ ] -''' +""" from __future__ import absolute_import import mock @@ -42,8 +42,7 @@ force_authenticate, ) -from student.models import CourseEnrollment - +from figures.compat import CourseEnrollment from figures.helpers import as_course_key from figures.serializers import LearnerCourseDetailsSerializer from figures.sites import get_course_enrollments_for_site