update nudge templates

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

@@ -14,7 +14,6 @@ from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
 
 from edx_ace.recipient_resolver import RecipientResolver
 
-
 LOG = logging.getLogger(__name__)
 
 
@@ -28,8 +27,28 @@ class ScheduleStartResolver(RecipientResolver):
         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)
@@ -45,14 +64,7 @@ class ScheduleStartResolver(RecipientResolver):
         except SiteConfiguration.DoesNotExist:
             org_list = None
             exclude_orgs = False
-
-        target_date = self.current_date - datetime.timedelta(days=day)
-        for hour in range(24):
-            target_hour = target_date + datetime.timedelta(hours=hour)
-            recurring_nudge_schedule_hour.apply_async(
-                (self.site.id, day, serialize(target_hour), org_list, exclude_orgs, override_recipient_email),
-                retry=False,
-            )
+        return exclude_orgs, org_list
 
 
 class Command(BaseCommand):
@@ -74,7 +86,11 @@ class Command(BaseCommand):
             *[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'))

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

@@ -143,20 +143,30 @@ class TestSendRecurringNudge(CacheIsolationTestCase):
         for config in (limited_config, unlimited_config):
             ScheduleConfigFactory.create(site=config.site)
 
+        user1 = UserFactory.create()
+        user2 = UserFactory.create()
+
         ScheduleFactory.create(
             start=datetime.datetime(2017, 8, 2, 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),
+            enrollment__course__org=unfiltered_org,
+            enrollment__user=user1,
+        )
+        ScheduleFactory.create(
+            start=datetime.datetime(2017, 8, 2, 17, 44, 30, tzinfo=pytz.UTC),
+            enrollment__course__org=unfiltered_org,
+            enrollment__user=user2,
         )
-        for _ in range(2):
-            ScheduleFactory.create(
-                start=datetime.datetime(2017, 8, 2, 17, 44, 30, tzinfo=pytz.UTC),
-                enrollment__course__org=unfiltered_org,
-            )
 
         test_time_str = serialize(datetime.datetime(2017, 8, 2, 17, tzinfo=pytz.UTC))
         with self.assertNumQueries(2):
             tasks.recurring_nudge_schedule_hour(
-                limited_config.site.id, 3, test_time_str, org_list=org_list, exclude_orgs=exclude_orgs,
+                limited_config.site.id, day=3, target_hour_str=test_time_str, org_list=org_list,
+                exclude_orgs=exclude_orgs,
             )
 
         self.assertEqual(mock_schedule_send.apply_async.call_count, expected_message_count)

openedx/core/djangoapps/schedules/tasks.py

 import datetime
 from itertools import groupby
-from logging import getLogger
+import logging
 from urlparse import urlparse
 
 from celery.task import task
@@ -12,6 +12,7 @@ from django.core.urlresolvers import reverse
 from django.db.models import Min
 from django.db.utils import DatabaseError
 from django.utils.http import urlquote
+
 from edx_ace import ace
 from edx_ace.message import Message
 from edx_ace.recipient import Recipient
@@ -22,7 +23,8 @@ from edxmako.shortcuts import marketing_link
 from openedx.core.djangoapps.schedules.message_type import ScheduleMessageType
 from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig
 
-log = getLogger(__name__)
+
+LOG = logging.getLogger(__name__)
 
 
 ROUTING_KEY = getattr(settings, 'ACE_ROUTING_KEY', None)
@@ -45,7 +47,7 @@ def update_course_schedules(self, **kwargs):
         )
     except Exception as exc:  # pylint: disable=broad-except
         if not isinstance(exc, KNOWN_RETRY_ERRORS):
-            log.exception("Unexpected failure: task id: %s, kwargs=%s".format(self.request.id, kwargs))
+            LOG.exception("Unexpected failure: task id: %s, kwargs=%s".format(self.request.id, kwargs))
         raise self.retry(kwargs=kwargs, exc=exc)
 
 
@@ -78,9 +80,11 @@ def recurring_nudge_schedule_hour(
 def _recurring_nudge_schedule_send(site_id, msg_str):
     site = Site.objects.get(pk=site_id)
     if not ScheduleConfig.current(site).deliver_recurring_nudge:
+        LOG.debug('Recurring Nudge: Message delivery disabled for site %s', site.domain)
         return
 
     msg = Message.from_string(msg_str)
+    LOG.debug('Recurring Nudge: Sending message = %s', msg_str)
     ace.send(msg)
 
 
@@ -97,12 +101,6 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals
         first_schedule__lt=target_hour + datetime.timedelta(minutes=60)
     )
 
-    if org_list is not None:
-        if exclude_orgs:
-            users = users.exclude(courseenrollment__course__org__in=org_list)
-        else:
-            users = users.filter(courseenrollment__course__org__in=org_list)
-
     schedules = Schedule.objects.select_related(
         'enrollment__user__profile',
         'enrollment__course',
@@ -122,6 +120,8 @@ def _recurring_nudge_schedules_for_hour(target_hour, org_list, exclude_orgs=Fals
     if "read_replica" in settings.DATABASES:
         schedules = schedules.using("read_replica")
 
+    LOG.debug('Scheduled Nudge: Query = %r', schedules.query.sql_with_params())
+
     dashboard_relative_url = reverse('dashboard')
 
     for (user, user_schedules) in groupby(schedules, lambda s: s.enrollment.user):

openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/base_body.html

@@ -15,13 +15,8 @@ email itself. -->
     {% block preview_text %}{% endblock %}
 </div>
 
-{# This "View on Web" link must appear in the document before any layout tables so that accessible email clients can #}
-{# send the user to a proper web browser which can parse the email correctly. #}
-
-{# Note {view_url} is not a template variable that is evaluated by the Django template engine. It is evaluated by #}
+{# Note {beacon_src} is not a template variable that is evaluated by the Django template engine. It is evaluated by #}
 {# Sailthru when the email is sent. Other email providers would need to replace this variable in the HTML as well. #}
-<p><a href="{view_url}">{% trans "View on Web" %}</a></p>
-{# Note this is another late-bound variable #}
 <img src="{beacon_src}" alt="" role="presentation" aria-hidden="true" />
 
 {% get_current_language as LANGUAGE_CODE %}
@@ -156,10 +151,17 @@ email itself. -->
                     <tr>
                         <!-- Actions -->
                         <td style="padding-bottom: 20px;">
-                            {# Note that this variable is evaluated by Sailthru, not the Django template engine #}
-                            <a href="{optout_confirm_url}" style="color: #005686">
-                                <font color="#005686"><b>{% trans "Unsubscribe from this list" %}</b></font>
-                            </a>
+                            {# Note that these variables are evaluated by Sailthru, not the Django template engine #}
+                            <p>
+                                <a href="{view_url}" style="color: #005686">
+                                    <font color="#005686"><b>{% trans "View on Web" %}</b></font>
+                                </a>
+                            </p>
+                            <p>
+                                <a href="{optout_confirm_url}" style="color: #005686">
+                                    <font color="#005686"><b>{% trans "Unsubscribe from this list" %}</b></font>
+                                </a>
+                            </p>
                         </td>
                     </tr>
                     <tr>

openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day3/email/subject.txt

 {% load i18n %}
-
 {% if courses|length > 1 %}
-{% blocktrans %}Keep learning on {{ platform_name }}!{% endblocktrans %}
+{% blocktrans %}Keep learning on {{ platform_name }}{% endblocktrans %}
 {% else %}
-{% blocktrans %}Keep learning in {{course_name}} !{% endblocktrans %}
+{% blocktrans %}Keep learning in {{course_name}}{% endblocktrans %}
 {% endif %}