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 d56ca49dea..775fa4b9a2 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 @@ -1,12 +1,14 @@ import datetime -import itertools from copy import deepcopy +import logging from unittest import skipUnless import attr import ddt import pytz from django.conf import settings +from edx_ace import Message +from freezegun import freeze_time from edx_ace.channel import ChannelType from edx_ace.test_utils import StubPolicy, patch_channels, patch_policies from edx_ace.utils.date import serialize @@ -17,42 +19,71 @@ 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.content.course_overviews.models import CourseOverview 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.djangoapps.waffle_utils.testutils import WAFFLE_TABLES -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms, FilteredQueryCountMixin +from openedx.core.djangolib.testing.utils import skip_unless_lms from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory -# 1) Load the current django site -# 2) Query the schedules to find all of the template context information -NUM_QUERIES_NO_MATCHING_SCHEDULES = 2 +SITE_QUERY = 1 +SCHEDULES_QUERY = 1 +COURSE_MODES_QUERY = 1 +GLOBAL_DEADLINE_SWITCH_QUERY = 1 +COMMERCE_CONFIG_QUERY = 1 -# 3) Query all course modes for all courses in returned schedules -NUM_QUERIES_WITH_MATCHES = NUM_QUERIES_NO_MATCHING_SCHEDULES + 1 +NUM_QUERIES_NO_MATCHING_SCHEDULES = SITE_QUERY + SCHEDULES_QUERY -# 1) Global dynamic deadline switch -# 2) Course dynamic deadline switch -# 2) E-commerce configuration -NUM_QUERIES_WITH_DEADLINE = 3 +NUM_QUERIES_WITH_MATCHES = ( + NUM_QUERIES_NO_MATCHING_SCHEDULES + + COURSE_MODES_QUERY +) -NUM_COURSE_MODES_QUERIES = 1 +NUM_QUERIES_FIRST_MATCH = ( + NUM_QUERIES_WITH_MATCHES + + GLOBAL_DEADLINE_SWITCH_QUERY + + COMMERCE_CONFIG_QUERY +) + +LOG = logging.getLogger(__name__) @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(FilteredQueryCountMixin, CacheIsolationTestCase): - # pylint: disable=protected-access +@freeze_time('2017-08-01 00:00:00', tz_offset=0, tick=True) +class TestUpgradeReminder(SharedModuleStoreTestCase): ENABLED_CACHES = ['default'] + @classmethod + def setUpClass(cls): + super(TestUpgradeReminder, cls).setUpClass() + + cls.course = CourseFactory.create( + org='edX', + number='test', + display_name='Test Course', + self_paced=True, + start=datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=30), + ) + cls.course_overview = CourseOverview.get_from_id(cls.course.id) + def setUp(self): super(TestUpgradeReminder, self).setUp() + CourseModeFactory( + course_id=self.course.id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=30), + ) + ScheduleFactory.create(upgrade_deadline=datetime.datetime(2017, 8, 1, 15, 44, 30, tzinfo=pytz.UTC)) ScheduleFactory.create(upgrade_deadline=datetime.datetime(2017, 8, 1, 17, 34, 30, tzinfo=pytz.UTC)) ScheduleFactory.create(upgrade_deadline=datetime.datetime(2017, 8, 2, 15, 34, 30, tzinfo=pytz.UTC)) @@ -96,22 +127,37 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): @patch.object(tasks, 'ace') @patch.object(tasks.ScheduleUpgradeReminder, 'async_send_task') def test_schedule_bin(self, schedule_count, mock_schedule_send, mock_ace): + upgrade_deadline = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=2) schedules = [ ScheduleFactory.create( - upgrade_deadline=datetime.datetime(2017, 8, 3, 18, 44, 30, tzinfo=pytz.UTC), - enrollment__course__id=CourseLocator('edX', 'toy', 'Bin') + upgrade_deadline=upgrade_deadline, + enrollment__course=self.course_overview, ) for i in range(schedule_count) ] bins_in_use = frozenset((s.enrollment.user.id % resolvers.UPGRADE_REMINDER_NUM_BINS) for s in schedules) + is_first_match = True - test_datetime = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) + course_switch_queries = len(set(s.enrollment.course.id for s in schedules)) + + test_datetime = datetime.datetime(upgrade_deadline.year, upgrade_deadline.month, upgrade_deadline.day, 18, + tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) + for b in range(resolvers.UPGRADE_REMINDER_NUM_BINS): + LOG.debug('Running bin %d', b) 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 + if is_first_match: + expected_queries = ( + # 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 + ) + is_first_match = False + else: + expected_queries = NUM_QUERIES_WITH_MATCHES with self.assertNumQueries(expected_queries, table_blacklist=WAFFLE_TABLES): tasks.ScheduleUpgradeReminder.delay( @@ -194,22 +240,27 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): ScheduleFactory.create( upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), enrollment__course__org=filtered_org, + enrollment__course__self_paced=True, enrollment__user=user1, ) ScheduleFactory.create( upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), enrollment__course__org=unfiltered_org, + enrollment__course__self_paced=True, enrollment__user=user1, ) ScheduleFactory.create( upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), enrollment__course__org=unfiltered_org, + enrollment__course__self_paced=True, enrollment__user=user2, ) test_datetime = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): + + course_switch_queries = 1 + with self.assertNumQueries(NUM_QUERIES_FIRST_MATCH + course_switch_queries, table_blacklist=WAFFLE_TABLES): tasks.ScheduleUpgradeReminder.delay( limited_config.site.id, target_day_str=test_datetime_str, day_offset=2, bin_num=0, org_list=org_list, exclude_orgs=exclude_orgs, @@ -226,14 +277,18 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): ScheduleFactory.create( upgrade_deadline=datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC), enrollment__user=user, + enrollment__course__self_paced=True, enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(course_num)) ) for course_num in (1, 2, 3) ] + num_courses = len(set(s.enrollment.course.id for s in schedules)) + test_datetime = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) test_datetime_str = serialize(test_datetime) - with self.assertNumQueries(NUM_QUERIES_WITH_MATCHES, table_blacklist=WAFFLE_TABLES): + + with self.assertNumQueries(NUM_QUERIES_FIRST_MATCH + num_courses, table_blacklist=WAFFLE_TABLES): tasks.ScheduleUpgradeReminder.delay( self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, bin_num=user.id % resolvers.UPGRADE_REMINDER_NUM_BINS, @@ -242,9 +297,8 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): self.assertEqual(mock_schedule_send.apply_async.call_count, 1) self.assertFalse(mock_ace.send.called) - @ddt.data(*itertools.product((1, 10, 100), (2, 10))) - @ddt.unpack - def test_templates(self, message_count, day): + @ddt.data(1, 10, 100) + def test_templates(self, message_count): now = datetime.datetime.now(pytz.UTC) future_datetime = now + datetime.timedelta(days=21) @@ -253,22 +307,22 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): ScheduleFactory.create( upgrade_deadline=future_datetime, enrollment__user=user, + enrollment__course__self_paced=True, + enrollment__course__end=future_datetime + datetime.timedelta(days=30), enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(course_num)) ) for course_num in range(message_count) ] for schedule in schedules: - schedule.enrollment.course.self_paced = True - schedule.enrollment.course.end = future_datetime + datetime.timedelta(days=30) - schedule.enrollment.course.save() - CourseModeFactory( course_id=schedule.enrollment.course.id, mode_slug=CourseMode.VERIFIED, expiration_datetime=future_datetime ) + num_courses = len(set(s.enrollment.course.id for s in schedules)) + test_datetime = future_datetime test_datetime_str = serialize(test_datetime) @@ -285,12 +339,11 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): with patch.object(tasks.ScheduleUpgradeReminder, 'async_send_task') as mock_schedule_send: 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, 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 + # 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_courses with self.assertNumQueries(num_expected_queries, table_blacklist=WAFFLE_TABLES): tasks.ScheduleUpgradeReminder.delay( - self.site_config.site.id, target_day_str=test_datetime_str, day_offset=day, + self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, bin_num=self._calculate_bin_for_user(user), org_list=[schedules[0].enrollment.course.org], ) @@ -316,4 +369,68 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase): return templates_override def _calculate_bin_for_user(self, user): - return user.id % resolvers.RECURRING_NUDGE_NUM_BINS + return user.id % resolvers.UPGRADE_REMINDER_NUM_BINS + + @patch.object(tasks, '_upgrade_reminder_schedule_send') + def test_dont_send_to_verified_learner(self, mock_schedule_send): + upgrade_deadline = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=2) + ScheduleFactory.create( + upgrade_deadline=upgrade_deadline, + enrollment__user=UserFactory.create(id=resolvers.UPGRADE_REMINDER_NUM_BINS), + enrollment__course=self.course_overview, + enrollment__mode=CourseMode.VERIFIED, + ) + test_datetime_str = serialize(datetime.datetime.now(pytz.UTC)) + + tasks.ScheduleUpgradeReminder.delay( + self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, bin_num=0, + org_list=[self.course.org], + ) + + self.assertFalse(mock_schedule_send.called) + self.assertFalse(mock_schedule_send.apply_async.called) + + def test_filter_out_verified_schedules(self): + now = datetime.datetime.now(pytz.UTC) + future_datetime = now + datetime.timedelta(days=21) + + user = UserFactory.create() + schedules = [ + ScheduleFactory.create( + upgrade_deadline=future_datetime, + enrollment__user=user, + enrollment__course__self_paced=True, + enrollment__course__end=future_datetime + datetime.timedelta(days=30), + enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(i)), + enrollment__mode=CourseMode.VERIFIED if i in (0, 3) else CourseMode.AUDIT, + ) + for i in range(5) + ] + + for schedule in schedules: + CourseModeFactory( + course_id=schedule.enrollment.course.id, + mode_slug=CourseMode.VERIFIED, + expiration_datetime=future_datetime + ) + + test_datetime = future_datetime + test_datetime_str = serialize(test_datetime) + bin_num = self._calculate_bin_for_user(user) + + sent_messages = [] + with patch.object(tasks.ScheduleUpgradeReminder, 'async_send_task') as mock_schedule_send: + mock_schedule_send.apply_async = lambda args, *_a, **_kw: sent_messages.append(args[1]) + + tasks.ScheduleUpgradeReminder.delay( + self.site_config.site.id, target_day_str=test_datetime_str, day_offset=2, bin_num=bin_num, + org_list=[schedules[0].enrollment.course.org], + ) + + messages = [Message.from_string(m) for m in sent_messages] + self.assertEqual(len(messages), 1) + message = messages[0] + self.assertItemsEqual( + message.context['course_ids'], + [str(schedules[i].enrollment.course.id) for i in (1, 2, 4)] + ) diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 3266d9b3ec..340dfbd9b7 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -165,11 +165,10 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): template_context['course_ids'] = course_id_strs first_schedule = user_schedules[0] - template_context.update(self.get_template_context(user, user_schedules)) - - # Information for including upsell messaging in template. - _add_upsell_button_information_to_template_context( - user, first_schedule, template_context) + try: + template_context.update(self.get_template_context(user, user_schedules)) + except InvalidContextError: + continue yield (user, first_schedule.enrollment.course.language, template_context) @@ -188,10 +187,19 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): dict: This dict must be JSON serializable (no datetime objects!). When rendering the message templates it it will be used as the template context. Note that it will also include several default values that injected into all template contexts. See `get_base_template_context` for more information. + + Raises: + InvalidContextError: If this user and set of schedules are not valid for this type of message. Raising this + exception will prevent this user from receiving the message, but allow other messages to be sent to other + users. """ return {} +class InvalidContextError(Exception): + pass + + class ScheduleStartResolver(BinnedSchedulesBaseResolver): """ Send a message to all users whose schedule started at ``self.current_date`` + ``day_offset``. @@ -202,13 +210,18 @@ class ScheduleStartResolver(BinnedSchedulesBaseResolver): def get_template_context(self, user, user_schedules): first_schedule = user_schedules[0] - return { + context = { 'course_name': first_schedule.enrollment.course.display_name, 'course_url': absolute_url( self.site, reverse('course_root', args=[str(first_schedule.enrollment.course_id)]) ), } + # Information for including upsell messaging in template. + context.update(_get_upsell_information_for_schedule(user, first_schedule)) + + return context + def _get_datetime_beginning_of_day(dt): """ @@ -226,20 +239,41 @@ class UpgradeReminderResolver(BinnedSchedulesBaseResolver): num_bins = UPGRADE_REMINDER_NUM_BINS def get_template_context(self, user, user_schedules): - first_schedule = user_schedules[0] - return { - 'course_links': [ - { - 'url': absolute_url(self.site, reverse('course_root', args=[str(s.enrollment.course_id)])), - 'name': s.enrollment.course.display_name - } for s in user_schedules - ], + course_id_strs = [] + course_links = [] + first_valid_upsell_context = None + first_schedule = None + for schedule in user_schedules: + upsell_context = _get_upsell_information_for_schedule(user, schedule) + if not upsell_context['show_upsell']: + continue + + if first_valid_upsell_context is None: + first_schedule = schedule + first_valid_upsell_context = upsell_context + course_id_str = str(schedule.enrollment.course_id) + course_id_strs.append(course_id_str) + course_links.append({ + 'url': absolute_url(self.site, reverse('course_root', args=[course_id_str])), + 'name': schedule.enrollment.course.display_name + }) + + if first_schedule is None: + self.log_debug('No courses eligible for upgrade for user.') + raise InvalidContextError() + + context = { + 'course_links': course_links, 'first_course_name': first_schedule.enrollment.course.display_name, 'cert_image': absolute_url(self.site, static('course_experience/images/verified-cert.png')), + 'course_ids': course_id_strs, } + context.update(first_valid_upsell_context) + return context -def _add_upsell_button_information_to_template_context(user, schedule, template_context): +def _get_upsell_information_for_schedule(user, schedule): + template_context = {} enrollment = schedule.enrollment course = enrollment.course @@ -258,6 +292,7 @@ def _add_upsell_button_information_to_template_context(user, schedule, template_ ) template_context['show_upsell'] = has_verified_upgrade_link + return template_context def _get_verified_upgrade_link(user, schedule): @@ -293,10 +328,9 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): course_id_str = str(enrollment.course_id) template_context.update({ - 'student_name': user.profile.name, 'course_name': schedule.enrollment.course.display_name, 'course_url': absolute_url( - self.site, reverse('course_root', args=[str(schedule.enrollment.course_id)]) + self.site, reverse('course_root', args=[course_id_str]) ), 'week_num': week_num, 'week_summary': week_summary, @@ -304,6 +338,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): # This is used by the bulk email optout policy 'course_ids': [course_id_str], }) + template_context.update(_get_upsell_information_for_schedule(user, schedule)) yield (user, schedule.enrollment.course.language, template_context)