diff --git a/figures/management/commands/populate_figures_metrics.py b/figures/management/commands/populate_figures_metrics.py index ec61cba0..dfbfe425 100644 --- a/figures/management/commands/populate_figures_metrics.py +++ b/figures/management/commands/populate_figures_metrics.py @@ -45,6 +45,10 @@ def add_arguments(self, parser): action='store_true', default=False, help='Run just the MAU pipeline') + parser.add_argument('--active-courses', + action='store_true', + default=False, + help='Populate only active courses') def handle(self, *args, **options): ''' @@ -78,9 +82,9 @@ def handle(self, *args, **options): experimental_populate_daily_metrics.delay(**kwargs) # pragma: no cover else: if options['no_delay']: - populate_daily_metrics(**kwargs) + populate_daily_metrics(active_courses=options['active_courses'], **kwargs) else: - populate_daily_metrics.delay(**kwargs) # pragma: no cover + populate_daily_metrics.delay(active_courses=options['active_courses'], **kwargs) # pragma: no cover # TODO: improve this message to say 'today' when options['date'] is None print('Management command populate_figures_metrics complete. date_for: {}'.format( diff --git a/figures/sites.py b/figures/sites.py index aa31261f..7a3b912e 100644 --- a/figures/sites.py +++ b/figures/sites.py @@ -11,6 +11,8 @@ """ from __future__ import absolute_import + +from datetime import datetime from django.contrib.auth import get_user_model from django.contrib.sites.models import Site from django.conf import settings @@ -168,17 +170,23 @@ def get_course_keys_for_sites_slugs(site_slugs): return [as_course_key(cid) for cid in course_ids] -def get_course_keys_for_site(site): +def get_course_keys_for_site(site, active_courses=False): if figures.helpers.is_multisite(): edx_orgs = EdlySubOrganization.objects.filter(lms_site=site).using(read_replica_or_default()).values_list( 'edx_organizations', flat=True) org_courses = organizations.models.OrganizationCourse.objects.filter(organization__in=edx_orgs).using( read_replica_or_default()) + if active_courses: + org_courses = org_courses.filter(active=True) + course_ids = org_courses.values_list('course_id', flat=True) else: - course_ids = CourseOverview.objects.using( - read_replica_or_default()).all().values_list('id', flat=True) + course_ids = CourseOverview.objects.using(read_replica_or_default()).all() + if active_courses: + course_ids = course_ids.filter(end_date__gte=datetime.now()) + + course_ids = course_ids.values_list('id', flat=True) return [as_course_key(cid) for cid in course_ids] @@ -196,16 +204,19 @@ def site_course_ids(site): 'id', flat=True)] -def get_courses_for_site(site): +def get_courses_for_site(site, active_courses=False): """Returns the courses accessible by the user on the site This function relies on Appsembler's fork of edx-organizations """ if figures.helpers.is_multisite(): - course_keys = get_course_keys_for_site(site) + course_keys = get_course_keys_for_site(site, active_courses) courses = CourseOverview.objects.filter(id__in=course_keys).using(read_replica_or_default()) else: courses = CourseOverview.objects.using(read_replica_or_default()).all() + if active_courses: + courses = courses.filter(active=True, end_date__gte=datetime.now()) + return courses diff --git a/figures/tasks.py b/figures/tasks.py index ef03466b..fc4906cf 100644 --- a/figures/tasks.py +++ b/figures/tasks.py @@ -167,7 +167,7 @@ def update_learners_progress_for_course(course): @shared_task -def populate_daily_metrics(date_for=None, force_update=False): +def populate_daily_metrics(date_for=None, force_update=False, active_courses=False): '''Populates the daily metrics models for the given date This method populates CourseDailyMetrics for all the courses in the site, @@ -198,7 +198,7 @@ def populate_daily_metrics(date_for=None, force_update=False): sites_count = len(lms_sites) for i, site in enumerate(Site.objects.using(read_replica_or_default()).filter(id__in=lms_sites)): try: - courses = figures.sites.get_courses_for_site(site) + courses = figures.sites.get_courses_for_site(site, active_courses) except Exception: # pylint: disable=broad-except courses = [] msg = ('FIGURES:FAIL populate_daily_metrics unhandled site level' diff --git a/tests/test_tasks.py b/tests/test_tasks.py index a7fae306..19d9a103 100644 --- a/tests/test_tasks.py +++ b/tests/test_tasks.py @@ -82,7 +82,7 @@ def test_populate_daily_metrics_site_level_error(transactional_db, error_message = dict(message=[u'expected failure']) assert not CourseOverview.objects.count() - def mock_get_courses_fail(site): + def mock_get_courses_fail(site, active_courses=False): raise Exception(error_message) assert SiteDailyMetrics.objects.count() == 0 @@ -92,9 +92,13 @@ def mock_get_courses_fail(site): figures.tasks.populate_daily_metrics(date_for=date_for) - last_log = caplog.records[0] - assert last_log.message.startswith( - 'FIGURES:FAIL populate_daily_metrics unhandled site level exception for site') + error_found = False + for record in caplog.records: + if record.message.startswith('FIGURES:FAIL populate_daily_metrics unhandled site level exception for site'): + error_found = True + break + + assert error_found, "Expected error message not found in logs" @pytest.mark.skipif(OPENEDX_RELEASE == GINKGO, @@ -105,7 +109,7 @@ def test_populate_daily_metrics_error(transactional_db, monkeypatch): error_message = dict(message=[u'expected failure']) assert not CourseOverview.objects.count() - def mock_get_courses(site): + def mock_get_courses(site, active_courses=False): CourseOverviewFactory() return CourseOverview.objects.all() @@ -142,7 +146,7 @@ def test_populate_daily_metrics_enrollment_data_error(transactional_db, error_message = dict(message=[u'expected failure']) assert not CourseOverview.objects.count() - def mock_get_courses(site): + def mock_get_courses(site, active_courses=False): CourseOverviewFactory() return CourseOverview.objects.all() @@ -167,10 +171,15 @@ def mock_update_learners_progress_for_course(course): figures.tasks, 'update_enrollment_data', mock_update_enrollment_data_fails) monkeypatch.setattr( figures.tasks, 'update_learners_progress_for_course', mock_update_learners_progress_for_course) - figures.tasks.populate_daily_metrics(date_for=date_for) - last_log = caplog.records[-1] - assert last_log.message.startswith( - 'FIGURES:FAIL figures.tasks update_enrollment_data') + figures.tasks.populate_daily_metrics(date_for=date_for, active_courses=False) + + error_found = False + for record in caplog.records: + if record.message.startswith('FIGURES:FAIL figures.tasks update_enrollment_data'): + error_found = True + break + + assert error_found, "Expected error message not found in logs" @pytest.mark.skipif(OPENEDX_RELEASE == GINKGO,