diff --git a/openedx/core/djangoapps/schedules/admin.py b/openedx/core/djangoapps/schedules/admin.py index 34ec24addd..c4dcc313f6 100644 --- a/openedx/core/djangoapps/schedules/admin.py +++ b/openedx/core/djangoapps/schedules/admin.py @@ -30,4 +30,6 @@ class ScheduleAdmin(admin.ModelAdmin): @admin.register(models.ScheduleConfig) class ScheduleConfigAdmin(admin.ModelAdmin): search_fields = ('site',) - list_display = ('site', 'create_schedules', 'enqueue_recurring_nudge', 'deliver_recurring_nudge') + list_display = ('site', 'create_schedules', 'enqueue_recurring_nudge', + 'deliver_recurring_nudge', 'enqueue_upgrade_reminder', + 'deliver_upgrade_reminder') diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index e69de29bb2..3be3b9f1a9 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -0,0 +1,42 @@ +import datetime + +import pytz +from django.contrib.sites.models import Site +from django.core.management.base import BaseCommand + +from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin + + +class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): + resolver_class = None # define in subclass + + def add_arguments(self, parser): + parser.add_argument( + '--date', + default=datetime.datetime.utcnow().date().isoformat(), + help='The date to compute weekly messages relative to, in YYYY-MM-DD format', + ) + parser.add_argument( + '--override-recipient-email', + help='Send all emails to this address instead of the actual recipient' + ) + parser.add_argument('site_domain_name') + + def handle(self, *args, **options): + resolver = self.make_resolver(*args, **options) + self.send_emails(resolver, *args, **options) + + def make_resolver(self, *args, **options): + current_date = datetime.datetime( + *[int(x) for x in options['date'].split('-')], + tzinfo=pytz.UTC + ) + self.log_debug('Args = %r', options) + self.log_debug('Current date = %s', current_date.isoformat()) + + site = Site.objects.get(domain__iexact=options['site_domain_name']) + self.log_debug('Running for site %s', site.domain) + return self.resolver_class(site, current_date) + + def send_emails(self, resolver, *args, **options): + pass # define in subclass diff --git a/openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py b/openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py index 7e2bea76db..5f3b316e60 100644 --- a/openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py +++ b/openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py @@ -1,96 +1,14 @@ -from __future__ import print_function - -import datetime -import logging - -from django.contrib.sites.models import Site -from django.core.management.base import BaseCommand -import pytz - -from edx_ace.utils.date import serialize -from openedx.core.djangoapps.schedules.models import ScheduleConfig -from openedx.core.djangoapps.schedules.tasks import recurring_nudge_schedule_hour -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration - -from edx_ace.recipient_resolver import RecipientResolver - -LOG = logging.getLogger(__name__) +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand +from openedx.core.djangoapps.schedules.resolvers import ScheduleStartResolver -class ScheduleStartResolver(RecipientResolver): - def __init__(self, site, current_date): - self.site = site - self.current_date = current_date.replace(hour=0, minute=0, second=0) +class Command(SendEmailBaseCommand): + resolver_class = ScheduleStartResolver - def send(self, day, override_recipient_email=None): - """ - Send a message to all users whose schedule started at ``self.current_date`` - ``day``. - """ - if not ScheduleConfig.current(self.site).enqueue_recurring_nudge: - LOG.debug('Recurring Nudge: Message queuing disabled for site %s', self.site.domain) - return + def __init__(self, *args, **kwargs): + super(Command, self).__init__(*args, **kwargs) + self.log_prefix = 'Scheduled Nudge' - exclude_orgs, org_list = self.get_org_filter() - - target_date = self.current_date - datetime.timedelta(days=day) - LOG.debug('Scheduled Nudge: Target date = %s', target_date.isoformat()) - for hour in range(24): - target_hour = target_date + datetime.timedelta(hours=hour) - task_args = (self.site.id, day, serialize(target_hour), org_list, exclude_orgs, override_recipient_email) - LOG.debug('Scheduled Nudge: Launching task with args = %r', task_args) - recurring_nudge_schedule_hour.apply_async(task_args, retry=False) - - def get_org_filter(self): - """ - Given the configuration of sites, get the list of orgs that should be included or excluded from this send. - - Returns: - tuple: Returns a tuple (exclude_orgs, org_list). If exclude_orgs is True, then org_list is a list of the - only orgs that should be included in this send. If exclude_orgs is False, then org_list is a list of - orgs that should be excluded from this send. All other orgs should be included. - """ - try: - site_config = SiteConfiguration.objects.get(site_id=self.site.id) - org_list = site_config.values.get('course_org_filter', None) - exclude_orgs = False - if not org_list: - not_orgs = set() - for other_site_config in SiteConfiguration.objects.all(): - not_orgs.update(other_site_config.values.get('course_org_filter', [])) - org_list = list(not_orgs) - exclude_orgs = True - elif not isinstance(org_list, list): - org_list = [org_list] - except SiteConfiguration.DoesNotExist: - org_list = None - exclude_orgs = False - return exclude_orgs, org_list - - -class Command(BaseCommand): - - def add_arguments(self, parser): - parser.add_argument( - '--date', - default=datetime.datetime.utcnow().date().isoformat(), - help='The date to compute weekly messages relative to, in YYYY-MM-DD format', - ) - parser.add_argument( - '--override-recipient-email', - help='Send all emails to this address instead of the actual recipient' - ) - parser.add_argument('site_domain_name') - - def handle(self, *args, **options): - current_date = datetime.datetime( - *[int(x) for x in options['date'].split('-')], - tzinfo=pytz.UTC - ) - LOG.debug('Scheduled Nudge: Args = %r', options) - LOG.debug('Scheduled Nudge: Current date = %s', current_date.isoformat()) - - site = Site.objects.get(domain__iexact=options['site_domain_name']) - LOG.debug('Scheduled Nudge: Running for site %s', site.domain) - resolver = ScheduleStartResolver(site, current_date) - for day in (3, 10): - resolver.send(day, options.get('override_recipient_email')) + def send_emails(self, resolver, *args, **options): + for day_offset in (-3, -10): + resolver.send(day_offset, options.get('override_recipient_email')) diff --git a/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py b/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py new file mode 100644 index 0000000000..c251c85275 --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py @@ -0,0 +1,13 @@ +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand +from openedx.core.djangoapps.schedules.resolvers import UpgradeReminderResolver + + +class Command(SendEmailBaseCommand): + resolver_class = UpgradeReminderResolver + + def __init__(self, *args, **kwargs): + super(Command, self).__init__(*args, **kwargs) + self.log_prefix = 'Upgrade Reminder' + + def send_emails(self, resolver, *args, **options): + resolver.send(2, options.get('override_recipient_email')) diff --git a/openedx/core/djangoapps/schedules/management/commands/send_verified_upgrade_deadline_reminder.py b/openedx/core/djangoapps/schedules/management/commands/send_verified_upgrade_deadline_reminder.py deleted file mode 100644 index a78fe79526..0000000000 --- a/openedx/core/djangoapps/schedules/management/commands/send_verified_upgrade_deadline_reminder.py +++ /dev/null @@ -1,116 +0,0 @@ -from __future__ import print_function - -import datetime - -from dateutil.tz import tzutc, gettz -from django.core.management.base import BaseCommand -from django.db.models import Prefetch -from django.conf import settings -from django.core.urlresolvers import reverse -from django.utils.http import urlquote - -from openedx.core.djangoapps.schedules.message_type import ScheduleMessageType -from openedx.core.djangoapps.schedules.models import Schedule -from openedx.core.djangoapps.user_api.models import UserPreference - -from edx_ace.recipient_resolver import RecipientResolver -from edx_ace import ace -from edx_ace.recipient import Recipient - - -from course_modes.models import CourseMode, format_course_price -from lms.djangoapps.experiments.utils import check_and_get_upgrade_link - - -class VerifiedUpgradeDeadlineReminder(ScheduleMessageType): - pass - - -class VerifiedDeadlineResolver(RecipientResolver): - def __init__(self, target_deadline): - self.target_deadline = target_deadline - - def send(self, msg_type): - for (user, language, context) in self.build_email_context(): - msg = msg_type.personalize( - Recipient( - user.username, - user.email, - ), - language, - context - ) - ace.send(msg) - - def build_email_context(self): - schedules = Schedule.objects.select_related( - 'enrollment__user__profile', - 'enrollment__course', - ).prefetch_related( - Prefetch( - 'enrollment__course__modes', - queryset=CourseMode.objects.filter(mode_slug=CourseMode.VERIFIED), - to_attr='verified_modes' - ), - Prefetch( - 'enrollment__user__preferences', - queryset=UserPreference.objects.filter(key='time_zone'), - to_attr='tzprefs' - ), - ).filter( - upgrade_deadline__year=self.schedule_deadline.year, - upgrade_deadline__month=self.schedule_deadline.month, - upgrade_deadline__day=self.schedule_deadline.day, - ) - - if "read_replica" in settings.DATABASES: - schedules = schedules.using("read_replica") - - for schedule in schedules: - enrollment = schedule.enrollment - user = enrollment.user - - user_time_zone = tzutc() - for preference in user.tzprefs: - user_time_zone = gettz(preference.value) - - course_id_str = str(enrollment.course_id) - course = enrollment.course - - course_root = reverse('course_root', kwargs={'course_id': urlquote(course_id_str)}) - - def absolute_url(relative_path): - return u'{}{}'.format(settings.LMS_ROOT_URL, relative_path) - - template_context = { - 'user_full_name': user.profile.name, - 'user_personal_address': user.profile.name if user.profile.name else user.username, - 'user_username': user.username, - 'user_time_zone': user_time_zone, - 'user_schedule_start_time': schedule.start, - 'user_schedule_verified_upgrade_deadline_time': schedule.upgrade_deadline, - 'course_id': course_id_str, - 'course_title': course.display_name, - 'course_url': absolute_url(course_root), - 'course_image_url': absolute_url(course.course_image_url), - 'course_end_time': course.end, - 'course_verified_upgrade_url': check_and_get_upgrade_link(course, user), - 'course_verified_upgrade_price': format_course_price(course.verified_modes[0].min_price), - } - - yield (user, course.language, template_context) - - -class Command(BaseCommand): - - def add_arguments(self, parser): - parser.add_argument('--date', default=datetime.datetime.utcnow().date().isoformat()) - - def handle(self, *args, **options): - current_date = datetime.date(*[int(x) for x in options['date'].split('-')]) - - msg_t = VerifiedUpgradeDeadlineReminder() - - for offset in (2, 9, 16): - target_date = current_date + datetime.timedelta(days=offset) - VerifiedDeadlineResolver(target_date).send(msg_t) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py new file mode 100644 index 0000000000..ce75828775 --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py @@ -0,0 +1,40 @@ +import datetime +from unittest import skipUnless + +import ddt +import pytz +from django.conf import settings +from mock import patch + +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand +from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms + + +@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 TestSendEmailBaseCommand(CacheIsolationTestCase): + def setUp(self): + self.command = SendEmailBaseCommand() + + def test_init_resolver_class(self): + assert self.command.resolver_class is None + + def test_make_resolver(self): + with patch.object(self.command, 'resolver_class') as resolver_class: + example_site = SiteFactory(domain='example.com') + self.command.make_resolver(site_domain_name='example.com', date='2017-09-29') + resolver_class.assert_called_once_with( + example_site, + datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC) + ) + + def test_handle(self): + with patch.object(self.command, 'make_resolver') as make_resolver: + make_resolver.return_value = 'resolver' + with patch.object(self.command, 'send_emails') as send_emails: + self.command.handle(date='2017-09-29') + make_resolver.assert_called_once_with(date='2017-09-29') + send_emails.assert_called_once_with('resolver', date='2017-09-29') 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 54d5aa0820..1eacc49303 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 @@ -7,7 +7,6 @@ import attr import ddt import pytz from django.conf import settings -from django.test import override_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 @@ -15,7 +14,7 @@ from mock import Mock, patch from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator -from openedx.core.djangoapps.schedules import tasks +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 @@ -41,28 +40,28 @@ class TestSendRecurringNudge(CacheIsolationTestCase): self.site_config = SiteConfigurationFactory.create(site=site) ScheduleConfigFactory.create(site=self.site_config.site) - @patch.object(nudge, 'ScheduleStartResolver') + @patch.object(nudge.Command, 'resolver_class') def test_handle(self, mock_resolver): test_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) nudge.Command().handle(date='2017-08-01', site_domain_name=self.site_config.site.domain) mock_resolver.assert_called_with(self.site_config.site, test_time) - for day in (3, 10): + for day in (-3, -10): mock_resolver().send.assert_any_call(day, None) @patch.object(tasks, 'ace') - @patch.object(nudge, 'recurring_nudge_schedule_hour') - def test_resolver_send(self, mock_schedule_hour, mock_ace): + @patch.object(resolvers.ScheduleStartResolver, 'async_send_task') + def test_resolver_send(self, mock_schedule_bin, mock_ace): current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) - nudge.ScheduleStartResolver(self.site_config.site, current_time).send(3) - test_time = current_time - datetime.timedelta(days=3) - self.assertFalse(mock_schedule_hour.called) - mock_schedule_hour.apply_async.assert_any_call( - (self.site_config.site.id, 3, serialize(test_time), [], True, None), + nudge.ScheduleStartResolver(self.site_config.site, current_time).send(-3) + test_time = current_time + datetime.timedelta(days=-3) + self.assertFalse(mock_schedule_bin.called) + mock_schedule_bin.apply_async.assert_any_call( + (self.site_config.site.id, serialize(test_time), -3, 0, [], True, None), retry=False, ) - mock_schedule_hour.apply_async.assert_any_call( - (self.site_config.site.id, 3, serialize(test_time + datetime.timedelta(hours=23)), [], True, None), + mock_schedule_bin.apply_async.assert_any_call( + (self.site_config.site.id, serialize(test_time), -3, tasks.RECURRING_NUDGE_NUM_BINS - 1, [], True, None), retry=False, ) self.assertFalse(mock_ace.send.called) @@ -70,17 +69,24 @@ class TestSendRecurringNudge(CacheIsolationTestCase): @ddt.data(1, 10, 100) @patch.object(tasks, 'ace') @patch.object(tasks, '_recurring_nudge_schedule_send') - def test_schedule_hour(self, schedule_count, mock_schedule_send, mock_ace): + def test_schedule_bin(self, schedule_count, mock_schedule_send, mock_ace): schedules = [ - ScheduleFactory.create(start=datetime.datetime(2017, 8, 1, 18, 34, 30, tzinfo=pytz.UTC)) - for _ in range(schedule_count) + 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) ] - test_time_str = serialize(datetime.datetime(2017, 8, 1, 18, tzinfo=pytz.UTC)) - with self.assertNumQueries(2): - tasks.recurring_nudge_schedule_hour( - self.site_config.site.id, 3, test_time_str, [schedules[0].enrollment.course.org], - ) + 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): + 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], + ) self.assertEqual(mock_schedule_send.apply_async.call_count, schedule_count) self.assertFalse(mock_ace.send.called) @@ -88,16 +94,20 @@ class TestSendRecurringNudge(CacheIsolationTestCase): def test_no_course_overview(self, mock_schedule_send): schedule = ScheduleFactory.create( - start=datetime.datetime(2017, 8, 1, 20, 34, 30, tzinfo=pytz.UTC), + start=datetime.datetime(2017, 8, 3, 20, 34, 30, tzinfo=pytz.UTC), ) schedule.enrollment.course_id = CourseKey.from_string('edX/toy/Not_2012_Fall') schedule.enrollment.save() - test_time_str = serialize(datetime.datetime(2017, 8, 1, 20, tzinfo=pytz.UTC)) - with self.assertNumQueries(2): - tasks.recurring_nudge_schedule_hour( - self.site_config.site.id, 3, test_time_str, [schedule.enrollment.course.org], - ) + 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): + 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], + ) # There is no database constraint that enforces that enrollment.course_id points # to a valid CourseOverview object. However, in that case, schedules isn't going @@ -116,14 +126,14 @@ class TestSendRecurringNudge(CacheIsolationTestCase): self.assertFalse(mock_ace.send.called) @patch.object(tasks, 'ace') - @patch.object(nudge, 'recurring_nudge_schedule_hour') - def test_enqueue_disabled(self, mock_schedule_hour, mock_ace): + @patch.object(resolvers.ScheduleStartResolver, 'async_send_task') + def test_enqueue_disabled(self, mock_schedule_bin, mock_ace): ScheduleConfigFactory.create(site=self.site_config.site, enqueue_recurring_nudge=False) current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) nudge.ScheduleStartResolver(self.site_config.site, current_time).send(3) - self.assertFalse(mock_schedule_hour.called) - self.assertFalse(mock_schedule_hour.apply_async.called) + self.assertFalse(mock_schedule_bin.called) + self.assertFalse(mock_schedule_bin.apply_async.called) self.assertFalse(mock_ace.send.called) @patch.object(tasks, 'ace') @@ -144,77 +154,76 @@ class TestSendRecurringNudge(CacheIsolationTestCase): for config in (limited_config, unlimited_config): ScheduleConfigFactory.create(site=config.site) - user1 = UserFactory.create() - user2 = UserFactory.create() + user1 = UserFactory.create(id=tasks.RECURRING_NUDGE_NUM_BINS) + user2 = UserFactory.create(id=tasks.RECURRING_NUDGE_NUM_BINS * 2) ScheduleFactory.create( - start=datetime.datetime(2017, 8, 2, 17, 44, 30, tzinfo=pytz.UTC), + start=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), enrollment__course__org=filtered_org, enrollment__user=user1, ) ScheduleFactory.create( - start=datetime.datetime(2017, 8, 2, 17, 44, 30, tzinfo=pytz.UTC), + start=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), enrollment__course__org=unfiltered_org, enrollment__user=user1, ) ScheduleFactory.create( - start=datetime.datetime(2017, 8, 2, 17, 44, 30, tzinfo=pytz.UTC), + start=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), enrollment__course__org=unfiltered_org, enrollment__user=user2, ) - test_time_str = serialize(datetime.datetime(2017, 8, 2, 17, tzinfo=pytz.UTC)) - with self.assertNumQueries(2): - tasks.recurring_nudge_schedule_hour( - limited_config.site.id, day=3, target_hour_str=test_time_str, org_list=org_list, - exclude_orgs=exclude_orgs, + test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) + with self.assertNumQueries(3): + 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, ) self.assertEqual(mock_schedule_send.apply_async.call_count, expected_message_count) self.assertFalse(mock_ace.send.called) - @ddt.data( - (19, 1), - (20, 0), - (21, 0) - ) - @ddt.unpack @patch.object(tasks, 'ace') @patch.object(tasks, '_recurring_nudge_schedule_send') - def test_multiple_enrollments(self, test_hour, messages_sent, mock_schedule_send, mock_ace): + def test_multiple_enrollments(self, mock_schedule_send, mock_ace): user = UserFactory.create() schedules = [ ScheduleFactory.create( - start=datetime.datetime(2017, 8, 1, hour, 44, 30, tzinfo=pytz.UTC), + start=datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC), enrollment__user=user, - enrollment__course__id=CourseLocator('edX', 'toy', 'Hour{}'.format(hour)) + enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(course_num)) ) - for hour in (19, 20, 21) + for course_num in (1, 2, 3) ] - test_time_str = serialize(datetime.datetime(2017, 8, 1, test_hour, tzinfo=pytz.UTC)) - with self.assertNumQueries(2): - tasks.recurring_nudge_schedule_hour( - self.site_config.site.id, 3, test_time_str, [schedules[0].enrollment.course.org], + test_time = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) + with self.assertNumQueries(3): + 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, + org_list=[schedules[0].enrollment.course.org], ) - self.assertEqual(mock_schedule_send.apply_async.call_count, messages_sent) + self.assertEqual(mock_schedule_send.apply_async.call_count, 1) self.assertFalse(mock_ace.send.called) - @ddt.data(*itertools.product((1, 10, 100), (3, 10))) + @ddt.data(*itertools.product((1, 10, 100), (-3, -10))) @ddt.unpack def test_templates(self, message_count, day): user = UserFactory.create() schedules = [ ScheduleFactory.create( - start=datetime.datetime(2017, 8, 1, 19, 44, 30, tzinfo=pytz.UTC), + start=datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC), enrollment__user=user, - enrollment__course__id=CourseLocator('edX', 'toy', 'Hour{}'.format(idx)) + enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(course_num)) ) - for idx in range(message_count) + for course_num in range(message_count) ] - test_time_str = serialize(datetime.datetime(2017, 8, 1, 19, tzinfo=pytz.UTC)) + test_time = datetime.datetime(2017, 8, 3, 19, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) patch_policies(self, [StubPolicy([ChannelType.PUSH])]) mock_channel = Mock( @@ -231,9 +240,10 @@ class TestSendRecurringNudge(CacheIsolationTestCase): 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(2): - tasks.recurring_nudge_schedule_hour( - self.site_config.site.id, day, test_time_str, [schedules[0].enrollment.course.org], + with self.assertNumQueries(3): + 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], ) self.assertEqual(len(sent_messages), 1) 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 new file mode 100644 index 0000000000..8a6ed80d0f --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py @@ -0,0 +1,259 @@ +import datetime +import itertools +from copy import deepcopy +from unittest import skipUnless + +import attr +import ddt +import pytz +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 mock import Mock, patch +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import CourseLocator + +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 student.tests.factories import UserFactory + + +@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): + # pylint: disable=protected-access + + def setUp(self): + super(TestUpgradeReminder, self).setUp() + + 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)) + + site = SiteFactory.create() + self.site_config = SiteConfigurationFactory.create(site=site) + ScheduleConfigFactory.create(site=self.site_config.site) + + @patch.object(reminder.Command, 'resolver_class') + def test_handle(self, mock_resolver): + test_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) + reminder.Command().handle(date='2017-08-01', site_domain_name=self.site_config.site.domain) + mock_resolver.assert_called_with(self.site_config.site, test_time) + + mock_resolver().send.assert_any_call(2, None) + + @patch.object(tasks, 'ace') + @patch.object(resolvers.UpgradeReminderResolver, 'async_send_task') + def test_resolver_send(self, mock_schedule_bin, mock_ace): + current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) + test_time = current_time + datetime.timedelta(days=2) + ScheduleFactory.create(upgrade_deadline=datetime.datetime(2017, 8, 3, 15, 34, 30, tzinfo=pytz.UTC)) + + reminder.UpgradeReminderResolver(self.site_config.site, current_time).send(2) + self.assertFalse(mock_schedule_bin.called) + mock_schedule_bin.apply_async.assert_any_call( + (self.site_config.site.id, serialize(test_time), 2, 0, [], True, None), + retry=False, + ) + mock_schedule_bin.apply_async.assert_any_call( + (self.site_config.site.id, serialize(test_time), 2, tasks.UPGRADE_REMINDER_NUM_BINS - 1, [], True, None), + retry=False, + ) + self.assertFalse(mock_ace.send.called) + + @ddt.data(1, 10, 100) + @patch.object(tasks, 'ace') + @patch.object(tasks, '_upgrade_reminder_schedule_send') + def test_schedule_bin(self, schedule_count, mock_schedule_send, mock_ace): + 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) + ] + + 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): + 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) + + @patch.object(tasks, '_upgrade_reminder_schedule_send') + def test_no_course_overview(self, mock_schedule_send): + + schedule = ScheduleFactory.create( + upgrade_deadline=datetime.datetime(2017, 8, 3, 20, 34, 30, tzinfo=pytz.UTC), + ) + schedule.enrollment.course_id = CourseKey.from_string('edX/toy/Not_2012_Fall') + schedule.enrollment.save() + + 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): + 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], + ) + + # There is no database constraint that enforces that enrollment.course_id points + # to a valid CourseOverview object. However, in that case, schedules isn't going + # to attempt to address it, and will instead simply skip those users. + # This happens 'transparently' because django generates an inner-join between + # enrollment and course_overview, and thus will skip any rows where course_overview + # is null. + self.assertEqual(mock_schedule_send.apply_async.call_count, 0) + + @patch.object(tasks, 'ace') + def test_delivery_disabled(self, mock_ace): + ScheduleConfigFactory.create(site=self.site_config.site, deliver_upgrade_reminder=False) + + mock_msg = Mock() + tasks._upgrade_reminder_schedule_send(self.site_config.site.id, mock_msg) + self.assertFalse(mock_ace.send.called) + + @patch.object(tasks, 'ace') + @patch.object(resolvers.UpgradeReminderResolver, 'async_send_task') + def test_enqueue_disabled(self, mock_schedule_bin, mock_ace): + ScheduleConfigFactory.create(site=self.site_config.site, enqueue_upgrade_reminder=False) + + current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) + reminder.UpgradeReminderResolver(self.site_config.site, current_time).send(3) + self.assertFalse(mock_schedule_bin.called) + self.assertFalse(mock_schedule_bin.apply_async.called) + self.assertFalse(mock_ace.send.called) + + @patch.object(tasks, 'ace') + @patch.object(tasks, '_upgrade_reminder_schedule_send') + @ddt.data( + ((['filtered_org'], False, 1)), + ((['filtered_org'], True, 2)) + ) + @ddt.unpack + def test_site_config(self, org_list, exclude_orgs, expected_message_count, mock_schedule_send, mock_ace): + filtered_org = 'filtered_org' + unfiltered_org = 'unfiltered_org' + site1 = SiteFactory.create(domain='foo1.bar', name='foo1.bar') + limited_config = SiteConfigurationFactory.create(values={'course_org_filter': [filtered_org]}, site=site1) + site2 = SiteFactory.create(domain='foo2.bar', name='foo2.bar') + unlimited_config = SiteConfigurationFactory.create(values={'course_org_filter': []}, site=site2) + + for config in (limited_config, unlimited_config): + ScheduleConfigFactory.create(site=config.site) + + user1 = UserFactory.create(id=tasks.UPGRADE_REMINDER_NUM_BINS) + user2 = UserFactory.create(id=tasks.UPGRADE_REMINDER_NUM_BINS * 2) + + ScheduleFactory.create( + upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), + enrollment__course__org=filtered_org, + enrollment__user=user1, + ) + ScheduleFactory.create( + upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), + enrollment__course__org=unfiltered_org, + enrollment__user=user1, + ) + ScheduleFactory.create( + upgrade_deadline=datetime.datetime(2017, 8, 3, 17, 44, 30, tzinfo=pytz.UTC), + enrollment__course__org=unfiltered_org, + enrollment__user=user2, + ) + + test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) + with self.assertNumQueries(3): + 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, + ) + + self.assertEqual(mock_schedule_send.apply_async.call_count, expected_message_count) + self.assertFalse(mock_ace.send.called) + + @patch.object(tasks, 'ace') + @patch.object(tasks, '_upgrade_reminder_schedule_send') + def test_multiple_enrollments(self, mock_schedule_send, mock_ace): + user = UserFactory.create() + schedules = [ + ScheduleFactory.create( + upgrade_deadline=datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC), + enrollment__user=user, + enrollment__course__id=CourseLocator('edX', 'toy', 'Course{}'.format(course_num)) + ) + for course_num in (1, 2, 3) + ] + + test_time = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) + with self.assertNumQueries(3): + 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, + org_list=[schedules[0].enrollment.course.org], + ) + self.assertEqual(mock_schedule_send.apply_async.call_count, 3) + self.assertFalse(mock_ace.send.called) + + @ddt.data(*itertools.product((1, 10, 100), (2, 10))) + @ddt.unpack + def test_templates(self, message_count, day): + + user = UserFactory.create() + schedules = [ + ScheduleFactory.create( + upgrade_deadline=datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC), + 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) + test_time_str = serialize(test_time) + + patch_policies(self, [StubPolicy([ChannelType.PUSH])]) + mock_channel = Mock( + name='test_channel', + channel_type=ChannelType.EMAIL + ) + patch_channels(self, [mock_channel]) + + 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 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): + 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, + org_list=[schedules[0].enrollment.course.org], + ) + + self.assertEqual(len(sent_messages), message_count) + + for args in sent_messages: + tasks._upgrade_reminder_schedule_send(*args) + + self.assertEqual(mock_channel.deliver.call_count, message_count) + for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls: + for template in attr.astuple(email): + self.assertNotIn("TEMPLATE WARNING", template) diff --git a/openedx/core/djangoapps/schedules/migrations/0004_auto_20170922_1428.py b/openedx/core/djangoapps/schedules/migrations/0004_auto_20170922_1428.py new file mode 100644 index 0000000000..f185696eda --- /dev/null +++ b/openedx/core/djangoapps/schedules/migrations/0004_auto_20170922_1428.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('schedules', '0003_scheduleconfig'), + ] + + operations = [ + migrations.AddField( + model_name='scheduleconfig', + name='deliver_upgrade_reminder', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='scheduleconfig', + name='enqueue_upgrade_reminder', + field=models.BooleanField(default=False), + ), + ] diff --git a/openedx/core/djangoapps/schedules/models.py b/openedx/core/djangoapps/schedules/models.py index bf2df3e41a..558da756bb 100644 --- a/openedx/core/djangoapps/schedules/models.py +++ b/openedx/core/djangoapps/schedules/models.py @@ -35,3 +35,5 @@ class ScheduleConfig(ConfigurationModel): create_schedules = models.BooleanField(default=False) enqueue_recurring_nudge = models.BooleanField(default=False) deliver_recurring_nudge = models.BooleanField(default=False) + enqueue_upgrade_reminder = models.BooleanField(default=False) + deliver_upgrade_reminder = models.BooleanField(default=False) diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py new file mode 100644 index 0000000000..bc86e1c5d8 --- /dev/null +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -0,0 +1,120 @@ +import datetime + +from edx_ace.recipient_resolver import RecipientResolver +from edx_ace.utils.date import serialize + +from openedx.core.djangoapps.schedules.models import ScheduleConfig +from openedx.core.djangoapps.schedules.tasks import ( + DEFAULT_NUM_BINS, + RECURRING_NUDGE_NUM_BINS, + UPGRADE_REMINDER_NUM_BINS, + recurring_nudge_schedule_bin, + upgrade_reminder_schedule_bin +) +from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + + +class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver): + """ + Starts num_bins number of async tasks, each of which sends emails to an equal group of learners. + + Arguments: + site -- Site object that filtered Schedules will be a part of + current_date -- datetime that will be used (with time zeroed-out) as the current date in the queries + + Static attributes: + async_send_task -- celery task function which this resolver will call out to + num_bins -- the int number of bins to split the users into + enqueue_config_var -- the string field name of the config variable on ScheduleConfig to check before enqueuing + """ + async_send_task = None # define in subclass + num_bins = DEFAULT_NUM_BINS + enqueue_config_var = None # define in subclass + + def __init__(self, site, current_date, *args, **kwargs): + super(BinnedSchedulesBaseResolver, self).__init__(*args, **kwargs) + self.site = site + self.current_date = current_date.replace(hour=0, minute=0, second=0) + + def send(self, day_offset, override_recipient_email=None): + if not self.is_enqueue_enabled(): + self.log_debug('Message queuing disabled for site %s', self.site.domain) + return + + exclude_orgs, org_list = self.get_course_org_filter() + + target_date = self.current_date + datetime.timedelta(days=day_offset) + self.log_debug('Target date = %s', target_date.isoformat()) + for bin in range(self.num_bins): + task_args = ( + self.site.id, serialize(target_date), day_offset, bin, org_list, exclude_orgs, override_recipient_email, + ) + self.log_debug('Launching task with args = %r', task_args) + self.async_send_task.apply_async( + task_args, + retry=False, + ) + + def is_enqueue_enabled(self): + if self.enqueue_config_var: + return getattr(ScheduleConfig.current(self.site), self.enqueue_config_var) + return False + + def get_course_org_filter(self): + """ + Given the configuration of sites, get the list of orgs that should be included or excluded from this send. + + Returns: + tuple: Returns a tuple (exclude_orgs, org_list). If exclude_orgs is True, then org_list is a list of the + only orgs that should be included in this send. If exclude_orgs is False, then org_list is a list of + orgs that should be excluded from this send. All other orgs should be included. + """ + try: + site_config = SiteConfiguration.objects.get(site_id=self.site.id) + org_list = site_config.get_value('course_org_filter') + exclude_orgs = False + if not org_list: + not_orgs = set() + for other_site_config in SiteConfiguration.objects.all(): + other = other_site_config.get_value('course_org_filter') + if not isinstance(other, list): + if other is not None: + not_orgs.add(other) + else: + not_orgs.update(other) + org_list = list(not_orgs) + exclude_orgs = True + elif not isinstance(org_list, list): + org_list = [org_list] + except SiteConfiguration.DoesNotExist: + org_list = None + exclude_orgs = False + finally: + return exclude_orgs, org_list + + +class ScheduleStartResolver(BinnedSchedulesBaseResolver): + """ + Send a message to all users whose schedule started at ``self.current_date`` + ``day_offset``. + """ + async_send_task = recurring_nudge_schedule_bin + num_bins = RECURRING_NUDGE_NUM_BINS + enqueue_config_var = 'enqueue_recurring_nudge' + + def __init__(self, *args, **kwargs): + super(ScheduleStartResolver, self).__init__(*args, **kwargs) + self.log_prefix = 'Scheduled Nudge' + + +class UpgradeReminderResolver(BinnedSchedulesBaseResolver): + """ + Send a message to all users whose verified upgrade deadline is at ``self.current_date`` + ``day_offset``. + """ + async_send_task = upgrade_reminder_schedule_bin + num_bins = UPGRADE_REMINDER_NUM_BINS + enqueue_config_var = 'enqueue_upgrade_reminder' + + def __init__(self, *args, **kwargs): + super(UpgradeReminderResolver, self).__init__(*args, **kwargs) + self.log_prefix = 'Upgrade Reminder' diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 4ef6301d9a..c9c7a7968b 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -1,17 +1,17 @@ import datetime from itertools import groupby import logging -from urlparse import urlparse from celery.task import task from django.conf import settings from django.contrib.auth.models import User from django.contrib.sites.models import Site +from django.contrib.staticfiles.templatetags.staticfiles import static from django.core.exceptions import ValidationError from django.core.urlresolvers import reverse -from django.db.models import Min +from django.db.models import F, Min from django.db.utils import DatabaseError -from django.utils.http import urlquote +from django.utils.formats import dateformat, get_format from edx_ace import ace from edx_ace.message import Message @@ -22,6 +22,12 @@ from opaque_keys.edx.keys import CourseKey from edxmako.shortcuts import marketing_link from openedx.core.djangoapps.schedules.message_type import ScheduleMessageType from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig +from openedx.core.djangoapps.schedules.template_context import ( + absolute_url, + encode_url, + encode_urls_in_dict, + get_base_template_context +) LOG = logging.getLogger(__name__) @@ -32,6 +38,9 @@ KNOWN_RETRY_ERRORS = ( # Errors we expect occasionally that could resolve on re DatabaseError, ValidationError, ) +DEFAULT_NUM_BINS = 24 +RECURRING_NUDGE_NUM_BINS = DEFAULT_NUM_BINS +UPGRADE_REMINDER_NUM_BINS = DEFAULT_NUM_BINS @task(bind=True, default_retry_delay=30, routing_key=ROUTING_KEY) @@ -57,6 +66,7 @@ class RecurringNudge(ScheduleMessageType): self.name = "recurringnudge_day{}".format(day) +# TODO: delete once recurring_nudge_schedule_bin is fully rolled out @task(ignore_result=True, routing_key=ROUTING_KEY) def recurring_nudge_schedule_hour( site_id, day, target_hour_str, org_list, exclude_orgs=False, override_recipient_email=None, @@ -64,7 +74,12 @@ def recurring_nudge_schedule_hour( target_hour = deserialize(target_hour_str) msg_type = RecurringNudge(day) - for (user, language, context) in _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs): + for (user, language, context) in _recurring_nudge_schedules_for_hour( + Site.objects.get(id=site_id), + target_hour, + org_list, + exclude_orgs + ): msg = msg_type.personalize( Recipient( user.username, @@ -88,7 +103,8 @@ def _recurring_nudge_schedule_send(site_id, msg_str): ace.send(msg) -def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=False): +# TODO: delete once _recurring_nudge_schedules_for_bin is fully rolled out +def _recurring_nudge_schedules_for_hour(site, target_hour, org_list, exclude_orgs=False): beginning_of_day = target_hour.replace(hour=0, minute=0, second=0) users = User.objects.filter( courseenrollment__schedule__start__gte=beginning_of_day, @@ -128,22 +144,19 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals user_schedules = list(user_schedules) course_id_strs = [str(schedule.enrollment.course_id) for schedule in user_schedules] - def absolute_url(relative_path): - return u'{}{}'.format(settings.LMS_ROOT_URL, urlquote(relative_path)) - first_schedule = user_schedules[0] template_context = { 'student_name': user.profile.name, 'course_name': first_schedule.enrollment.course.display_name, - 'course_url': absolute_url(reverse('course_root', args=[str(first_schedule.enrollment.course_id)])), + 'course_url': absolute_url(site, reverse('course_root', args=[str(first_schedule.enrollment.course_id)])), # This is used by the bulk email optout policy 'course_ids': course_id_strs, # Platform information 'homepage_url': encode_url(marketing_link('ROOT')), - 'dashboard_url': absolute_url(dashboard_relative_url), + 'dashboard_url': absolute_url(site, dashboard_relative_url), 'template_revision': settings.EDX_PLATFORM_REVISION, 'platform_name': settings.PLATFORM_NAME, 'contact_mailing_address': settings.CONTACT_MAILING_ADDRESS, @@ -153,23 +166,197 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals yield (user, first_schedule.enrollment.course.language, template_context) -def encode_url(url): - # Sailthru has a bug where URLs that contain "+" characters in their path components are misinterpreted - # when GA instrumentation is enabled. We need to percent-encode the path segments of all URLs that are - # injected into our templates to work around this issue. - parsed_url = urlparse(url) - modified_url = parsed_url._replace(path=urlquote(parsed_url.path)) - return modified_url.geturl() +@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, +): + target_day = deserialize(target_day_str) + msg_type = RecurringNudge(abs(day_offset)) + + for (user, language, context) in _recurring_nudge_schedules_for_bin( + Site.objects.get(id=site_id), + target_day, + bin_num, + org_list, + exclude_orgs + ): + msg = msg_type.personalize( + Recipient( + user.username, + override_recipient_email or user.email, + ), + language, + context, + ) + _recurring_nudge_schedule_send.apply_async((site_id, str(msg)), retry=False) -def absolute_url(relative_path): - root = settings.LMS_ROOT_URL.rstrip('/') - relative_path = relative_path.lstrip('/') - return encode_url(u'{root}/{path}'.format(root=root, path=relative_path)) +def _recurring_nudge_schedules_for_bin(site, target_day, bin_num, org_list, exclude_orgs=False): + beginning_of_day = target_day.replace(hour=0, minute=0, second=0) + schedules = get_schedules_with_target_date_by_bin_and_orgs( + schedule_date_field='start', + target_date=beginning_of_day, + bin_num=bin_num, + num_bins=RECURRING_NUDGE_NUM_BINS, + org_list=org_list, + exclude_orgs=exclude_orgs, + ) + + LOG.debug('Recurring Nudge: Query = %r', schedules.query.sql_with_params()) + + for (user, user_schedules) in groupby(schedules, lambda s: s.enrollment.user): + user_schedules = list(user_schedules) + course_id_strs = [str(schedule.enrollment.course_id) for schedule in user_schedules] + + first_schedule = user_schedules[0] + template_context = get_base_template_context(site) + template_context.update({ + 'student_name': user.profile.name, + + 'course_name': first_schedule.enrollment.course.display_name, + 'course_url': absolute_url(site, reverse('course_root', args=[str(first_schedule.enrollment.course_id)])), + + # This is used by the bulk email optout policy + 'course_ids': course_id_strs, + }) + yield (user, first_schedule.enrollment.course.language, template_context) -def encode_urls_in_dict(mapping): - urls = {} - for key, value in mapping.iteritems(): - urls[key] = encode_url(value) - return urls +class UpgradeReminder(ScheduleMessageType): + pass + + +@task(ignore_result=True, routing_key=ROUTING_KEY) +def upgrade_reminder_schedule_bin( + site_id, target_day_str, day_offset, bin_num, org_list, exclude_orgs=False, override_recipient_email=None, +): + target_day = deserialize(target_day_str) + msg_type = UpgradeReminder() + + for (user, language, context) in _upgrade_reminder_schedules_for_bin( + Site.objects.get(id=site_id), + target_day, + bin_num, + org_list, + exclude_orgs + ): + msg = msg_type.personalize( + Recipient( + user.username, + override_recipient_email or user.email, + ), + language, + context, + ) + _upgrade_reminder_schedule_send.apply_async((site_id, str(msg)), retry=False) + + +@task(ignore_result=True, routing_key=ROUTING_KEY) +def _upgrade_reminder_schedule_send(site_id, msg_str): + site = Site.objects.get(pk=site_id) + if not ScheduleConfig.current(site).deliver_upgrade_reminder: + return + + msg = Message.from_string(msg_str) + ace.send(msg) + + +def _upgrade_reminder_schedules_for_bin(site, target_day, bin_num, org_list, exclude_orgs=False): + beginning_of_day = target_day.replace(hour=0, minute=0, second=0) + + schedules = get_schedules_with_target_date_by_bin_and_orgs( + schedule_date_field='upgrade_deadline', + target_date=beginning_of_day, + bin_num=bin_num, + num_bins=RECURRING_NUDGE_NUM_BINS, + org_list=org_list, + exclude_orgs=exclude_orgs, + ) + + LOG.debug('Upgrade Reminder: Query = %r', schedules.query.sql_with_params()) + + for schedule in schedules: + enrollment = schedule.enrollment + user = enrollment.user + + course_id_str = str(enrollment.course_id) + + # TODO: group by schedule and user like recurring nudge + course_id_strs = [course_id_str] + first_schedule = schedule + + template_context = get_base_template_context(site) + template_context.update({ + 'student_name': user.profile.name, + 'user_personal_address': user.profile.name if user.profile.name else user.username, + 'user_schedule_upgrade_deadline_time': dateformat.format( + schedule.upgrade_deadline, + get_format( + 'DATE_FORMAT', + lang=first_schedule.enrollment.course.language, + use_l10n=True + ) + ), + + 'course_name': first_schedule.enrollment.course.display_name, + 'course_url': absolute_url(site, reverse('course_root', args=[str(first_schedule.enrollment.course_id)])), + + # This is used by the bulk email optout policy + 'course_ids': course_id_strs, + 'cert_image': absolute_url(site, static('course_experience/images/verified-cert.png')), + }) + + yield (user, first_schedule.enrollment.course.language, template_context) + + +def get_schedules_with_target_date_by_bin_and_orgs(schedule_date_field, target_date, bin_num, num_bins=DEFAULT_NUM_BINS, + org_list=None, exclude_orgs=False): + """ + Returns Schedules with the target_date, related to Users whose id matches the bin_num, and filtered by org_list. + + Arguments: + schedule_date_field -- string field name to query on the User's Schedule model + target_date -- datetime day (with zeroed-out time) that the User's Schedule's schedule_date_field value should fall + under + bin_num -- int for selecting the bin of Users whose id % num_bins == bin_num + num_bin -- int specifying the number of bins to separate the Users into (default: DEFAULT_NUM_BINS) + org_list -- list of course_org names (strings) that the returned Schedules must or must not be in (default: None) + exclude_orgs -- boolean indicating whether the returned Schedules should exclude (True) the course_orgs in org_list + or strictly include (False) them (default: False) + """ + schedule_date_equals_target_date_filter = { + 'courseenrollment__schedule__{}__gte'.format(schedule_date_field): target_date, + 'courseenrollment__schedule__{}__lt'.format(schedule_date_field): target_date + datetime.timedelta(days=1), + } + users = User.objects.filter( + courseenrollment__is_active=True, + **schedule_date_equals_target_date_filter + ).annotate( + id_mod=F('id') % num_bins + ).filter( + id_mod=bin_num + ) + + schedule_date_equals_target_date_filter = { + '{}__gte'.format(schedule_date_field): target_date, + '{}__lt'.format(schedule_date_field): target_date + datetime.timedelta(days=1), + } + schedules = Schedule.objects.select_related( + 'enrollment__user__profile', + 'enrollment__course', + ).filter( + enrollment__user__in=users, + enrollment__is_active=True, + **schedule_date_equals_target_date_filter + ).order_by('enrollment__user__id') + + if org_list is not None: + if exclude_orgs: + schedules = schedules.exclude(enrollment__course__org__in=org_list) + else: + schedules = schedules.filter(enrollment__course__org__in=org_list) + + if "read_replica" in settings.DATABASES: + schedules = schedules.using("read_replica") + + return schedules diff --git a/openedx/core/djangoapps/schedules/template_context.py b/openedx/core/djangoapps/schedules/template_context.py new file mode 100644 index 0000000000..d0b86f3d4b --- /dev/null +++ b/openedx/core/djangoapps/schedules/template_context.py @@ -0,0 +1,43 @@ +from urlparse import urlparse + +from django.conf import settings +from django.core.urlresolvers import reverse +from django.utils.http import urlquote + +from edxmako.shortcuts import marketing_link + + +def get_base_template_context(site): + """Dict with entries needed for all templates that use the base template""" + return { + # Platform information + 'homepage_url': encode_url(marketing_link('ROOT')), + 'dashboard_url': absolute_url(site, reverse('dashboard')), + 'template_revision': settings.EDX_PLATFORM_REVISION, + 'platform_name': settings.PLATFORM_NAME, + 'contact_mailing_address': settings.CONTACT_MAILING_ADDRESS, + 'social_media_urls': encode_urls_in_dict(getattr(settings, 'SOCIAL_MEDIA_FOOTER_URLS', {})), + 'mobile_store_urls': encode_urls_in_dict(getattr(settings, 'MOBILE_STORE_URLS', {})), + } + + +def encode_url(url): + # Sailthru has a bug where URLs that contain "+" characters in their path components are misinterpreted + # when GA instrumentation is enabled. We need to percent-encode the path segments of all URLs that are + # injected into our templates to work around this issue. + parsed_url = urlparse(url) + modified_url = parsed_url._replace(path=urlquote(parsed_url.path)) + return modified_url.geturl() + + +def absolute_url(site, relative_path): + root = site.domain.rstrip('/') + relative_path = relative_path.lstrip('/') + return encode_url(u'https://{root}/{path}'.format(root=root, path=relative_path)) + + +def encode_urls_in_dict(mapping): + urls = {} + for key, value in mapping.iteritems(): + urls[key] = encode_url(value) + return urls diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html new file mode 100644 index 0000000000..d33c12d25f --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html @@ -0,0 +1,74 @@ +{% extends 'schedules/edx_ace/common/base_body.html' %} +{% load i18n %} +{% load static %} + +{% block preview_text %} + {% blocktrans trimmed %} + We hope you are enjoying learning with us so far in {{ course_name }}! A verified certificate + will allow you to highlight your new knowledge and skills. It's official, and easily shareable. + + Upgrade by {{ user_schedule_upgrade_deadline_time }}. + {% endblocktrans %} +{% endblock %} + +{% block content %} + + + + +
+

{% trans "Upgrade now" %}

+ +

+ {% blocktrans trimmed %} + We hope you are enjoying learning with us so far in {{ course_name }}! A + verified certificate will allow you to highlight your new knowledge and skills. It's official, + and easily shareable. + {% endblocktrans %} +

+

+ {% blocktrans trimmed %} + Upgrade by {{ user_schedule_upgrade_deadline_time }}. + {% endblocktrans %} +

+ + + {% blocktrans %}Example print-out of a verified certificate{% endblocktrans %} + + +

+ + + + {% trans "Upgrade now" %} + +

+
+{% endblock %} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.txt new file mode 100644 index 0000000000..3bbf29c78e --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.txt @@ -0,0 +1,14 @@ +{% load i18n %} + +{% blocktrans trimmed %} +Dear {{ user_personal_address }}, +{% endblocktrans %} + +{% blocktrans trimmed %} + We hope you are enjoying learning with us so far in {{ course_name }}! A verified certificate + will allow you to highlight your new knowledge and skills. It's official, and easily shareable. + + Upgrade by {{ user_schedule_upgrade_deadline_time }}. +{% endblocktrans %} + +{% trans "Upgrade now at" %} <{{ course_url }}> diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/from_name.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/from_name.txt new file mode 100644 index 0000000000..08e86bddd0 --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/from_name.txt @@ -0,0 +1 @@ +{{ course_name }} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/head.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/head.html new file mode 100644 index 0000000000..588357ec65 --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/head.html @@ -0,0 +1 @@ +{% extends 'schedules/edx_ace/common/base_head.html' %} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt new file mode 100644 index 0000000000..7daea3543a --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt @@ -0,0 +1,3 @@ +{% load i18n %} + +{% blocktrans %}Upgrade to earn a verified certificate in {{ course_name }}{% endblocktrans %} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.html deleted file mode 100644 index 8da84e439a..0000000000 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.html +++ /dev/null @@ -1,7 +0,0 @@ -Dear {{ user_personal_address }}, -
-We hope you are enjoying {{ course_title }}. -Upgrade by {{ user_schedule_verified_upgrade_deadline_time|date:"l, F dS, Y" }} -to get a shareable certificate! -
-Upgrade now diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.txt deleted file mode 100644 index beb0ae45ba..0000000000 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.txt +++ /dev/null @@ -1,6 +0,0 @@ -Dear {{ user_personal_address }}, - -We hope you are enjoying {{ course_title }}. -Upgrade by {{ user_schedule_verified_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! - -Upgrade now at {{course_verified_upgrade_url}} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/subject.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/subject.txt deleted file mode 100644 index 8ca024cf72..0000000000 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/subject.txt +++ /dev/null @@ -1 +0,0 @@ -Only two days left to upgrade! \ No newline at end of file diff --git a/openedx/core/djangoapps/schedules/tests/factories.py b/openedx/core/djangoapps/schedules/tests/factories.py index 7c286afb7f..13c88e403d 100644 --- a/openedx/core/djangoapps/schedules/tests/factories.py +++ b/openedx/core/djangoapps/schedules/tests/factories.py @@ -23,3 +23,5 @@ class ScheduleConfigFactory(factory.DjangoModelFactory): create_schedules = True enqueue_recurring_nudge = True deliver_recurring_nudge = True + enqueue_upgrade_reminder = True + deliver_upgrade_reminder = True diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py new file mode 100644 index 0000000000..0854c52b56 --- /dev/null +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -0,0 +1,110 @@ +import datetime +from unittest import skipUnless + +import ddt +from django.conf import settings +from mock import patch + +from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver +from openedx.core.djangoapps.schedules.tasks import DEFAULT_NUM_BINS +from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory +from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms + + +@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 TestBinnedSchedulesBaseResolver(CacheIsolationTestCase): + def setUp(self): + super(TestBinnedSchedulesBaseResolver, self).setUp() + + self.site = SiteFactory.create() + self.site_config = SiteConfigurationFactory.create(site=self.site) + self.schedule_config = ScheduleConfigFactory.create(site=self.site) + + def setup_resolver(self, site=None, current_date=None): + if site is None: + site = self.site + if current_date is None: + current_date = datetime.datetime.now() + resolver = BinnedSchedulesBaseResolver(self.site, current_date) + return resolver + + def test_init_site(self): + resolver = self.setup_resolver() + assert resolver.site == self.site + + def test_init_current_date(self): + current_time = datetime.datetime.now() + resolver = self.setup_resolver(current_date=current_time) + current_date = current_time.replace(hour=0, minute=0, second=0) + assert resolver.current_date == current_date + + def test_init_async_send_task(self): + resolver = self.setup_resolver() + assert resolver.async_send_task is None + + def test_init_num_bins(self): + resolver = self.setup_resolver() + assert resolver.num_bins == DEFAULT_NUM_BINS + + def test_send_enqueue_disabled(self): + resolver = self.setup_resolver() + resolver.is_enqueue_enabled = lambda: False + with patch.object(resolver, 'async_send_task') as send: + with patch.object(resolver, 'log_debug') as log_debug: + resolver.send(day_offset=2) + log_debug.assert_called_once_with('Message queuing disabled for site %s', self.site.domain) + send.apply_async.assert_not_called() + + @ddt.data(0, 2, -3) + def test_send_enqueue_enabled(self, day_offset): + resolver = self.setup_resolver() + resolver.is_enqueue_enabled = lambda: True + resolver.get_course_org_filter = lambda: (False, None) + with patch.object(resolver, 'async_send_task') as send: + with patch.object(resolver, 'log_debug') as log_debug: + resolver.send(day_offset=day_offset) + target_date = resolver.current_date + datetime.timedelta(day_offset) + log_debug.assert_any_call('Target date = %s', target_date.isoformat()) + assert send.apply_async.call_count == DEFAULT_NUM_BINS + + @ddt.data(True, False) + def test_is_enqueue_enabled(self, enabled): + resolver = self.setup_resolver() + resolver.enqueue_config_var = 'enqueue_recurring_nudge' + self.schedule_config.enqueue_recurring_nudge = enabled + self.schedule_config.save() + assert resolver.is_enqueue_enabled() == enabled + + @ddt.unpack + @ddt.data( + ('course1', ['course1']), + (['course1', 'course2'], ['course1', 'course2']) + ) + def test_get_course_org_filter_include(self, course_org_filter, expected_org_list): + resolver = self.setup_resolver() + self.site_config.values['course_org_filter'] = course_org_filter + self.site_config.save() + exclude_orgs, org_list = resolver.get_course_org_filter() + assert not exclude_orgs + assert org_list == expected_org_list + + @ddt.unpack + @ddt.data( + (None, []), + ('course1', [u'course1']), + (['course1', 'course2'], [u'course1', u'course2']) + ) + def test_get_course_org_filter_exclude(self, course_org_filter, expected_org_list): + resolver = self.setup_resolver() + self.other_site = SiteFactory.create() + self.other_site_config = SiteConfigurationFactory.create( + site=self.other_site, + values={'course_org_filter': course_org_filter}, + ) + exclude_orgs, org_list = resolver.get_course_org_filter() + assert exclude_orgs + self.assertItemsEqual(org_list, expected_org_list) diff --git a/openedx/core/djangoapps/schedules/utils.py b/openedx/core/djangoapps/schedules/utils.py new file mode 100644 index 0000000000..5705d95d22 --- /dev/null +++ b/openedx/core/djangoapps/schedules/utils.py @@ -0,0 +1,14 @@ +import logging + +LOG = logging.getLogger(__name__) + + +# TODO: consider using a LoggerAdapter instead of this mixin: +# https://docs.python.org/2/library/logging.html#logging.LoggerAdapter +class PrefixedDebugLoggerMixin(object): + def __init__(self, *args, **kwargs): + super(PrefixedDebugLoggerMixin, self).__init__(*args, **kwargs) + self.log_prefix = self.__class__.__name__ + + def log_debug(self, message, *args, **kwargs): + LOG.debug(self.log_prefix + ': ' + message, *args, **kwargs) diff --git a/openedx/core/djangoapps/site_configuration/tests/factories.py b/openedx/core/djangoapps/site_configuration/tests/factories.py index 08dde09734..3813d76077 100644 --- a/openedx/core/djangoapps/site_configuration/tests/factories.py +++ b/openedx/core/djangoapps/site_configuration/tests/factories.py @@ -3,7 +3,7 @@ Model factories for unit testing views or models. """ from django.contrib.sites.models import Site from factory.django import DjangoModelFactory -from factory import SubFactory, Sequence, SelfAttribute +from factory import SubFactory, Sequence, SelfAttribute, lazy_attribute from openedx.core.djangoapps.site_configuration.models import SiteConfiguration @@ -27,6 +27,9 @@ class SiteConfigurationFactory(DjangoModelFactory): class Meta(object): model = SiteConfiguration - values = {} enabled = True site = SubFactory(SiteFactory) + + @lazy_attribute + def values(self): + return {}