diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 934c250f2b..c313955a5d 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1700,12 +1700,9 @@ class CourseEnrollment(models.Model): def upgrade_deadline(self): """ Returns the upgrade deadline for this enrollment, if it is upgradeable. - If the seat cannot be upgraded, None is returned. - Note: When loading this model, use `select_related` to retrieve the associated schedule object. - Returns: datetime|None """ @@ -1717,40 +1714,61 @@ class CourseEnrollment(models.Model): ) return None + if self.dynamic_upgrade_deadline is not None: + return self.dynamic_upgrade_deadline + + return self.course_upgrade_deadline + + @cached_property + def dynamic_upgrade_deadline(self): + try: - schedule_driven_deadlines_enabled = ( - DynamicUpgradeDeadlineConfiguration.is_enabled() - or CourseDynamicUpgradeDeadlineConfiguration.is_enabled(self.course_id) + course_overview = self.course + except CourseOverview.DoesNotExist: + course_overview = self.course_overview + + if not course_overview.self_paced: + return None + + if not DynamicUpgradeDeadlineConfiguration.is_enabled(): + return None + + course_config = CourseDynamicUpgradeDeadlineConfiguration.current(self.course_id) + if course_config.enabled and course_config.opt_out: + return None + + try: + if not self.schedule: + return None + + log.debug( + 'Schedules: Pulling upgrade deadline for CourseEnrollment %d from Schedule %d.', + self.id, self.schedule.id ) - if ( - schedule_driven_deadlines_enabled - and self.course_overview.self_paced - and self.schedule - and self.schedule.upgrade_deadline is not None - ): - log.debug( - 'Schedules: Pulling upgrade deadline for CourseEnrollment %d from Schedule %d.', - self.id, self.schedule.id - ) - return self.schedule.upgrade_deadline + upgrade_deadline = self.schedule.upgrade_deadline except ObjectDoesNotExist: # NOTE: Schedule has a one-to-one mapping with CourseEnrollment. If no schedule is associated # with this enrollment, Django will raise an exception rather than return None. log.debug('Schedules: No schedule exists for CourseEnrollment %d.', self.id) - pass + return None + if upgrade_deadline is None or datetime.now(UTC) >= upgrade_deadline: + return None + + return upgrade_deadline + + @cached_property + def course_upgrade_deadline(self): try: if self.verified_mode: log.debug('Schedules: Defaulting to verified mode expiration date-time for %s.', self.course_id) return self.verified_mode.expiration_datetime else: log.debug('Schedules: No verified mode located for %s.', self.course_id) + return None except CourseMode.DoesNotExist: log.debug('Schedules: %s has no verified mode.', self.course_id) - pass - - log.debug('Schedules: Returning default of `None`') - return None + return None def is_verified_enrollment(self): """ diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 9b08398237..d8422e0960 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -14,6 +14,7 @@ from django.db.models.functions import Lower from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory from courseware.models import DynamicUpgradeDeadlineConfiguration +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview 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 @@ -142,9 +143,14 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): course_id=course.id, mode_slug=CourseMode.VERIFIED, # 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), + expiration_datetime=datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=30), + ) + course_overview = CourseOverview.load_from_module_store(course.id) + enrollment = CourseEnrollmentFactory( + course_id=course.id, + mode=CourseMode.AUDIT, + course=course_overview, ) - enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) # The schedule's upgrade deadline should be used if a schedule exists DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 7ed0b0566f..eb5dbde16a 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): (18, 1), - ('no_overrides', 2, True, False): (18, 1), - ('no_overrides', 3, True, False): (18, 1), - ('ccx', 1, True, False): (18, 1), - ('ccx', 2, True, False): (18, 1), - ('ccx', 3, True, False): (18, 1), - ('no_overrides', 1, False, False): (18, 1), - ('no_overrides', 2, False, False): (18, 1), - ('no_overrides', 3, False, False): (18, 1), - ('ccx', 1, False, False): (18, 1), - ('ccx', 2, False, False): (18, 1), - ('ccx', 3, False, False): (18, 1), + ('no_overrides', 1, True, False): (16, 1), + ('no_overrides', 2, True, False): (16, 1), + ('no_overrides', 3, True, False): (16, 1), + ('ccx', 1, True, False): (16, 1), + ('ccx', 2, True, False): (16, 1), + ('ccx', 3, True, False): (16, 1), + ('no_overrides', 1, False, False): (16, 1), + ('no_overrides', 2, False, False): (16, 1), + ('no_overrides', 3, False, False): (16, 1), + ('ccx', 1, False, False): (16, 1), + ('ccx', 2, False, False): (16, 1), + ('ccx', 3, False, False): (16, 1), } @@ -260,19 +260,19 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True TEST_DATA = { - ('no_overrides', 1, True, False): (18, 3), - ('no_overrides', 2, True, False): (18, 3), - ('no_overrides', 3, True, False): (18, 3), - ('ccx', 1, True, False): (18, 3), - ('ccx', 2, True, False): (18, 3), - ('ccx', 3, True, False): (18, 3), - ('ccx', 1, True, True): (19, 3), - ('ccx', 2, True, True): (19, 3), - ('ccx', 3, True, True): (19, 3), - ('no_overrides', 1, False, False): (18, 3), - ('no_overrides', 2, False, False): (18, 3), - ('no_overrides', 3, False, False): (18, 3), - ('ccx', 1, False, False): (18, 3), - ('ccx', 2, False, False): (18, 3), - ('ccx', 3, False, False): (18, 3), + ('no_overrides', 1, True, False): (16, 3), + ('no_overrides', 2, True, False): (16, 3), + ('no_overrides', 3, True, False): (16, 3), + ('ccx', 1, True, False): (16, 3), + ('ccx', 2, True, False): (16, 3), + ('ccx', 3, True, False): (16, 3), + ('ccx', 1, True, True): (17, 3), + ('ccx', 2, True, True): (17, 3), + ('ccx', 3, True, True): (17, 3), + ('no_overrides', 1, False, False): (16, 3), + ('no_overrides', 2, False, False): (16, 3), + ('no_overrides', 3, False, False): (16, 3), + ('ccx', 1, False, False): (16, 3), + ('ccx', 2, False, False): (16, 3), + ('ccx', 3, False, False): (16, 3), } diff --git a/lms/djangoapps/courseware/date_summary.py b/lms/djangoapps/courseware/date_summary.py index 98478899fb..b885beaf7c 100644 --- a/lms/djangoapps/courseware/date_summary.py +++ b/lms/djangoapps/courseware/date_summary.py @@ -396,7 +396,7 @@ def verified_upgrade_deadline_link(user, course=None, course_id=None): course_id (:class:`.CourseKey`): The course_id of the course to render for. Returns: - The formatted link to that will allow the user to upgrade to verified + The formatted link that will allow the user to upgrade to verified in this course. """ if course is not None: diff --git a/lms/djangoapps/courseware/migrations/0004_auto_20171010_1639.py b/lms/djangoapps/courseware/migrations/0004_auto_20171010_1639.py new file mode 100644 index 0000000000..9fa9ec6b4c --- /dev/null +++ b/lms/djangoapps/courseware/migrations/0004_auto_20171010_1639.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('courseware', '0003_auto_20170825_0935'), + ] + + operations = [ + migrations.AlterField( + model_name='coursedynamicupgradedeadlineconfiguration', + name='opt_out', + field=models.BooleanField(default=False, help_text='Disable the dynamic upgrade deadline for this course run.'), + ), + ] diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index c69e22e3eb..00b58c82d8 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -398,5 +398,5 @@ class CourseDynamicUpgradeDeadlineConfiguration(ConfigurationModel): ) opt_out = models.BooleanField( default=False, - help_text=_('This does not do anything and is no longer used. Setting enabled=False has the same effect.') + help_text=_('Disable the dynamic upgrade deadline for this course run.') ) diff --git a/lms/djangoapps/courseware/tests/test_date_summary.py b/lms/djangoapps/courseware/tests/test_date_summary.py index bee9bd9599..340e7883a6 100644 --- a/lms/djangoapps/courseware/tests/test_date_summary.py +++ b/lms/djangoapps/courseware/tests/test_date_summary.py @@ -617,18 +617,6 @@ class TestScheduleOverrides(SharedModuleStoreTestCase): block = VerifiedUpgradeDeadlineDate(course, enrollment.user) self.assertEqual(block.date, expected) - @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) - def test_date_with_self_paced_with_single_course(self): - """ If the global switch is off, a single course can still be enabled. """ - course = create_self_paced_course_run(days_till_start=-1) - DynamicUpgradeDeadlineConfiguration.objects.create(enabled=False) - course_config = CourseDynamicUpgradeDeadlineConfiguration.objects.create(enabled=True, course_id=course.id) - enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT) - - block = VerifiedUpgradeDeadlineDate(course, enrollment.user) - expected = enrollment.created + timedelta(days=course_config.deadline_days) - self.assertEqual(block.date, expected) - @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True) def test_date_with_existing_schedule(self): """ If a schedule is created while deadlines are disabled, they shouldn't magically appear once the feature is diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 50dda88ea4..b65169d5e5 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -213,8 +213,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 146), - (ModuleStoreEnum.Type.split, 4, 146), + (ModuleStoreEnum.Type.mongo, 10, 145), + (ModuleStoreEnum.Type.split, 4, 145), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1457,13 +1457,13 @@ 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(35, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): + with self.assertNumQueries(34 if self_paced else 33, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST), check_mongo_calls(1): self._get_progress_page() @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 42, 26), - (True, 35, 22) + (False, 40, 26), + (True, 33, 22) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py index bc86491251..5674a9e3ea 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py @@ -10,42 +10,38 @@ from django.conf import settings from edx_ace.channel import ChannelType from edx_ace.test_utils import StubPolicy, patch_channels, patch_policies from edx_ace.utils.date import serialize +from edx_ace.message import Message from mock import Mock, patch from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory +from courseware.models import DynamicUpgradeDeadlineConfiguration from openedx.core.djangoapps.schedules import resolvers, tasks from openedx.core.djangoapps.schedules.management.commands import send_recurring_nudge as nudge from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms, FilteredQueryCountMixin from student.tests.factories import UserFactory -# Populating recurring nudge emails requires three queries -# 1a) Find all users whose first enrollment during a day was in the specified hour window -# 1b) All schedules for all enrollments for that day for users in that window (with 1a as a subquery) -# 2) Check whether debugging is enabled -CONST_QUERIES = 2 - -# 1) Prefetch all course modes for those schedules -# 2) (When not cached) load the DynamicUpgradeDeadlineConfiguration -SCHEDULE_QUERIES = 2 - # 1) Load the current django site -# 2) Load the ScheduleConfig -SEND_QUERIES = 2 +# 2) Query the schedules to find all of the template context information +NUM_QUERIES_NO_MATCHING_SCHEDULES = 2 -# 1) (When not cached) load the CourseDynamicUpgradeDeadlineConfiguration -# 2) Load the VERIFIED course mode for the course -PER_COURSE_QUERIES = 2 +# 3) Query all course modes for all courses in returned schedules +NUM_QUERIES_WITH_MATCHES = NUM_QUERIES_NO_MATCHING_SCHEDULES + 1 + +NUM_COURSE_MODES_QUERIES = 1 @ddt.ddt @skip_unless_lms @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, "Can't test schedules if the app isn't installed") -class TestSendRecurringNudge(CacheIsolationTestCase): +class TestSendRecurringNudge(FilteredQueryCountMixin, CacheIsolationTestCase): # pylint: disable=protected-access ENABLED_CACHES = ['default'] @@ -61,6 +57,8 @@ class TestSendRecurringNudge(CacheIsolationTestCase): self.site_config = SiteConfigurationFactory.create(site=site) ScheduleConfigFactory.create(site=self.site_config.site) + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + @patch.object(nudge.Command, 'resolver_class') def test_handle(self, mock_resolver): test_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) @@ -94,16 +92,21 @@ class TestSendRecurringNudge(CacheIsolationTestCase): schedules = [ ScheduleFactory.create( start=datetime.datetime(2017, 8, 3, 18, 44, 30, tzinfo=pytz.UTC), - enrollment__user=UserFactory.create(), enrollment__course__id=CourseLocator('edX', 'toy', 'Bin') - ) for _ in range(schedule_count) + ) for i in range(schedule_count) ] + bins_in_use = frozenset((s.enrollment.user.id % tasks.RECURRING_NUDGE_NUM_BINS) for s in schedules) + test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_time_str = serialize(test_time) for b in range(tasks.RECURRING_NUDGE_NUM_BINS): - # waffle flag takes an extra query before it is cached - with self.assertNumQueries(3 if b == 0 else 2): + expected_queries = NUM_QUERIES_NO_MATCHING_SCHEDULES + if b in bins_in_use: + # to fetch course modes for valid schedules + expected_queries += NUM_COURSE_MODES_QUERIES + + with self.assertNumQueries(expected_queries, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b, org_list=[schedules[0].enrollment.course.org], @@ -113,9 +116,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase): @patch.object(tasks, '_recurring_nudge_schedule_send') def test_no_course_overview(self, mock_schedule_send): - schedule = ScheduleFactory.create( start=datetime.datetime(2017, 8, 3, 20, 34, 30, tzinfo=pytz.UTC), + enrollment__user=UserFactory.create(), ) schedule.enrollment.course_id = CourseKey.from_string('edX/toy/Not_2012_Fall') schedule.enrollment.save() @@ -123,8 +126,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_time_str = serialize(test_time) for b in range(tasks.RECURRING_NUDGE_NUM_BINS): - # waffle flag takes an extra query before it is cached - with self.assertNumQueries(3 if b == 0 else 2): + with self.assertNumQueries(NUM_QUERIES_NO_MATCHING_SCHEDULES, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b, org_list=[schedule.enrollment.course.org], @@ -196,7 +198,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(3): + with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( limited_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=0, org_list=org_list, exclude_orgs=exclude_orgs, @@ -220,7 +222,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(3): + with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=user.id % tasks.RECURRING_NUDGE_NUM_BINS, @@ -255,21 +257,21 @@ class TestSendRecurringNudge(CacheIsolationTestCase): sent_messages = [] - templates_override = deepcopy(settings.TEMPLATES) - templates_override[0]['OPTIONS']['string_if_invalid'] = "TEMPLATE WARNING - MISSING VARIABLE [%s]" - with self.settings(TEMPLATES=templates_override): + with self.settings(TEMPLATES=self._get_template_overrides()): with patch.object(tasks, '_recurring_nudge_schedule_send') as mock_schedule_send: mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args) - with self.assertNumQueries(3): + with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.recurring_nudge_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=day, - bin_num=user.id % tasks.RECURRING_NUDGE_NUM_BINS, org_list=[schedules[0].enrollment.course.org], + bin_num=self._calculate_bin_for_user(user), org_list=[schedules[0].enrollment.course.org], ) self.assertEqual(len(sent_messages), 1) - with self.assertNumQueries(SEND_QUERIES): + # Load the site + # Check the schedule config + with self.assertNumQueries(2): for args in sent_messages: tasks._recurring_nudge_schedule_send(*args) @@ -277,3 +279,142 @@ class TestSendRecurringNudge(CacheIsolationTestCase): for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls: for template in attr.astuple(email): self.assertNotIn("TEMPLATE WARNING", template) + + def test_user_in_course_with_verified_coursemode_receives_upsell(self): + user = UserFactory.create() + course_id = CourseLocator('edX', 'toy', 'Course1') + + first_day_of_schedule = datetime.datetime.now(pytz.UTC) + verification_deadline = first_day_of_schedule + datetime.timedelta(days=21) + target_day = first_day_of_schedule + target_hour_as_string = serialize(target_day) + nudge_day = 3 + + schedule = ScheduleFactory.create(start=first_day_of_schedule, + enrollment__user=user, + enrollment__course__id=course_id) + schedule.enrollment.course.self_paced = True + schedule.enrollment.course.save() + + CourseModeFactory( + course_id=course_id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=verification_deadline + ) + schedule.upgrade_deadline = verification_deadline + + bin_task_parameters = [ + target_hour_as_string, + nudge_day, + user, + schedule.enrollment.course.org + ] + sent_messages = self._stub_sender_and_collect_sent_messages(bin_task=tasks.recurring_nudge_schedule_bin, + stubbed_send_task=patch.object(tasks, '_recurring_nudge_schedule_send'), + bin_task_params=bin_task_parameters) + + self.assertEqual(len(sent_messages), 1) + + message_attributes = sent_messages[0][1] + self.assertTrue(self._contains_upsell_attribute(message_attributes)) + + def test_no_upsell_button_when_DUDConfiguration_is_off(self): + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=False) + + user = UserFactory.create() + course_id = CourseLocator('edX', 'toy', 'Course1') + + first_day_of_schedule = datetime.datetime.now(pytz.UTC) + target_day = first_day_of_schedule + target_hour_as_string = serialize(target_day) + nudge_day = 3 + + schedule = ScheduleFactory.create(start=first_day_of_schedule, + enrollment__user=user, + enrollment__course__id=course_id) + schedule.enrollment.course.self_paced = True + schedule.enrollment.course.save() + + bin_task_parameters = [ + target_hour_as_string, + nudge_day, + user, + schedule.enrollment.course.org + ] + sent_messages = self._stub_sender_and_collect_sent_messages(bin_task=tasks.recurring_nudge_schedule_bin, + stubbed_send_task=patch.object(tasks, '_recurring_nudge_schedule_send'), + bin_task_params=bin_task_parameters) + + self.assertEqual(len(sent_messages), 1) + + message_attributes = sent_messages[0][1] + self.assertFalse(self._contains_upsell_attribute(message_attributes)) + + def test_user_with_no_upgrade_deadline_is_not_upsold(self): + user = UserFactory.create() + course_id = CourseLocator('edX', 'toy', 'Course1') + + first_day_of_schedule = datetime.datetime.now(pytz.UTC) + target_day = first_day_of_schedule + target_hour_as_string = serialize(target_day) + nudge_day = 3 + + schedule = ScheduleFactory.create(start=first_day_of_schedule, + upgrade_deadline=None, + enrollment__user=user, + enrollment__course__id=course_id) + schedule.enrollment.course.self_paced = True + schedule.enrollment.course.save() + + verification_deadline = first_day_of_schedule + datetime.timedelta(days=21) + CourseModeFactory( + course_id=course_id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=verification_deadline + ) + schedule.upgrade_deadline = verification_deadline + + bin_task_parameters = [ + target_hour_as_string, + nudge_day, + user, + schedule.enrollment.course.org + ] + sent_messages = self._stub_sender_and_collect_sent_messages(bin_task=tasks.recurring_nudge_schedule_bin, + stubbed_send_task=patch.object(tasks, '_recurring_nudge_schedule_send'), + bin_task_params=bin_task_parameters) + + self.assertEqual(len(sent_messages), 1) + + message_attributes = sent_messages[0][1] + self.assertFalse(self._contains_upsell_attribute(message_attributes)) + + def _stub_sender_and_collect_sent_messages(self, bin_task, stubbed_send_task, bin_task_params): + sent_messages = [] + + with self.settings(TEMPLATES=self._get_template_overrides()), stubbed_send_task as mock_schedule_send: + + mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args) + + bin_task( + self.site_config.site.id, + target_day_str=bin_task_params[0], + day_offset=bin_task_params[1], + bin_num=self._calculate_bin_for_user(bin_task_params[2]), + org_list=[bin_task_params[3]] + ) + + return sent_messages + + def _get_template_overrides(self): + templates_override = deepcopy(settings.TEMPLATES) + templates_override[0]['OPTIONS']['string_if_invalid'] = "TEMPLATE WARNING - MISSING VARIABLE [%s]" + return templates_override + + def _calculate_bin_for_user(self, user): + return user.id % tasks.RECURRING_NUDGE_NUM_BINS + + def _contains_upsell_attribute(self, msg_attr): + msg = Message.from_string(msg_attr) + tmp = msg.context["show_upsell"] + return msg.context["show_upsell"] 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 8a6ed80d0f..9003aa45fd 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 @@ -14,21 +14,41 @@ from mock import Mock, patch from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory +from courseware.models import DynamicUpgradeDeadlineConfiguration from openedx.core.djangoapps.schedules import resolvers, tasks from openedx.core.djangoapps.schedules.management.commands import send_upgrade_reminder as reminder from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms, FilteredQueryCountMixin from student.tests.factories import UserFactory +# 1) Load the current django site +# 2) Query the schedules to find all of the template context information +NUM_QUERIES_NO_MATCHING_SCHEDULES = 2 + +# 3) Query all course modes for all courses in returned schedules +NUM_QUERIES_WITH_MATCHES = NUM_QUERIES_NO_MATCHING_SCHEDULES + 1 + +# 1) Global dynamic deadline switch +# 2) E-commerce configuration +NUM_QUERIES_WITH_DEADLINE = 2 + +NUM_COURSE_MODES_QUERIES = 1 + + @ddt.ddt @skip_unless_lms @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, "Can't test schedules if the app isn't installed") -class TestUpgradeReminder(CacheIsolationTestCase): +class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): # pylint: disable=protected-access + ENABLED_CACHES = ['default'] + def setUp(self): super(TestUpgradeReminder, self).setUp() @@ -74,20 +94,26 @@ class TestUpgradeReminder(CacheIsolationTestCase): schedules = [ ScheduleFactory.create( upgrade_deadline=datetime.datetime(2017, 8, 3, 18, 44, 30, tzinfo=pytz.UTC), - enrollment__user=UserFactory.create(), enrollment__course__id=CourseLocator('edX', 'toy', 'Bin') - ) for _ in range(schedule_count) + ) for i in range(schedule_count) ] + bins_in_use = frozenset((s.enrollment.user.id % tasks.UPGRADE_REMINDER_NUM_BINS) for s in schedules) + test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_time_str = serialize(test_time) for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): - # waffle flag takes an extra query before it is cached - with self.assertNumQueries(3 if b == 0 else 2): + expected_queries = NUM_QUERIES_NO_MATCHING_SCHEDULES + if b in bins_in_use: + # to fetch course modes for valid schedules + expected_queries += NUM_COURSE_MODES_QUERIES + + with self.assertNumQueries(expected_queries, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=b, org_list=[schedules[0].enrollment.course.org], ) + self.assertEqual(mock_schedule_send.apply_async.call_count, schedule_count) self.assertFalse(mock_ace.send.called) @@ -103,8 +129,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_time_str = serialize(test_time) for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): - # waffle flag takes an extra query before it is cached - with self.assertNumQueries(3 if b == 0 else 2): + with self.assertNumQueries(NUM_QUERIES_NO_MATCHING_SCHEDULES, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=b, org_list=[schedule.enrollment.course.org], @@ -176,7 +201,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(3): + with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( limited_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=0, org_list=org_list, exclude_orgs=exclude_orgs, @@ -200,7 +225,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(3): + with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=user.id % tasks.UPGRADE_REMINDER_NUM_BINS, @@ -212,18 +237,31 @@ class TestUpgradeReminder(CacheIsolationTestCase): @ddt.data(*itertools.product((1, 10, 100), (2, 10))) @ddt.unpack def test_templates(self, message_count, day): + DynamicUpgradeDeadlineConfiguration.objects.create(enabled=True) + now = datetime.datetime.now(pytz.UTC) + future_date = now + datetime.timedelta(days=21) user = UserFactory.create() schedules = [ ScheduleFactory.create( - upgrade_deadline=datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC), + upgrade_deadline=future_date, enrollment__user=user, enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(course_num)) ) for course_num in range(message_count) ] - test_time = datetime.datetime(2017, 8, 3, 19, tzinfo=pytz.UTC) + for schedule in schedules: + schedule.enrollment.course.self_paced = True + schedule.enrollment.course.save() + + CourseModeFactory( + course_id=schedule.enrollment.course.id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=future_date + ) + + test_time = future_date test_time_str = serialize(test_time) patch_policies(self, [StubPolicy([ChannelType.PUSH])]) @@ -241,7 +279,10 @@ class TestUpgradeReminder(CacheIsolationTestCase): with patch.object(tasks, '_upgrade_reminder_schedule_send') as mock_schedule_send: mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args) - with self.assertNumQueries(3): + # we execute one query per course to see if it's opted out of dynamic upgrade deadlines, however, + # since we create a new course for each schedule in this test, we expect there to be one per message + num_expected_queries = NUM_QUERIES_WITH_MATCHES + NUM_QUERIES_WITH_DEADLINE + message_count + with self.assertNumQueries(num_expected_queries, table_blacklist=WAFFLE_TABLES): tasks.upgrade_reminder_schedule_bin( self.site_config.site.id, target_day_str=test_time_str, day_offset=day, bin_num=user.id % tasks.UPGRADE_REMINDER_NUM_BINS, diff --git a/openedx/core/djangoapps/schedules/signals.py b/openedx/core/djangoapps/schedules/signals.py index cbf5201fd4..685118aa64 100644 --- a/openedx/core/djangoapps/schedules/signals.py +++ b/openedx/core/djangoapps/schedules/signals.py @@ -112,7 +112,7 @@ def _get_upgrade_deadline_delta_setting(course_id): # Check if the course has a deadline course_config = CourseDynamicUpgradeDeadlineConfiguration.current(course_id) - if course_config.enabled: + if course_config.enabled and not course_config.opt_out: delta = course_config.deadline_days return delta diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 67cd0af11f..821c7997c5 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -12,13 +12,15 @@ from django.core.urlresolvers import reverse from django.db.models import F, Min from django.db.utils import DatabaseError from django.utils.formats import dateformat, get_format +import pytz from edx_ace import ace from edx_ace.message import Message from edx_ace.recipient import Recipient from edx_ace.utils.date import deserialize from opaque_keys.edx.keys import CourseKey -from lms.djangoapps.experiments.utils import check_and_get_upgrade_link_and_date + +from courseware.date_summary import verified_upgrade_deadline_link, verified_upgrade_link_is_valid from edxmako.shortcuts import marketing_link from openedx.core.djangoapps.schedules.message_type import ScheduleMessageType @@ -134,7 +136,7 @@ def _recurring_nudge_schedules_for_hour(site, target_hour, org_list, exclude_org } # Information for including upsell messaging in template. - _add_upsell_button_to_email_template(user, first_schedule, template_context) + _add_upsell_button_information_to_template_context(user, first_schedule, template_context) yield (user, first_schedule.enrollment.course.language, template_context) @@ -178,27 +180,6 @@ def _gather_users_and_schedules_for_target_hour(target_hour, org_list, exclude_o return users, schedules -def _add_upsell_button_to_email_template(a_user, a_schedule, template_context): - # Check and upgrade link performs a query on CourseMode, which is triggering failures in - # test_send_recurring_nudge.py - upgrade_link, upgrade_date = check_and_get_upgrade_link_and_date(a_user, a_schedule.enrollment) - has_dynamic_deadline = a_schedule.upgrade_deadline is not None - has_upgrade_link = upgrade_link is not None - show_upsell = has_dynamic_deadline and has_upgrade_link - - template_context['show_upsell'] = show_upsell - if show_upsell: - template_context['upsell_link'] = upgrade_link - template_context['user_schedule_upgrade_deadline_time'] = dateformat.format( - upgrade_date, - get_format( - 'DATE_FORMAT', - lang=a_schedule.enrollment.course.language, - use_l10n=True - ) - ) - - @task(ignore_result=True, routing_key=ROUTING_KEY) def recurring_nudge_schedule_bin( site_id, target_day_str, day_offset, bin_num, org_list, exclude_orgs=False, override_recipient_email=None, @@ -254,7 +235,7 @@ def _recurring_nudge_schedules_for_bin(site, target_day, bin_num, org_list, excl }) # Information for including upsell messaging in template. - _add_upsell_button_to_email_template(user, first_schedule, template_context) + _add_upsell_button_information_to_template_context(user, first_schedule, template_context) yield (user, first_schedule.enrollment.course.language, template_context) @@ -335,7 +316,7 @@ def _upgrade_reminder_schedules_for_bin(site, target_day, bin_num, org_list, exc 'cert_image': absolute_url(site, static('course_experience/images/verified-cert.png')), }) - _add_upsell_button_to_email_template(user, first_schedule, template_context) + _add_upsell_button_information_to_template_context(user, first_schedule, template_context) yield (user, first_schedule.enrollment.course.language, template_context) @@ -393,3 +374,32 @@ def get_schedules_with_target_date_by_bin_and_orgs(schedule_date_field, target_d schedules = schedules.using("read_replica") return schedules + + +def _add_upsell_button_information_to_template_context(user, schedule, template_context): + enrollment = schedule.enrollment + course = enrollment.course + + verified_upgrade_link = _get_link_to_purchase_verified_certificate(user, schedule) + has_verified_upgrade_link = verified_upgrade_link is not None + + if has_verified_upgrade_link: + template_context['upsell_link'] = verified_upgrade_link + template_context['user_schedule_upgrade_deadline_time'] = dateformat.format( + enrollment.dynamic_upgrade_deadline, + get_format( + 'DATE_FORMAT', + lang=course.language, + use_l10n=True + ) + ) + + template_context['show_upsell'] = has_verified_upgrade_link + + +def _get_link_to_purchase_verified_certificate(a_user, a_schedule): + enrollment = a_schedule.enrollment + if enrollment.dynamic_upgrade_deadline is None or not verified_upgrade_link_is_valid(enrollment): + return None + + return verified_upgrade_deadline_link(a_user, enrollment.course) 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 4bcae0c965..ab0bb2806c 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -126,7 +126,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(30, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url)