Commit b7558636 by Matjaz Gregoric

Don't hardcode https protocol in enrollment emails.

Use http or https, depending on the value of `request.is_secure()`.
That is how links in most other emails are handled.
parent ddfffdaf
...@@ -223,7 +223,7 @@ def _reset_module_attempts(studentmodule): ...@@ -223,7 +223,7 @@ def _reset_module_attempts(studentmodule):
studentmodule.save() studentmodule.save()
def get_email_params(course, auto_enroll): def get_email_params(course, auto_enroll, secure=True):
""" """
Generate parameters used when parsing email templates. Generate parameters used when parsing email templates.
...@@ -231,27 +231,32 @@ def get_email_params(course, auto_enroll): ...@@ -231,27 +231,32 @@ def get_email_params(course, auto_enroll):
Returns a dict of parameters Returns a dict of parameters
""" """
protocol = 'https' if secure else 'http'
stripped_site_name = microsite.get_value( stripped_site_name = microsite.get_value(
'SITE_NAME', 'SITE_NAME',
settings.SITE_NAME settings.SITE_NAME
) )
# TODO: Use request.build_absolute_uri rather than 'https://{}{}'.format # TODO: Use request.build_absolute_uri rather than '{proto}://{site}{path}'.format
# and check with the Services team that this works well with microsites # and check with the Services team that this works well with microsites
registration_url = u'https://{}{}'.format( registration_url = u'{proto}://{site}{path}'.format(
stripped_site_name, proto=protocol,
reverse('student.views.register_user') site=stripped_site_name,
path=reverse('student.views.register_user')
) )
course_url = u'https://{}{}'.format( course_url = u'{proto}://{site}{path}'.format(
stripped_site_name, proto=protocol,
reverse('course_root', kwargs={'course_id': course.id.to_deprecated_string()}) site=stripped_site_name,
path=reverse('course_root', kwargs={'course_id': course.id.to_deprecated_string()})
) )
# We can't get the url to the course's About page if the marketing site is enabled. # We can't get the url to the course's About page if the marketing site is enabled.
course_about_url = None course_about_url = None
if not settings.FEATURES.get('ENABLE_MKTG_SITE', False): if not settings.FEATURES.get('ENABLE_MKTG_SITE', False):
course_about_url = u'https://{}{}'.format( course_about_url = u'{proto}://{site}{path}'.format(
stripped_site_name, proto=protocol,
reverse('about_course', kwargs={'course_id': course.id.to_deprecated_string()}) site=stripped_site_name,
path=reverse('about_course', kwargs={'course_id': course.id.to_deprecated_string()})
) )
is_shib_course = uses_shib(course) is_shib_course = uses_shib(course)
......
...@@ -3,6 +3,7 @@ Unit tests for enrollment methods in views.py ...@@ -3,6 +3,7 @@ Unit tests for enrollment methods in views.py
""" """
import ddt
from mock import patch from mock import patch
from django.test.utils import override_settings from django.test.utils import override_settings
...@@ -20,6 +21,7 @@ from django.core import mail ...@@ -20,6 +21,7 @@ from django.core import mail
USER_COUNT = 4 USER_COUNT = 4
@ddt.ddt
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase): class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase):
""" """
...@@ -189,7 +191,8 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -189,7 +191,8 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
cleaned_string, cleaned_string_lc = get_and_clean_student_list(string) cleaned_string, cleaned_string_lc = get_and_clean_student_list(string)
self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com', 'mno@test.com']) self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com', 'mno@test.com'])
def test_enrollment_email_on(self): @ddt.data('http', 'https')
def test_enrollment_email_on(self, protocol):
""" """
Do email on enroll test Do email on enroll test
""" """
...@@ -200,7 +203,9 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -200,7 +203,9 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
UserFactory.create(username="student3_0", email="student3_0@test.com", first_name='Autoenrolled') UserFactory.create(username="student3_0", email="student3_0@test.com", first_name='Autoenrolled')
url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()})
response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student3_0@test.com, student3_1@test.com, student3_2@test.com', 'auto_enroll': 'on', 'email_students': 'on'}) params = {'action': 'Enroll multiple students', 'multiple_students': 'student3_0@test.com, student3_1@test.com, student3_2@test.com', 'auto_enroll': 'on', 'email_students': 'on'}
environ = {'wsgi.url_scheme': protocol}
response = self.client.post(url, params, **environ)
# Check the page output # Check the page output
self.assertContains(response, '<td>student3_0@test.com</td>') self.assertContains(response, '<td>student3_0@test.com</td>')
...@@ -221,8 +226,8 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -221,8 +226,8 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
"at edx.org by a member of the course staff. " "at edx.org by a member of the course staff. "
"The course should now appear on your edx.org dashboard.\n\n" "The course should now appear on your edx.org dashboard.\n\n"
"To start accessing course materials, please visit " "To start accessing course materials, please visit "
"https://edx.org/courses/MITx/999/Robot_Super_Course/\n\n" "{}://edx.org/courses/MITx/999/Robot_Super_Course/\n\n"
"----\nThis email was automatically sent from edx.org to Autoenrolled Test" "----\nThis email was automatically sent from edx.org to Autoenrolled Test".format(protocol)
) )
self.assertEqual( self.assertEqual(
...@@ -235,12 +240,12 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -235,12 +240,12 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
"Robot Super Course at edx.org by a member of the " "Robot Super Course at edx.org by a member of the "
"course staff.\n\n" "course staff.\n\n"
"To finish your registration, please visit " "To finish your registration, please visit "
"https://edx.org/register and fill out the registration form " "{}://edx.org/register and fill out the registration form "
"making sure to use student3_1@test.com in the E-mail field.\n" "making sure to use student3_1@test.com in the E-mail field.\n"
"Once you have registered and activated your account, you will " "Once you have registered and activated your account, you will "
"see Robot Super Course listed on your dashboard.\n\n" "see Robot Super Course listed on your dashboard.\n\n"
"----\nThis email was automatically sent from edx.org to " "----\nThis email was automatically sent from edx.org to "
"student3_1@test.com" "student3_1@test.com".format(protocol)
) )
def test_unenrollment_email_on(self): def test_unenrollment_email_on(self):
...@@ -291,8 +296,9 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -291,8 +296,9 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
send_mail_ret = send_mail_to_student('student0@test.com', d) send_mail_ret = send_mail_to_student('student0@test.com', d)
self.assertFalse(send_mail_ret) self.assertFalse(send_mail_ret)
@ddt.data('http', 'https')
@patch('instructor.views.legacy.uses_shib') @patch('instructor.views.legacy.uses_shib')
def test_enrollment_email_on_shib_on(self, mock_uses_shib): def test_enrollment_email_on_shib_on(self, protocol, mock_uses_shib):
# Do email on enroll, shibboleth on test # Do email on enroll, shibboleth on test
course = self.course course = self.course
...@@ -302,7 +308,9 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -302,7 +308,9 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
UserFactory.create(username="student5_0", email="student5_0@test.com", first_name="ShibTest", last_name="Enrolled") UserFactory.create(username="student5_0", email="student5_0@test.com", first_name="ShibTest", last_name="Enrolled")
url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()}) url = reverse('instructor_dashboard_legacy', kwargs={'course_id': course.id.to_deprecated_string()})
response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student5_0@test.com, student5_1@test.com', 'auto_enroll': 'on', 'email_students': 'on'}) params = {'action': 'Enroll multiple students', 'multiple_students': 'student5_0@test.com, student5_1@test.com', 'auto_enroll': 'on', 'email_students': 'on'}
environ = {'wsgi.url_scheme': protocol}
response = self.client.post(url, params, **environ)
# Check the page output # Check the page output
self.assertContains(response, '<td>student5_0@test.com</td>') self.assertContains(response, '<td>student5_0@test.com</td>')
...@@ -322,8 +330,8 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -322,8 +330,8 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
"at edx.org by a member of the course staff. " "at edx.org by a member of the course staff. "
"The course should now appear on your edx.org dashboard.\n\n" "The course should now appear on your edx.org dashboard.\n\n"
"To start accessing course materials, please visit " "To start accessing course materials, please visit "
"https://edx.org/courses/MITx/999/Robot_Super_Course/\n\n" "{}://edx.org/courses/MITx/999/Robot_Super_Course/\n\n"
"----\nThis email was automatically sent from edx.org to ShibTest Enrolled" "----\nThis email was automatically sent from edx.org to ShibTest Enrolled".format(protocol)
) )
self.assertEqual( self.assertEqual(
...@@ -335,7 +343,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) ...@@ -335,7 +343,7 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase)
"Dear student,\n\nYou have been invited to join " "Dear student,\n\nYou have been invited to join "
"Robot Super Course at edx.org by a member of the " "Robot Super Course at edx.org by a member of the "
"course staff.\n\n" "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 {}://edx.org/courses/MITx/999/Robot_Super_Course/ and login.\n\n"
"----\nThis email was automatically sent from edx.org to " "----\nThis email was automatically sent from edx.org to "
"student5_1@test.com" "student5_1@test.com".format(protocol)
) )
...@@ -257,7 +257,7 @@ def students_update_enrollment(request, course_id): ...@@ -257,7 +257,7 @@ def students_update_enrollment(request, course_id):
email_params = {} email_params = {}
if email_students: if email_students:
course = get_course_by_id(course_id) course = get_course_by_id(course_id)
email_params = get_email_params(course, auto_enroll) email_params = get_email_params(course, auto_enroll, secure=request.is_secure())
results = [] results = []
for identifier in identifiers: for identifier in identifiers:
...@@ -348,7 +348,8 @@ def bulk_beta_modify_access(request, course_id): ...@@ -348,7 +348,8 @@ def bulk_beta_modify_access(request, course_id):
email_params = {} email_params = {}
if email_students: if email_students:
email_params = get_email_params(course, auto_enroll=auto_enroll) secure = request.is_secure()
email_params = get_email_params(course, auto_enroll=auto_enroll, secure=secure)
for identifier in identifiers: for identifier in identifiers:
try: try:
......
...@@ -813,7 +813,8 @@ def instructor_dashboard(request, course_id): ...@@ -813,7 +813,8 @@ def instructor_dashboard(request, course_id):
students = request.POST.get('multiple_students', '') students = request.POST.get('multiple_students', '')
auto_enroll = bool(request.POST.get('auto_enroll')) auto_enroll = bool(request.POST.get('auto_enroll'))
email_students = bool(request.POST.get('email_students')) email_students = bool(request.POST.get('email_students'))
ret = _do_enroll_students(course, course_key, students, auto_enroll=auto_enroll, email_students=email_students, is_shib_course=is_shib_course) secure = request.is_secure()
ret = _do_enroll_students(course, course_key, students, secure=secure, auto_enroll=auto_enroll, email_students=email_students, is_shib_course=is_shib_course)
datatable = ret['datatable'] datatable = ret['datatable']
elif action == 'Unenroll multiple students': elif action == 'Unenroll multiple students':
...@@ -839,7 +840,8 @@ def instructor_dashboard(request, course_id): ...@@ -839,7 +840,8 @@ def instructor_dashboard(request, course_id):
if not 'List' in action: if not 'List' in action:
students = ','.join([x['email'] for x in datatable['retdata']]) students = ','.join([x['email'] for x in datatable['retdata']])
overload = 'Overload' in action overload = 'Overload' in action
ret = _do_enroll_students(course, course_key, students, overload=overload) secure = request.is_secure()
ret = _do_enroll_students(course, course_key, students, secure=secure, overload=overload)
datatable = ret['datatable'] datatable = ret['datatable']
#---------------------------------------- #----------------------------------------
...@@ -1439,7 +1441,7 @@ def grade_summary(request, course_key): ...@@ -1439,7 +1441,7 @@ def grade_summary(request, course_key):
#----------------------------------------------------------------------------- #-----------------------------------------------------------------------------
# enrollment # enrollment
def _do_enroll_students(course, course_key, students, overload=False, auto_enroll=False, email_students=False, is_shib_course=False): def _do_enroll_students(course, course_key, students, secure=False, overload=False, auto_enroll=False, email_students=False, is_shib_course=False):
""" """
Do the actual work of enrolling multiple students, presented as a string Do the actual work of enrolling multiple students, presented as a string
of emails separated by commas or returns of emails separated by commas or returns
...@@ -1468,26 +1470,30 @@ def _do_enroll_students(course, course_key, students, overload=False, auto_enrol ...@@ -1468,26 +1470,30 @@ def _do_enroll_students(course, course_key, students, overload=False, auto_enrol
ceaset.delete() ceaset.delete()
if email_students: if email_students:
protocol = 'https' if secure else 'http'
stripped_site_name = microsite.get_value( stripped_site_name = microsite.get_value(
'SITE_NAME', 'SITE_NAME',
settings.SITE_NAME settings.SITE_NAME
) )
# TODO: Use request.build_absolute_uri rather than 'https://{}{}'.format # TODO: Use request.build_absolute_uri rather than '{proto}://{site}{path}'.format
# and check with the Services team that this works well with microsites # and check with the Services team that this works well with microsites
registration_url = 'https://{}{}'.format( registration_url = '{proto}://{site}{path}'.format(
stripped_site_name, proto=protocol,
reverse('student.views.register_user') site=stripped_site_name,
path=reverse('student.views.register_user')
) )
course_url = 'https://{}{}'.format( course_url = '{proto}://{site}{path}'.format(
stripped_site_name, proto=protocol,
reverse('course_root', kwargs={'course_id': course_key.to_deprecated_string()}) site=stripped_site_name,
path=reverse('course_root', kwargs={'course_id': course_key.to_deprecated_string()})
) )
# We can't get the url to the course's About page if the marketing site is enabled. # We can't get the url to the course's About page if the marketing site is enabled.
course_about_url = None course_about_url = None
if not settings.FEATURES.get('ENABLE_MKTG_SITE', False): if not settings.FEATURES.get('ENABLE_MKTG_SITE', False):
course_about_url = u'https://{}{}'.format( course_about_url = u'{proto}://{site}{path}'.format(
stripped_site_name, proto=protocol,
reverse('about_course', kwargs={'course_id': course_key.to_deprecated_string()}) site=stripped_site_name,
path=reverse('about_course', kwargs={'course_id': course_key.to_deprecated_string()})
) )
# Composition of email # Composition of email
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment