From ed980f218965c8975ea7c6a14b1cd96404e6d52b Mon Sep 17 00:00:00 2001 From: David Adams Date: Fri, 1 Nov 2013 11:41:35 -0700 Subject: [PATCH] Port of invitation/enroll/unenroll email functionality into beta dashboard. Also includes new functionality, in both legacy and beta dashboard, to include appropriate email text for a Shibboleth course. --- lms/djangoapps/instructor/enrollment.py | 111 ++++++- lms/djangoapps/instructor/tests/test_api.py | 284 ++++++++++++++++-- .../instructor/tests/test_enrollment.py | 1 - .../tests/test_legacy_enrollment.py | 52 +++- lms/djangoapps/instructor/views/api.py | 18 +- .../instructor/views/instructor_dashboard.py | 1 + lms/djangoapps/instructor/views/legacy.py | 36 ++- .../instructor_dashboard/membership.coffee | 56 +++- .../sass/course/instructor/_instructor_2.scss | 17 ++ .../emails/enroll_email_allowedmessage.txt | 18 +- .../emails/unenroll_email_enrolledmessage.txt | 4 +- .../instructor_dashboard_2/membership.html | 18 +- 12 files changed, 557 insertions(+), 59 deletions(-) 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, 'student5_0@test.com') + self.assertContains(response, 'student5_1@test.com') + self.assertContains(response, 'added, email sent') + self.assertContains(response, 'user does not exist, enrollment allowed, pending with auto enrollment on, email sent') + + # Check the outbox + self.assertEqual(len(mail.outbox), 2) + self.assertEqual( + mail.outbox[0].subject, + 'You have been enrolled in Robot Super Course' + ) + self.assertEqual( + mail.outbox[0].body, + "Dear ShibTest Enrolled\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" + "----\nThis email was automatically sent from edx.org to ShibTest Enrolled" + ) + + self.assertEqual( + mail.outbox[1].subject, + 'You have been invited to register for Robot Super Course' + ) + self.assertEqual( + mail.outbox[1].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" + "----\nThis email was automatically sent from edx.org to " + "student5_1@test.com" + ) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 2364e85cc2..5596ba9c41 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -33,7 +33,7 @@ import instructor_task.api from instructor_task.api_helper import AlreadyRunningError from instructor_task.views import get_task_completion_info import instructor.enrollment as enrollment -from instructor.enrollment import enroll_email, unenroll_email +from instructor.enrollment import enroll_email, unenroll_email, get_email_params from instructor.views.tools import strip_if_string, get_student_from_identifier from instructor.access import list_with_level, allow_access, revoke_access, update_forum_role import analytics.basic @@ -189,7 +189,10 @@ def students_update_enrollment(request, course_id): - emails is string containing a list of emails separated by anything split_input_list can handle. - auto_enroll is a boolean (defaults to false) If auto_enroll is false, students will be allowed to enroll. - If auto_enroll is true, students will be enroled as soon as they register. + If auto_enroll is true, students will be enrolled as soon as they register. + - email_students is a boolean (defaults to false) + If email_students is true, students will be sent email notification + If email_students is false, students will not be sent email notification Returns an analog to this JSON structure: { "action": "enroll", @@ -213,18 +216,25 @@ def students_update_enrollment(request, course_id): ] } """ + action = request.GET.get('action') emails_raw = request.GET.get('emails') emails = _split_input_list(emails_raw) auto_enroll = request.GET.get('auto_enroll') in ['true', 'True', True] + email_students = request.GET.get('email_students') in ['true', 'True', True] + + email_params = {} + if email_students: + course = get_course_by_id(course_id) + email_params = get_email_params(course, auto_enroll) results = [] for email in emails: try: if action == 'enroll': - before, after = enroll_email(course_id, email, auto_enroll) + before, after = enroll_email(course_id, email, auto_enroll, email_students, email_params) elif action == 'unenroll': - before, after = unenroll_email(course_id, email) + before, after = unenroll_email(course_id, email, email_students, email_params) else: return HttpResponseBadRequest("Unrecognized action '{}'".format(action)) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index bfa85c9f09..7c3fd32a0a 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -26,6 +26,7 @@ from student.models import CourseEnrollment from bulk_email.models import CourseAuthorization from lms.lib.xblock.runtime import handler_prefix + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) def instructor_dashboard_2(request, course_id): diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 04df8e181c..98c18dc68b 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -66,6 +66,9 @@ log = logging.getLogger(__name__) FORUM_ROLE_ADD = 'add' FORUM_ROLE_REMOVE = 'remove' +# For determining if a shibboleth course +SHIBBOLETH_DOMAIN_PREFIX = 'shib:' + def split_by_comma_and_whitespace(a_str): """ @@ -382,8 +385,8 @@ def instructor_dashboard(request, course_id): module_state_key, err.message ) log.exception("Encountered exception from rescore: student '{0}' problem '{1}'".format( - unique_student_identifier, module_state_key - ) + unique_student_identifier, module_state_key + ) ) elif "Get link to student's progress page" in action: @@ -445,7 +448,7 @@ def instructor_dashboard(request, course_id): try: ddata.append([x.email, x.grades[aidx]]) except IndexError: - log.debug('No grade for assignment %s (%s) for student %s' % (aidx, aname, x.email)) + log.debug('No grade for assignment %s (%s) for student %s', aidx, aname, x.email) datatable['data'] = ddata datatable['title'] = 'Grades for assignment "%s"' % aname @@ -632,10 +635,11 @@ def instructor_dashboard(request, course_id): elif action == 'Enroll multiple students': + is_shib_course = uses_shib(course) students = request.POST.get('multiple_students', '') auto_enroll = bool(request.POST.get('auto_enroll')) email_students = bool(request.POST.get('email_students')) - ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll, email_students=email_students) + ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll, email_students=email_students, is_shib_course=is_shib_course) datatable = ret['datatable'] elif action == 'Unenroll multiple students': @@ -1190,7 +1194,7 @@ def grade_summary(request, course_id): #----------------------------------------------------------------------------- # enrollment -def _do_enroll_students(course, course_id, students, overload=False, auto_enroll=False, email_students=False): +def _do_enroll_students(course, course_id, students, overload=False, auto_enroll=False, email_students=False, is_shib_course=False): """ Do the actual work of enrolling multiple students, presented as a string of emails separated by commas or returns @@ -1219,7 +1223,7 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll ceaset.delete() if email_students: - stripped_site_name = _remove_preview(settings.SITE_NAME) + stripped_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, @@ -1227,6 +1231,8 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll '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, } for student in new_students: @@ -1308,7 +1314,7 @@ def _do_unenroll_students(course_id, students, email_students=False): old_students, _ = get_and_clean_student_list(students) status = dict([x, 'unprocessed'] for x in old_students) - stripped_site_name = _remove_preview(settings.SITE_NAME) + stripped_site_name = settings.SITE_NAME if email_students: course = course_from_id(course_id) #Composition of email @@ -1377,6 +1383,7 @@ def send_mail_to_student(student, param_dict): `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. """ @@ -1403,12 +1410,6 @@ def send_mail_to_student(student, param_dict): return False -def _remove_preview(site_name): - if site_name[:8] == "preview.": - return site_name[8:] - return site_name - - def get_and_clean_student_list(students): """ Separate out individual student email from the comma, or space separated string. @@ -1593,3 +1594,12 @@ def get_background_task_table(course_id, problem_url=None, student=None, task_ty datatable['title'] = "{course_id} > {location}".format(course_id=course_id, location=problem_url) return msg, datatable + + +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/static/coffee/src/instructor_dashboard/membership.coffee b/lms/static/coffee/src/instructor_dashboard/membership.coffee index 03c6b705b6..590fd86f3e 100644 --- a/lms/static/coffee/src/instructor_dashboard/membership.coffee +++ b/lms/static/coffee/src/instructor_dashboard/membership.coffee @@ -8,6 +8,7 @@ such that the value can be defined later than this assignment (file load order). plantTimeout = -> window.InstructorDashboard.util.plantTimeout.apply this, arguments std_ajax_err = -> window.InstructorDashboard.util.std_ajax_err.apply this, arguments +emailStudents = false class MemberListWidget @@ -189,16 +190,20 @@ class BatchEnrollment @$btn_enroll = @$container.find("input[name='enroll']'") @$btn_unenroll = @$container.find("input[name='unenroll']'") @$checkbox_autoenroll = @$container.find("input[name='auto-enroll']'") + @$checkbox_emailstudents = @$container.find("input[name='email-students']'") @$task_response = @$container.find(".request-response") @$request_response_error = @$container.find(".request-response-error") # attach click handlers @$btn_enroll.click => + emailStudents = @$checkbox_emailstudents.is(':checked') + send_data = action: 'enroll' emails: @$emails_input.val() auto_enroll: @$checkbox_autoenroll.is(':checked') + email_students: emailStudents $.ajax dataType: 'json' @@ -208,10 +213,13 @@ class BatchEnrollment error: std_ajax_err => @fail_with_error "Error enrolling/unenrolling students." @$btn_unenroll.click => + emailStudents = @$checkbox_emailstudents.is(':checked') + send_data = action: 'unenroll' emails: @$emails_input.val() auto_enroll: @$checkbox_autoenroll.is(':checked') + email_students: emailStudents $.ajax dataType: 'json' @@ -244,6 +252,8 @@ class BatchEnrollment autoenrolled = [] # students who are now not enrolled in the course notenrolled = [] + # students who were not enrolled or allowed prior to unenroll action + notunenrolled = [] # categorize student results into the above arrays. for student_results in data_from_server.results @@ -272,15 +282,23 @@ class BatchEnrollment if student_results.error errors.push student_results + else if student_results.after.enrollment enrolled.push student_results + else if student_results.after.allowed if student_results.after.auto_enroll autoenrolled.push student_results else allowed.push student_results + + # The instructor is trying to unenroll someone who is not enrolled or allowed to enroll; non-sensical action. + else if data_from_server.action is 'unenroll' and not (student_results.before.enrollment) and not (student_results.before.allowed) + notunenrolled.push student_results + else if not student_results.after.enrollment notenrolled.push student_results + else console.warn 'student results not reported to user' console.warn student_results @@ -310,21 +328,43 @@ class BatchEnrollment for student_results in errors render_list errors_label, (sr.email for sr in errors) - if enrolled.length - render_list "Students Enrolled:", (sr.email for sr in enrolled) + if enrolled.length and emailStudents + render_list gettext("Successfully enrolled and sent email to the following students:"), (sr.email for sr in enrolled) - if allowed.length - render_list "These students will be allowed to enroll once they register:", + if enrolled.length and not emailStudents + render_list gettext("Successfully enrolled the following students:"), (sr.email for sr in enrolled) + + # Student hasn't registered so we allow them to enroll + if allowed.length and emailStudents + render_list gettext("Successfully sent enrollment emails to the following students. They will be allowed to enroll once they register:"), (sr.email for sr in allowed) - if autoenrolled.length - render_list "These students will be enrolled once they register:", + # Student hasn't registered so we allow them to enroll + if allowed.length and not emailStudents + render_list gettext("These students will be allowed to enroll once they register:"), + (sr.email for sr in allowed) + + # Student hasn't registered so we allow them to enroll with autoenroll + if autoenrolled.length and emailStudents + render_list gettext("Successfully sent enrollment emails to the following students. They will be enrolled once they register:"), (sr.email for sr in autoenrolled) - if notenrolled.length - render_list "These students are now not enrolled:", + # Student hasn't registered so we allow them to enroll with autoenroll + if autoenrolled.length and not emailStudents + render_list gettext("These students will be enrolled once they register:"), + (sr.email for sr in autoenrolled) + + if notenrolled.length and emailStudents + render_list gettext("Emails successfully sent. The following students are no longer enrolled in the course:"), (sr.email for sr in notenrolled) + if notenrolled.length and not emailStudents + render_list gettext("The following students are no longer enrolled in the course:"), + (sr.email for sr in notenrolled) + + if notunenrolled.length + render_list gettext("These students were not affliliated with the course so could not be unenrolled:"), + (sr.email for sr in notunenrolled) # Wrapper for auth list subsection. # manages a list of users who have special access. diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index e0c0e3a6bb..d1a04ce185 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -309,6 +309,23 @@ section.instructor-dashboard-content-2 { font-weight: bold; } } + + label[for="email-students"]:hover + .email-students-hint { + display: block; + } + + .email-students-hint { + position: absolute; + display: none; + padding: $baseline; + width: $half_width; + border: 1px solid $light-gray; + background-color: $white; + + span.emph { + font-weight: bold; + } + } .batch-enrollment { textarea { diff --git a/lms/templates/emails/enroll_email_allowedmessage.txt b/lms/templates/emails/enroll_email_allowedmessage.txt index ad25c23eaf..a41cb6d11a 100644 --- a/lms/templates/emails/enroll_email_allowedmessage.txt +++ b/lms/templates/emails/enroll_email_allowedmessage.txt @@ -7,10 +7,19 @@ ${_("You have been invited to join {course_name} at {site_name} by a " course_name=course.display_name_with_default, 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: +${_("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 " - "out the registration form making sure to use {email_address} in the " - "E-mail field.").format( + "out the registration form making sure to use {email_address} in the E-mail field.").format( registration_url=registration_url, email_address=email_address )} @@ -20,10 +29,11 @@ ${_("Once you have registered and activated your account, you will see " course_name=course.display_name_with_default )} % else: -${_("Once you have registered and activated your account, visit {course_url} " - "to join the course.").format(course_url=course_url)} +${_("Once you have registered and activated your account, visit {course_about_url} " + "to join the course.").format(course_about_url=course_about_url)} % endif +% endif ---- ${_("This email was automatically sent from {site_name} to " "{email_address}").format( diff --git a/lms/templates/emails/unenroll_email_enrolledmessage.txt b/lms/templates/emails/unenroll_email_enrolledmessage.txt index f3e1bf8bdc..9a6e5d9161 100644 --- a/lms/templates/emails/unenroll_email_enrolledmessage.txt +++ b/lms/templates/emails/unenroll_email_enrolledmessage.txt @@ -11,7 +11,7 @@ ${_("You have been un-enrolled in {course_name} at {site_name} by a member " ${_("Your other courses have not been affected.")} ---- -${_("This email was automatically sent from ${site_name} to " - "${full_name}").format( +${_("This email was automatically sent from {site_name} to " + "{full_name}").format( full_name=full_name, site_name=site_name )} \ 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 eea52cdfc6..aa05b47e17 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -32,8 +32,7 @@

${_("Enter student emails separated by new lines or commas.")}


- - +
@@ -41,6 +40,21 @@ ${_("If auto enroll is left unchecked, students who have not yet registered for edX will not be enrolled, but will be allowed to enroll.")}

+ +
+ + + +
+ +
+ + +
+