From 806114a3ef7820b426336b85695a89cd9d475a4c Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Wed, 25 Oct 2017 15:57:57 -0400 Subject: [PATCH] Check org before course deadline config w/ tests --- common/djangoapps/student/models.py | 13 ++- lms/djangoapps/courseware/models.py | 46 +++++---- .../courseware/tests/test_date_summary.py | 95 +++++++++++++++++-- .../tests/test_send_upgrade_reminder.py | 20 ++-- openedx/core/djangoapps/schedules/signals.py | 17 +++- 5 files changed, 151 insertions(+), 40 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 39fd943eff..a20f25f95d 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -50,7 +50,11 @@ import request_cache from student.signals import UNENROLL_DONE, ENROLL_STATUS_CHANGE, REFUND_ORDER, ENROLLMENT_TRACK_UPDATED from certificates.models import GeneratedCertificate from course_modes.models import CourseMode -from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration +from courseware.models import ( + CourseDynamicUpgradeDeadlineConfiguration, + DynamicUpgradeDeadlineConfiguration, + OrgDynamicUpgradeDeadlineConfiguration +) from enrollment.api import _default_course_mode from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -1740,7 +1744,12 @@ class CourseEnrollment(models.Model): return None course_config = CourseDynamicUpgradeDeadlineConfiguration.current(self.course_id) - if course_config.enabled and course_config.opt_out: + if course_config.opted_out(): + # Course-level config should be checked first since it overrides the org-level config + return None + + org_config = OrgDynamicUpgradeDeadlineConfiguration.current(self.course_id.org) + if org_config.opted_out() and not course_config.opted_in(): return None try: diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index d1eba09d08..c0f855e123 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -379,47 +379,45 @@ class DynamicUpgradeDeadlineConfiguration(ConfigurationModel): ) -class CourseDynamicUpgradeDeadlineConfiguration(ConfigurationModel): +class OptOutDynamicUpgradeDeadlineConfiguration(DynamicUpgradeDeadlineConfiguration): + """ Dynamic upgrade deadline configuration with opt-out switch. + + This is an abstract model that both CourseDynamicUpgradeDeadlineConfiguration and + OrgDynamicUpgradeDeadlineConfiguration inherit. + """ + opt_out = models.BooleanField( + default=False, + help_text=_('Disable the dynamic upgrade deadline for this course run.') + ) + + def opted_in(self): + """Convienence function that returns True if this config model is both enabled and opt_out is False""" + return self.enabled and not self.opt_out + + def opted_out(self): + """Convienence function that returns True if this config model is both enabled and opt_out is True""" + return self.enabled and self.opt_out + + +class CourseDynamicUpgradeDeadlineConfiguration(OptOutDynamicUpgradeDeadlineConfiguration): """ Per-course run configuration for dynamic upgrade deadlines. This model controls dynamic upgrade deadlines on a per-course run level, allowing course runs to have different deadlines or opt out of the functionality altogether. """ - class Meta(object): - app_label = 'courseware' - KEY_FIELDS = ('course_id',) course_id = CourseKeyField(max_length=255, db_index=True) - deadline_days = models.PositiveSmallIntegerField( - default=21, - help_text=_('Number of days a learner has to upgrade after content is made available') - ) - opt_out = models.BooleanField( - default=False, - help_text=_('Disable the dynamic upgrade deadline for this course run.') - ) -class OrgDynamicUpgradeDeadlineConfiguration(ConfigurationModel): +class OrgDynamicUpgradeDeadlineConfiguration(OptOutDynamicUpgradeDeadlineConfiguration): """ Per-org configuration for dynamic upgrade deadlines. This model controls dynamic upgrade deadlines on a per-org level, allowing organizations to have different deadlines or opt out of the functionality altogether. """ - class Meta(object): - app_label = 'courseware' - KEY_FIELDS = ('org_id',) org_id = models.CharField(max_length=255, db_index=True) - deadline_days = models.PositiveSmallIntegerField( - default=21, - help_text=_('Number of days a learner has to upgrade after content is made available') - ) - opt_out = models.BooleanField( - default=False, - help_text=_('Disable the dynamic upgrade deadline for this organization.') - ) diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index 3dcb787e39..052cea4f83 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -24,10 +24,15 @@ from courseware.date_summary import ( VerifiedUpgradeDeadlineDate, CertificateAvailableDate ) -from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration +from courseware.models import ( + CourseDynamicUpgradeDeadlineConfiguration, + DynamicUpgradeDeadlineConfiguration, + OrgDynamicUpgradeDeadlineConfiguration +) from lms.djangoapps.verify_student.models import VerificationDeadline from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.schedules.signals import CREATE_SCHEDULE_WAFFLE_FLAG from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory @@ -562,6 +567,7 @@ class TestDateAlerts(SharedModuleStoreTestCase): self.assertEqual(len(messages), 0) +@ddt.ddt @attr(shard=1) class TestScheduleOverrides(SharedModuleStoreTestCase): @@ -599,15 +605,28 @@ class TestScheduleOverrides(SharedModuleStoreTestCase): @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) def test_date_with_self_paced_with_enrollment_after_course_start(self): """ Enrolling after a course begins should result in the upgrade deadline being set relative to the - enrollment date. """ + enrollment date. + + Additionally, OrgDynamicUpgradeDeadlineConfiguration should override the number of days until the deadline, + and CourseDynamicUpgradeDeadlineConfiguration should override the org-level override. + """ global_config = DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) - course = create_self_paced_course_run(days_till_start=-1) + course = create_self_paced_course_run(days_till_start=-1, org_id='TestOrg') enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) block = VerifiedUpgradeDeadlineDate(course, enrollment.user) expected = enrollment.created + timedelta(days=global_config.deadline_days) self.assertEqual(block.date, expected) - # Courses should be able to override the deadline + # Orgs should be able to override the deadline + org_config = OrgDynamicUpgradeDeadlineConfiguration.objects.create( + enabled=True, org_id=course.org, deadline_days=4 + ) + enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) + block = VerifiedUpgradeDeadlineDate(course, enrollment.user) + expected = enrollment.created + timedelta(days=org_config.deadline_days) + self.assertEqual(block.date, expected) + + # Courses should be able to override the deadline (and the org-level override) course_config = CourseDynamicUpgradeDeadlineConfiguration.objects.create( enabled=True, course_id=course.id, deadline_days=3 ) @@ -650,6 +669,68 @@ class TestScheduleOverrides(SharedModuleStoreTestCase): block = VerifiedUpgradeDeadlineDate(course, enrollment.user) self.assertEqual(block.date, expected) + @ddt.data( + # (enroll before configs, org enabled, org opt-out, course enabled, course opt-out, expected dynamic deadline) + (False, False, False, False, False, True), + (False, False, False, False, True, True), + (False, False, False, True, False, True), + (False, False, False, True, True, False), + (False, False, True, False, False, True), + (False, False, True, False, True, True), + (False, False, True, True, False, True), + (False, False, True, True, True, False), + (False, True, False, False, False, True), + (False, True, False, False, True, True), + (False, True, False, True, False, True), + (False, True, False, True, True, False), # course-level overrides org-level + (False, True, True, False, False, False), + (False, True, True, False, True, False), + (False, True, True, True, False, True), # course-level overrides org-level + (False, True, True, True, True, False), + + (True, False, False, False, False, True), + (True, False, False, False, True, True), + (True, False, False, True, False, True), + (True, False, False, True, True, False), + (True, False, True, False, False, True), + (True, False, True, False, True, True), + (True, False, True, True, False, True), + (True, False, True, True, True, False), + (True, True, False, False, False, True), + (True, True, False, False, True, True), + (True, True, False, True, False, True), + (True, True, False, True, True, False), # course-level overrides org-level + (True, True, True, False, False, False), + (True, True, True, False, True, False), + (True, True, True, True, False, True), # course-level overrides org-level + (True, True, True, True, True, False), + ) + @ddt.unpack + @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) + def test_date_with_org_and_course_config_overrides(self, enroll_first, org_config_enabled, org_config_opt_out, + course_config_enabled, course_config_opt_out, + expected_dynamic_deadline): + """ Runs through every combination of org-level plus course-level DynamicUpgradeDeadlineConfiguration enabled + and opt-out states to verify that course-level overrides the org-level config. """ + course = create_self_paced_course_run(days_till_start=-1, org_id='TestOrg') + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + if enroll_first: + enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT, course__self_paced=True) + OrgDynamicUpgradeDeadlineConfiguration.objects.create( + enabled=org_config_enabled, opt_out=org_config_opt_out, org_id=course.id.org + ) + CourseDynamicUpgradeDeadlineConfiguration.objects.create( + enabled=course_config_enabled, opt_out=course_config_opt_out, course_id=course.id + ) + if not enroll_first: + enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT, course__self_paced=True) + + # The enrollment has a schedule, and the upgrade_deadline is set when expected_dynamic_deadline is True + if not enroll_first: + self.assertEqual(enrollment.schedule.upgrade_deadline is not None, expected_dynamic_deadline) + # The CourseEnrollment.upgrade_deadline property method is checking the configs + self.assertEqual(enrollment.dynamic_upgrade_deadline is not None, expected_dynamic_deadline) + def create_user(verification_status=None): """ Create a new User instance. @@ -705,7 +786,7 @@ def create_course_run( return course -def create_self_paced_course_run(days_till_start=1): +def create_self_paced_course_run(days_till_start=1, org_id=None): """ Create a new course run and course modes. All date-related arguments are relative to the current date-time (now) unless otherwise specified. @@ -714,9 +795,11 @@ def create_self_paced_course_run(days_till_start=1): Arguments: days_till_start (int): Number of days until the course starts. + org_id (string): String org id to assign the course to (default: None; use CourseFactory default) """ now = datetime.now(utc) - course = CourseFactory.create(start=now + timedelta(days=days_till_start), self_paced=True) + course = CourseFactory.create(start=now + timedelta(days=days_till_start), self_paced=True, + org=org_id if org_id else 'TestedX') CourseModeFactory( course_id=course.id, diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py index 95a9bc3f5c..6fb9963c84 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py @@ -139,6 +139,7 @@ class TestUpgradeReminder(SharedModuleStoreTestCase): bins_in_use = frozenset((self._calculate_bin_for_user(s.enrollment.user)) for s in schedules) is_first_match = True course_switch_queries = len(set(s.enrollment.course.id for s in schedules)) + org_switch_queries = len(set(s.enrollment.course.id.org for s in schedules)) test_datetime = upgrade_deadline test_datetime_str = serialize(test_datetime) @@ -151,7 +152,7 @@ class TestUpgradeReminder(SharedModuleStoreTestCase): # Since this is the first match, we need to cache all of the config models, so we run a query # for each of those... NUM_QUERIES_FIRST_MATCH - + course_switch_queries + + course_switch_queries + org_switch_queries ) is_first_match = False else: @@ -257,7 +258,8 @@ class TestUpgradeReminder(SharedModuleStoreTestCase): test_datetime_str = serialize(test_datetime) course_switch_queries = 1 - expected_queries = NUM_QUERIES_FIRST_MATCH + course_switch_queries + org_switch_queries = 1 + expected_queries = NUM_QUERIES_FIRST_MATCH + course_switch_queries + org_switch_queries if not this_org_list: expected_queries += NUM_QUERIES_NO_ORG_LIST @@ -283,11 +285,14 @@ class TestUpgradeReminder(SharedModuleStoreTestCase): for course_num in (1, 2, 3) ] - num_courses = len(set(s.enrollment.course.id for s in schedules)) + course_switch_queries = len(set(s.enrollment.course.id for s in schedules)) + org_switch_queries = len(set(s.enrollment.course.id.org for s in schedules)) test_datetime = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - expected_query_count = NUM_QUERIES_FIRST_MATCH + num_courses + NUM_QUERIES_NO_ORG_LIST + expected_query_count = ( + NUM_QUERIES_FIRST_MATCH + course_switch_queries + org_switch_queries + NUM_QUERIES_NO_ORG_LIST + ) with self.assertNumQueries(expected_query_count, table_blacklist=WAFFLE_TABLES): tasks.ScheduleUpgradeReminder.apply(kwargs=dict( site_id=self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, @@ -320,7 +325,8 @@ class TestUpgradeReminder(SharedModuleStoreTestCase): expiration_datetime=future_datetime ) - num_courses = len(set(s.enrollment.course.id for s in schedules)) + course_switch_queries = len(set(s.enrollment.course.id for s in schedules)) + org_switch_queries = len(set(s.enrollment.course.id.org for s in schedules)) test_datetime = future_datetime test_datetime_str = serialize(test_datetime) @@ -339,7 +345,9 @@ class TestUpgradeReminder(SharedModuleStoreTestCase): mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args) # we execute one query per course to see if it's opted out of dynamic upgrade deadlines - num_expected_queries = NUM_QUERIES_FIRST_MATCH + NUM_QUERIES_NO_ORG_LIST + num_courses + num_expected_queries = ( + NUM_QUERIES_FIRST_MATCH + NUM_QUERIES_NO_ORG_LIST + course_switch_queries + org_switch_queries + ) with self.assertNumQueries(num_expected_queries, table_blacklist=WAFFLE_TABLES): tasks.ScheduleUpgradeReminder.apply(kwargs=dict( diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index 685118aa64..1af925f22e 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -6,7 +6,11 @@ from django.dispatch import receiver from django.utils import timezone from course_modes.models import CourseMode -from courseware.models import DynamicUpgradeDeadlineConfiguration, CourseDynamicUpgradeDeadlineConfiguration +from courseware.models import ( + CourseDynamicUpgradeDeadlineConfiguration, + DynamicUpgradeDeadlineConfiguration, + OrgDynamicUpgradeDeadlineConfiguration +) from edx_ace.utils import date from openedx.core.djangoapps.signals.signals import COURSE_START_DATE_CHANGED from openedx.core.djangoapps.theming.helpers import get_current_site @@ -110,9 +114,18 @@ def _get_upgrade_deadline_delta_setting(course_id): # Use the default from this model whether or not the feature is enabled delta = global_config.deadline_days + # Check if the org has a deadline + org_config = OrgDynamicUpgradeDeadlineConfiguration.current(course_id.org) + if org_config.opted_in(): + delta = org_config.deadline_days + elif org_config.opted_out(): + delta = None + # Check if the course has a deadline course_config = CourseDynamicUpgradeDeadlineConfiguration.current(course_id) - if course_config.enabled and not course_config.opt_out: + if course_config.opted_in(): delta = course_config.deadline_days + elif course_config.opted_out(): + delta = None return delta