From 986afbfa383e2e6bcebfe801c86713f2bae1b9de Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Wed, 2 Aug 2017 16:26:16 -0400 Subject: [PATCH] Powering courseware deadline with schedules --- .../djangoapps/student/tests/test_models.py | 9 ++- .../tests/test_field_override_performance.py | 54 +++++++------- lms/djangoapps/courseware/date_summary.py | 38 +--------- .../courseware/tests/test_date_summary.py | 8 ++- lms/djangoapps/courseware/tests/test_views.py | 10 +-- lms/djangoapps/courseware/testutils.py | 4 +- openedx/core/djangoapps/schedules/__init__.py | 1 + openedx/core/djangoapps/schedules/apps.py | 11 +++ openedx/core/djangoapps/schedules/signals.py | 70 +++++++++++++++++++ .../schedules/tests/test_signals.py | 24 +++++++ .../tests/views/test_course_home.py | 2 +- .../tests/views/test_course_updates.py | 2 +- 12 files changed, 157 insertions(+), 76 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/apps.py create mode 100644 openedx/core/djangoapps/schedules/signals.py create mode 100644 openedx/core/djangoapps/schedules/tests/test_signals.py diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 34c6cef9fe..ad8d68d071 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -4,12 +4,15 @@ import datetime import hashlib import ddt +import factory import pytz from django.contrib.auth.models import AnonymousUser from django.core.cache import cache +from django.db.models import signals from django.db.models.functions import Lower from course_modes.models import CourseMode +from openedx.core.djangoapps.schedules.models import Schedule from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from openedx.core.djangolib.testing.utils import skip_unless_lms from student.models import CourseEnrollment @@ -112,14 +115,18 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): self.assertListEqual([self.user, self.user_2], all_enrolled_users) @skip_unless_lms + # NOTE: We mute the post_save signal to prevent Schedules from being created for new enrollments + @factory.django.mute_signals(signals.post_save) def test_upgrade_deadline(self): """ The property should use either the CourseMode or related Schedule to determine the deadline. """ course_mode = CourseModeFactory( course_id=self.course.id, mode_slug=CourseMode.VERIFIED, - expiration_datetime=datetime.datetime.now(pytz.UTC) + # This must be in the future to ensure it is returned by downstream code. + expiration_datetime=datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1) ) enrollment = CourseEnrollmentFactory(course_id=self.course.id, mode=CourseMode.AUDIT) + self.assertEqual(Schedule.objects.all().count(), 0) self.assertEqual(enrollment.upgrade_deadline, course_mode.expiration_datetime) # The schedule's upgrade deadline should be used if a schedule exists diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 0e0f54dd38..2e68180ab0 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -237,18 +237,18 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): # # of sql queries to default, # # of mongo queries, # ) - ('no_overrides', 1, True, False): (24, 1), - ('no_overrides', 2, True, False): (24, 1), - ('no_overrides', 3, True, False): (24, 1), - ('ccx', 1, True, False): (24, 1), - ('ccx', 2, True, False): (24, 1), - ('ccx', 3, True, False): (24, 1), - ('no_overrides', 1, False, False): (24, 1), - ('no_overrides', 2, False, False): (24, 1), - ('no_overrides', 3, False, False): (24, 1), - ('ccx', 1, False, False): (24, 1), - ('ccx', 2, False, False): (24, 1), - ('ccx', 3, False, False): (24, 1), + ('no_overrides', 1, True, False): (25, 1), + ('no_overrides', 2, True, False): (25, 1), + ('no_overrides', 3, True, False): (25, 1), + ('ccx', 1, True, False): (25, 1), + ('ccx', 2, True, False): (25, 1), + ('ccx', 3, True, False): (25, 1), + ('no_overrides', 1, False, False): (25, 1), + ('no_overrides', 2, False, False): (25, 1), + ('no_overrides', 3, False, False): (25, 1), + ('ccx', 1, False, False): (25, 1), + ('ccx', 2, False, False): (25, 1), + ('ccx', 3, False, False): (25, 1), } @@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (24, 3), - ('no_overrides', 2, True, False): (24, 3), - ('no_overrides', 3, True, False): (24, 3), - ('ccx', 1, True, False): (24, 3), - ('ccx', 2, True, False): (24, 3), - ('ccx', 3, True, False): (24, 3), - ('ccx', 1, True, True): (25, 3), - ('ccx', 2, True, True): (25, 3), - ('ccx', 3, True, True): (25, 3), - ('no_overrides', 1, False, False): (24, 3), - ('no_overrides', 2, False, False): (24, 3), - ('no_overrides', 3, False, False): (24, 3), - ('ccx', 1, False, False): (24, 3), - ('ccx', 2, False, False): (24, 3), - ('ccx', 3, False, False): (24, 3), + ('no_overrides', 1, True, False): (25, 3), + ('no_overrides', 2, True, False): (25, 3), + ('no_overrides', 3, True, False): (25, 3), + ('ccx', 1, True, False): (25, 3), + ('ccx', 2, True, False): (25, 3), + ('ccx', 3, True, False): (25, 3), + ('ccx', 1, True, True): (26, 3), + ('ccx', 2, True, True): (26, 3), + ('ccx', 3, True, True): (26, 3), + ('no_overrides', 1, False, False): (25, 3), + ('no_overrides', 2, False, False): (25, 3), + ('no_overrides', 3, False, False): (25, 3), + ('ccx', 1, False, False): (25, 3), + ('ccx', 2, False, False): (25, 3), + ('ccx', 3, False, False): (25, 3), } diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index a8f80fb430..4b381093d6 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -14,10 +14,8 @@ from lazy import lazy from pytz import timezone, utc from course_modes.models import CourseMode -from courseware.models import CourseDynamicUpgradeDeadlineConfiguration, DynamicUpgradeDeadlineConfiguration from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification, VerificationDeadline -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment @@ -224,10 +222,6 @@ class VerifiedUpgradeDeadlineDate(DateSummary): def enrollment(self): return CourseEnrollment.get_enrollment(self.user, self.course_id) - @cached_property - def course_overview(self): - return CourseOverview.get_from_id(self.course_id) - @property def is_enabled(self): """ @@ -258,36 +252,8 @@ class VerifiedUpgradeDeadlineDate(DateSummary): def date(self): deadline = None - try: - verified_mode = CourseMode.objects.get(course_id=self.course_id, mode_slug=CourseMode.VERIFIED) - deadline = verified_mode.expiration_datetime - except CourseMode.DoesNotExist: - pass - - if self.course and self.course_overview.self_paced and self.enrollment: - global_config = DynamicUpgradeDeadlineConfiguration.current() - if global_config.enabled: - delta = global_config.deadline_days - - # Check if the given course has opted out of the feature - course_config = CourseDynamicUpgradeDeadlineConfiguration.current(self.course.id) - if course_config.enabled: - if course_config.opt_out: - return deadline - - delta = course_config.deadline_days - - # This represents the first date at which the learner can access the content. This will be the - # latter of either the enrollment date or the course's start date. - content_availability_date = max(self.enrollment.created, self.course_overview.start) - user_deadline = content_availability_date + datetime.timedelta(days=delta) - - # If the deadline from above is None, make sure we have a value for comparison - deadline = deadline or datetime.date.max - - # The user-specific deadline should never occur after the verified mode's expiration date, - # if one is set. - deadline = min(deadline, user_deadline) + if self.enrollment: + deadline = self.enrollment.upgrade_deadline return deadline diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 83168c3d45..ef59e085fc 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -3,6 +3,7 @@ from datetime import datetime, timedelta import ddt +import waffle from django.core.urlresolvers import reverse from freezegun import freeze_time from nose.plugins.attrib import attr @@ -34,6 +35,7 @@ from xmodule.modulestore.tests.factories import CourseFactory @attr(shard=1) @ddt.ddt +@waffle.testutils.override_switch('schedules.enable-create-schedule-receiver', True) class CourseDateSummaryTest(SharedModuleStoreTestCase): """Tests for course date summary blocks.""" @@ -171,12 +173,12 @@ class CourseDateSummaryTest(SharedModuleStoreTestCase): @ddt.data( # Course not started - ({}, (CourseStartDate, TodaysDate, CourseEndDate, VerifiedUpgradeDeadlineDate)), + ({}, (CourseStartDate, TodaysDate, CourseEndDate)), # Course active - ({'days_till_start': -1}, (TodaysDate, CourseEndDate, VerifiedUpgradeDeadlineDate)), + ({'days_till_start': -1}, (TodaysDate, CourseEndDate)), # Course ended ({'days_till_start': -10, 'days_till_end': -5}, - (TodaysDate, CourseEndDate, VerifiedUpgradeDeadlineDate)), + (TodaysDate, CourseEndDate)), ) @ddt.unpack def test_enabled_block_types_without_enrollment(self, course_kwargs, expected_blocks): diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index b54855b36e..57a21bb9c6 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -211,8 +211,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 145), - (ModuleStoreEnum.Type.split, 4, 145), + (ModuleStoreEnum.Type.mongo, 10, 146), + (ModuleStoreEnum.Type.split, 4, 146), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1444,12 +1444,12 @@ class ProgressPageTests(ProgressPageBaseTests): """Test that query counts remain the same for self-paced and instructor-paced courses.""" SelfPacedConfiguration(enabled=self_paced_enabled).save() self.setup_course(self_paced=self_paced) - with self.assertNumQueries(41, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): + with self.assertNumQueries(42, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): self._get_progress_page() @ddt.data( - (False, 41, 27), - (True, 34, 23) + (False, 42, 28), + (True, 35, 24) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index 1374e50a27..91ba8d813a 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -148,9 +148,9 @@ class RenderXBlockTestMixin(object): return response @ddt.data( - ('vertical_block', ModuleStoreEnum.Type.mongo, 14), + ('vertical_block', ModuleStoreEnum.Type.mongo, 10), ('vertical_block', ModuleStoreEnum.Type.split, 6), - ('html_block', ModuleStoreEnum.Type.mongo, 15), + ('html_block', ModuleStoreEnum.Type.mongo, 11), ('html_block', ModuleStoreEnum.Type.split, 6), ) @ddt.unpack diff --git a/openedx/core/djangoapps/schedules/__init__.py b/openedx/core/djangoapps/schedules/__init__.py index e69de29bb2..1813393f87 100644 --- a/openedx/core/djangoapps/schedules/__init__.py +++ b/openedx/core/djangoapps/schedules/__init__.py @@ -0,0 +1 @@ +default_app_config = 'openedx.core.djangoapps.schedules.apps.SchedulesConfig' diff --git a/openedx/core/djangoapps/schedules/apps.py b/openedx/core/djangoapps/schedules/apps.py new file mode 100644 index 0000000000..87f6f1321c --- /dev/null +++ b/openedx/core/djangoapps/schedules/apps.py @@ -0,0 +1,11 @@ +from django.apps import AppConfig +from django.utils.translation import ugettext_lazy as _ + + +class SchedulesConfig(AppConfig): + name = 'openedx.core.djangoapps.schedules' + verbose_name = _('Schedules') + + def ready(self): + # noinspection PyUnresolvedReferences + from . import signals # pylint: disable=unused-variable diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py new file mode 100644 index 0000000000..13b139a5f1 --- /dev/null +++ b/openedx/core/djangoapps/schedules/signals.py @@ -0,0 +1,70 @@ +import datetime +import logging + +from django.db.models.signals import post_save +from django.dispatch import receiver +from django.utils import timezone + +from course_modes.models import CourseMode +from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace +from student.models import CourseEnrollment +from .models import Schedule + +log = logging.getLogger(__name__) + + +def _get_upgrade_deadline(enrollment): + """ Returns the upgrade deadline for the given enrollment. + + The deadline is determined based on the following data (in priority order): + 1. Course run-specific deadline configuration (CourseDynamicUpgradeDeadlineConfiguration) + 2. Global deadline configuration (DynamicUpgradeDeadlineConfiguration) + 3. Verified course mode expiration + """ + course_key = enrollment.course_id + upgrade_deadline = None + + try: + verified_mode = CourseMode.verified_mode_for_course(course_key) + if verified_mode: + upgrade_deadline = verified_mode.expiration_datetime + except CourseMode.DoesNotExist: + pass + + global_config = DynamicUpgradeDeadlineConfiguration.current() + if global_config.enabled: + delta = global_config.deadline_days + + # Check if the given course has opted out of the feature + course_config = CourseDynamicUpgradeDeadlineConfiguration.current(course_key) + if course_config.enabled: + if course_config.opt_out: + return upgrade_deadline + + delta = course_config.deadline_days + + course_overview = CourseOverview.get_from_id(course_key) + + # This represents the first date at which the learner can access the content. This will be the latter of + # either the enrollment date or the course's start date. + content_availability_date = max(enrollment.created, course_overview.start) + cav_based_deadline = content_availability_date + datetime.timedelta(days=delta) + + # If the deadline from above is None, make sure we have a value for comparison + upgrade_deadline = upgrade_deadline or datetime.date.max + + # The content availability-based deadline should never occur after the verified mode's + # expiration date, if one is set. + upgrade_deadline = min(upgrade_deadline, cav_based_deadline) + + return upgrade_deadline + + +@receiver(post_save, sender=CourseEnrollment, dispatch_uid='create_schedule_for_enrollment') +def create_schedule(sender, **kwargs): + if WaffleSwitchNamespace('schedules').is_enabled('enable-create-schedule-receiver') and kwargs['created']: + enrollment = kwargs['instance'] + upgrade_deadline = _get_upgrade_deadline(enrollment) + Schedule.objects.create(enrollment=enrollment, start=timezone.now(), upgrade_deadline=upgrade_deadline) diff --git a/openedx/core/djangoapps/schedules/tests/test_signals.py b/openedx/core/djangoapps/schedules/tests/test_signals.py new file mode 100644 index 0000000000..5661c73385 --- /dev/null +++ b/openedx/core/djangoapps/schedules/tests/test_signals.py @@ -0,0 +1,24 @@ +from django.test import TestCase + +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace +from openedx.core.djangolib.testing.utils import skip_unless_lms +from student.tests.factories import CourseEnrollmentFactory +from ..models import Schedule + + +@skip_unless_lms +class CreateScheduleTests(TestCase): + def test_create_schedule(self): + """ A schedule should be created for every new enrollment if the switch is active. """ + + SWITCH_NAME = 'enable-create-schedule-receiver' + switch_namesapce = WaffleSwitchNamespace('schedules') + + with switch_namesapce.override(SWITCH_NAME, True): + enrollment = CourseEnrollmentFactory() + self.assertIsNotNone(enrollment.schedule) + + with switch_namesapce.override(SWITCH_NAME, False): + enrollment = CourseEnrollmentFactory() + with self.assertRaises(Schedule.DoesNotExist): + enrollment.schedule diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 6248aa9a9f..a2c398567b 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -160,7 +160,7 @@ class TestCourseHomePage(CourseHomePageTestCase): course_home_url(self.course) # Fetch the view and verify the query counts - with self.assertNumQueries(41, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(42, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index aa7f3f1ec8..2f47917d80 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -127,7 +127,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): course_updates_url(self.course) # Fetch the view and verify that the query counts haven't changed - with self.assertNumQueries(32, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(33, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url)