From 4cd78a71d1213b86bf730bf70550030a0dcef2cc Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Fri, 25 Feb 2022 00:03:59 -0500 Subject: [PATCH] figures.course is a new module to simplify Figures This PR adds `figures.course`, a new module that has the `Course` class The goal of this class is to simplify Figures code This class provides data specific to and associated with a course. Our initial version provides only the essential data needed for our pipeline performance improvement --- figures/course.py | 115 +++++++++++++++++++++++++++++++++++++++++++ tests/test_course.py | 112 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 figures/course.py create mode 100644 tests/test_course.py diff --git a/figures/course.py b/figures/course.py new file mode 100644 index 000000000..207921fd7 --- /dev/null +++ b/figures/course.py @@ -0,0 +1,115 @@ +"""Course specific module for Figures + +## This module defined a `Course` class for data retrieval + +Initialy created to do the following: + +1. Reduce duplication in Figures "pipeline" +2. Build stronger course context to make Figures programming easier + +## Background summary + +A course id is globally unique as it has the identify of the organization and +organizations are globally unique. + +## Design to think about - Enrollment class + +Build on Django's lazy eval for querysets to create also an `Enrollment` class +that provides interfaces for `enrollment.date_for` and abstracts this bit of +mess that enrollments and student modules do NOT associate and instead we need +to query back and forth with `user_id` and `course_id`. +""" +from __future__ import absolute_import +from django.db.models import Q +from figures.compat import CourseEnrollment, StudentModule +from figures.helpers import ( + as_course_key, + as_date, +) +from figures.sites import ( + get_site_for_course, +) + + +class Course(object): + """Representation of a Course. + + The impetus for this class was dealing with querying for course enrollment + and student module records for a specific course and for dates and date + ranges for the course + + ## Architecture goal + + **Start simple and don't build the kitchen sink into here right away just + because this class exists** + + ## Data under consideration to have this class handle + + * enrollments created on a date, before, after, between. However this would + just be a convenience as the `.enrollments` property returns a queryset that + can be filtered on `.created` + """ + def __init__(self, course_id): + """ + Initial version, we pass in a course ID and cast to a course key as an + instance attribute. Later on, add `CourseLike` to abstract course identity + so we can stop worrying about "Is it a string repretentation of a course or + is it a CourseKey?" + """ + self.course_key = as_course_key(course_id) + + # Improvement: Consider doing lazy evaluation + self.site = get_site_for_course(self.course_id) + + def __repr__(self): + return '{}.{} <{}>'.format(self.__module__, + self.__class__.__name__, + str(self.course_key)) + + def __str__(self): + return self.__repr__() + + @property + def course_id(self): + """Returns string representation of the course id + """ + return str(self.course_key) + + @property + def enrollments(self): + """Returns CourseEnrollment queryset for the course + """ + return CourseEnrollment.objects.filter(course_id=self.course_key) + + @property + def student_modules(self): + """Returns StudentModule queryset for enrollments in the course + """ + return StudentModule.objects.filter(course_id=self.course_key) + + def student_modules_active_on_date(self, date_for): + """Returns StudentModule queryset active on the date + Active is if there was a `created` or `modified` field for the given date + + NOTE: We need to do this instead of simplly `modified__date=date_for` + because we still have to support Django 1.8/Ginkgo + """ + date_for = as_date(date_for) + q_created = Q(created__year=date_for.year, + created__month=date_for.month, + created__day=date_for.day) + q_modified = Q(modified__year=date_for.year, + modified__month=date_for.month, + modified__day=date_for.day) + return self.student_modules.filter(q_created | q_modified) + + def enrollments_active_on_date(self, date_for): + """Return CourseEnrollment queryset for enrollments active on the date + + Looks for student modules modified on the specified date and returns + matching CourseEnrollment records + """ + sm = self.student_modules_active_on_date(date_for) + user_ids = sm.values('student_id').distinct() + return CourseEnrollment.objects.filter(course_id=self.course_key, + user_id__in=user_ids) diff --git a/tests/test_course.py b/tests/test_course.py new file mode 100644 index 000000000..ed699b42f --- /dev/null +++ b/tests/test_course.py @@ -0,0 +1,112 @@ +"""Test `figures.course` module + +""" +from __future__ import absolute_import +import pytest +from faker import Faker +from opaque_keys.edx.keys import CourseKey + +from figures.course import Course +from figures.helpers import days_from +from figures.sites import get_site_for_course + +from tests.factories import ( + CourseEnrollmentFactory, + CourseOverviewFactory, + StudentModuleFactory, +) + +fake = Faker() + + +@pytest.mark.django_db +class TestCourse: + """ + Starting with just basic tests + """ + @pytest.fixture(autouse=True) + def setup(self, db): + self.course_overview = CourseOverviewFactory() + # Little sanity check making sure our Factory has the right class type + # for the course key + assert isinstance(self.course_overview.id, CourseKey) + + def assert_construction(self, course): + assert course.course_key == self.course_overview.id + assert course.course_id == str(self.course_overview.id) + assert course.site == get_site_for_course(self.course_overview.id) + + def simple_test_property_course_id_association(self, factory_class, property_name): + """Helper method to DRY tests for property methods on simple associations + + This method can be used to test the Course class property methods return + expected results for property methods that return objects that can be + associated with a course with just the course id. Initially implemented + to test the enrollments and student modules property methods. Could also + be used to test for other models like CourseAccessGroup, CourseAccessRole, + Figures CourseDailyMetrics, + EnrollmentData, or LearnerCourseGradeMetrics if we wanted to implement + property methods for those + + If we did use this for models that use a string course id instead of a + CourseKey, then we'll need to make sure the handled course id form is + used. We could do this with a method parameter that either casts to the + expected form or defines which one to use and we can explicitly cast in + this method + """ + our_objects = [factory_class(course_id=self.course_overview.id) + for _ in range(2)] + other_object = factory_class() + assert not other_object.course_id == self.course_overview.id + course = Course(self.course_overview.id) + assert set(our_objects) == set(getattr(course, property_name)) + + def test_str_constructor(self): + self.assert_construction(Course(str(self.course_overview.id))) + + def test_course_key_constructor(self): + self.assert_construction(Course(self.course_overview.id)) + + def test_enrollments(self): + self.simple_test_property_course_id_association(CourseEnrollmentFactory, + 'enrollments') + + def test_student_modules(self): + self.simple_test_property_course_id_association(StudentModuleFactory, + 'student_modules') + + def test_student_modules_active_on_date(self): + our_date_for = fake.date_this_year() + our_created_sm = [StudentModuleFactory(course_id=self.course_overview.id, + created=our_date_for) for _ in range(2)] + our_modified_sm = [StudentModuleFactory(course_id=self.course_overview.id, + modified=our_date_for) for _ in range(2)] + # Create record with a different date + StudentModuleFactory(course_id=self.course_overview.id, + created=days_from(our_date_for, -2), + modified=days_from(our_date_for, -1)) + course = Course(self.course_overview.id) + found_sm = course.student_modules_active_on_date(our_date_for) + assert set(our_created_sm + our_modified_sm) == set(found_sm) + + def test_enrollments_active_on_date(self): + our_date_for = fake.date_this_year() + other_date_for = days_from(our_date_for, -1) + our_ce = [CourseEnrollmentFactory(course_id=self.course_overview.id) + for _ in range(2)] + our_sm = [] + for ce in our_ce: + our_sm.extend([StudentModuleFactory(course_id=ce.course_id, + student=ce.user, + modified=our_date_for), + StudentModuleFactory(course_id=ce.course_id, + student=ce.user, + created=our_date_for)]) + # Create enrollment we should not get in our results + other_ce = CourseEnrollmentFactory(course_id=self.course_overview.id) + StudentModuleFactory.from_course_enrollment(other_ce, + created=other_date_for, + modified=other_date_for) + course = Course(self.course_overview.id) + found_ce = course.enrollments_active_on_date(our_date_for) + assert set(found_ce) == set(our_ce)