Skip to content

E
edx-platform
Commit 9876f597 by Tyler Hallada
Options

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
parent efe814c6
Showing with 367 additions and 388 deletions
openedx/core/djangoapps/schedules/management/commands/__init__.py
import datetime import datetime
import logging
import pytz import pytz
from django.contrib.sites.models import Site from django.contrib.sites.models import Site
from django.core.management.base import BaseCommand 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.utils import PrefixedDebugLoggerMixin
from openedx.core.djangoapps.schedules.tasks import DEFAULT_NUM_BINS
from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
LOG = logging.getLogger(__name__) class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand):
resolver_class = None # define in subclass
# 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__
def add_arguments(self, parser): def add_arguments(self, parser):
parser.add_argument( parser.add_argument(
...@@ -130,4 +39,4 @@ class SendEmailBaseCommand(BaseCommand, PrefixedDebugLoggerMixin): ...@@ -130,4 +39,4 @@ class SendEmailBaseCommand(BaseCommand, PrefixedDebugLoggerMixin):
return self.resolver_class(site, current_date) return self.resolver_class(site, current_date)
def send_emails(self, resolver, *args, **options): def send_emails(self, resolver, *args, **options):
resolver.send(0, options.get('override_recipient_email')) pass # define in subclass
openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py
from __future__ import print_function from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
from openedx.core.djangoapps.schedules.resolvers import ScheduleStartResolver
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'
class Command(SendEmailBaseCommand): class Command(SendEmailBaseCommand):
resolver_class = ScheduleStartResolver
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(Command, self).__init__(*args, **kwargs) super(Command, self).__init__(*args, **kwargs)
self.resolver_class = ScheduleStartResolver
self.log_prefix = 'Scheduled Nudge' self.log_prefix = 'Scheduled Nudge'
def send_emails(self, resolver, *args, **options): def send_emails(self, resolver, *args, **options):
......
openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py
from __future__ import print_function from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
from openedx.core.djangoapps.schedules.resolvers import UpgradeReminderResolver
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): class Command(SendEmailBaseCommand):
resolver_class = UpgradeReminderResolver
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(Command, self).__init__(*args, **kwargs) super(Command, self).__init__(*args, **kwargs)
self.resolver_class = UpgradeReminderResolver
self.log_prefix = 'Upgrade Reminder' self.log_prefix = 'Upgrade Reminder'
def send_emails(self, resolver, *args, **options): def send_emails(self, resolver, *args, **options):
logging.basicConfig(level=logging.DEBUG)
resolver.send(2, options.get('override_recipient_email')) resolver.send(2, options.get('override_recipient_email'))
openedx/core/djangoapps/schedules/management/commands/tests/test_base.py
...@@ -4,15 +4,10 @@ from unittest import skipUnless ...@@ -4,15 +4,10 @@ from unittest import skipUnless
import ddt import ddt
import pytz import pytz
from django.conf import settings from django.conf import settings
from mock import patch, Mock from mock import patch
from openedx.core.djangoapps.schedules.management.commands import ( from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
DEFAULT_NUM_BINS, from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
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.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
...@@ -20,110 +15,12 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un ...@@ -20,110 +15,12 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un
@skip_unless_lms @skip_unless_lms
@skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS, @skipUnless('openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS,
"Can't test schedules if the app isn't installed") "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,
"Can't test schedules if the app isn't installed")
class TestSendEmailBaseCommand(CacheIsolationTestCase): class TestSendEmailBaseCommand(CacheIsolationTestCase):
def setUp(self): def setUp(self):
self.command = SendEmailBaseCommand() self.command = SendEmailBaseCommand()
def test_init_resolver_class(self): def test_init_resolver_class(self):
assert self.command.resolver_class == BinnedSchedulesBaseResolver assert self.command.resolver_class is None
def test_make_resolver(self): def test_make_resolver(self):
with patch.object(self.command, 'resolver_class') as resolver_class: with patch.object(self.command, 'resolver_class') as resolver_class:
...@@ -134,11 +31,6 @@ class TestSendEmailBaseCommand(CacheIsolationTestCase): ...@@ -134,11 +31,6 @@ class TestSendEmailBaseCommand(CacheIsolationTestCase):
datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC) 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): def test_handle(self):
with patch.object(self.command, 'make_resolver') as make_resolver: with patch.object(self.command, 'make_resolver') as make_resolver:
make_resolver.return_value = 'resolver' make_resolver.return_value = 'resolver'
......
openedx/core/djangoapps/schedules/management/commands/tests/test_send_recurring_nudge.py
...@@ -14,7 +14,7 @@ from mock import Mock, patch ...@@ -14,7 +14,7 @@ from mock import Mock, patch
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import CourseLocator 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.management.commands import send_recurring_nudge as nudge
from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
...@@ -40,7 +40,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): ...@@ -40,7 +40,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
self.site_config = SiteConfigurationFactory.create(site=site) self.site_config = SiteConfigurationFactory.create(site=site)
ScheduleConfigFactory.create(site=self.site_config.site) ScheduleConfigFactory.create(site=self.site_config.site)
@patch.object(nudge, 'ScheduleStartResolver') @patch.object(nudge.Command, 'resolver_class')
def test_handle(self, mock_resolver): def test_handle(self, mock_resolver):
test_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) 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) nudge.Command().handle(date='2017-08-01', site_domain_name=self.site_config.site.domain)
...@@ -50,7 +50,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): ...@@ -50,7 +50,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
mock_resolver().send.assert_any_call(day, None) mock_resolver().send.assert_any_call(day, None)
@patch.object(tasks, 'ace') @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): def test_resolver_send(self, mock_schedule_bin, mock_ace):
current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
nudge.ScheduleStartResolver(self.site_config.site, current_time).send(-3) nudge.ScheduleStartResolver(self.site_config.site, current_time).send(-3)
...@@ -80,8 +80,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase): ...@@ -80,8 +80,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC)
test_time_str = serialize(test_time) 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( tasks.recurring_nudge_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b, self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b,
org_list=[schedules[0].enrollment.course.org], org_list=[schedules[0].enrollment.course.org],
...@@ -100,8 +101,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase): ...@@ -100,8 +101,9 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC)
test_time_str = serialize(test_time) 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( tasks.recurring_nudge_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b, self.site_config.site.id, target_day_str=test_time_str, day_offset=-3, bin_num=b,
org_list=[schedule.enrollment.course.org], org_list=[schedule.enrollment.course.org],
...@@ -124,7 +126,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase): ...@@ -124,7 +126,7 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
self.assertFalse(mock_ace.send.called) self.assertFalse(mock_ace.send.called)
@patch.object(tasks, 'ace') @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): def test_enqueue_disabled(self, mock_schedule_bin, mock_ace):
ScheduleConfigFactory.create(site=self.site_config.site, enqueue_recurring_nudge=False) ScheduleConfigFactory.create(site=self.site_config.site, enqueue_recurring_nudge=False)
......
openedx/core/djangoapps/schedules/management/commands/tests/test_send_upgrade_reminder.py
...@@ -14,7 +14,7 @@ from mock import Mock, patch ...@@ -14,7 +14,7 @@ from mock import Mock, patch
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import CourseLocator 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.management.commands import send_upgrade_reminder as reminder
from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory from openedx.core.djangoapps.schedules.tests.factories import ScheduleConfigFactory, ScheduleFactory
from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory
...@@ -40,7 +40,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): ...@@ -40,7 +40,7 @@ class TestUpgradeReminder(CacheIsolationTestCase):
self.site_config = SiteConfigurationFactory.create(site=site) self.site_config = SiteConfigurationFactory.create(site=site)
ScheduleConfigFactory.create(site=self.site_config.site) ScheduleConfigFactory.create(site=self.site_config.site)
@patch.object(reminder, 'UpgradeReminderResolver') @patch.object(reminder.Command, 'resolver_class')
def test_handle(self, mock_resolver): def test_handle(self, mock_resolver):
test_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) 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) reminder.Command().handle(date='2017-08-01', site_domain_name=self.site_config.site.domain)
...@@ -49,7 +49,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): ...@@ -49,7 +49,7 @@ class TestUpgradeReminder(CacheIsolationTestCase):
mock_resolver().send.assert_any_call(2, None) mock_resolver().send.assert_any_call(2, None)
@patch.object(tasks, 'ace') @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): def test_resolver_send(self, mock_schedule_bin, mock_ace):
current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC) current_time = datetime.datetime(2017, 8, 1, tzinfo=pytz.UTC)
test_time = current_time + datetime.timedelta(days=2) test_time = current_time + datetime.timedelta(days=2)
...@@ -81,8 +81,9 @@ class TestUpgradeReminder(CacheIsolationTestCase): ...@@ -81,8 +81,9 @@ class TestUpgradeReminder(CacheIsolationTestCase):
test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC) test_time = datetime.datetime(2017, 8, 3, 18, tzinfo=pytz.UTC)
test_time_str = serialize(test_time) 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( tasks.upgrade_reminder_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=b, self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=b,
org_list=[schedules[0].enrollment.course.org], org_list=[schedules[0].enrollment.course.org],
...@@ -101,8 +102,9 @@ class TestUpgradeReminder(CacheIsolationTestCase): ...@@ -101,8 +102,9 @@ class TestUpgradeReminder(CacheIsolationTestCase):
test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC) test_time = datetime.datetime(2017, 8, 3, 20, tzinfo=pytz.UTC)
test_time_str = serialize(test_time) 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( tasks.upgrade_reminder_schedule_bin(
self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=b, self.site_config.site.id, target_day_str=test_time_str, day_offset=2, bin_num=b,
org_list=[schedule.enrollment.course.org], org_list=[schedule.enrollment.course.org],
...@@ -125,7 +127,7 @@ class TestUpgradeReminder(CacheIsolationTestCase): ...@@ -125,7 +127,7 @@ class TestUpgradeReminder(CacheIsolationTestCase):
self.assertFalse(mock_ace.send.called) self.assertFalse(mock_ace.send.called)
@patch.object(tasks, 'ace') @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): def test_enqueue_disabled(self, mock_schedule_bin, mock_ace):
ScheduleConfigFactory.create(site=self.site_config.site, enqueue_upgrade_reminder=False) ScheduleConfigFactory.create(site=self.site_config.site, enqueue_upgrade_reminder=False)
......
openedx/core/djangoapps/schedules/resolvers.py 0 → 100644
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'
openedx/core/djangoapps/schedules/tasks.py
...@@ -181,36 +181,16 @@ def recurring_nudge_schedule_bin( ...@@ -181,36 +181,16 @@ def recurring_nudge_schedule_bin(
def _recurring_nudge_schedules_for_bin(target_day, bin_num, org_list, exclude_orgs=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) beginning_of_day = target_day.replace(hour=0, minute=0, second=0)
users = User.objects.filter( schedules = get_schedules_with_target_date_by_bin_and_orgs(
courseenrollment__schedule__start__gte=beginning_of_day, schedule_date_field='start',
courseenrollment__schedule__start__lt=beginning_of_day + datetime.timedelta(days=1), target_date=beginning_of_day,
courseenrollment__is_active=True, bin_num=bin_num,
).annotate( num_bins=RECURRING_NUDGE_NUM_BINS,
first_schedule=Min('courseenrollment__schedule__start') org_list=org_list,
).annotate( exclude_orgs=exclude_orgs,
id_mod=F('id') % RECURRING_NUDGE_NUM_BINS
).filter(
id_mod=bin_num
) )
schedules = Schedule.objects.select_related( LOG.debug('Recurring Nudge: Query = %r', schedules.query.sql_with_params())
'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): for (user, user_schedules) in groupby(schedules, lambda s: s.enrollment.user):
user_schedules = list(user_schedules) user_schedules = list(user_schedules)
...@@ -265,36 +245,17 @@ def _upgrade_reminder_schedule_send(site_id, msg_str): ...@@ -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): 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) 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',
).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: schedules = get_schedules_with_target_date_by_bin_and_orgs(
if exclude_orgs: schedule_date_field='upgrade_deadline',
schedules = schedules.exclude(enrollment__course__org__in=org_list) target_date=beginning_of_day,
else: bin_num=bin_num,
schedules = schedules.filter(enrollment__course__org__in=org_list) num_bins=RECURRING_NUDGE_NUM_BINS,
org_list=org_list,
exclude_orgs=exclude_orgs,
)
if "read_replica" in settings.DATABASES: LOG.debug('Upgrade Reminder: Query = %r', schedules.query.sql_with_params())
schedules = schedules.using("read_replica")
for schedule in schedules: for schedule in schedules:
enrollment = schedule.enrollment enrollment = schedule.enrollment
...@@ -327,3 +288,56 @@ def _upgrade_reminder_schedules_for_bin(target_day, bin_num, org_list, exclude_o ...@@ -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) 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
openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html
...@@ -2,22 +2,12 @@ ...@@ -2,22 +2,12 @@
{% load i18n %} {% load i18n %}
{% block preview_text %} {% block preview_text %}
{% if course_ids|length > 1 %} {% blocktrans trimmed %}
{% blocktrans trimmed %} We hope you are enjoying learning with us so far in {{ course_name }}! A verified certificate
We hope you are enjoying learning with us so far in {{ course_name }}, and other courses on will allow you to highlight your new knowledge and skills. It's official, and easily shareable.
{{ 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 }}. Upgrade by {{ user_schedule_upgrade_deadline_time }}.
{% endblocktrans %} {% 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 %}
{% endblock %} {% endblock %}
{% block content %} {% block content %}
...@@ -26,33 +16,18 @@ ...@@ -26,33 +16,18 @@
<td> <td>
<h1>{% trans "Upgrade now" %}</h1> <h1>{% trans "Upgrade now" %}</h1>
{% if course_ids|length > 1 %} <p>
<p> {% blocktrans trimmed %}
{% blocktrans trimmed %} We hope you are enjoying learning with us so far in <strong>{{ course_name }}</strong>! A
We hope you are enjoying learning with us so far in <strong>{{ course_name }}</strong>, and verified certificate will allow you to highlight your new knowledge and skills. It's official,
other courses on {{ platform_name }}! A verified certificate will allow you to highlight your and easily shareable.
new knowledge and skills. It's official, and easily shareable. {% endblocktrans %}
{% endblocktrans %} </p>
</p> <p>
<p> {% blocktrans trimmed %}
{% blocktrans trimmed %} Upgrade by <strong>{{ user_schedule_upgrade_deadline_time }}</strong>.
Upgrade by <strong>{{ user_schedule_upgrade_deadline_time }}</strong>. {% endblocktrans %}
{% endblocktrans %} </p>
</p>
{% else %}
<p>
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in <strong>{{ course_name }}</strong>! A
verified certificate will allow you to highlight your new knowledge and skills. It's official,
and easily shareable.
{% endblocktrans %}
</p>
<p>
{% blocktrans trimmed %}
Upgrade by <strong>{{ user_schedule_upgrade_deadline_time }}</strong>.
{% endblocktrans %}
</p>
{% endif %}
<p> <p>
<!-- email client support for style sheets is pretty spotty, so we have to inline all of these styles --> <!-- email client support for style sheets is pretty spotty, so we have to inline all of these styles -->
......
openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.txt
...@@ -4,17 +4,6 @@ ...@@ -4,17 +4,6 @@
Dear {{ user_personal_address }}, Dear {{ user_personal_address }},
{% endblocktrans %} {% 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 %} {% blocktrans trimmed %}
We hope you are enjoying learning with us so far in {{ course_name }}! A verified 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. will allow you to highlight your new knowledge and skills. It's official, and easily shareable.
...@@ -23,4 +12,3 @@ Dear {{ user_personal_address }}, ...@@ -23,4 +12,3 @@ Dear {{ user_personal_address }},
{% endblocktrans %} {% endblocktrans %}
{% trans "Upgrade now at" %} <{{ course_url }}> {% trans "Upgrade now at" %} <{{ course_url }}>
{% endif %}
openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/from_name.txt
{% if course_ids|length > 1 %}
{{ platform_name }}
{% else %}
{{ course_name }} {{ course_name }}
{% endif %}
openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/subject.txt
{% load i18n %} {% 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 %} {% blocktrans %}Upgrade to earn a verified certificate in {{ course_name }}{% endblocktrans %}
{% endif %}
openedx/core/djangoapps/schedules/tests/test_resolvers.py 0 → 100644
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)
openedx/core/djangoapps/schedules/utils.py 0 → 100644
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)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment