From 338a242e22d9ef23be3868abe4a4a396fee9cdf1 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Wed, 2 Apr 2014 18:09:54 -0400 Subject: [PATCH 1/2] Fix course urls in enrollment emails (LMS-2217) --- lms/djangoapps/instructor/enrollment.py | 27 +++++++-- lms/djangoapps/instructor/tests/test_api.py | 60 +++++++++++++++++-- .../instructor/tests/test_enrollment.py | 49 +++++++++++++++ .../tests/test_legacy_enrollment.py | 6 +- lms/djangoapps/instructor/views/legacy.py | 36 +++++++---- .../emails/add_beta_tester_email_message.txt | 4 ++ .../emails/enroll_email_allowedmessage.txt | 14 +++-- .../instructor_dashboard_2/membership.html | 10 +++- 8 files changed, 175 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index d9627e03ff..e62851d6cb 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/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 diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 9a59cdd35c..227d26c15e 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/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" @@ -608,6 +622,22 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org" ) + @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): @@ -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}) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 98b91732cf..8c34c2b95d 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/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) diff --git a/lms/djangoapps/instructor/tests/test_legacy_enrollment.py b/lms/djangoapps/instructor/tests/test_legacy_enrollment.py index f8c8f5559f..2a3b29c742 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_enrollment.py +++ b/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" ) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index f80d6ef617..9d515db359 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/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: diff --git a/lms/templates/emails/add_beta_tester_email_message.txt b/lms/templates/emails/add_beta_tester_email_message.txt index c6b3f3cff4..b08a100d4b 100644 --- a/lms/templates/emails/add_beta_tester_email_message.txt +++ b/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 " diff --git a/lms/templates/emails/enroll_email_allowedmessage.txt b/lms/templates/emails/enroll_email_allowedmessage.txt index a41cb6d11a..49ad62d8ab 100644 --- a/lms/templates/emails/enroll_email_allowedmessage.txt +++ b/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 -% 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 + )} diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 17b3acd010..00636e0f69 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -40,8 +40,10 @@
-

${_("If this option is checked, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)} - ${_("If this option is left unchecked, 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)}

+

+ ${_("If this option is checked, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)} + ${_("If this option is left unchecked, 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)} +

@@ -50,7 +52,9 @@
-

${_("If this option is checked, users will receive an email notification.")}

+

+ ${_("If this option is checked, users will receive an email notification.")} +

From 9a586edf02bc6c90e542aa0e72708becb2f21217 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Fri, 11 Apr 2014 16:21:57 -0400 Subject: [PATCH 2/2] InstDash: check Auto-Enroll and Email Students boxes by default --- .../instructor/instructor_dashboard_2/membership.html | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 00636e0f69..df639a37d6 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -36,19 +36,21 @@

- +

${_("If this option is checked, users who have not yet registered for {platform_name} will be automatically enrolled.").format(platform_name=settings.PLATFORM_NAME)} ${_("If this option is left unchecked, 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)} +

+ ${_("Checking this box has no effect if 'Unenroll' is selected.")}

- +