Refactor: Moved a number of edx-platform imports to figures.compat

Improved abstraction: This commit refactors how Figures imports edx-platform
classes. It centralizes imports to figures.compat. The rest of Figures then
imports the platform classes from figures.compat.

This is preliminary work to help abstract platform dependencies for Open
edX named release structures and improve future testing efforts

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):

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,

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'

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

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,

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,30 +173,30 @@ 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)
     return user_ids
 
 
 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()
     return users
 
 
 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()

figures/tasks.py

@@ -6,18 +6,17 @@
 import datetime
 import time
 
+import six
+
 from django.contrib.sites.models import Site
 from django.utils.timezone import utc
 
 from celery import chord
 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__)

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,

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',

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

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,

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,

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
 

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
 

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

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
 

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