diff --git a/figures/enrollment.py b/figures/enrollment.py new file mode 100644 index 00000000..68598f3f --- /dev/null +++ b/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 diff --git a/figures/models.py b/figures/models.py index 64192be6..8762e66c 100644 --- a/figures/models.py +++ b/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 diff --git a/figures/tasks.py b/figures/tasks.py index b731d29c..57f40564 100644 --- a/figures/tasks.py +++ b/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))) # diff --git a/tests/factories.py b/tests/factories.py index 90fddda5..9cbf0290 100644 --- a/tests/factories.py +++ b/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: diff --git a/tests/models/test_enrollment_data_model.py b/tests/models/test_enrollment_data_model.py index 501cc3c9..d97fa18d 100644 --- a/tests/models/test_enrollment_data_model.py +++ b/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, diff --git a/tests/tasks/test_backfill_tasks.py b/tests/tasks/test_backfill_tasks.py index 7c5420fe..2e870d6e 100644 --- a/tests/tasks/test_backfill_tasks.py +++ b/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)) diff --git a/tests/test_enrollment.py b/tests/test_enrollment.py new file mode 100644 index 00000000..fbafb443 --- /dev/null +++ b/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