From 0c0a5a93c048d14d6983f5d97fc7f1c4bae6b8ed Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Wed, 20 Sep 2017 16:22:22 -0400 Subject: [PATCH 1/8] Create func for common template context entries --- openedx/core/djangoapps/schedules/tasks.py | 41 ++---------------- .../djangoapps/schedules/template_context.py | 43 +++++++++++++++++++ 2 files changed, 47 insertions(+), 37 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/template_context.py diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 4ef6301d9a..8de9b0ad5a 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -19,9 +19,9 @@ from edx_ace.recipient import Recipient from edx_ace.utils.date import deserialize 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, get_base_template_context LOG = logging.getLogger(__name__) @@ -128,11 +128,9 @@ 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 = { + template_context = get_base_template_context() + template_context.update({ 'student_name': user.profile.name, 'course_name': first_schedule.enrollment.course.display_name, @@ -140,36 +138,5 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals # 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), - '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', {})), - } + }) 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() - - -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 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/template_context.py b/openedx/core/djangoapps/schedules/template_context.py new file mode 100644 index 0000000000..f04b0dd9d9 --- /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(): + """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(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(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 encode_urls_in_dict(mapping): + urls = {} + for key, value in mapping.iteritems(): + urls[key] = encode_url(value) + return urls From 1bcd3a45ca145baa35b8ed072983f3fe5ed08e24 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Wed, 27 Sep 2017 10:12:29 -0400 Subject: [PATCH 2/8] Refactor upgrade_reminder to use async celery task Finish test_base.py tests Address some PR comments Some test_send_recurring_nudge fixes Fix test_schedule_bin Fix test_site_config Fix test_multiple_enrollments Tests pass now! Use consistent naming: upgrade_reminder --- .../schedules/management/commands/__init__.py | 131 ++++++++++++ .../commands/send_recurring_nudge.py | 105 ++-------- .../commands/send_upgrade_reminder.py | 35 ++++ ...send_verified_upgrade_deadline_reminder.py | 116 ----------- .../management/commands/tests/test_base.py | 149 ++++++++++++++ .../tests/test_send_recurring_nudge.py | 126 ++++++------ .../migrations/0004_auto_20170922_1428.py | 24 +++ openedx/core/djangoapps/schedules/models.py | 2 + openedx/core/djangoapps/schedules/tasks.py | 187 +++++++++++++++++- .../edx_ace/upgradereminder/email/body.html | 67 +++++++ .../edx_ace/upgradereminder/email/body.txt | 21 ++ .../upgradereminder/email/from_name.txt | 5 + .../edx_ace/upgradereminder/email/head.html | 1 + .../edx_ace/upgradereminder/email/subject.txt | 7 + .../email/body.html | 7 - .../email/body.txt | 6 - .../email/subject.txt | 1 - .../site_configuration/tests/factories.py | 7 +- 18 files changed, 717 insertions(+), 280 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py delete mode 100644 openedx/core/djangoapps/schedules/management/commands/send_verified_upgrade_deadline_reminder.py create mode 100644 openedx/core/djangoapps/schedules/management/commands/tests/test_base.py create mode 100644 openedx/core/djangoapps/schedules/migrations/0004_auto_20170922_1428.py create mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html create mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.txt create mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/from_name.txt create mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/head.html create mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt delete mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.html delete mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/body.txt delete mode 100644 openedx/core/djangoapps/schedules/templates/schedules/edx_ace/verifiedupgradedeadlinereminder/email/subject.txt diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index e69de29bb2..1aaf31701f 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -0,0 +1,131 @@ +import datetime +import logging + +import pytz +from django.contrib.sites.models import Site +from django.core.management.base import BaseCommand +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 +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + + +LOG = logging.getLogger(__name__) + + +class PrefixedDebugLoggerMixin(object): + def __init__(self, *args, **kwargs): + self.log_prefix = self.__class__.__name__ + + def log_debug(self, message, *args, **kwargs): + LOG.debug(self.log_prefix + ': ' + message, *args, **kwargs) + + +class BinnedSchedulesBaseResolver(RecipientResolver, PrefixedDebugLoggerMixin): + """ + Starts num_bins number of async tasks, each of which sends emails to an equal group of learners. + """ + 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) + self.async_send_task = None # define in subclasses + self.num_bins = DEFAULT_NUM_BINS + self.enqueue_config_var = None # define in subclasses + self.log_prefix = self.__class__.__name__ + + 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 SendEmailBaseCommand(BaseCommand, PrefixedDebugLoggerMixin): + def __init__(self, *args, **kwargs): + super(SendEmailBaseCommand, self).__init__(*args, **kwargs) + self.resolver_class = BinnedSchedulesBaseResolver + self.log_prefix = self.__class__.__name__ + + 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): + resolver.send(0, options.get('override_recipient_email')) 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..26902bbaab 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,31 @@ 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 +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand, BinnedSchedulesBaseResolver +from openedx.core.djangoapps.schedules.tasks import RECURRING_NUDGE_NUM_BINS, recurring_nudge_schedule_bin LOG = logging.getLogger(__name__) -class ScheduleStartResolver(RecipientResolver): - def __init__(self, site, current_date): - self.site = site - self.current_date = current_date.replace(hour=0, minute=0, second=0) - - 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 - - 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 ScheduleStartResolver(BinnedSchedulesBaseResolver): + """ + Send a message to all users whose schedule started at ``self.current_date`` + ``day_offset``. + """ + def __init__(self, *args, **kwargs): + super(ScheduleStartResolver, self).__init__(*args, **kwargs) + self.async_send_task = recurring_nudge_schedule_bin + self.num_bins = RECURRING_NUDGE_NUM_BINS + self.log_prefix = 'Scheduled Nudge' + self.enqueue_config_var = 'enqueue_recurring_nudge' -class Command(BaseCommand): +class Command(SendEmailBaseCommand): + def __init__(self, *args, **kwargs): + super(Command, self).__init__(*args, **kwargs) + self.resolver_class = ScheduleStartResolver + self.log_prefix = 'Scheduled Nudge' - 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..9d5956d013 --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py @@ -0,0 +1,35 @@ +from __future__ import print_function + +import logging + +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand, BinnedSchedulesBaseResolver +from openedx.core.djangoapps.schedules.tasks import ( + UPGRADE_REMINDER_NUM_BINS, + upgrade_reminder_schedule_bin +) + + +LOG = logging.getLogger(__name__) + + +class UpgradeReminderResolver(BinnedSchedulesBaseResolver): + """ + Send a message to all users whose verified upgrade deadline is at ``self.current_date`` + ``day_offset``. + """ + def __init__(self, *args, **kwargs): + super(UpgradeReminderResolver, self).__init__(*args, **kwargs) + self.async_send_task = upgrade_reminder_schedule_bin + self.num_bins = UPGRADE_REMINDER_NUM_BINS + self.log_prefix = 'Upgrade Reminder' + self.enqueue_config_var = 'enqueue_upgrade_reminder' + + +class Command(SendEmailBaseCommand): + def __init__(self, *args, **kwargs): + super(Command, self).__init__(*args, **kwargs) + self.resolver_class = UpgradeReminderResolver + self.log_prefix = 'Upgrade Reminder' + + def send_emails(self, resolver, *args, **options): + logging.basicConfig(level=logging.DEBUG) + 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..6147204d5e --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py @@ -0,0 +1,149 @@ +import datetime +from unittest import skipUnless + +import ddt +import pytz +from django.conf import settings +from mock import patch, Mock + +from openedx.core.djangoapps.schedules.management.commands import ( + DEFAULT_NUM_BINS, + SendEmailBaseCommand, + BinnedSchedulesBaseResolver +) +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 + + +@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 + + # factory_boy doesn't make sense at all + @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) + + +@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 == BinnedSchedulesBaseResolver + + 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_send_emails(self): + resolver = Mock() + self.command.send_emails(resolver, override_recipient_email='foo@example.com') + resolver.send.assert_called_once_with(0, 'foo@example.com') + + 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..14ff129f45 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 @@ -47,22 +46,22 @@ class TestSendRecurringNudge(CacheIsolationTestCase): 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(nudge, 'recurring_nudge_schedule_bin') + 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, 23, [], True, None), retry=False, ) self.assertFalse(mock_ace.send.called) @@ -70,17 +69,23 @@ 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) + with self.assertNumQueries(25): + for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + 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 +93,19 @@ 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) + with self.assertNumQueries(25): + for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + 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 +124,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(nudge, 'recurring_nudge_schedule_bin') + 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 +152,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)) + test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) 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, + 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)) + test_time = datetime.datetime(2017, 8, 3, 19, 44, 30, tzinfo=pytz.UTC) + test_time_str = serialize(test_time) with self.assertNumQueries(2): - tasks.recurring_nudge_schedule_hour( - self.site_config.site.id, 3, test_time_str, [schedules[0].enrollment.course.org], + 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( @@ -232,8 +239,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase): 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], + 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/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/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 8de9b0ad5a..c687150c53 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -1,7 +1,6 @@ import datetime from itertools import groupby import logging -from urlparse import urlparse from celery.task import task from django.conf import settings @@ -9,9 +8,8 @@ from django.contrib.auth.models import User from django.contrib.sites.models import Site 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, Prefetch from django.db.utils import DatabaseError -from django.utils.http import urlquote from edx_ace import ace from edx_ace.message import Message @@ -19,9 +17,17 @@ from edx_ace.recipient import Recipient from edx_ace.utils.date import deserialize from opaque_keys.edx.keys import CourseKey +from course_modes.models import CourseMode +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, get_base_template_context +from openedx.core.djangoapps.schedules.template_context import ( + absolute_url, + encode_url, + encode_urls_in_dict, + get_base_template_context +) +from openedx.core.djangoapps.user_api.models import UserPreference 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, @@ -88,6 +98,7 @@ def _recurring_nudge_schedule_send(site_id, msg_str): ace.send(msg) +# TODO: delete once _recurring_nudge_schedules_for_bin is fully rolled out def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=False): beginning_of_day = target_hour.replace(hour=0, minute=0, second=0) users = User.objects.filter( @@ -124,6 +135,84 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals dashboard_relative_url = reverse('dashboard') + 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 = { + '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)])), + + # 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), + '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', {})), + } + yield (user, first_schedule.enrollment.course.language, template_context) + + +@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(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 _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=False): + beginning_of_day = target_day.replace(hour=0, minute=0, second=0) + users = User.objects.filter( + courseenrollment__schedule__start__gte=beginning_of_day, + courseenrollment__schedule__start__lt=beginning_of_day + datetime.timedelta(days=1), + courseenrollment__is_active=True, + ).annotate( + first_schedule=Min('courseenrollment__schedule__start') + ).annotate( + id_mod=F('id') % RECURRING_NUDGE_NUM_BINS + ).filter( + id_mod=bin_num + ) + + schedules = Schedule.objects.select_related( + 'enrollment__user__profile', + 'enrollment__course', + ).filter( + enrollment__user__in=users, + start__gte=beginning_of_day, + start__lt=beginning_of_day + datetime.timedelta(days=1), + enrollment__is_active=True, + ).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") + 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] @@ -140,3 +229,93 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals 'course_ids': course_id_strs, }) yield (user, first_schedule.enrollment.course.language, template_context) + + +class UpgradeReminder(ScheduleMessageType): + def __init__(self, day, *args, **kwargs): + super(UpgradeReminder, self).__init__(*args, **kwargs) + self.name = "upgradereminder".format(day) + + +@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(abs(day_offset)) + + for (user, language, context) in _upgrade_reminder_schedules_for_bin(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(target_day, bin_num, org_list, exclude_orgs=False): + 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' + ), + ).annotate( + id_mod=F('enrollment__user__id') % UPGRADE_REMINDER_NUM_BINS + ).filter( + id_mod=bin_num + ).filter( + upgrade_deadline__year=target_day.year, + upgrade_deadline__month=target_day.month, + upgrade_deadline__day=target_day.day, + ) + + if "read_replica" in settings.DATABASES: + schedules = schedules.using("read_replica") + + for schedule in schedules: + enrollment = schedule.enrollment + user = enrollment.user + + course_id_str = str(enrollment.course_id) + course = enrollment.course + + # TODO: group by schedule and user like recurring nudge + course_id_strs = [course_id_str] + first_schedule = schedule + + template_context = get_base_template_context() + 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': schedule.upgrade_deadline, + + 'course_name': first_schedule.enrollment.course.display_name, + 'course_url': absolute_url(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, course.language, template_context) 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..70f6fc3299 --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html @@ -0,0 +1,67 @@ +{% extends 'schedules/edx_ace/common/base_body.html' %} +{% load i18n %} + +{% block preview_text %} + {% if courses|length > 1 %} + {% blocktrans trimmed %} + We hope you are enjoying {{ course_name }}, and other courses on edX.org. + Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + {% endblocktrans %} + {% else %} + {% blocktrans trimmed %} + We hope you are enjoying {{ course_name }}. + Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + {% endblocktrans %} + {% endif %} +{% endblock %} + +{% block content %} + + + + +
+

{% trans "Upgrade now" %}

+ +

+ {% if courses|length > 1 %} + {% blocktrans trimmed %} + We hope you are enjoying {{ course_name }}, and other courses on edX.org. + {{ user_schedule_upgrade_deadline_time|date }} + Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + {% endblocktrans %} + {% else %} + {% blocktrans trimmed %} + We hope you are enjoying {{ course_name }}. + Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + {% endblocktrans %} + {% endif %} +

+ +

+ + 1 %} + href="{{ dashboard_url }}" + {% else %} + href="{{ course_url }}" + {% endif %} + style=" + color: #ffffff; + text-decoration: none; + border-radius: 4px; + -webkit-border-radius: 4px; + -moz-border-radius: 4px; + background-color: #005686; + border-top: 10px solid #005686; + border-bottom: 10px solid #005686; + border-right: 16px solid #005686; + border-left: 16px solid #005686; + display: inline-block; + "> + + {% 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..83d76022de --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.txt @@ -0,0 +1,21 @@ +{% load i18n %} + +{% blocktrans trimmed %} +Dear {{ user_personal_address }}, +{% endblocktrans %} + +{% if courses|length > 1 %} +{% blocktrans trimmed %} + We hope you are enjoying {{ course_name }}, and other courses on edX.org. + Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! +{% endblocktrans %} + +{% trans "Upgrade now at" %} <{{ dashboard_url }}> +{% else %} +{% blocktrans trimmed %} + We hope you are enjoying {{ course_name }}. + Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! +{% endblocktrans %} + +{% trans "Upgrade now at" %} <{{ course_url }}> +{% endif %} 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..f07331f68a --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/from_name.txt @@ -0,0 +1,5 @@ +{% if courses|length > 1 %} +{{ platform_name }} +{% else %} +{{ course_name }} +{% endif %} 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..bcc0c6d217 --- /dev/null +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt @@ -0,0 +1,7 @@ +{% load i18n %} + +{% if courses|length > 1 %} +{% blocktrans %}Only two days left to upgrade on {{ platform_name }}!{% endblocktrans %} +{% else %} +{% blocktrans %}Only two days left to upgrade in {{course_name}} !{% endblocktrans %} +{% endif %} 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/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 {} From 004ca4800e2cb5d088b3847d217657113fef5050 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Mon, 2 Oct 2017 18:25:42 -0400 Subject: [PATCH 3/8] Add tests for upgrade_reminder Fix test_templates By passing the date pre-formatted to ace. Address PR comments Fix linting Add TODO comment about using LoggerAdapter More lint fixes Fix upgrade_reminder task from silently failing --- .../schedules/management/commands/__init__.py | 2 + .../management/commands/tests/test_base.py | 3 +- .../tests/test_send_recurring_nudge.py | 2 +- .../tests/test_send_upgrade_reminder.py | 257 ++++++++++++++++++ openedx/core/djangoapps/schedules/tasks.py | 66 +++-- .../edx_ace/upgradereminder/email/body.html | 15 +- .../edx_ace/upgradereminder/email/body.txt | 6 +- .../upgradereminder/email/from_name.txt | 2 +- .../edx_ace/upgradereminder/email/subject.txt | 2 +- .../djangoapps/schedules/tests/factories.py | 2 + 10 files changed, 312 insertions(+), 45 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index 1aaf31701f..d39d4f01eb 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -15,6 +15,8 @@ from openedx.core.djangoapps.site_configuration.models import SiteConfiguration 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): self.log_prefix = self.__class__.__name__ diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py index 6147204d5e..e2cb00a870 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py @@ -11,7 +11,7 @@ from openedx.core.djangoapps.schedules.management.commands import ( SendEmailBaseCommand, BinnedSchedulesBaseResolver ) -from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory +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 @@ -96,7 +96,6 @@ class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase): assert not exclude_orgs assert org_list == expected_org_list - # factory_boy doesn't make sense at all @ddt.unpack @ddt.data( (None, []), 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 14ff129f45..3b83a1d952 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 @@ -61,7 +61,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): retry=False, ) mock_schedule_bin.apply_async.assert_any_call( - (self.site_config.site.id, serialize(test_time), -3, 23, [], True, None), + (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) 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..329bcdd065 --- /dev/null +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py @@ -0,0 +1,257 @@ +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 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, 'UpgradeReminderResolver') + 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(reminder, 'upgrade_reminder_schedule_bin') + 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) + with self.assertNumQueries(25): + for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + 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) + with self.assertNumQueries(25): + for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + 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(reminder, 'upgrade_reminder_schedule_bin') + 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(2): + 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(2): + 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(2): + 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/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index c687150c53..a9cdcfcd51 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -8,8 +8,9 @@ from django.contrib.auth.models import User from django.contrib.sites.models import Site from django.core.exceptions import ValidationError from django.core.urlresolvers import reverse -from django.db.models import F, Min, Prefetch +from django.db.models import F, Min from django.db.utils import DatabaseError +from django.utils.formats import dateformat, get_format from edx_ace import ace from edx_ace.message import Message @@ -17,7 +18,6 @@ from edx_ace.recipient import Recipient from edx_ace.utils.date import deserialize from opaque_keys.edx.keys import CourseKey -from course_modes.models import CourseMode from edxmako.shortcuts import marketing_link from openedx.core.djangoapps.schedules.message_type import ScheduleMessageType from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig @@ -27,7 +27,6 @@ from openedx.core.djangoapps.schedules.template_context import ( encode_urls_in_dict, get_base_template_context ) -from openedx.core.djangoapps.user_api.models import UserPreference LOG = logging.getLogger(__name__) @@ -232,9 +231,7 @@ def _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_or class UpgradeReminder(ScheduleMessageType): - def __init__(self, day, *args, **kwargs): - super(UpgradeReminder, self).__init__(*args, **kwargs) - self.name = "upgradereminder".format(day) + pass @task(ignore_result=True, routing_key=ROUTING_KEY) @@ -242,7 +239,7 @@ 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(abs(day_offset)) + msg_type = UpgradeReminder() for (user, language, context) in _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs): msg = msg_type.personalize( @@ -267,29 +264,34 @@ def _upgrade_reminder_schedule_send(site_id, msg_str): def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=False): + beginning_of_day = target_day.replace(hour=0, minute=0, second=0) + users = User.objects.filter( + courseenrollment__schedule__upgrade_deadline__gte=beginning_of_day, + courseenrollment__schedule__upgrade_deadline__lt=beginning_of_day + datetime.timedelta(days=1), + courseenrollment__is_active=True, + ).annotate( + first_schedule=Min('courseenrollment__schedule__upgrade_deadline') + ).annotate( + id_mod=F('id') % UPGRADE_REMINDER_NUM_BINS + ).filter( + id_mod=bin_num + ) + 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' - ), - ).annotate( - id_mod=F('enrollment__user__id') % UPGRADE_REMINDER_NUM_BINS ).filter( - id_mod=bin_num - ).filter( - upgrade_deadline__year=target_day.year, - upgrade_deadline__month=target_day.month, - upgrade_deadline__day=target_day.day, - ) + enrollment__user__in=users, + upgrade_deadline__gte=beginning_of_day, + upgrade_deadline__lt=beginning_of_day + datetime.timedelta(days=1), + enrollment__is_active=True, + ).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") @@ -299,7 +301,6 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o user = enrollment.user course_id_str = str(enrollment.course_id) - course = enrollment.course # TODO: group by schedule and user like recurring nudge course_id_strs = [course_id_str] @@ -309,7 +310,14 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o 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': schedule.upgrade_deadline, + '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(reverse('course_root', args=[str(first_schedule.enrollment.course_id)])), @@ -318,4 +326,4 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o 'course_ids': course_id_strs, }) - yield (user, course.language, template_context) + yield (user, first_schedule.enrollment.course.language, template_context) 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 index 70f6fc3299..448b656628 100644 --- 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 @@ -2,15 +2,15 @@ {% load i18n %} {% block preview_text %} - {% if courses|length > 1 %} + {% if course_ids|length > 1 %} {% blocktrans trimmed %} We hope you are enjoying {{ course_name }}, and other courses on edX.org. - Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! {% endblocktrans %} {% else %} {% blocktrans trimmed %} We hope you are enjoying {{ course_name }}. - Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! {% endblocktrans %} {% endif %} {% endblock %} @@ -22,16 +22,15 @@

{% trans "Upgrade now" %}

- {% if courses|length > 1 %} + {% if course_ids|length > 1 %} {% blocktrans trimmed %} We hope you are enjoying {{ course_name }}, and other courses on edX.org. - {{ user_schedule_upgrade_deadline_time|date }} - Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! {% endblocktrans %} {% else %} {% blocktrans trimmed %} We hope you are enjoying {{ course_name }}. - Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! {% endblocktrans %} {% endif %}

@@ -39,7 +38,7 @@

1 %} + {% if course_ids|length > 1 %} href="{{ dashboard_url }}" {% else %} href="{{ course_url }}" 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 index 83d76022de..9eee300a2b 100644 --- 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 @@ -4,17 +4,17 @@ Dear {{ user_personal_address }}, {% endblocktrans %} -{% if courses|length > 1 %} +{% if course_ids|length > 1 %} {% blocktrans trimmed %} We hope you are enjoying {{ course_name }}, and other courses on edX.org. - Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! {% endblocktrans %} {% trans "Upgrade now at" %} <{{ dashboard_url }}> {% else %} {% blocktrans trimmed %} We hope you are enjoying {{ course_name }}. - Upgrade by {{ user_schedule_upgrade_deadline_time|date:"l, F dS, Y" }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! {% 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 index f07331f68a..d25f5d3e4a 100644 --- 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 @@ -1,4 +1,4 @@ -{% if courses|length > 1 %} +{% if course_ids|length > 1 %} {{ platform_name }} {% else %} {{ course_name }} 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 index bcc0c6d217..1f1a6e4988 100644 --- 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 @@ -1,6 +1,6 @@ {% load i18n %} -{% if courses|length > 1 %} +{% if course_ids|length > 1 %} {% blocktrans %}Only two days left to upgrade on {{ platform_name }}!{% endblocktrans %} {% else %} {% blocktrans %}Only two days left to upgrade in {{course_name}} !{% endblocktrans %} 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 From 4acdcc698e6d58865a1f2472c15dbd09696b4295 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Wed, 4 Oct 2017 15:51:33 -0400 Subject: [PATCH 4/8] Display new flags in the ScheduleConfig admin list --- openedx/core/djangoapps/schedules/admin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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') From efe814c64f74f08fad8df2fdc17283858b8ec0f3 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Wed, 4 Oct 2017 15:51:57 -0400 Subject: [PATCH 5/8] Update copy for the upgrade_reminder emails --- .../edx_ace/upgradereminder/email/body.html | 45 +++++++++++++------ .../edx_ace/upgradereminder/email/body.txt | 13 ++++-- .../edx_ace/upgradereminder/email/subject.txt | 4 +- 3 files changed, 43 insertions(+), 19 deletions(-) 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 index 448b656628..1746524cf3 100644 --- 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 @@ -4,13 +4,18 @@ {% block preview_text %} {% if course_ids|length > 1 %} {% blocktrans trimmed %} - We hope you are enjoying {{ course_name }}, and other courses on edX.org. - Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! + We hope you are enjoying learning with us so far in {{ course_name }}, and other courses on + {{ platform_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 %} {% else %} {% blocktrans trimmed %} - We hope you are enjoying {{ course_name }}. - Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! + 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 %} {% endif %} {% endblock %} @@ -21,19 +26,33 @@

{% trans "Upgrade now" %}

-

- {% if course_ids|length > 1 %} + {% if course_ids|length > 1 %} +

{% blocktrans trimmed %} - We hope you are enjoying {{ course_name }}, and other courses on edX.org. - Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! + We hope you are enjoying learning with us so far in {{ course_name }}, and + other courses on {{ platform_name }}! A verified certificate will allow you to highlight your + new knowledge and skills. It's official, and easily shareable. {% endblocktrans %} - {% else %} +

+

{% blocktrans trimmed %} - We hope you are enjoying {{ course_name }}. - Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! + Upgrade by {{ user_schedule_upgrade_deadline_time }}. {% endblocktrans %} - {% endif %} -

+

+ {% else %} +

+ {% 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 %} +

+ {% endif %}

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 index 9eee300a2b..0efa861d4b 100644 --- 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 @@ -6,15 +6,20 @@ Dear {{ user_personal_address }}, {% if course_ids|length > 1 %} {% blocktrans trimmed %} - We hope you are enjoying {{ course_name }}, and other courses on edX.org. - Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! + We hope you are enjoying learning with us so far in {{ course_name }}, and other courses on + {{ platform_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" %} <{{ dashboard_url }}> {% else %} {% blocktrans trimmed %} - We hope you are enjoying {{ course_name }}. - Upgrade by {{ user_schedule_upgrade_deadline_time }} to get a shareable certificate! + 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/subject.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt index 1f1a6e4988..b207b935fe 100644 --- 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 @@ -1,7 +1,7 @@ {% load i18n %} {% if course_ids|length > 1 %} -{% blocktrans %}Only two days left to upgrade on {{ platform_name }}!{% endblocktrans %} +{% blocktrans %}Upgrade to earn a verified certificate on {{ platform_name }}{% endblocktrans %} {% else %} -{% blocktrans %}Only two days left to upgrade in {{course_name}} !{% endblocktrans %} +{% blocktrans %}Upgrade to earn a verified certificate in {{ course_name }}{% endblocktrans %} {% endif %} From 9876f597e6bd489d6ae44710fe8e7d63acdfa7e9 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Wed, 4 Oct 2017 17:35:00 -0400 Subject: [PATCH 6/8] Refactor common task querying into a separate func Address some of Cale's PR comments Combine query functions into one. No debug logging Pull int variables out into static class variables Mixin needs to call super __init__ too Remove multi-course copy from upgrade reminder Address Cale's round 2 comments --- .../schedules/management/commands/__init__.py | 99 +------------- .../commands/send_recurring_nudge.py | 25 +--- .../commands/send_upgrade_reminder.py | 30 +---- .../management/commands/tests/test_base.py | 116 +--------------- .../tests/test_send_recurring_nudge.py | 18 +-- .../tests/test_send_upgrade_reminder.py | 18 +-- .../core/djangoapps/schedules/resolvers.py | 120 +++++++++++++++++ openedx/core/djangoapps/schedules/tasks.py | 126 ++++++++++-------- .../edx_ace/upgradereminder/email/body.html | 59 +++----- .../edx_ace/upgradereminder/email/body.txt | 12 -- .../upgradereminder/email/from_name.txt | 4 - .../edx_ace/upgradereminder/email/subject.txt | 4 - .../schedules/tests/test_resolvers.py | 110 +++++++++++++++ openedx/core/djangoapps/schedules/utils.py | 14 ++ 14 files changed, 367 insertions(+), 388 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/resolvers.py create mode 100644 openedx/core/djangoapps/schedules/tests/test_resolvers.py create mode 100644 openedx/core/djangoapps/schedules/utils.py diff --git a/openedx/core/djangoapps/schedules/management/commands/__init__.py b/openedx/core/djangoapps/schedules/management/commands/__init__.py index d39d4f01eb..3be3b9f1a9 100644 --- a/openedx/core/djangoapps/schedules/management/commands/__init__.py +++ b/openedx/core/djangoapps/schedules/management/commands/__init__.py @@ -1,105 +1,14 @@ import datetime -import logging import pytz from django.contrib.sites.models import Site from django.core.management.base import BaseCommand -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 -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration +from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin -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): - self.log_prefix = self.__class__.__name__ - - def log_debug(self, message, *args, **kwargs): - LOG.debug(self.log_prefix + ': ' + message, *args, **kwargs) - - -class BinnedSchedulesBaseResolver(RecipientResolver, PrefixedDebugLoggerMixin): - """ - Starts num_bins number of async tasks, each of which sends emails to an equal group of learners. - """ - 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) - self.async_send_task = None # define in subclasses - self.num_bins = DEFAULT_NUM_BINS - self.enqueue_config_var = None # define in subclasses - self.log_prefix = self.__class__.__name__ - - 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 SendEmailBaseCommand(BaseCommand, PrefixedDebugLoggerMixin): - def __init__(self, *args, **kwargs): - super(SendEmailBaseCommand, self).__init__(*args, **kwargs) - self.resolver_class = BinnedSchedulesBaseResolver - self.log_prefix = self.__class__.__name__ +class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand): + resolver_class = None # define in subclass def add_arguments(self, parser): parser.add_argument( @@ -130,4 +39,4 @@ class SendEmailBaseCommand(BaseCommand, PrefixedDebugLoggerMixin): return self.resolver_class(site, current_date) def send_emails(self, resolver, *args, **options): - resolver.send(0, options.get('override_recipient_email')) + 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 26902bbaab..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,29 +1,12 @@ -from __future__ import print_function - -import logging - -from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand, BinnedSchedulesBaseResolver -from openedx.core.djangoapps.schedules.tasks import RECURRING_NUDGE_NUM_BINS, recurring_nudge_schedule_bin - -LOG = logging.getLogger(__name__) - - -class ScheduleStartResolver(BinnedSchedulesBaseResolver): - """ - Send a message to all users whose schedule started at ``self.current_date`` + ``day_offset``. - """ - def __init__(self, *args, **kwargs): - super(ScheduleStartResolver, self).__init__(*args, **kwargs) - self.async_send_task = recurring_nudge_schedule_bin - self.num_bins = RECURRING_NUDGE_NUM_BINS - self.log_prefix = 'Scheduled Nudge' - self.enqueue_config_var = 'enqueue_recurring_nudge' +from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand +from openedx.core.djangoapps.schedules.resolvers import ScheduleStartResolver class Command(SendEmailBaseCommand): + resolver_class = ScheduleStartResolver + def __init__(self, *args, **kwargs): super(Command, self).__init__(*args, **kwargs) - self.resolver_class = ScheduleStartResolver self.log_prefix = 'Scheduled Nudge' def send_emails(self, resolver, *args, **options): diff --git a/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py b/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py index 9d5956d013..c251c85275 100644 --- a/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py +++ b/openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py @@ -1,35 +1,13 @@ -from __future__ import print_function - -import logging - -from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand, BinnedSchedulesBaseResolver -from openedx.core.djangoapps.schedules.tasks import ( - UPGRADE_REMINDER_NUM_BINS, - upgrade_reminder_schedule_bin -) - - -LOG = logging.getLogger(__name__) - - -class UpgradeReminderResolver(BinnedSchedulesBaseResolver): - """ - Send a message to all users whose verified upgrade deadline is at ``self.current_date`` + ``day_offset``. - """ - def __init__(self, *args, **kwargs): - super(UpgradeReminderResolver, self).__init__(*args, **kwargs) - self.async_send_task = upgrade_reminder_schedule_bin - self.num_bins = UPGRADE_REMINDER_NUM_BINS - self.log_prefix = 'Upgrade Reminder' - self.enqueue_config_var = 'enqueue_upgrade_reminder' +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.resolver_class = UpgradeReminderResolver self.log_prefix = 'Upgrade Reminder' def send_emails(self, resolver, *args, **options): - logging.basicConfig(level=logging.DEBUG) resolver.send(2, options.get('override_recipient_email')) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py index e2cb00a870..ce75828775 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/test_base.py @@ -4,116 +4,13 @@ from unittest import skipUnless import ddt import pytz from django.conf import settings -from mock import patch, Mock +from mock import patch -from openedx.core.djangoapps.schedules.management.commands import ( - DEFAULT_NUM_BINS, - SendEmailBaseCommand, - BinnedSchedulesBaseResolver -) -from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory -from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory +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 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) - - @ddt.ddt @skip_unless_lms @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, @@ -123,7 +20,7 @@ class TestSendEmailBaseCommand(CacheIsolationTestCase): self.command = SendEmailBaseCommand() def test_init_resolver_class(self): - assert self.command.resolver_class == BinnedSchedulesBaseResolver + assert self.command.resolver_class is None def test_make_resolver(self): with patch.object(self.command, 'resolver_class') as resolver_class: @@ -134,11 +31,6 @@ class TestSendEmailBaseCommand(CacheIsolationTestCase): datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC) ) - def test_send_emails(self): - resolver = Mock() - self.command.send_emails(resolver, override_recipient_email='foo@example.com') - resolver.send.assert_called_once_with(0, 'foo@example.com') - def test_handle(self): with patch.object(self.command, 'make_resolver') as make_resolver: make_resolver.return_value = 'resolver' 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 3b83a1d952..4dcec134c7 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 @@ -14,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 @@ -40,7 +40,7 @@ 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) @@ -50,7 +50,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): mock_resolver().send.assert_any_call(day, None) @patch.object(tasks, 'ace') - @patch.object(nudge, 'recurring_nudge_schedule_bin') + @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) @@ -80,8 +80,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(25): - for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + # waffle flag takes an extra query before it is cached + with self.assertNumQueries(2 if b == 0 else 1): 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], @@ -100,8 +101,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(25): - for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + for b in range(tasks.RECURRING_NUDGE_NUM_BINS): + # waffle flag takes an extra query before it is cached + with self.assertNumQueries(2 if b == 0 else 1): 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], @@ -124,7 +126,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): self.assertFalse(mock_ace.send.called) @patch.object(tasks, 'ace') - @patch.object(nudge, 'recurring_nudge_schedule_bin') + @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) 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 329bcdd065..7c4507b144 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,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_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 @@ -40,7 +40,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): self.site_config = SiteConfigurationFactory.create(site=site) ScheduleConfigFactory.create(site=self.site_config.site) - @patch.object(reminder, 'UpgradeReminderResolver') + @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) @@ -49,7 +49,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): mock_resolver().send.assert_any_call(2, None) @patch.object(tasks, 'ace') - @patch.object(reminder, 'upgrade_reminder_schedule_bin') + @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) @@ -81,8 +81,9 @@ class TestUpgradeReminder(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(25): - for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + # waffle flag takes an extra query before it is cached + with self.assertNumQueries(2 if b == 0 else 1): 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], @@ -101,8 +102,9 @@ class TestUpgradeReminder(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(25): - for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + for b in range(tasks.UPGRADE_REMINDER_NUM_BINS): + # waffle flag takes an extra query before it is cached + with self.assertNumQueries(2 if b == 0 else 1): 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], @@ -125,7 +127,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): self.assertFalse(mock_ace.send.called) @patch.object(tasks, 'ace') - @patch.object(reminder, 'upgrade_reminder_schedule_bin') + @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) 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 a9cdcfcd51..2e8dcff8f9 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -181,36 +181,16 @@ def recurring_nudge_schedule_bin( def _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=False): beginning_of_day = target_day.replace(hour=0, minute=0, second=0) - users = User.objects.filter( - courseenrollment__schedule__start__gte=beginning_of_day, - courseenrollment__schedule__start__lt=beginning_of_day + datetime.timedelta(days=1), - courseenrollment__is_active=True, - ).annotate( - first_schedule=Min('courseenrollment__schedule__start') - ).annotate( - id_mod=F('id') % RECURRING_NUDGE_NUM_BINS - ).filter( - id_mod=bin_num + 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, ) - schedules = Schedule.objects.select_related( - 'enrollment__user__profile', - 'enrollment__course', - ).filter( - enrollment__user__in=users, - start__gte=beginning_of_day, - start__lt=beginning_of_day + datetime.timedelta(days=1), - enrollment__is_active=True, - ).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") + 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) @@ -265,36 +245,17 @@ def _upgrade_reminder_schedule_send(site_id, msg_str): def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=False): beginning_of_day = target_day.replace(hour=0, minute=0, second=0) - users = User.objects.filter( - courseenrollment__schedule__upgrade_deadline__gte=beginning_of_day, - courseenrollment__schedule__upgrade_deadline__lt=beginning_of_day + datetime.timedelta(days=1), - courseenrollment__is_active=True, - ).annotate( - first_schedule=Min('courseenrollment__schedule__upgrade_deadline') - ).annotate( - id_mod=F('id') % UPGRADE_REMINDER_NUM_BINS - ).filter( - id_mod=bin_num + + 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, ) - schedules = Schedule.objects.select_related( - 'enrollment__user__profile', - 'enrollment__course', - ).filter( - enrollment__user__in=users, - upgrade_deadline__gte=beginning_of_day, - upgrade_deadline__lt=beginning_of_day + datetime.timedelta(days=1), - enrollment__is_active=True, - ).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") + LOG.debug('Upgrade Reminder: Query = %r', schedules.query.sql_with_params()) for schedule in schedules: enrollment = schedule.enrollment @@ -327,3 +288,56 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o }) 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/templates/schedules/edx_ace/upgradereminder/email/body.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html index 1746524cf3..6d02528764 100644 --- 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 @@ -2,22 +2,12 @@ {% load i18n %} {% block preview_text %} - {% if course_ids|length > 1 %} - {% blocktrans trimmed %} - We hope you are enjoying learning with us so far in {{ course_name }}, and other courses on - {{ platform_name }}! A verified certificate will allow you to highlight your new knowledge and - skills. It's official, and easily shareable. + {% 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 %} - {% else %} - {% 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 %} - {% endif %} + Upgrade by {{ user_schedule_upgrade_deadline_time }}. + {% endblocktrans %} {% endblock %} {% block content %} @@ -26,33 +16,18 @@

{% trans "Upgrade now" %}

- {% if course_ids|length > 1 %} -

- {% blocktrans trimmed %} - We hope you are enjoying learning with us so far in {{ course_name }}, and - other courses on {{ platform_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 %} -

- {% else %} -

- {% 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 %} -

- {% endif %} +

+ {% 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 %} +

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 index 0efa861d4b..3bbf29c78e 100644 --- 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 @@ -4,17 +4,6 @@ Dear {{ user_personal_address }}, {% endblocktrans %} -{% if course_ids|length > 1 %} -{% blocktrans trimmed %} - We hope you are enjoying learning with us so far in {{ course_name }}, and other courses on - {{ platform_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" %} <{{ dashboard_url }}> -{% else %} {% 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. @@ -23,4 +12,3 @@ Dear {{ user_personal_address }}, {% endblocktrans %} {% trans "Upgrade now at" %} <{{ course_url }}> -{% endif %} 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 index d25f5d3e4a..08e86bddd0 100644 --- 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 @@ -1,5 +1 @@ -{% if course_ids|length > 1 %} -{{ platform_name }} -{% else %} {{ course_name }} -{% endif %} 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 index b207b935fe..7daea3543a 100644 --- 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 @@ -1,7 +1,3 @@ {% load i18n %} -{% if course_ids|length > 1 %} -{% blocktrans %}Upgrade to earn a verified certificate on {{ platform_name }}{% endblocktrans %} -{% else %} {% blocktrans %}Upgrade to earn a verified certificate in {{ course_name }}{% endblocktrans %} -{% endif %} 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) From e7b3590448cc782314aa0d4e9e59d961e871ce67 Mon Sep 17 00:00:00 2001 From: Tyler Hallada Date: Thu, 5 Oct 2017 17:27:54 -0400 Subject: [PATCH 7/8] Add verified cert image to email template Make image url in the task and pass to template --- openedx/core/djangoapps/schedules/tasks.py | 2 + .../edx_ace/upgradereminder/email/body.html | 46 ++++++++++++------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 2e8dcff8f9..b1b01eefae 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -6,6 +6,7 @@ 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 F, Min @@ -285,6 +286,7 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o # This is used by the bulk email optout policy 'course_ids': course_id_strs, + 'cert_image': absolute_url(static('course_experience/images/verified-cert.png')), }) yield (user, first_schedule.enrollment.course.language, template_context) 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 index 6d02528764..d33c12d25f 100644 --- 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 @@ -1,5 +1,6 @@ {% extends 'schedules/edx_ace/common/base_body.html' %} {% load i18n %} +{% load static %} {% block preview_text %} {% blocktrans trimmed %} @@ -29,26 +30,39 @@ {% endblocktrans %}

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

1 %} - href="{{ dashboard_url }}" - {% else %} href="{{ course_url }}" - {% endif %} - style=" - color: #ffffff; - text-decoration: none; - border-radius: 4px; - -webkit-border-radius: 4px; - -moz-border-radius: 4px; - background-color: #005686; - border-top: 10px solid #005686; - border-bottom: 10px solid #005686; - border-right: 16px solid #005686; - border-left: 16px solid #005686; - display: inline-block; + style=" + color: #ffffff; + text-decoration: none; + border-radius: 4px; + -webkit-border-radius: 4px; + -moz-border-radius: 4px; + background-color: #005686; + border-top: 10px solid #005686; + border-bottom: 10px solid #005686; + border-right: 16px solid #005686; + border-left: 16px solid #005686; + display: inline-block; "> {% trans "Upgrade now" %} From d5b6b170ddf6176750417185a7ca0029ac5eaa40 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 6 Oct 2017 13:22:29 -0400 Subject: [PATCH 8/8] Use the current site to generate absolute urls for emails --- .../tests/test_send_recurring_nudge.py | 10 ++--- .../tests/test_send_upgrade_reminder.py | 10 ++--- openedx/core/djangoapps/schedules/tasks.py | 43 +++++++++++++------ .../djangoapps/schedules/template_context.py | 10 ++--- 4 files changed, 45 insertions(+), 28 deletions(-) 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 4dcec134c7..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 @@ -82,7 +82,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): 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(2 if b == 0 else 1): + 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], @@ -103,7 +103,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): 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(2 if b == 0 else 1): + 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], @@ -175,7 +175,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(2): + 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, @@ -199,7 +199,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(2): + 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, @@ -240,7 +240,7 @@ 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): + 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], 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 7c4507b144..8a6ed80d0f 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 @@ -83,7 +83,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): 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(2 if b == 0 else 1): + 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], @@ -104,7 +104,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): 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(2 if b == 0 else 1): + 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], @@ -176,7 +176,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): test_time = datetime.datetime(2017, 8, 3, 17, tzinfo=pytz.UTC) test_time_str = serialize(test_time) - with self.assertNumQueries(2): + 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, @@ -200,7 +200,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(2): + 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, @@ -241,7 +241,7 @@ 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(2): + 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, diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index b1b01eefae..c9c7a7968b 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -74,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, @@ -99,7 +104,7 @@ def _recurring_nudge_schedule_send(site_id, msg_str): # TODO: delete once _recurring_nudge_schedules_for_bin is fully rolled out -def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=False): +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, @@ -144,14 +149,14 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals '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, @@ -168,7 +173,13 @@ def recurring_nudge_schedule_bin( target_day = deserialize(target_day_str) msg_type = RecurringNudge(abs(day_offset)) - for (user, language, context) in _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs): + 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, @@ -180,7 +191,7 @@ def recurring_nudge_schedule_bin( _recurring_nudge_schedule_send.apply_async((site_id, str(msg)), retry=False) -def _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=False): +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', @@ -198,12 +209,12 @@ def _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_or course_id_strs = [str(schedule.enrollment.course_id) for schedule in user_schedules] first_schedule = user_schedules[0] - template_context = get_base_template_context() + 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(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, @@ -222,7 +233,13 @@ def upgrade_reminder_schedule_bin( target_day = deserialize(target_day_str) msg_type = UpgradeReminder() - for (user, language, context) in _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs): + 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, @@ -244,7 +261,7 @@ def _upgrade_reminder_schedule_send(site_id, msg_str): ace.send(msg) -def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=False): +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( @@ -268,7 +285,7 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o course_id_strs = [course_id_str] first_schedule = schedule - template_context = get_base_template_context() + 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, @@ -282,11 +299,11 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o ), '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, - 'cert_image': absolute_url(static('course_experience/images/verified-cert.png')), + 'cert_image': absolute_url(site, static('course_experience/images/verified-cert.png')), }) yield (user, first_schedule.enrollment.course.language, template_context) diff --git a/openedx/core/djangoapps/schedules/template_context.py b/openedx/core/djangoapps/schedules/template_context.py index f04b0dd9d9..d0b86f3d4b 100644 --- a/openedx/core/djangoapps/schedules/template_context.py +++ b/openedx/core/djangoapps/schedules/template_context.py @@ -7,12 +7,12 @@ from django.utils.http import urlquote from edxmako.shortcuts import marketing_link -def get_base_template_context(): +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(reverse('dashboard')), + '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, @@ -30,10 +30,10 @@ def encode_url(url): return modified_url.geturl() -def absolute_url(relative_path): - root = settings.LMS_ROOT_URL.rstrip('/') +def absolute_url(site, relative_path): + root = site.domain.rstrip('/') relative_path = relative_path.lstrip('/') - return encode_url(u'{root}/{path}'.format(root=root, path=relative_path)) + return encode_url(u'https://{root}/{path}'.format(root=root, path=relative_path)) def encode_urls_in_dict(mapping):