Merge pull request #16293 from edx/cale/refactor-upgrade-email-resolvers

Cale/refactor upgrade email resolvers

openedx/core/djangoapps/schedules/management/commands/__init__.py

@@ -8,7 +8,7 @@ from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin
 
 
 class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand):
-    resolver_class = None  # define in subclass
+    async_send_task = None  # define in subclass
 
     def add_arguments(self, parser):
         parser.add_argument(
@@ -23,20 +23,27 @@ class SendEmailBaseCommand(PrefixedDebugLoggerMixin, BaseCommand):
         parser.add_argument('site_domain_name')
 
     def handle(self, *args, **options):
-        resolver = self.make_resolver(*args, **options)
-        self.send_emails(resolver, *args, **options)
+        self.log_debug('Args = %r', options)
 
-    def make_resolver(self, *args, **options):
         current_date = datetime.datetime(
             *[int(x) for x in options['date'].split('-')],
             tzinfo=pytz.UTC
         )
-        self.log_debug('Args = %r', options)
         self.log_debug('Current date = %s', current_date.isoformat())
 
         site = Site.objects.get(domain__iexact=options['site_domain_name'])
         self.log_debug('Running for site %s', site.domain)
-        return self.resolver_class(site, current_date)
 
-    def send_emails(self, resolver, *args, **options):
-        pass  # define in subclass
+        override_recipient_email = options.get('override_recipient_email')
+        self.send_emails(site, current_date, override_recipient_email)
+
+    def send_emails(self, *args, **kwargs):
+        raise NotImplementedError
+
+    def enqueue(self, day_offset, site, current_date, override_recipient_email=None):
+        self.async_send_task.enqueue(
+            site,
+            current_date,
+            day_offset,
+            override_recipient_email,
+        )

openedx/core/djangoapps/schedules/management/commands/send_course_update.py

 from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
-from openedx.core.djangoapps.schedules.resolvers import CourseUpdateResolver
+from openedx.core.djangoapps.schedules.tasks import ScheduleCourseUpdate
 
 
 class Command(SendEmailBaseCommand):
-    resolver_class = CourseUpdateResolver
+    async_send_task = ScheduleCourseUpdate
 
     def __init__(self, *args, **kwargs):
         super(Command, self).__init__(*args, **kwargs)
         self.log_prefix = 'Upgrade Reminder'
 
-    def send_emails(self, resolver, *args, **options):
+    def send_emails(self, *args, **kwargs):
         for day_offset in xrange(-7, -77, -7):
-            resolver.send(day_offset, options.get('override_recipient_email'))
+            self.enqueue(day_offset, *args, **kwargs)

openedx/core/djangoapps/schedules/management/commands/send_recurring_nudge.py

 from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
-from openedx.core.djangoapps.schedules.resolvers import ScheduleStartResolver
+from openedx.core.djangoapps.schedules.tasks import ScheduleRecurringNudge
 
 
 class Command(SendEmailBaseCommand):
-    resolver_class = ScheduleStartResolver
+    async_send_task = ScheduleRecurringNudge
 
     def __init__(self, *args, **kwargs):
         super(Command, self).__init__(*args, **kwargs)
         self.log_prefix = 'Scheduled Nudge'
 
-    def send_emails(self, resolver, *args, **options):
+    def send_emails(self, *args, **kwargs):
         for day_offset in (-3, -10):
-            resolver.send(day_offset, options.get('override_recipient_email'))
+            self.enqueue(day_offset, *args, **kwargs)

openedx/core/djangoapps/schedules/management/commands/send_upgrade_reminder.py

 from openedx.core.djangoapps.schedules.management.commands import SendEmailBaseCommand
-from openedx.core.djangoapps.schedules.resolvers import UpgradeReminderResolver
+from openedx.core.djangoapps.schedules.tasks import ScheduleUpgradeReminder
 
 
 class Command(SendEmailBaseCommand):
-    resolver_class = UpgradeReminderResolver
+    async_send_task = ScheduleUpgradeReminder
 
     def __init__(self, *args, **kwargs):
         super(Command, self).__init__(*args, **kwargs)
         self.log_prefix = 'Upgrade Reminder'
 
-    def send_emails(self, resolver, *args, **options):
-        resolver.send(2, options.get('override_recipient_email'))
+    def send_emails(self, *args, **kwargs):
+        self.enqueue(2, *args, **kwargs)

openedx/core/djangoapps/schedules/management/commands/tests/test_base.py

@@ -18,23 +18,13 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un
 class TestSendEmailBaseCommand(CacheIsolationTestCase):
     def setUp(self):
         self.command = SendEmailBaseCommand()
-
-    def test_init_resolver_class(self):
-        assert self.command.resolver_class is None
-
-    def test_make_resolver(self):
-        with patch.object(self.command, 'resolver_class') as resolver_class:
-            example_site = SiteFactory(domain='example.com')
-            self.command.make_resolver(site_domain_name='example.com', date='2017-09-29')
-            resolver_class.assert_called_once_with(
-                example_site,
-                datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC)
-            )
+        self.site = SiteFactory()
 
     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')
+        with patch.object(self.command, 'send_emails') as send_emails:
+            self.command.handle(site_domain_name=self.site.domain, date='2017-09-29')
+            send_emails.assert_called_once_with(
+                self.site,
+                datetime.datetime(2017, 9, 29, tzinfo=pytz.UTC),
+                None
+            )

openedx/core/djangoapps/schedules/message_type.py → openedx/core/djangoapps/schedules/message_types.py

@@ -9,3 +9,17 @@ class ScheduleMessageType(MessageType):
     def __init__(self, *args, **kwargs):
         super(ScheduleMessageType, self).__init__(*args, **kwargs)
         self.log_level = logging.DEBUG if DEBUG_MESSAGE_WAFFLE_FLAG.is_enabled() else None
+
+
+class RecurringNudge(ScheduleMessageType):
+    def __init__(self, day, *args, **kwargs):
+        super(RecurringNudge, self).__init__(*args, **kwargs)
+        self.name = "recurringnudge_day{}".format(day)
+
+
+class UpgradeReminder(ScheduleMessageType):
+    pass
+
+
+class CourseUpdate(ScheduleMessageType):
+    pass

openedx/core/djangoapps/schedules/tests/test_resolvers.py → openedx/core/djangoapps/schedules/tests/test_tasks.py

@@ -3,10 +3,10 @@ from unittest import skipUnless
 
 import ddt
 from django.conf import settings
-from mock import patch
+from mock import patch, DEFAULT, Mock
 
-from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver
-from openedx.core.djangoapps.schedules.tasks import DEFAULT_NUM_BINS
+from openedx.core.djangoapps.schedules.tasks import ScheduleMessageBaseTask
+from openedx.core.djangoapps.schedules.resolvers 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
@@ -16,68 +16,61 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un
 @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):
+class TestScheduleMessageBaseTask(CacheIsolationTestCase):
     def setUp(self):
-        super(TestBinnedSchedulesBaseResolver, self).setUp()
+        super(TestScheduleMessageBaseTask, 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
+        self.basetask = ScheduleMessageBaseTask
 
     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()
+        send = Mock(name='async_send_task')
+        with patch.multiple(
+            self.basetask,
+            is_enqueue_enabled=Mock(return_value=False),
+            log_debug=DEFAULT,
+            run=send,
+        ) as patches:
+            self.basetask.enqueue(
+                site=self.site,
+                current_date=datetime.datetime.now(),
+                day_offset=2
+            )
+            patches['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
+        send = Mock(name='async_send_task')
+        current_date = datetime.datetime.now()
+        with patch.multiple(
+            self.basetask,
+            is_enqueue_enabled=Mock(return_value=True),
+            get_course_org_filter=Mock(return_value=(False, None)),
+            log_debug=DEFAULT,
+            run=send,
+        ) as patches:
+            self.basetask.enqueue(
+                site=self.site,
+                current_date=current_date,
+                day_offset=day_offset
+            )
+            target_date = current_date.replace(hour=0, minute=0, second=0, microsecond=0) + \
+                datetime.timedelta(day_offset)
+            print(patches['log_debug'].mock_calls)
+            patches['log_debug'].assert_any_call(
+                'Target date = %s', target_date.isoformat())
+            assert send.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
+        with patch.object(self.basetask, 'enqueue_config_var', 'enqueue_recurring_nudge'):
+            self.schedule_config.enqueue_recurring_nudge = enabled
+            self.schedule_config.save()
+            assert self.basetask.is_enqueue_enabled(self.site) == enabled
 
     @ddt.unpack
     @ddt.data(
@@ -85,10 +78,9 @@ class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase):
         (['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()
+        exclude_orgs, org_list = self.basetask.get_course_org_filter(self.site)
         assert not exclude_orgs
         assert org_list == expected_org_list
 
@@ -99,12 +91,9 @@ class TestBinnedSchedulesBaseResolver(CacheIsolationTestCase):
         (['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,
+        SiteConfigurationFactory.create(
             values={'course_org_filter': course_org_filter},
         )
-        exclude_orgs, org_list = resolver.get_course_org_filter()
+        exclude_orgs, org_list = self.basetask.get_course_org_filter(self.site)
         assert exclude_orgs
         self.assertItemsEqual(org_list, expected_org_list)

openedx/core/djangoapps/schedules/utils.py

@@ -6,9 +6,12 @@ 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):
+    log_prefix = None
+
     def __init__(self, *args, **kwargs):
         super(PrefixedDebugLoggerMixin, self).__init__(*args, **kwargs)
-        self.log_prefix = self.__class__.__name__
+        if self.log_prefix is None:
+            self.log_prefix = self.__class__.__name__
 
     def log_debug(self, message, *args, **kwargs):
         LOG.debug(self.log_prefix + ': ' + message, *args, **kwargs)

requirements/edx/base.txt

@@ -4,6 +4,7 @@
 #   * @edx/ospr - to check licensing
 #   * @edx/devops - to check system requirements
 
+attrs==17.2.0
 beautifulsoup4==4.1.3
 beautifulsoup==3.2.1
 bleach==1.4