Enable course updates for instructor led courses (#22422)

Currently there is no option to schedule bulk emails to be sent
out at a specific time for instructor led courses. It would reduce
the effort required to create an engaging course if instructor led
course teams had the option to turn on weekly highlight emails as
well.

PROD-575

cms/static/js/views/pages/course_outline.js

@@ -82,7 +82,7 @@ define([
                 }
 
                 /* globals course */
-                if (this.model.get('highlights_enabled') && course.get('self_paced')) {
+                if (this.model.get('highlights_enabled')) {
                     this.highlightsEnableView = new CourseHighlightsEnableView({
                         el: this.$('.status-highlights-enabled'),
                         model: this.model

cms/templates/js/course-outline.underscore

@@ -212,7 +212,7 @@ if (is_proctored_exam) {
                 </p>
               </div>
             <% } %>
-            <% if (xblockInfo.get('highlights_enabled') && course.get('self_paced') && xblockInfo.isChapter()) { %>
+            <% if (xblockInfo.get('highlights_enabled') && xblockInfo.isChapter()) { %>
                 <div class="block-highlights">
                     <% var number_of_highlights = (xblockInfo.get('highlights') || []).length; %>
                         <button class="block-highlights-value highlights-button action-button">

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

@@ -7,6 +7,7 @@ from unittest import skipUnless
 
 import ddt
 from django.conf import settings
+from django.core import mail
 from edx_ace.utils.date import serialize
 from mock import patch
 from six.moves import range
@@ -34,6 +35,9 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
     "Can't test schedules if the app isn't installed",
 )
 class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestMixin, ModuleStoreTestCase):
+    """
+    Tests for django management command 'send_course_update'
+    """
     __test__ = True
 
     # pylint: disable=protected-access
@@ -55,6 +59,28 @@ class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestMixin,
         mock_highlights.return_value = [u'Highlight {}'.format(num + 1) for num in range(3)]
         self.addCleanup(self.stop_highlights_patcher)
 
+    def prepare_course_data(self, mock_get_current_site, is_self_paced=True):
+        """
+        Prepare course data with highlights
+        """
+        self.highlights_patcher.stop()
+        self.highlights_patcher = None
+        mock_get_current_site.return_value = self.site_config.site
+
+        course = CourseFactory(highlights_enabled_for_messaging=True, self_paced=is_self_paced)
+        with self.store.bulk_operations(course.id):
+            ItemFactory.create(parent=course, category='chapter', highlights=[u'highlights'])
+
+        enrollment = CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=u'audit')
+        self.assertEqual(enrollment.schedule.get_experience_type(), ScheduleExperience.EXPERIENCES.course_updates)
+
+        _, offset, target_day, _ = self._get_dates(offset=self.expected_offsets[0])
+        enrollment.schedule.start = target_day
+        enrollment.schedule.start_date = target_day
+        enrollment.schedule.save()
+
+        return offset, target_day, enrollment
+
     def stop_highlights_patcher(self):
         """
         Stops the patcher for the get_week_highlights method
@@ -74,21 +100,7 @@ class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestMixin,
     @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
     @patch('openedx.core.djangoapps.schedules.signals.get_current_site')
     def test_with_course_data(self, mock_get_current_site):
-        self.highlights_patcher.stop()
-        self.highlights_patcher = None
-        mock_get_current_site.return_value = self.site_config.site
-
-        course = CourseFactory(highlights_enabled_for_messaging=True, self_paced=True)
-        with self.store.bulk_operations(course.id):
-            ItemFactory.create(parent=course, category='chapter', highlights=[u'highlights'])
-
-        enrollment = CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=u'audit')
-        self.assertEqual(enrollment.schedule.get_experience_type(), ScheduleExperience.EXPERIENCES.course_updates)
-
-        _, offset, target_day, _ = self._get_dates(offset=self.expected_offsets[0])
-        enrollment.schedule.start = target_day
-        enrollment.schedule.start_date = target_day
-        enrollment.schedule.save()
+        offset, target_day, enrollment = self.prepare_course_data(mock_get_current_site)
 
         with patch.object(tasks, 'ace') as mock_ace:
             self.task().apply(kwargs=dict(
@@ -99,3 +111,20 @@ class TestSendCourseUpdate(ScheduleUpsellTestMixin, ScheduleSendEmailTestMixin,
             ))
 
             self.assertTrue(mock_ace.send.called)
+
+    @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
+    @patch('openedx.core.djangoapps.schedules.signals.get_current_site')
+    def test_template_for_instructor_led_courses(self, mock_get_current_site):
+        """
+        Test that InstructorLedCourseUpdate template is picked for instructor led
+        courses
+        """
+        offset, target_day, enrollment = self.prepare_course_data(mock_get_current_site, is_self_paced=False)
+
+        self.task().apply(kwargs=dict(
+            site_id=self.site_config.site.id,
+            target_day_str=serialize(target_day),
+            day_offset=offset,
+            bin_num=self._calculate_bin_for_user(enrollment.user),
+        ))
+        self.assertEqual(u'{} Weekly Update'.format(enrollment.course.display_name), mail.outbox[0].subject)

openedx/core/djangoapps/schedules/message_types.py

@@ -1,4 +1,6 @@
-
+"""
+ACE message types for the schedules module.
+"""
 
 import logging
 
@@ -24,3 +26,7 @@ class UpgradeReminder(ScheduleMessageType):
 
 class CourseUpdate(ScheduleMessageType):
     pass
+
+
+class InstructorLedCourseUpdate(ScheduleMessageType):
+    pass

openedx/core/djangoapps/schedules/resolvers.py

@@ -20,6 +20,7 @@ from openedx.core.djangoapps.ace_common.template_context import get_base_templat
 from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_SHOW_UNSUBSCRIBE_WAFFLE_SWITCH
 from openedx.core.djangoapps.schedules.content_highlights import get_week_highlights
 from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist
+from openedx.core.djangoapps.schedules.message_types import CourseUpdate, InstructorLedCourseUpdate
 from openedx.core.djangoapps.schedules.models import Schedule, ScheduleExperience
 from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin
 from openedx.core.djangoapps.site_configuration.models import SiteConfiguration
@@ -254,6 +255,8 @@ class RecurringNudgeResolver(BinnedSchedulesBaseResolver):
 
     def get_template_context(self, user, user_schedules):
         first_schedule = user_schedules[0]
+        if not first_schedule.enrollment.course.self_paced:
+            raise InvalidContextError
         context = {
             'course_name': first_schedule.enrollment.course.display_name,
             'course_url': _get_trackable_course_home_url(first_schedule.enrollment.course_id),
@@ -286,6 +289,10 @@ class UpgradeReminderResolver(BinnedSchedulesBaseResolver):
         first_valid_upsell_context = None
         first_schedule = None
         for schedule in user_schedules:
+            if not schedule.enrollment.course.self_paced:
+                # We don't want to include instructor led courses in this email
+                continue
+
             upsell_context = _get_upsell_information_for_schedule(user, schedule)
             if not upsell_context['show_upsell']:
                 continue
@@ -349,6 +356,20 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver):
     num_bins = COURSE_UPDATE_NUM_BINS
     experience_filter = Q(experience__experience_type=ScheduleExperience.EXPERIENCES.course_updates)
 
+    def send(self, msg_type):
+        for (user, language, context, is_self_paced) in self.schedules_for_bin():
+            msg_type = CourseUpdate() if is_self_paced else InstructorLedCourseUpdate()
+            msg = msg_type.personalize(
+                Recipient(
+                    user.username,
+                    self.override_recipient_email or user.email,
+                ),
+                language,
+                context,
+            )
+            with function_trace('enqueue_send_task'):
+                self.async_send_task.apply_async((self.site.id, str(msg)), retry=False)  # pylint: disable=no-member
+
     def schedules_for_bin(self):
         week_num = abs(self.day_offset) // 7
         schedules = self.get_schedules_with_target_date_by_bin_and_orgs(
@@ -358,6 +379,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver):
         template_context = get_base_template_context(self.site)
         for schedule in schedules:
             enrollment = schedule.enrollment
+            course = schedule.enrollment.course
             user = enrollment.user
 
             try:
@@ -391,7 +413,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver):
                 })
                 template_context.update(_get_upsell_information_for_schedule(user, schedule))
 
-                yield (user, schedule.enrollment.course.closest_released_language, template_context)
+                yield (user, schedule.enrollment.course.closest_released_language, template_context, course.self_paced)
 
 
 def _get_trackable_course_home_url(course_id):

openedx/core/djangoapps/schedules/signals.py

@@ -1,4 +1,6 @@
-
+"""
+CourseEnrollment related signal handlers.
+"""
 
 import datetime
 import logging
@@ -175,10 +177,6 @@ def _create_schedule(enrollment, enrollment_created):
         log.debug('Schedules: Creation not enabled for this course or for this site')
         return
 
-    if not enrollment.course_overview.self_paced:
-        log.debug('Schedules: Creation only enabled for self-paced courses')
-        return
-
     # This represents the first date at which the learner can access the content. This will be the latter of
     # either the enrollment date or the course's start date.
     content_availability_date = max(enrollment.created, enrollment.course_overview.start)

openedx/core/djangoapps/schedules/templates/schedules/edx_ace/instructorledcourseupdate/email/body.html

@@ -0,0 +1,45 @@
+{% extends 'ace_common/edx_ace/common/base_body.html' %}
+{% load i18n %}
+{% load django_markup %}
+
+{% block preview_text %}
+    {% filter force_escape %}
+    {% blocktrans trimmed %}
+        Welcome to week {{ week_num }} of {{ course_name }}!
+    {% endblocktrans %}
+    {% endfilter %}
+{% endblock %}
+
+{% block content %}
+<table width="100%" align="left" border="0" cellpadding="0" cellspacing="0" role="presentation">
+    <tr>
+        <td>
+            <p>
+                {% blocktrans trimmed asvar tmsg %}
+                    We hope you're enjoying {start_strong}{course_name}{end_strong}!
+                    We want to let you know what you can look forward to in the coming weeks:
+                {% endblocktrans %}
+                {% interpolate_html tmsg start_strong='<strong>'|safe end_strong='</strong>'|safe course_name=course_name|force_escape|safe week_num=week_num|force_escape|safe %}
+                <ul>
+                    {% for highlight in week_highlights %}
+                        <li>{{ highlight }}</li>
+                    {% endfor %}
+                </ul>
+            </p>
+            <p>
+                {% filter force_escape %}
+                {% blocktrans trimmed %}
+                    We encourage you to spend time with the course each week.
+                    Your focused attention will pay off in the end!
+                {% endblocktrans %}
+                {% endfilter %}
+            </p>
+            {# xss-lint: disable=django-trans-missing-escape #}
+            {% trans "Resume your course now" as course_cta_text %}
+            {% include "ace_common/edx_ace/common/return_to_course_cta.html" with course_cta_text=course_cta_text%}
+
+            {% include "ace_common/edx_ace/common/upsell_cta.html"%}
+        </td>
+    </tr>
+</table>
+{% endblock %}

openedx/core/djangoapps/schedules/templates/schedules/edx_ace/instructorledcourseupdate/email/body.txt

@@ -0,0 +1,17 @@
+{% autoescape off %}
+{% load i18n %}
+
+{% blocktrans trimmed %}
+We hope you're enjoying {{ course_name }}!
+We want to let you know what you can look forward to in the coming weeks:
+{% endblocktrans %}
+
+{% for highlight in week_highlights %}
+  * {{ highlight }}
+{% endfor %}
+
+{% blocktrans trimmed %}
+We encourage you to spend time with the course each week. Your focused attention will pay off in the end!
+{% endblocktrans %}
+{% include "ace_common/edx_ace/common/upsell_cta.txt"%}
+{% endautoescape %}

openedx/core/djangoapps/schedules/templates/schedules/edx_ace/instructorledcourseupdate/email/from_name.txt

@@ -0,0 +1,3 @@
+{% autoescape off %}
+{{ course_name }}
+{% endautoescape %}

openedx/core/djangoapps/schedules/templates/schedules/edx_ace/instructorledcourseupdate/email/head.html

@@ -0,0 +1 @@
+{% extends 'ace_common/edx_ace/common/base_head.html' %}

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

@@ -0,0 +1,5 @@
+{% autoescape off %}
+{% load i18n %}
+
+{% blocktrans trimmed %}{{ course_name }} Weekly Update {% endblocktrans %}
+{% endautoescape %}

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

@@ -147,7 +147,7 @@ class TestCourseUpdateResolver(SchedulesResolverTestMixin, ModuleStoreTestCase):
             'week_highlights': ['good stuff'],
             'week_num': 1,
         }
-        self.assertEqual(schedules, [(self.user, None, expected_context)])
+        self.assertEqual(schedules, [(self.user, None, expected_context, True)])
 
     @override_waffle_flag(COURSE_UPDATE_WAFFLE_FLAG, True)
     @override_switch('schedules.course_update_show_unsubscribe', True)

openedx/core/djangoapps/schedules/tests/test_signals.py

@@ -7,23 +7,23 @@ import datetime
 
 import ddt
 import pytest
-from mock import patch
-from pytz import utc
-
 from course_modes.models import CourseMode
 from course_modes.tests.factories import CourseModeFactory
-from lms.djangoapps.courseware.models import DynamicUpgradeDeadlineConfiguration
-from openedx.core.djangoapps.schedules.models import ScheduleExperience
-from openedx.core.djangoapps.schedules.signals import CREATE_SCHEDULE_WAFFLE_FLAG
-from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
-from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
-from openedx.core.djangolib.testing.utils import skip_unless_lms
+from mock import patch
+from pytz import utc
 from student.models import CourseEnrollment
 from student.tests.factories import CourseEnrollmentFactory
+from testfixtures import LogCapture
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
+from lms.djangoapps.courseware.models import DynamicUpgradeDeadlineConfiguration
+from openedx.core.djangoapps.schedules.models import ScheduleExperience
+from openedx.core.djangoapps.schedules.signals import CREATE_SCHEDULE_WAFFLE_FLAG, log
+from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
+from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
+from openedx.core.djangolib.testing.utils import skip_unless_lms
 from ..models import Schedule
 from ..tests.factories import ScheduleConfigFactory
 
@@ -33,8 +33,12 @@ from ..tests.factories import ScheduleConfigFactory
 @skip_unless_lms
 class CreateScheduleTests(SharedModuleStoreTestCase):
 
-    def assert_schedule_created(self, experience_type=ScheduleExperience.EXPERIENCES.default):
-        course = _create_course_run(self_paced=True)
+    def assert_schedule_created(self, is_self_paced=True, experience_type=ScheduleExperience.EXPERIENCES.default):
+        """
+        Checks whether schedule is created and that it is created with the correct
+        experience type
+        """
+        course = _create_course_run(self_paced=is_self_paced)
         enrollment = CourseEnrollmentFactory(
             course_id=course.id,
             mode=CourseMode.AUDIT,
@@ -83,17 +87,17 @@ class CreateScheduleTests(SharedModuleStoreTestCase):
         site = SiteFactory.create()
         mock_get_current_site.return_value = site
         ScheduleConfigFactory.create(site=site, create_schedules=False)
-        self.assert_schedule_not_created()
+        with LogCapture(log.name) as log_capture:
+            self.assert_schedule_not_created()
+            log_capture.check((log.name, 'DEBUG', 'Schedules: Creation not enabled for this course or for this site'))
 
     @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True)
-    def test_schedule_config_creation_enabled_instructor_paced(self, mock_get_current_site):
+    @patch('openedx.core.djangoapps.schedules.signals.course_has_highlights')
+    def test_schedule_config_creation_enabled_instructor_paced(self, mock_course_has_highlights, mock_get_current_site):
         site = SiteFactory.create()
+        mock_course_has_highlights.return_value = True
         mock_get_current_site.return_value = site
-        ScheduleConfigFactory.create(site=site, enabled=True, create_schedules=True)
-        course = _create_course_run(self_paced=False)
-        enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT)
-        with pytest.raises(Schedule.DoesNotExist):
-            enrollment.schedule
+        self.assert_schedule_created(is_self_paced=False, experience_type=ScheduleExperience.EXPERIENCES.course_updates)
 
     @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True)
     @patch('openedx.core.djangoapps.schedules.signals.course_has_highlights')
@@ -141,6 +145,30 @@ class CreateScheduleTests(SharedModuleStoreTestCase):
         mock_log.assert_called_once()
         assert 'Encountered error in creating a Schedule for CourseEnrollment' in mock_log.call_args[0][0]
 
+    @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True)
+    def test_course_start_date_in_future(self, mock_get_current_site):
+        """
+        Test that the schedule start date will be set to course's start date
+        if course starts after enrollment
+        """
+        site = SiteFactory.create()
+        mock_get_current_site.return_value = site
+        course = _create_course_run(self_paced=True, start_day_offset=5)  # course starts in future
+        enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT)
+        assert _strip_secs(enrollment.schedule.start) == _strip_secs(course.start)
+
+    @override_waffle_flag(CREATE_SCHEDULE_WAFFLE_FLAG, True)
+    def test_course_already_started(self, mock_get_current_site):
+        """
+        Test that the schedule start date will be set to the date enrollment was
+        created if course has already started
+        """
+        site = SiteFactory.create()
+        mock_get_current_site.return_value = site
+        course = _create_course_run(self_paced=True, start_day_offset=-5)  # course already started
+        enrollment = CourseEnrollmentFactory(course_id=course.id, mode=CourseMode.AUDIT)
+        assert _strip_secs(enrollment.schedule.start) == _strip_secs(enrollment.created)
+
 
 @ddt.ddt
 @skip_unless_lms