Fix backfill_enrollment_data_for_course task

* Problem was that `update_enrollment_data_for_course` returns a list of
tuples. Each tuple is `(object, created)`, where `object` is the
EnrollmentData record and `created` tells if the record was created
* Added logging to the backfill task
* Updated tests
* Added `get_from_enrollment` method to `EnrollmentDataManager`

figures/enrollment.py

@@ -0,0 +1,57 @@
+"""This module handles enrollments
+
+Enrollment specific functionality should go here unless the functionality would
+only be run in the pipeline.
+
+At some point we might make an `Enrollment` class. But for now, this module is
+here to build functionality to shed more light into what such a class would
+look like.
+"""
+from figures.compat import StudentModule
+from figures.models import EnrollmentData
+
+
+def student_modules_for_enrollment(enrollment):
+    """Return an enrollment's StudentModule records, if they exist
+
+    This function exists because edx-platform does not have a model relationship between
+    `CourseEnrollment` and `StudentModule`.
+    """
+    return StudentModule.objects.filter(student_id=enrollment.user_id,
+                                        course_id=enrollment.course_id)
+
+
+def student_modules_for_enrollment_after_date(enrollment, date_for):
+    """Return StudentModule records modified for the enrollment after a date
+
+    TODO: Test if we need to do `modified__gte=next_day(date_for)` or if
+    `modified__gt=date_for` will trigger for times after midnight but before
+    the next day
+
+    We can ignore `StudentModule.created` because a new record also sets the
+    `modified` field.
+    """
+    return student_modules_for_enrollment(enrollment).filter(modified__gte=date_for)
+
+
+def is_enrollment_data_out_of_date(enrollment):
+    """
+    Assumes being called with an enrollment with student modules
+    """
+    # has EnrollmentData records? if not, then out of date
+    edrec = EnrollmentData.objects.get_for_enrollment(enrollment)
+    if edrec:
+        sm = student_modules_for_enrollment_after_date(enrollment, edrec.date_for)
+        # All we care about is if there are student modules modified
+        # after the EnrollmentData record was `date_for`
+        # out_of_date.append(enrollment)
+        out_of_date = True if sm.exists() else False
+    else:
+        # Just check if there are student modules for the enrollment, ever
+        # If there are student modules (and we've already check there is no
+        # enrollment), then we need to update
+        # If there are no student modules, then the enrollment had no activity
+        # then return whether there are StudentModule records or not
+        out_of_date = student_modules_for_enrollment(enrollment).exists()
+
+    return out_of_date

figures/models.py

@@ -22,6 +22,7 @@
 from figures.progress import EnrollmentProgress
 
 
+# Remove this. Import from figures.sites or will we get dependency issues?
 def default_site():
     """
     Wrapper aroung `django.conf.settings.SITE_ID` so we do not have to create a
@@ -204,6 +205,19 @@ class EnrollmentDataManager(models.Manager):
     EnrollmentData instances.
 
     """
+
+    def get_for_enrollment(self, course_enrollment):
+        """Returns EnrollmentData object or None for the given CourseEnrollment
+
+        This is a context specific `get_or_none` function that uses the `user_id`
+        and `course_id` from the enrollment argument.
+        """
+        try:
+            return self.get(user_id=course_enrollment.user_id,
+                            course_id=str(course_enrollment.course_id))
+        except EnrollmentData.DoesNotExist:
+            return None
+
     def set_enrollment_data(self, site, user, course_id, course_enrollment=None):
         """
         This is an expensive call as it needs to call CourseGradeFactory if

figures/tasks.py

@@ -326,13 +326,16 @@ def populate_daily_metrics_next(site_id=None, force_update=False):
 def backfill_enrollment_data_for_course(course_id):
     """Create or update EnrollmentData records for the course
 
-    Simple wrapper to run the enrollment update as a Celery task
+    This is a simple wrapper to run the enrollment update as a Celery task
 
     We usually run this task through the Figures Django management command,
     `backfill_figures_enrollment_data`
     """
-    ed_objects = update_enrollment_data_for_course(course_id)
-    return [obj.id for obj in ed_objects]
+    # results are a list of (object, created_flag) tuples
+    updated = update_enrollment_data_for_course(course_id)
+    msg = ('figures.tasks.backfill_enrollment_data_for_course "{course_id}".'
+           ' Updated {edrec_count} enrollment data records.')
+    logger.info(msg.format(course_id=course_id, edrec_count=len(updated)))
 
 
 #

tests/factories.py

@@ -1,12 +1,10 @@
-'''Helpers to generate model instances for testing.
+"""Helpers to generate model instances for testing.
 
 Defines model factories for Figures, edX platform, and other models that we
 need to create for our tests.
 
 Uses Factory Boy: https://factoryboy.readthedocs.io/en/latest/
-
-'''
-
+"""
 from __future__ import absolute_import
 import datetime
 from dateutil.relativedelta import relativedelta
@@ -223,7 +221,7 @@ class Meta:
 
     @classmethod
     def from_course_enrollment(cls, course_enrollment, **kwargs):
-        """Contruct a StudentModule  for the given CourseEnrollment
+        """Contruct a StudentModule for the given CourseEnrollment
 
         kwargs provides for additional optional parameters if you need to
         override the default factory assignment
@@ -340,6 +338,19 @@ class Meta:
     sections_worked = 5
     sections_possible = 10
 
+    @classmethod
+    def from_course_enrollment(cls, course_enrollment, **kwargs):
+        """Construct an EnrollmentData for the given CourseEnrollment
+
+        kwargs provides for additional optional parameters if you need to
+        override the default factory assignment
+        """
+        kwargs.update({
+            'user': course_enrollment.user,
+            'course_id': course_enrollment.course_id,
+            })
+        return cls(**kwargs)
+
 
 class LearnerCourseGradeMetricsFactory(DjangoModelFactory):
     class Meta:

tests/models/test_enrollment_data_model.py

@@ -102,15 +102,37 @@ def site_data(db, settings):
 
     lcgm = [
         LearnerCourseGradeMetricsFactory(site=site_data['site'],
-            user=ce.user,
-            course_id=str(ce.course_id),
-            date_for='2020-10-01'),
+                                         user=ce.user,
+                                         course_id=str(ce.course_id),
+                                         date_for='2020-10-01'),
     ]
 
     site_data['lcgm'] = lcgm
     return site_data
 
 
+@pytest.mark.django_db
+def test_get_for_enrollment_has():
+    """Test we get an EnrollmentData record
+
+    We have a CourseEnrollment and we have an EnrollmentData record.
+    """
+    ce = CourseEnrollmentFactory()
+    ed = EnrollmentDataFactory.from_course_enrollment(ce)
+
+    assert EnrollmentData.objects.get_for_enrollment(ce) == ed
+
+
+@pytest.mark.django_db
+def test_get_for_enrollment_has_not():
+    """Test we get `None` for enrollment w/out EnrollmentData record
+
+    We have a CourseEnrollment, but no EnrollmentData record.
+    """
+    ce = CourseEnrollmentFactory()
+    assert EnrollmentData.objects.get_for_enrollment(ce) is None
+
+
 @pytest.mark.skipif(OPENEDX_RELEASE == GINKGO, reason='Breaks on CourseEnrollmentFactory')
 def test_set_enrollment_data_new_record(site_data):
     """Test we create a new EnrollmentData record
@@ -140,10 +162,9 @@ def test_set_enrollment_data_update_existing(site_data):
     """
     site = site_data['site']
     ce = site_data['enrollments'][0]
-    lcgm = site_data['lcgm'][0]
-    ed = EnrollmentDataFactory(site=site,
-                               course_id=str(ce.course_id),
-                               user=ce.user)
+    EnrollmentDataFactory(site=site,
+                          course_id=str(ce.course_id),
+                          user=ce.user)
     assert EnrollmentData.objects.count() == 1
     obj, created = EnrollmentData.objects.set_enrollment_data(
         site=site,

tests/tasks/test_backfill_tasks.py

@@ -1,20 +1,55 @@
 """Test Figures backfill Celery tasks
 """
 from __future__ import absolute_import
+import logging
+import pytest
 
 from figures.tasks import backfill_enrollment_data_for_course
 
 from tests.factories import EnrollmentDataFactory
 
 
-def test_backfill_enrollment_data_for_course(transactional_db, monkeypatch):
-    """
-    The Celery task is a simple wrapper around the pipeline function
-    """
-    course_id = 'course-v1:SomeOrg+SomeNum+SomeRun'
-    ed_recs = [EnrollmentDataFactory() for _ in range(2)]
+@pytest.mark.django_db
+class TestBackfillEnrollmentDataForCourse(object):
 
-    func_path = 'figures.tasks.update_enrollment_data_for_course'
-    monkeypatch.setattr(func_path, lambda course_id: ed_recs)
-    ed_ids = backfill_enrollment_data_for_course(course_id)
-    assert set(ed_ids) == set([obj.id for obj in ed_recs])
+    @pytest.fixture(autouse=True)
+    def setup(self, db):
+        self.expected_message_template = (
+            'figures.tasks.backfill_enrollment_data_for_course "{course_id}".'
+            ' Updated {edrec_count} enrollment data records.')
+
+    def test_backfill_enrollment_data_for_course_no_update(self, transactional_db,
+                                                           monkeypatch, caplog):
+        """
+        The Celery task is a simple wrapper around the pipeline function
+        """
+        course_id = 'course-v1:SomeOrg+SomeNum+SomeRun'
+
+        # The function returns a list of tuples with (object, created)
+        # ed_recs = [(EnrollmentDataFactory(), False) for _ in range(2)]
+        caplog.set_level(logging.INFO)
+        func_path = 'figures.tasks.update_enrollment_data_for_course'
+        monkeypatch.setattr(func_path, lambda course_id: [])
+        backfill_enrollment_data_for_course(course_id)
+        assert len(caplog.records) == 1
+        assert caplog.records[0].message == self.expected_message_template.format(
+            course_id=course_id,
+            edrec_count=0)
+
+    def test_backfill_enrollment_data_for_course_with_updates(self, transactional_db,
+                                                              monkeypatch, caplog):
+        """
+        The Celery task is a simple wrapper around the pipeline function
+        """
+        course_id = 'course-v1:SomeOrg+SomeNum+SomeRun'
+
+        # The function returns a list of tuples with (object, created)
+        ed_recs = [(EnrollmentDataFactory(), False) for _ in range(2)]
+        caplog.set_level(logging.INFO)
+        func_path = 'figures.tasks.update_enrollment_data_for_course'
+        monkeypatch.setattr(func_path, lambda course_id: ed_recs)
+        backfill_enrollment_data_for_course(course_id)
+        assert len(caplog.records) == 1
+        assert caplog.records[0].message == self.expected_message_template.format(
+            course_id=course_id,
+            edrec_count=len(ed_recs))

tests/test_enrollment.py

@@ -0,0 +1,77 @@
+"""Tests figures.enrollment module
+"""
+from __future__ import absolute_import
+from datetime import timedelta
+
+import pytest
+from faker import Faker
+
+from figures.enrollment import (
+    student_modules_for_enrollment,
+    student_modules_for_enrollment_after_date,
+    is_enrollment_data_out_of_date
+)
+from figures.helpers import as_date, as_datetime
+
+from tests.factories import (
+    CourseEnrollmentFactory,
+    EnrollmentDataFactory,
+    StudentModuleFactory,
+)
+
+
+fake = Faker()
+
+
+@pytest.mark.django_db
+class TestStudentModulesForEnrollmentAfterDate(object):
+    """Tests figures.enrollment.student_modules_for_enrollment_after_date
+    This test also exercises `student_modules_for_enrollment` function.
+    """
+    @pytest.fixture(autouse=True)
+    def setup(self, db):
+        self.enrollment = CourseEnrollmentFactory()
+
+    def test_no_student_modules(self):
+        qs = student_modules_for_enrollment_after_date(self.enrollment, fake.date())
+        assert not qs.exists()
+
+    def test_none_after(self):
+        sm_date = fake.date_this_year()
+        td_params = dict(days=1)
+        check_date = sm_date + timedelta(**td_params)
+        sm = StudentModuleFactory.from_course_enrollment(self.enrollment,
+            modified=as_datetime(sm_date))
+        qs = student_modules_for_enrollment_after_date(self.enrollment, check_date)
+        assert not qs.exists()
+
+
+
+
+@pytest.mark.django_db
+class TestIsEnrollmentDataOutOfDate(object):
+    """Tests figures.enrollment.is_enrollment_data_out_of_date
+    """
+    @pytest.fixture(autouse=True)
+    def setup(self, db):
+        self.enrollment = CourseEnrollmentFactory()
+
+    def test_no_edrec_no_sm(self):
+        assert is_enrollment_data_out_of_date(self.enrollment) == False
+
+    def test_no_edrec_has_sm(self):
+        StudentModuleFactory.from_course_enrollment(self.enrollment)
+        assert is_enrollment_data_out_of_date(self.enrollment) == True
+
+    def test_has_edrec_but_out_of_date(self):
+        edrec = EnrollmentDataFactory.from_course_enrollment(
+            self.enrollment, date_for=fake.date_this_year())
+
+        sm_date_for = edrec.date_for + timedelta(days=1)
+        StudentModuleFactory.from_course_enrollment(
+            self.enrollment, modified=sm_date_for)
+        assert is_enrollment_data_out_of_date(self.enrollment) == True
+
+    def test_has_edrec_and_up_to_date(self):
+        edrec = EnrollmentDataFactory.from_course_enrollment(self.enrollment)
+        assert is_enrollment_data_out_of_date(self.enrollment) == False