diff --git a/figures/pipeline/course_daily_metrics.py b/figures/pipeline/course_daily_metrics.py index 9bc4e2a0..d301b691 100644 --- a/figures/pipeline/course_daily_metrics.py +++ b/figures/pipeline/course_daily_metrics.py @@ -28,6 +28,7 @@ from figures.models import CourseDailyMetrics, PipelineError from figures.pipeline.logger import log_error import figures.pipeline.loaders +from figures.pipeline.enrollment_metrics import bulk_calculate_course_progress_data from figures.serializers import CourseIndexSerializer from figures.compat import GeneratedCertificate import figures.sites @@ -81,7 +82,7 @@ def get_active_learner_ids_today(course_id, date_for): ).values_list('student__id', flat=True).distinct() -def get_average_progress(course_id, date_for, course_enrollments): +def get_average_progress_deprecated(course_id, date_for, course_enrollments): """Collects and aggregates raw course grades data """ progress = [] @@ -225,6 +226,9 @@ def extract(self, course_id, date_for=None, **_kwargs): average_days_to_complete=data.get('average_days_to_complete, None'), num_learners_completed=data['num_learners_completed'], ) + TODO: refactor this class + Add lazy loading method to load course enrollments + - Create a method for each metric field """ # Update args if not assigned @@ -247,18 +251,23 @@ def extract(self, course_id, date_for=None, **_kwargs): # we can do a lambda for course_enrollments to get the count data['enrollment_count'] = course_enrollments.count() + active_learner_ids_today = get_active_learner_ids_today( course_id, date_for,) if active_learner_ids_today: active_learners_today = active_learner_ids_today.count() else: active_learners_today = 0 - data['active_learners_today'] = active_learners_today - data['average_progress'] = get_average_progress( - course_id, date_for, course_enrollments,) + + # Average progress + progress_data = bulk_calculate_course_progress_data(course_id=course_id, + date_for=date_for) + data['average_progress'] = progress_data['average_progress'] + data['average_days_to_complete'] = get_average_days_to_complete( course_id, date_for,) + data['num_learners_completed'] = get_num_learners_completed( course_id, date_for,) diff --git a/tests/pipeline/test_course_daily_metrics.py b/tests/pipeline/test_course_daily_metrics.py index d898c8f7..daa687d6 100644 --- a/tests/pipeline/test_course_daily_metrics.py +++ b/tests/pipeline/test_course_daily_metrics.py @@ -158,7 +158,7 @@ def test_get_active_learner_ids_today(self): course_id=self.course_overview.id, date_for=self.today) assert recs.count() == len(self.course_enrollments) - def test_get_average_progress(self): + def test_get_average_progress_deprecated(self): """ [John] This test needs work. The function it is testing needs work too for testability. We don't want to reproduce the function's behavior, we @@ -167,7 +167,7 @@ def test_get_average_progress(self): """ course_enrollments = CourseEnrollment.objects.filter( course_id=self.course_overview.id) - actual = pipeline_cdm.get_average_progress( + actual = pipeline_cdm.get_average_progress_deprecated( course_id=self.course_overview.id, date_for=self.today, course_enrollments=course_enrollments @@ -183,13 +183,13 @@ def test_get_average_progress(self): 'figures.metrics.LearnerCourseGrades.course_progress', side_effect=PermissionDenied('mock-failure') ) - def test_get_average_progress_error(self, mock_lcg): + def test_get_average_progress_deprecated_has_error(self, mock_lcg): assert PipelineError.objects.count() == 0 course_enrollments = CourseEnrollment.objects.filter( course_id=self.course_overview.id) - results = pipeline_cdm.get_average_progress( + results = pipeline_cdm.get_average_progress_deprecated( course_id=self.course_overview.id, date_for=self.today, course_enrollments=course_enrollments @@ -236,8 +236,12 @@ def setup(self, db): self.course_enrollments = [CourseEnrollmentFactory() for i in range(1, 5)] self.student_module = StudentModuleFactory() - def test_extract(self): + def test_extract(self, monkeypatch): course_id = self.course_enrollments[0].course_id + monkeypatch.setattr(figures.pipeline.course_daily_metrics, + 'bulk_calculate_course_progress_data', + lambda **_kwargs: dict(average_progress=0.5)) + results = pipeline_cdm.CourseDailyMetricsExtractor().extract(course_id) assert results @@ -298,14 +302,11 @@ def test_load_existing(self): @pytest.mark.parametrize('average_progress', [-1.0, -0.01, 1.01]) def test_load_invalid_data(self, monkeypatch, average_progress): - def mock_get_average_progress(course_id, date_for, course_enrollments): - return average_progress - course_id = self.course_enrollments[0].course_id - monkeypatch.setattr( - figures.pipeline.course_daily_metrics, - 'get_average_progress', - mock_get_average_progress) + monkeypatch.setattr(figures.pipeline.course_daily_metrics, + 'bulk_calculate_course_progress_data', + lambda **_kwargs: dict(average_progress=average_progress)) + with pytest.raises(ValidationError) as execinfo: assert CourseDailyMetrics.objects.count() == 0 cdm, created = pipeline_cdm.CourseDailyMetricsLoader(course_id).load()