Fix course urls in enrollment emails (LMS-2217)

lms/djangoapps/instructor/enrollment.py

@@ -211,8 +211,27 @@ def get_email_params(course, auto_enroll):
     Returns a dict of parameters
     """
 
-    stripped_site_name = settings.SITE_NAME
-    registration_url = 'https://' + stripped_site_name + reverse('student.views.register_user')
+    stripped_site_name = microsite.get_value(
+        'SITE_NAME',
+        settings.SITE_NAME
+    )
+    registration_url = u'https://{}{}'.format(
+        stripped_site_name,
+        reverse('student.views.register_user')
+    )
+    course_url = u'https://{}{}'.format(
+        stripped_site_name,
+        reverse('course_root', kwargs={'course_id': course.id})
+    )
+
+    # We can't get the url to the course's About page if the marketing site is enabled.
+    course_about_url = None
+    if not settings.FEATURES.get('ENABLE_MKTG_SITE', False):
+        course_about_url = u'https://{}{}'.format(
+            stripped_site_name,
+            reverse('about_course', kwargs={'course_id': course.id})
+        )
+
     is_shib_course = uses_shib(course)
 
     # Composition of email
@@ -221,8 +240,8 @@ def get_email_params(course, auto_enroll):
         'registration_url': registration_url,
         'course': course,
         'auto_enroll': auto_enroll,
-        'course_url': 'https://' + stripped_site_name + '/courses/' + course.id,
-        'course_about_url': 'https://' + stripped_site_name + '/courses/' + course.id + '/about',
+        'course_url': course_url,
+        'course_about_url': course_about_url,
         'is_shib_course': is_shib_course,
     }
     return email_params

lms/djangoapps/instructor/tests/test_api.py

@@ -403,14 +403,13 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
             "at edx.org by a member of the course staff. "
             "The course should now appear on your edx.org dashboard.\n\n"
             "To start accessing course materials, please visit "
-            "https://edx.org/courses/MITx/999/Robot_Super_Course\n\n----\n"
+            "https://edx.org/courses/MITx/999/Robot_Super_Course/\n\n----\n"
             "This email was automatically sent from edx.org to NotEnrolled Student"
         )
 
     def test_enroll_with_email_not_registered(self):
         url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True})
-        print "type(self.notregistered_email): {}".format(type(self.notregistered_email))
         self.assertEqual(response.status_code, 200)
 
         # Check the outbox
@@ -429,6 +428,22 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
             "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org"
         )
 
+    def test_enroll_email_not_registered_mktgsite(self):
+        url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id})
+        # Try with marketing site enabled
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
+            response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            mail.outbox[0].body,
+            "Dear student,\n\nYou have been invited to join Robot Super Course at edx.org by a member of the course staff.\n\n"
+            "To finish your registration, please visit https://edx.org/register and fill out the registration form "
+            "making sure to use robot-not-an-email-yet@robot.org in the E-mail field.\n"
+            "You can then enroll in Robot Super Course.\n\n----\n"
+            "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org"
+        )
+
     def test_enroll_with_email_not_registered_autoenroll(self):
         url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True, 'auto_enroll': True})
@@ -587,12 +602,10 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
     @patch('instructor.enrollment.uses_shib')
     def test_enroll_with_email_not_registered_with_shib(self, mock_uses_shib):
-
         mock_uses_shib.return_value = True
 
         url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id})
         response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True})
-        print "type(self.notregistered_email): {}".format(type(self.notregistered_email))
         self.assertEqual(response.status_code, 200)
 
         # Check the outbox
@@ -601,6 +614,7 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
             mail.outbox[0].subject,
             'You have been invited to register for Robot Super Course'
         )
+
         self.assertEqual(
             mail.outbox[0].body,
             "Dear student,\n\nYou have been invited to join Robot Super Course at edx.org by a member of the course staff.\n\n"
@@ -609,6 +623,22 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
         )
 
     @patch('instructor.enrollment.uses_shib')
+    def test_enroll_email_not_registered_shib_mktgsite(self, mock_uses_shib):
+        mock_uses_shib.return_value = True
+
+        url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id})
+        # Try with marketing site enabled
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
+            response = self.client.get(url, {'emails': self.notregistered_email, 'action': 'enroll', 'email_students': True})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            mail.outbox[0].body,
+            "Dear student,\n\nYou have been invited to join Robot Super Course at edx.org by a member of the course staff.\n\n----\n"
+            "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org"
+        )
+
+    @patch('instructor.enrollment.uses_shib')
     def test_enroll_with_email_not_registered_with_shib_autoenroll(self, mock_uses_shib):
 
         mock_uses_shib.return_value = True
@@ -624,10 +654,11 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase):
             mail.outbox[0].subject,
             'You have been invited to register for Robot Super Course'
         )
+
         self.assertEqual(
             mail.outbox[0].body,
             "Dear student,\n\nYou have been invited to join Robot Super Course at edx.org by a member of the course staff.\n\n"
-            "To access the course visit https://edx.org/courses/MITx/999/Robot_Super_Course and login.\n\n----\n"
+            "To access the course visit https://edx.org/courses/MITx/999/Robot_Super_Course/ and login.\n\n----\n"
             "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org"
         )
 
@@ -721,6 +752,7 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe
             mail.outbox[0].subject,
             'You have been invited to a beta test for Robot Super Course'
         )
+
         self.assertEqual(
             mail.outbox[0].body,
             u"Dear {0}\n\nYou have been invited to be a beta tester "
@@ -733,6 +765,24 @@ class TestInstructorAPIBulkBetaEnrollment(ModuleStoreTestCase, LoginEnrollmentTe
             )
         )
 
+    def test_add_notenrolled_email_mktgsite(self):
+        url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id})
+        # Try with marketing site enabled
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
+            response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'add', 'email_students': True})
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(
+            mail.outbox[0].body,
+            u"Dear {0}\n\nYou have been invited to be a beta tester "
+            "for Robot Super Course at edx.org by a member of the course staff.\n\n"
+            "Visit edx.org to enroll in the course and begin the beta test.\n\n----\n"
+            "This email was automatically sent from edx.org to {1}".format(
+                self.notenrolled_student.profile.name,
+                self.notenrolled_student.email
+            )
+        )
+
     def test_enroll_with_email_not_registered(self):
         # User doesn't exist
         url = reverse('bulk_beta_modify_access', kwargs={'course_id': self.course.id})

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -3,15 +3,21 @@ Unit tests for instructor.enrollment methods.
 """
 
 import json
+import mock
 from abc import ABCMeta
 from courseware.models import StudentModule
+from django.conf import settings
 from django.test import TestCase
+from django.test.utils import override_settings
 from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.factories import CourseFactory
+from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
 
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
 from instructor.enrollment import (
     EmailEnrollmentState,
     enroll_email,
+    get_email_params,
     reset_student_attempts,
     send_beta_role_email,
     unenroll_email
@@ -385,3 +391,46 @@ class TestSendBetaRoleEmail(TestCase):
         error_msg = "Unexpected action received '{}' - expected 'add' or 'remove'".format(bad_action)
         with self.assertRaisesRegexp(ValueError, error_msg):
             send_beta_role_email(bad_action, self.user, self.email_params)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class TestGetEmailParams(TestCase):
+    """
+    Test what URLs the function get_email_params returns under different
+    production-like conditions.
+    """
+    def setUp(self):
+        self.course = CourseFactory.create()
+
+        # Explicitly construct what we expect the course URLs to be
+        site = settings.SITE_NAME
+        self.course_url = u'https://{}/courses/{}/'.format(
+            site,
+            self.course.id
+        )
+        self.course_about_url = self.course_url + 'about'
+        self.registration_url = u'https://{}/register'.format(
+            site,
+        )
+
+    def test_normal_params(self):
+        # For a normal site, what do we expect to get for the URLs?
+        # Also make sure `auto_enroll` is properly passed through.
+        result = get_email_params(self.course, False)
+
+        self.assertEqual(result['auto_enroll'], False)
+        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)
+
+    def test_marketing_params(self):
+        # For a site with a marketing front end, what do we expect to get for the URLs?
+        # Also make sure `auto_enroll` is properly passed through.
+        with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
+            result = get_email_params(self.course, True)
+
+        self.assertEqual(result['auto_enroll'], True)
+        # We should *not* get a course about url (LMS doesn't know what the marketing site URLs are)
+        self.assertEqual(result['course_about_url'], None)
+        self.assertEqual(result['registration_url'], self.registration_url)
+        self.assertEqual(result['course_url'], self.course_url)

lms/djangoapps/instructor/tests/test_legacy_enrollment.py

@@ -221,7 +221,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
             "at edx.org by a member of the course staff. "
             "The course should now appear on your edx.org dashboard.\n\n"
             "To start accessing course materials, please visit "
-            "https://edx.org/courses/MITx/999/Robot_Super_Course\n\n"
+            "https://edx.org/courses/MITx/999/Robot_Super_Course/\n\n"
             "----\nThis email was automatically sent from edx.org to Autoenrolled Test"
         )
 
@@ -322,7 +322,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
             "at edx.org by a member of the course staff. "
             "The course should now appear on your edx.org dashboard.\n\n"
             "To start accessing course materials, please visit "
-            "https://edx.org/courses/MITx/999/Robot_Super_Course\n\n"
+            "https://edx.org/courses/MITx/999/Robot_Super_Course/\n\n"
             "----\nThis email was automatically sent from edx.org to ShibTest Enrolled"
         )
 
@@ -335,7 +335,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
             "Dear student,\n\nYou have been invited to join "
             "Robot Super Course at edx.org by a member of the "
             "course staff.\n\n"
-            "To access the course visit https://edx.org/courses/MITx/999/Robot_Super_Course and login.\n\n"
+            "To access the course visit https://edx.org/courses/MITx/999/Robot_Super_Course/ and login.\n\n"
             "----\nThis email was automatically sent from edx.org to "
             "student5_1@test.com"
         )

lms/djangoapps/instructor/views/legacy.py

@@ -1381,16 +1381,32 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
             'SITE_NAME',
             settings.SITE_NAME
         )
-        registration_url = 'https://' + stripped_site_name + reverse('student.views.register_user')
-        #Composition of email
-        d = {'site_name': stripped_site_name,
-             'registration_url': registration_url,
-             'course': course,
-             'auto_enroll': auto_enroll,
-             'course_url': 'https://' + stripped_site_name + '/courses/' + course_id,
-             'course_about_url': 'https://' + stripped_site_name + '/courses/' + course_id + '/about',
-             'is_shib_course': is_shib_course
-             }
+        registration_url = 'https://{}{}'.format(
+            stripped_site_name,
+            reverse('student.views.register_user')
+        )
+        course_url = 'https://{}{}'.format(
+            stripped_site_name,
+            reverse('course_root', kwargs={'course_id': course_id})
+        )
+        # We can't get the url to the course's About page if the marketing site is enabled.
+        course_about_url = None
+        if not settings.FEATURES.get('ENABLE_MKTG_SITE', False):
+            course_about_url = u'https://{}{}'.format(
+                stripped_site_name,
+                reverse('about_course', kwargs={'course_id': course.id})
+            )
+
+        # Composition of email
+        d = {
+            'site_name': stripped_site_name,
+            'registration_url': registration_url,
+            'course': course,
+            'auto_enroll': auto_enroll,
+            'course_url': course_url,
+            'course_about_url': course_about_url,
+            'is_shib_course': is_shib_course
+        }
 
     for student in new_students:
         try:

lms/templates/emails/add_beta_tester_email_message.txt

@@ -8,7 +8,11 @@ ${_("You have been invited to be a beta tester for {course_name} at {site_name}
 		site_name=site_name
 	)}
 
+% if course_about_url is not None:
 ${_("Visit {course_about_url} to join the course and begin the beta test.").format(course_about_url=course_about_url)}
+% else:
+${_("Visit {site_name} to enroll in the course and begin the beta test.").format(site_name=site_name)}
+% endif
 
 ----
 ${_("This email was automatically sent from {site_name} to "

lms/templates/emails/enroll_email_allowedmessage.txt

@@ -8,14 +8,14 @@ ${_("You have been invited to join {course_name} at {site_name} by a "
 		site_name=site_name
 	)}
 % if is_shib_course:
-
 % if auto_enroll:
+
 ${_("To access the course visit {course_url} and login.").format(course_url=course_url)}
-% else:
+% elif course_about_url is not None:
+
 ${_("To access the course visit {course_about_url} and register for the course.").format(
 		course_about_url=course_about_url)}
 % endif
-
 % else:
 
 ${_("To finish your registration, please visit {registration_url} and fill "
@@ -28,14 +28,16 @@ ${_("Once you have registered and activated your account, you will see "
 	"{course_name} listed on your dashboard.").format(
 		course_name=course.display_name_with_default
 	)}
-% else:
+% elif course_about_url is not None:
 ${_("Once you have registered and activated your account, visit {course_about_url} "
 	"to join the course.").format(course_about_url=course_about_url)}
+% else:
+${_("You can then enroll in {course_name}.").format(course_name=course.display_name_with_default)}
 % endif
-
 % endif
+
 ----
 ${_("This email was automatically sent from {site_name} to "
 	"{email_address}").format(
 		site_name=site_name, email_address=email_address
-	)}
+	)}
\ No newline at end of file

lms/templates/instructor/instructor_dashboard_2/membership.html

@@ -40,8 +40,10 @@
     <label for="auto-enroll">${_("Auto Enroll")}</label>
     <div class="hint auto-enroll-hint">
       <span class="hint-caret"></span>
-      <p> ${_("If this option is <em>checked</em>, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)}
-          ${_("If this option is left <em>unchecked</em>, users who have not yet registered for {platform_name} will not be enrolled, but will be allowed to enroll once they make an account.").format(platform_name=settings.PLATFORM_NAME)}</p>
+      <p> 
+	${_("If this option is <em>checked</em>, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)}
+	${_("If this option is left <em>unchecked</em>, users who have not yet registered for {platform_name} will not be enrolled, but will be allowed to enroll once they make an account.").format(platform_name=settings.PLATFORM_NAME)}
+      </p>
     </div>
   </div>
 
@@ -50,7 +52,9 @@
     <label for="email-students">${_("Notify users by email")}</label>
     <div class="hint email-students-hint">
       <span class="hint-caret"></span>
-      <p> ${_("If this option is <em>checked</em>, users will receive an email notification.")}</p>
+      <p> 
+	${_("If this option is <em>checked</em>, users will receive an email notification.")}
+      </p>
     </div>
   </div>