Merge pull request #9448 from mitocw/fix/aq/email_ccx

Fixed URL and course title in CCX enrollment/unenrollment email

lms/djangoapps/ccx/views.py

@@ -410,7 +410,7 @@ def ccx_invite(request, course, ccx=None):
         try:
             validate_email(email)
             course_key = CCXLocator.from_course_locator(course.id, ccx.id)
-            email_params = get_email_params(course, auto_enroll)
+            email_params = get_email_params(course, auto_enroll, course_key=course_key, display_name=ccx.display_name)
             if action == 'Enroll':
                 enroll_email(
                     course_key,

lms/djangoapps/instructor/enrollment.py

@@ -261,7 +261,7 @@ def _reset_module_attempts(studentmodule):
     studentmodule.save()
 
 
-def get_email_params(course, auto_enroll, secure=True):
+def get_email_params(course, auto_enroll, secure=True, course_key=None, display_name=None):
     """
     Generate parameters used when parsing email templates.
 
@@ -270,6 +270,8 @@ def get_email_params(course, auto_enroll, secure=True):
     """
 
     protocol = 'https' if secure else 'http'
+    course_key = course_key or course.id.to_deprecated_string()
+    display_name = display_name or course.display_name_with_default
 
     stripped_site_name = microsite.get_value(
         'SITE_NAME',
@@ -285,7 +287,7 @@ def get_email_params(course, auto_enroll, secure=True):
     course_url = u'{proto}://{site}{path}'.format(
         proto=protocol,
         site=stripped_site_name,
-        path=reverse('course_root', kwargs={'course_id': course.id.to_deprecated_string()})
+        path=reverse('course_root', kwargs={'course_id': course_key})
     )
 
     # We can't get the url to the course's About page if the marketing site is enabled.
@@ -294,7 +296,7 @@ def get_email_params(course, auto_enroll, secure=True):
         course_about_url = u'{proto}://{site}{path}'.format(
             proto=protocol,
             site=stripped_site_name,
-            path=reverse('about_course', kwargs={'course_id': course.id.to_deprecated_string()})
+            path=reverse('about_course', kwargs={'course_id': course_key})
         )
 
     is_shib_course = uses_shib(course)
@@ -304,6 +306,7 @@ def get_email_params(course, auto_enroll, secure=True):
         'site_name': stripped_site_name,
         'registration_url': registration_url,
         'course': course,
+        'display_name': display_name,
         'auto_enroll': auto_enroll,
         'course_url': course_url,
         'course_about_url': course_about_url,
@@ -321,6 +324,7 @@ def send_mail_to_student(student, param_dict, language=None):
     [
         `site_name`: name given to edX instance (a `str`)
         `registration_url`: url for registration (a `str`)
+        `display_name` : display name of a course (a `str`)
         `course_id`: id of course (a `str`)
         `auto_enroll`: user input option (a `str`)
         `course_url`: url of course (a `str`)
@@ -338,8 +342,8 @@ def send_mail_to_student(student, param_dict, language=None):
     """
 
     # add some helpers and microconfig subsitutions
-    if 'course' in param_dict:
-        param_dict['course_name'] = param_dict['course'].display_name_with_default
+    if 'display_name' in param_dict:
+        param_dict['course_name'] = param_dict['display_name']
 
     param_dict['site_name'] = microsite.get_value(
         'SITE_NAME',

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -5,6 +5,7 @@ Unit tests for instructor.enrollment methods.
 
 import json
 import mock
+from mock import patch
 from abc import ABCMeta
 from courseware.models import StudentModule
 from django.conf import settings
@@ -12,11 +13,17 @@ from django.test import TestCase
 from django.utils.translation import get_language
 from django.utils.translation import override as override_language
 from nose.plugins.attrib import attr
+from ccx_keys.locator import CCXLocator
 from student.tests.factories import UserFactory
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
+from ccx.tests.factories import CcxFactory
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
+from student.roles import CourseCcxCoachRole  # pylint: disable=import-error
+from student.tests.factories import (  # pylint: disable=import-error
+    AdminFactory
+)
 from instructor.enrollment import (
     EmailEnrollmentState,
     enroll_email,
@@ -30,7 +37,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
 from submissions import api as sub_api
 from student.models import anonymous_id_for_user
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE
 
 
 @attr('shard_1')
@@ -567,6 +574,53 @@ class TestSendBetaRoleEmail(TestCase):
 
 
 @attr('shard_1')
+class TestGetEmailParamsCCX(ModuleStoreTestCase):
+    """
+    Test what URLs the function get_email_params for CCX student enrollment.
+    """
+
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def setUp(self):
+        super(TestGetEmailParamsCCX, self).setUp()
+
+        self.course = CourseFactory.create()
+        self.coach = AdminFactory.create()
+        role = CourseCcxCoachRole(self.course.id)
+        role.add_users(self.coach)
+        self.ccx = CcxFactory(course_id=self.course.id, coach=self.coach)
+        self.course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id)
+        # Explicitly construct what we expect the course URLs to be
+        site = settings.SITE_NAME
+        self.course_url = u'https://{}/courses/{}/'.format(
+            site,
+            self.course_key
+        )
+        self.course_about_url = self.course_url + 'about'
+        self.registration_url = u'https://{}/register'.format(
+            site,
+        )
+
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def test_ccx_enrollment_email_params(self):
+        # For a CCX, what do we expect to get for the URLs?
+        # Also make sure `auto_enroll` is properly passed through.
+        result = get_email_params(
+            self.course,
+            True,
+            course_key=self.course_key,
+            display_name=self.ccx.display_name
+        )
+
+        self.assertEqual(result['display_name'], self.ccx.display_name)
+        self.assertEqual(result['auto_enroll'], True)
+        self.assertEqual(result['course_about_url'], self.course_about_url)
+        self.assertEqual(result['registration_url'], self.registration_url)
+        self.assertEqual(result['course_url'], self.course_url)
+
+
+@attr('shard_1')
 class TestGetEmailParams(ModuleStoreTestCase):
     """
     Test what URLs the function get_email_params returns under different
@@ -615,13 +669,18 @@ class TestGetEmailParams(ModuleStoreTestCase):
 class TestRenderMessageToString(ModuleStoreTestCase):
     """
     Test that email templates can be rendered in a language chosen manually.
+    Test CCX enrollmet email.
     """
 
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     def setUp(self):
         super(TestRenderMessageToString, self).setUp()
         self.subject_template = 'emails/enroll_email_allowedsubject.txt'
         self.message_template = 'emails/enroll_email_allowedmessage.txt'
         self.course = CourseFactory.create()
+        self.course_key = None
+        self.ccx = None
 
     def get_email_params(self):
         """
@@ -633,6 +692,27 @@ class TestRenderMessageToString(ModuleStoreTestCase):
 
         return email_params
 
+    def get_email_params_ccx(self):
+        """
+        Returns a dictionary of parameters used to render an email for CCX.
+        """
+        coach = AdminFactory.create()
+        role = CourseCcxCoachRole(self.course.id)
+        role.add_users(coach)
+        self.ccx = CcxFactory(course_id=self.course.id, coach=coach)
+        self.course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id)
+
+        email_params = get_email_params(
+            self.course,
+            True,
+            course_key=self.course_key,
+            display_name=self.ccx.display_name
+        )
+        email_params["email_address"] = "user@example.com"
+        email_params["full_name"] = "Jean Reno"
+
+        return email_params
+
     def get_subject_and_message(self, language):
         """
         Returns the subject and message rendered in the specified language.
@@ -644,6 +724,18 @@ class TestRenderMessageToString(ModuleStoreTestCase):
             language=language
         )
 
+    def get_subject_and_message_ccx(self):
+        """
+        Returns the subject and message rendered in the specified language for CCX.
+        """
+        subject_template = 'emails/enroll_email_enrolledsubject.txt'
+        message_template = 'emails/enroll_email_enrolledmessage.txt'
+        return render_message_to_string(
+            subject_template,
+            message_template,
+            self.get_email_params_ccx()
+        )
+
     def test_subject_and_message_translation(self):
         subject, message = self.get_subject_and_message('fr')
         language_after_rendering = get_language()
@@ -658,3 +750,18 @@ class TestRenderMessageToString(ModuleStoreTestCase):
             subject, message = self.get_subject_and_message(None)
             self.assertIn("You have been", subject)
             self.assertIn("You have been", message)
+
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def test_render_message_ccx(self):
+        """
+        Test email template renders for CCX.
+        """
+        subject, message = self.get_subject_and_message_ccx()
+        self.assertIn(self.ccx.display_name, subject)
+        self.assertIn(self.ccx.display_name, message)
+        site = settings.SITE_NAME
+        course_url = u'https://{}/courses/{}/'.format(
+            site,
+            self.course_key
+        )
+        self.assertIn(course_url, message)

lms/templates/emails/enroll_email_enrolledmessage.txt

@@ -5,7 +5,7 @@ ${_("Dear {full_name}").format(full_name=full_name)}
 ${_("You have been enrolled in {course_name} at {site_name} by a member "
 	"of the course staff. The course should now appear on your {site_name} "
 	"dashboard.").format(
-		course_name=course.display_name_with_default,
+		course_name=display_name or course.display_name_with_default,
 		site_name=site_name
 	)}
 

lms/templates/emails/enroll_email_enrolledsubject.txt

 <%! from django.utils.translation import ugettext as _ %>
 
 ${_("You have been enrolled in {course_name}").format(
-		course_name=course.display_name_with_default
+		course_name=display_name or course.display_name_with_default
 	)}
\ No newline at end of file