Commit ddd9e0e4 by dcadams

Merge pull request #1643 from edx/dcadams/fix_invitation_emails

Email port to beta dash and new option for shibboleth courses
parents 7fbd47ce ed980f21
......@@ -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)
......@@ -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
......
......@@ -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, '<td>student5_0@test.com</td>')
self.assertContains(response, '<td>student5_1@test.com</td>')
self.assertContains(response, '<td>added, email sent</td>')
self.assertContains(response, '<td>user does not exist, enrollment allowed, pending with auto enrollment on, email sent</td>')
# 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"
)
......@@ -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))
......
......@@ -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):
......
......@@ -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)
......@@ -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)
# 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)
if autoenrolled.length
render_list "These students will be enrolled once they register:",
# 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.
......
......@@ -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 {
......
......@@ -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(
......
......@@ -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
......@@ -32,8 +32,7 @@
<p> ${_("Enter student emails separated by new lines or commas.")} </p>
<textarea rows="6" cols="50" name="student-emails" placeholder="${_("Student Emails")}" spellcheck="false"></textarea>
<br>
<input type="button" name="enroll" value="${_("Enroll")}" data-endpoint="${ section_data['enroll_button_url'] }" >
<input type="button" name="unenroll" value="${_("Unenroll")}" data-endpoint="${ section_data['unenroll_button_url'] }" >
<input type="checkbox" name="auto-enroll" value="${_("Auto-Enroll")}" style="margin-top: 1em;">
<label for="auto-enroll">${_("Auto Enroll")}</label>
<div class="auto-enroll-hint">
......@@ -41,6 +40,21 @@
${_("If auto enroll is left <em>unchecked</em>, students who have not yet registered for edX will not be enrolled, but will be allowed to enroll.")}
</p>
</div>
<div>
<input type="checkbox" name="email-students" value="${_("Notify-students-by-email")}">
<label for="email-students">${_("Notify students by email")}</label>
<div class="email-students-hint">
<p> ${_("If email students is <em>checked</em> students will receive an email notification.")}
</p>
</div>
</div>
<div>
<input type="button" name="enroll" value="${_("Enroll")}" data-endpoint="${ section_data['enroll_button_url'] }" >
<input type="button" name="unenroll" value="${_("Unenroll")}" data-endpoint="${ section_data['unenroll_button_url'] }" >
</div>
<div class="request-response"></div>
<div class="request-response-error"></div>
</div>
......
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