diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 7205cffe06..508271fa8b 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -6,8 +6,16 @@ Does not include any access control, be sure to check access before calling. import json from django.contrib.auth.models import User +from django.conf import settings +from django.core.urlresolvers import reverse +from django.core.mail import send_mail + from student.models import CourseEnrollment, CourseEnrollmentAllowed from courseware.models import StudentModule +from mitxmako.shortcuts import render_to_string + +# For determining if a shibboleth course +SHIBBOLETH_DOMAIN_PREFIX = 'shib:' class EmailEnrollmentState(object): @@ -17,8 +25,10 @@ class EmailEnrollmentState(object): if exists_user: user = User.objects.get(email=email) exists_ce = CourseEnrollment.is_enrolled(user, course_id) + full_name = user.profile.name else: exists_ce = False + full_name = None ceas = CourseEnrollmentAllowed.objects.filter(course_id=course_id, email=email).all() exists_allowed = len(ceas) > 0 state_auto_enroll = exists_allowed and ceas[0].auto_enroll @@ -27,6 +37,7 @@ class EmailEnrollmentState(object): self.enrollment = exists_ce self.allowed = exists_allowed self.auto_enroll = bool(state_auto_enroll) + self.full_name = full_name def __repr__(self): return "{}(user={}, enrollment={}, allowed={}, auto_enroll={})".format( @@ -54,7 +65,7 @@ class EmailEnrollmentState(object): } -def enroll_email(course_id, student_email, auto_enroll=False): +def enroll_email(course_id, student_email, auto_enroll=False, email_students=False, email_params=None): """ Enroll a student by email. @@ -62,6 +73,8 @@ def enroll_email(course_id, student_email, auto_enroll=False): `auto_enroll` determines what is put in CourseEnrollmentAllowed.auto_enroll if auto_enroll is set, then when the email registers, they will be enrolled in the course automatically. + `email_students` determines if student should be notified of action by email. + `email_params` parameters used while parsing email templates (a `dict`). returns two EmailEnrollmentState's representing state before and after the action. @@ -71,21 +84,32 @@ def enroll_email(course_id, student_email, auto_enroll=False): if previous_state.user: CourseEnrollment.enroll_by_email(student_email, course_id) + if email_students: + email_params['message'] = 'enrolled_enroll' + email_params['email_address'] = student_email + email_params['full_name'] = previous_state.full_name + send_mail_to_student(student_email, email_params) else: cea, _ = CourseEnrollmentAllowed.objects.get_or_create(course_id=course_id, email=student_email) cea.auto_enroll = auto_enroll cea.save() + if email_students: + email_params['message'] = 'allowed_enroll' + email_params['email_address'] = student_email + send_mail_to_student(student_email, email_params) after_state = EmailEnrollmentState(course_id, student_email) return previous_state, after_state -def unenroll_email(course_id, student_email): +def unenroll_email(course_id, student_email, email_students=False, email_params=None): """ Unenroll a student by email. `student_email` is student's emails e.g. "foo@bar.com" + `email_students` determines if student should be notified of action by email. + `email_params` parameters used while parsing email templates (a `dict`). returns two EmailEnrollmentState's representing state before and after the action. @@ -95,9 +119,19 @@ def unenroll_email(course_id, student_email): if previous_state.enrollment: CourseEnrollment.unenroll_by_email(student_email, course_id) + if email_students: + email_params['message'] = 'enrolled_unenroll' + email_params['email_address'] = student_email + email_params['full_name'] = previous_state.full_name + send_mail_to_student(student_email, email_params) if previous_state.allowed: CourseEnrollmentAllowed.objects.get(course_id=course_id, email=student_email).delete() + if email_students: + email_params['message'] = 'allowed_unenroll' + email_params['email_address'] = student_email + # Since no User object exists for this student there is no "full_name" available. + send_mail_to_student(student_email, email_params) after_state = EmailEnrollmentState(course_id, student_email) @@ -141,3 +175,76 @@ def _reset_module_attempts(studentmodule): # save studentmodule.state = json.dumps(problem_state) studentmodule.save() + + +def get_email_params(course, auto_enroll): + """ + Generate parameters used when parsing email templates. + + `auto_enroll` is a flag for auto enrolling non-registered students: (a `boolean`) + Returns a dict of parameters + """ + + stripped_site_name = settings.SITE_NAME + registration_url = 'https://' + stripped_site_name + reverse('student.views.register_user') + is_shib_course = uses_shib(course) + + # Composition of email + email_params = { + '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, + } + return email_params + + +def send_mail_to_student(student, param_dict): + """ + Construct the email using templates and then send it. + `student` is the student's email address (a `str`), + + `param_dict` is a `dict` with keys + [ + `site_name`: name given to edX instance (a `str`) + `registration_url`: url for registration (a `str`) + `course_id`: id of course (a `str`) + `auto_enroll`: user input option (a `str`) + `course_url`: url of course (a `str`) + `email_address`: email of student (a `str`) + `full_name`: student full name (a `str`) + `message`: type of email to send and template to use (a `str`) + `is_shib_course`: (a `boolean`) + ] + + Returns a boolean indicating whether the email was sent successfully. + """ + + email_template_dict = {'allowed_enroll': ('emails/enroll_email_allowedsubject.txt', 'emails/enroll_email_allowedmessage.txt'), + 'enrolled_enroll': ('emails/enroll_email_enrolledsubject.txt', 'emails/enroll_email_enrolledmessage.txt'), + 'allowed_unenroll': ('emails/unenroll_email_subject.txt', 'emails/unenroll_email_allowedmessage.txt'), + 'enrolled_unenroll': ('emails/unenroll_email_subject.txt', 'emails/unenroll_email_enrolledmessage.txt')} + + subject_template, message_template = email_template_dict.get(param_dict['message'], (None, None)) + if subject_template is not None and message_template is not None: + subject = render_to_string(subject_template, param_dict) + message = render_to_string(message_template, param_dict) + + # Remove leading and trailing whitespace from body + message = message.strip() + + # Email subject *must not* contain newlines + subject = ''.join(subject.splitlines()) + send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, [student], fail_silently=False) + + +def uses_shib(course): + """ + Used to return whether course has Shibboleth as the enrollment domain + + Returns a boolean indicating if Shibboleth authentication is set for this course. + """ + return course.enrollment_domain and course.enrollment_domain.startswith(SHIBBOLETH_DOMAIN_PREFIX) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 35c7e22c60..6744031c6f 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -14,6 +14,7 @@ from django.test.utils import override_settings from django.core.urlresolvers import reverse from django.http import HttpRequest, HttpResponse from django_comment_common.models import FORUM_ROLE_COMMUNITY_TA +from django.core import mail from django.contrib.auth.models import User from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE @@ -23,7 +24,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from student.tests.factories import UserFactory from courseware.tests.factories import StaffFactory, InstructorFactory -from student.models import CourseEnrollment +from student.models import CourseEnrollment, CourseEnrollmentAllowed from courseware.models import StudentModule # modules which are mocked in test cases. @@ -252,12 +253,17 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): self.instructor = InstructorFactory(course=self.course.location) self.client.login(username=self.instructor.username, password='test') - self.enrolled_student = UserFactory() + self.enrolled_student = UserFactory(username='EnrolledStudent', first_name='Enrolled', last_name='Student') CourseEnrollment.enroll( self.enrolled_student, self.course.id ) - self.notenrolled_student = UserFactory() + self.notenrolled_student = UserFactory(username='NotEnrolledStudent', first_name='NotEnrolled', last_name='Student') + + # Create invited, but not registered, user + cea = CourseEnrollmentAllowed(email='robot-allowed@robot.org', course_id=self.course.id) + cea.save() + self.allowed_email = 'robot-allowed@robot.org' self.notregistered_email = 'robot-not-an-email-yet@robot.org' self.assertEqual(User.objects.filter(email=self.notregistered_email).count(), 0) @@ -280,19 +286,15 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': action}) self.assertEqual(response.status_code, 400) - def test_enroll(self): + def test_enroll_without_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'enroll'}) + response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'enroll', 'email_students': False}) print "type(self.notenrolled_student.email): {}".format(type(self.notenrolled_student.email)) self.assertEqual(response.status_code, 200) - # test that the user is now enrolled - self.assertEqual( - self.notenrolled_student.courseenrollment_set.filter( - course_id=self.course.id - ).count(), - 1 - ) + # test that the user is now enrolled + user = User.objects.get(email=self.notenrolled_student.email) + self.assertTrue(CourseEnrollment.is_enrolled(user, self.course.id)) # test the response data expected = { @@ -320,20 +322,113 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): res_json = json.loads(response.content) self.assertEqual(res_json, expected) - def test_unenroll(self): + # Check the outbox + self.assertEqual(len(mail.outbox), 0) + + def test_enroll_with_email(self): url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': 'unenroll'}) + response = self.client.get(url, {'emails': self.notenrolled_student.email, 'action': 'enroll', 'email_students': True}) + print "type(self.notenrolled_student.email): {}".format(type(self.notenrolled_student.email)) + self.assertEqual(response.status_code, 200) + + # test that the user is now enrolled + user = User.objects.get(email=self.notenrolled_student.email) + self.assertTrue(CourseEnrollment.is_enrolled(user, self.course.id)) + + # test the response data + expected = { + "action": "enroll", + "auto_enroll": False, + "results": [ + { + "email": self.notenrolled_student.email, + "before": { + "enrollment": False, + "auto_enroll": False, + "user": True, + "allowed": False, + }, + "after": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + } + } + ] + } + + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + mail.outbox[0].subject, + 'You have been enrolled in Robot Super Course' + ) + self.assertEqual( + mail.outbox[0].body, + "Dear NotEnrolled Student\n\nYou have been enrolled in Robot Super Course " + "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" + "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 + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + 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 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" + "Once you have registered and activated your account, " + "visit https://edx.org/courses/MITx/999/Robot_Super_Course/about to join the 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}) + print "type(self.notregistered_email): {}".format(type(self.notregistered_email)) + self.assertEqual(response.status_code, 200) + + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + 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 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" + "Once you have registered and activated your account, you will see Robot Super Course listed on your dashboard.\n\n----\n" + "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org" + ) + + def test_unenroll_without_email(self): + url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) + response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': 'unenroll', 'email_students': False}) print "type(self.enrolled_student.email): {}".format(type(self.enrolled_student.email)) self.assertEqual(response.status_code, 200) - # test that the user is now unenrolled - self.assertEqual( - self.enrolled_student.courseenrollment_set.filter( - course_id=self.course.id, - is_active=1, - ).count(), - 0 - ) + # test that the user is now unenrolled + user = User.objects.get(email=self.enrolled_student.email) + self.assertFalse(CourseEnrollment.is_enrolled(user, self.course.id)) # test the response data expected = { @@ -361,6 +456,151 @@ class TestInstructorAPIEnrollment(ModuleStoreTestCase, LoginEnrollmentTestCase): res_json = json.loads(response.content) self.assertEqual(res_json, expected) + # Check the outbox + self.assertEqual(len(mail.outbox), 0) + + def test_unenroll_with_email(self): + url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) + response = self.client.get(url, {'emails': self.enrolled_student.email, 'action': 'unenroll', 'email_students': True}) + print "type(self.enrolled_student.email): {}".format(type(self.enrolled_student.email)) + self.assertEqual(response.status_code, 200) + + # test that the user is now unenrolled + user = User.objects.get(email=self.enrolled_student.email) + self.assertFalse(CourseEnrollment.is_enrolled(user, self.course.id)) + + # test the response data + expected = { + "action": "unenroll", + "auto_enroll": False, + "results": [ + { + "email": self.enrolled_student.email, + "before": { + "enrollment": True, + "auto_enroll": False, + "user": True, + "allowed": False, + }, + "after": { + "enrollment": False, + "auto_enroll": False, + "user": True, + "allowed": False, + } + } + ] + } + + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + mail.outbox[0].subject, + 'You have been un-enrolled from Robot Super Course' + ) + self.assertEqual( + mail.outbox[0].body, + "Dear Enrolled Student\n\nYou have been un-enrolled in Robot Super Course " + "at edx.org by a member of the course staff. " + "The course will no longer appear on your edx.org dashboard.\n\n" + "Your other courses have not been affected.\n\n----\n" + "This email was automatically sent from edx.org to Enrolled Student" + ) + + def test_unenroll_with_email_allowed_student(self): + url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) + response = self.client.get(url, {'emails': self.allowed_email, 'action': 'unenroll', 'email_students': True}) + print "type(self.allowed_email): {}".format(type(self.allowed_email)) + self.assertEqual(response.status_code, 200) + + # test the response data + expected = { + "action": "unenroll", + "auto_enroll": False, + "results": [ + { + "email": self.allowed_email, + "before": { + "enrollment": False, + "auto_enroll": False, + "user": False, + "allowed": True, + }, + "after": { + "enrollment": False, + "auto_enroll": False, + "user": False, + "allowed": False, + } + } + ] + } + + res_json = json.loads(response.content) + self.assertEqual(res_json, expected) + + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + mail.outbox[0].subject, + 'You have been un-enrolled from Robot Super Course' + ) + self.assertEqual( + mail.outbox[0].body, + "Dear Student,\n\nYou have been un-enrolled from course Robot Super Course by a member of the course staff. " + "Please disregard the invitation previously sent.\n\n----\n" + "This email was automatically sent from edx.org to robot-allowed@robot.org" + ) + + @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 + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + 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/about and register for the course.\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 + + 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}) + print "type(self.notregistered_email): {}".format(type(self.notregistered_email)) + self.assertEqual(response.status_code, 200) + + # Check the outbox + self.assertEqual(len(mail.outbox), 1) + self.assertEqual( + 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" + "This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org" + ) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase): diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 3af71bb466..d8202bf534 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -4,7 +4,6 @@ Unit tests for instructor.enrollment methods. import json from abc import ABCMeta -from django.contrib.auth.models import User from courseware.models import StudentModule from django.test import TestCase from student.tests.factories import UserFactory diff --git a/lms/djangoapps/instructor/tests/test_legacy_enrollment.py b/lms/djangoapps/instructor/tests/test_legacy_enrollment.py index a072a2768e..f8c8f5559f 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_legacy_enrollment.py @@ -3,6 +3,8 @@ Unit tests for enrollment methods in views.py """ +from mock import patch + from django.test.utils import override_settings from django.contrib.auth.models import User from django.core.urlresolvers import reverse @@ -279,7 +281,6 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) 'You have been un-enrolled from Robot Super Course' ) - def test_send_mail_to_student(self): """ Do invalid mail template test @@ -289,3 +290,52 @@ class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase) send_mail_ret = send_mail_to_student('student0@test.com', d) self.assertFalse(send_mail_ret) + + @patch('instructor.views.legacy.uses_shib') + def test_enrollment_email_on_shib_on(self, mock_uses_shib): + # Do email on enroll, shibboleth on test + + course = self.course + mock_uses_shib.return_value = True + + # Create activated, but not enrolled, user + UserFactory.create(username="student5_0", email="student5_0@test.com", first_name="ShibTest", last_name="Enrolled") + + url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) + 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'}) + + # Check the page output + self.assertContains(response, '
${_("Enter student emails separated by new lines or commas.")}
${_("If email students is checked students will receive an email notification.")} +
+