Skip to content

E
edx-platform
Commit 94ac02e8 by Miles Steele
Options

refactor enrollment action reporting

parent 096f7dcd
Showing with 288 additions and 182 deletions
lms/djangoapps/instructor/enrollment.py
......@@ -4,137 +4,101 @@ Enrollment operations for use by instructor APIs.
Does not include any access control, be sure to check access before calling.
"""
import re
import json
from django.contrib.auth.models import User
from student.models import CourseEnrollment, CourseEnrollmentAllowed
from courseware.models import StudentModule
def enroll_emails(course_id, student_emails, auto_enroll=False):
class EmailEnrollmentState(object):
""" Store the complete enrollment state of an email in a class """
def __init__(self, course_id, email):
exists_user = User.objects.filter(email=email).exists()
exists_ce = CourseEnrollment.objects.filter(course_id=course_id, user__email=email).exists()
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
self.user = exists_user
self.enrollment = exists_ce
self.allowed = exists_allowed
self.auto_enroll = bool(state_auto_enroll)
def __repr__(self):
return "{}(user={}, enrollment={}, allowed={}, auto_enroll={})".format(
self.__class__.__name__,
self.user,
self.enrollment,
self.allowed,
self.auto_enroll,
)
def to_dict(self):
"""
example: {
'user': False,
'enrollment': False,
'allowed': True,
'auto_enroll': True,
}
"""
return {
'user': self.user,
'enrollment': self.enrollment,
'allowed': self.allowed,
'auto_enroll': self.auto_enroll,
}
def enroll_email(course_id, student_email, auto_enroll=False):
"""
Enroll multiple students by email.
Enroll a student by email.
students is a list of student emails e.g. ["foo@bar.com", "bar@foo.com]
each of whom possibly does not exist in db.
`student_email` is student's emails e.g. "foo@bar.com"
`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.
status contains the relevant prior state and action performed on the user.
ce stands for CourseEnrollment
cea stands for CourseEnrollmentAllowed
! stands for the object not existing prior to the action
return a mapping from status to emails.
returns two EmailEnrollmentState's
representing state before and after the action.
"""
auto_string = 'willautoenroll' if auto_enroll else 'allowed'
status_map = {
'user/ce/alreadyenrolled': [],
'user/!ce/enrolled': [],
'user/!ce/rejected': [],
'!user/cea/' + auto_string: [],
'!user/!cea/' + auto_string: [],
}
for student_email in student_emails:
# status: user
try:
user = User.objects.get(email=student_email)
# status: user/ce
try:
CourseEnrollment.objects.get(user=user, course_id=course_id)
status_map['user/ce/alreadyenrolled'].append(student_email)
# status: user/!ce
except CourseEnrollment.DoesNotExist:
# status: user/!ce/enrolled
try:
cenr = CourseEnrollment(user=user, course_id=course_id)
cenr.save()
status_map['user/!ce/enrolled'].append(student_email)
# status: user/!ce/rejected
except Exception:
status_map['user/!ce/rejected'].append(student_email)
# status: !user
except User.DoesNotExist:
# status: !user/cea
try:
cea = CourseEnrollmentAllowed.objects.get(course_id=course_id, email=student_email)
cea.auto_enroll = auto_enroll
cea.save()
status_map['!user/cea/' + auto_string].append(student_email)
# status: !user/!cea
except CourseEnrollmentAllowed.DoesNotExist:
cea = CourseEnrollmentAllowed(course_id=course_id, email=student_email, auto_enroll=auto_enroll)
cea.save()
status_map['!user/!cea/' + auto_string].append(student_email)
return status_map
def unenroll_emails(course_id, student_emails):
"""
Unenroll multiple students by email.
previous_state = EmailEnrollmentState(course_id, student_email)
if previous_state.user:
user = User.objects.get(email=student_email)
CourseEnrollment.objects.get_or_create(course_id=course_id, user=user)
else:
cea, _ = CourseEnrollmentAllowed.objects.get_or_create(course_id=course_id, email=student_email)
cea.auto_enroll = auto_enroll
cea.save()
`students` is a list of student emails e.g. ["foo@bar.com", "bar@foo.com]
each of whom possibly does not exist in db.
after_state = EmailEnrollmentState(course_id, student_email)
Fail quietly on student emails that do not match any users or allowed enrollments.
return previous_state, after_state
status contains the relevant prior state and action performed on the user.
ce stands for CourseEnrollment
cea stands for CourseEnrollmentAllowed
! stands for the object not existing prior to the action
return a mapping from status to emails.
"""
# NOTE these are not mutually exclusive
status_map = {
'cea/disallowed': [],
'ce/unenrolled': [],
'ce/rejected': [],
'!ce/notenrolled': [],
}
for student_email in student_emails:
# delete CourseEnrollmentAllowed
try:
cea = CourseEnrollmentAllowed.objects.get(course_id=course_id, email=student_email)
cea.delete()
status_map['cea/disallowed'].append(student_email)
except CourseEnrollmentAllowed.DoesNotExist:
pass
# delete CourseEnrollment
try:
cenr = CourseEnrollment.objects.get(course_id=course_id, user__email=student_email)
try:
cenr.delete()
status_map['ce/unenrolled'].append(student_email)
except Exception:
status_map['ce/rejected'].append(student_email)
except CourseEnrollment.DoesNotExist:
status_map['!ce/notenrolled'].append(student_email)
return status_map
def split_input_list(str_list):
def unenroll_email(course_id, student_email):
"""
Separate out individual student email from the comma, or space separated string.
Unenroll a student by email.
e.g.
in: "Lorem@ipsum.dolor, sit@amet.consectetur\nadipiscing@elit.Aenean\r convallis@at.lacus\r, ut@lacinia.Sed"
out: ['Lorem@ipsum.dolor', 'sit@amet.consectetur', 'adipiscing@elit.Aenean', 'convallis@at.lacus', 'ut@lacinia.Sed']
`student_email` is student's emails e.g. "foo@bar.com"
`str_list` is a string coming from an input text area
returns a list of separated values
returns two EmailEnrollmentState's
representing state before and after the action.
"""
new_list = re.split(r'[\n\r\s,]', str_list)
new_list = [s.strip() for s in new_list]
new_list = [s for s in new_list if s != '']
previous_state = EmailEnrollmentState(course_id, student_email)
if previous_state.enrollment:
CourseEnrollment.objects.get(course_id=course_id, user__email=student_email).delete()
if previous_state.allowed:
CourseEnrollmentAllowed.objects.get(course_id=course_id, email=student_email).delete()
after_state = EmailEnrollmentState(course_id, student_email)
return new_list
return previous_state, after_state
def reset_student_attempts(course_id, student, module_state_key, delete_module=False):
......
lms/djangoapps/instructor/tests/test_api.py 0 → 100644
"""
Unit tests for instructor.enrollment methods.
"""
from django.test import TestCase
from instructor.views.api import _split_input_list
class TestInstructorAPIHelpers(TestCase):
""" Test helpers for instructor.api """
def test_split_input_list(self):
strings = []
lists = []
strings.append("Lorem@ipsum.dolor, sit@amet.consectetur\nadipiscing@elit.Aenean\r convallis@at.lacus\r, ut@lacinia.Sed")
lists.append(['Lorem@ipsum.dolor', 'sit@amet.consectetur', 'adipiscing@elit.Aenean', 'convallis@at.lacus', 'ut@lacinia.Sed'])
for (stng, lst) in zip(strings, lists):
self.assertEqual(_split_input_list(stng), lst)
lms/djangoapps/instructor/views/api.py
......@@ -7,6 +7,7 @@ TODO add tracking
TODO a lot of these GETs should be PUTs
"""
import re
import json
from django_future.csrf import ensure_csrf_cookie
from django.views.decorators.cache import cache_control
......@@ -23,7 +24,7 @@ from django_comment_common.models import (Role,
from courseware.models import StudentModule
import instructor_task.api
import instructor.enrollment as enrollment
from instructor.enrollment import split_input_list, enroll_emails, unenroll_emails
from instructor.enrollment import enroll_email, unenroll_email
import instructor.access as access
import analytics.basic
import analytics.distributions
......@@ -40,9 +41,49 @@ def common_exceptions_400(func):
return wrapped
def require_query_params(*args, **kwargs):
"""
Checks for required paremters or renders a 400 error.
`args` is a *list of required GET parameter names.
`kwargs` is a **dict of required GET parameter names
to string explanations of the parameter
"""
required_params = []
required_params += [(arg, None) for arg in args]
required_params += [(key, kwargs[key]) for key in kwargs]
# required_params = e.g. [('action', 'enroll or unenroll'), ['emails', None]]
def decorator(func):
def wrapped(*args, **kwargs):
request = args[0]
error_response_data = {
'error': 'Missing required query parameter(s)',
'parameters': [],
'info': {},
}
for (param, extra) in required_params:
default = object()
if request.GET.get(param, default) == default:
error_response_data['parameters'] += [param]
error_response_data['info'][param] = extra
if len(error_response_data['parameters']) > 0:
return HttpResponseBadRequest(
json.dumps(error_response_data),
mimetype="application/json",
)
else:
return func(*args, **kwargs)
return wrapped
return decorator
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
def students_update_enrollment_email(request, course_id):
@require_query_params(action="enroll or unenroll", emails="stringified list of emails")
def students_update_enrollment(request, course_id):
"""
Enroll or unenroll students by email.
Requires staff access.
......@@ -57,15 +98,36 @@ def students_update_enrollment_email(request, course_id):
)
action = request.GET.get('action')
emails = split_input_list(request.GET.get('emails'))
emails = _split_input_list(request.GET.get('emails'))
auto_enroll = request.GET.get('auto_enroll') in ['true', 'True', True]
def format_result(func, email):
""" Act on a single email and format response or errors. """
try:
before, after = func()
return {
'email': email,
'before': before.to_dict(),
'after': after.to_dict(),
}
except Exception:
return {
'email': email,
'error': True,
}
if action == 'enroll':
results = enroll_emails(course_id, emails, auto_enroll=auto_enroll)
results = [format_result(
lambda: enroll_email(course_id, email, auto_enroll),
email
) for email in emails]
elif action == 'unenroll':
results = unenroll_emails(course_id, emails)
results = [format_result(
lambda: unenroll_email(course_id, email),
email
) for email in emails]
else:
raise ValueError("unrecognized action '{}'".format(action))
return HttpResponseBadRequest("Unrecognized action '{}'".format(action))
response_payload = {
'action': action,
......@@ -81,6 +143,11 @@ def students_update_enrollment_email(request, course_id):
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@common_exceptions_400
@require_query_params(
email="user email",
rolename="'instructor', 'staff', or 'beta'",
mode="'allow' or 'revoke'"
)
def access_allow_revoke(request, course_id):
"""
Modify staff/instructor access.
......@@ -119,6 +186,7 @@ def access_allow_revoke(request, course_id):
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@require_query_params(rolename="'instructor', 'staff', or 'beta'")
def list_course_role_members(request, course_id):
"""
List instructors and staff.
......@@ -306,6 +374,7 @@ def get_student_progress_url(request, course_id):
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@common_exceptions_400
@require_query_params(student_email="student email")
def redirect_to_student_progress(request, course_id):
"""
Redirects to the specified students progress page
......@@ -318,9 +387,6 @@ def redirect_to_student_progress(request, course_id):
)
student_email = request.GET.get('student_email')
if not student_email:
return HttpResponseBadRequest("Must provide an email.")
user = User.objects.get(email=student_email)
progress_url = reverse('student_progress', kwargs={'course_id': course_id, 'student_id': user.id})
......@@ -363,7 +429,7 @@ def reset_student_attempts(request, course_id):
if will_delete_module and all_students:
return HttpResponseBadRequest()
module_state_key = _module_state_key_from_problem_urlname(course_id, problem_to_reset)
module_state_key = _msk_from_problem_urlname(course_id, problem_to_reset)
response_payload = {}
response_payload['problem_to_reset'] = problem_to_reset
......@@ -412,7 +478,7 @@ def rescore_problem(request, course_id):
if not (problem_to_reset and (all_students or student_email)):
return HttpResponseBadRequest()
module_state_key = _module_state_key_from_problem_urlname(course_id, problem_to_reset)
module_state_key = _msk_from_problem_urlname(course_id, problem_to_reset)
response_payload = {}
response_payload['problem_to_reset'] = problem_to_reset
......@@ -456,7 +522,7 @@ def list_instructor_tasks(request, course_id):
return HttpResponseBadRequest()
if problem_urlname:
module_state_key = _module_state_key_from_problem_urlname(course_id, problem_urlname)
module_state_key = _msk_from_problem_urlname(course_id, problem_urlname)
if student_email:
student = User.objects.get(email=student_email)
tasks = instructor_task.api.get_instructor_task_history(course_id, module_state_key, student)
......@@ -540,7 +606,7 @@ def update_forum_role_membership(request, course_id):
rolename = request.GET.get('rolename')
mode = request.GET.get('mode')
if not rolename in [access.FORUM_ROLE_ADMINISTRATOR, access.FORUM_ROLE_MODERATOR, access.FORUM_ROLE_COMMUNITY_TA]:
if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]:
return HttpResponseBadRequest()
try:
......@@ -560,7 +626,30 @@ def update_forum_role_membership(request, course_id):
return response
def _module_state_key_from_problem_urlname(course_id, urlname):
def _split_input_list(str_list):
"""
Separate out individual student email from the comma, or space separated string.
e.g.
in: "Lorem@ipsum.dolor, sit@amet.consectetur\nadipiscing@elit.Aenean\r convallis@at.lacus\r, ut@lacinia.Sed"
out: ['Lorem@ipsum.dolor', 'sit@amet.consectetur', 'adipiscing@elit.Aenean', 'convallis@at.lacus', 'ut@lacinia.Sed']
`str_list` is a string coming from an input text area
returns a list of separated values
"""
new_list = re.split(r'[\n\r\s,]', str_list)
new_list = [s.strip() for s in new_list]
new_list = [s for s in new_list if s != '']
return new_list
def _msk_from_problem_urlname(course_id, urlname):
"""
Convert a 'problem urlname' (instructor input name)
to a module state key (db field)
"""
if urlname[-4:] == ".xml":
urlname = urlname[:-4]
......
lms/djangoapps/instructor/views/instructor_dashboard.py
......@@ -105,8 +105,8 @@ def _section_membership(course_id, access):
'section_key': 'membership',
'section_display_name': 'Membership',
'access': access,
'enroll_button_url': reverse('students_update_enrollment_email', kwargs={'course_id': course_id}),
'unenroll_button_url': reverse('students_update_enrollment_email', kwargs={'course_id': course_id}),
'enroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}),
'unenroll_button_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}),
'list_course_role_members_url': reverse('list_course_role_members', kwargs={'course_id': course_id}),
'access_allow_revoke_url': reverse('access_allow_revoke', kwargs={'course_id': course_id}),
'list_forum_members_url': reverse('list_forum_members', kwargs={'course_id': course_id}),
......@@ -121,7 +121,7 @@ def _section_student_admin(course_id):
'section_key': 'student_admin',
'section_display_name': 'Student Admin',
'get_student_progress_url': reverse('get_student_progress_url', kwargs={'course_id': course_id}),
'enrollment_url': reverse('students_update_enrollment_email', kwargs={'course_id': course_id}),
'enrollment_url': reverse('students_update_enrollment', kwargs={'course_id': course_id}),
'reset_student_attempts_url': reverse('reset_student_attempts', kwargs={'course_id': course_id}),
'rescore_problem_url': reverse('rescore_problem', kwargs={'course_id': course_id}),
'list_instructor_tasks_url': reverse('list_instructor_tasks', kwargs={'course_id': course_id}),
......
lms/static/coffee/src/instructor_dashboard/membership.coffee
......@@ -60,65 +60,100 @@ class BatchEnrollment
$task_response.empty()
$request_response_error.empty()
response_code_dict = _.extend {}, data_from_server.results
# response_code_dict e.g. {'code': ['email1', 'email2'], ...}
message_ordering = [
'msg_error_enroll'
'msg_error_unenroll'
'msg_enrolled'
'msg_unenrolled'
'msg_willautoenroll'
'msg_allowed'
'msg_disallowed'
'msg_already_enrolled'
'msg_notenrolled'
]
# these results arrays contain student_results
# only populated arrays will be rendered
#
# students for which there was an error during the action
errors = []
# students who are now enrolled in the course
enrolled = []
# students who are now allowed to enroll in the course
allowed = []
# students who will be autoenrolled on registration
autoenrolled = []
# students who are now not enrolled in the course
notenrolled = []
# categorize student results into the above arrays.
for student_results in data_from_server.results
# for a successful action.
# student_results is of the form {
# "email": "jd405@edx.org",
# "before": {
# "enrollment": true,
# "auto_enroll": false,
# "user": true,
# "allowed": false
# }
# "after": {
# "enrollment": true,
# "auto_enroll": false,
# "user": true,
# "allowed": false
# },
# }
#
# for an action error.
# student_results is of the form {
# 'email': email,
# 'error': True,
# }
if student_results.error != undefined
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
else if not student_results.after.enrollment
notenrolled.push student_results
else
console.warn 'student results not reported to user'
console.warn student_results
msg_to_txt = {
msg_already_enrolled: "Already enrolled:"
msg_enrolled: "Enrolled:"
msg_error_enroll: "There was an error enrolling these students:"
msg_allowed: "These students will be allowed to enroll once they register:"
msg_willautoenroll: "These students will be enrolled once they register:"
msg_unenrolled: "Unenrolled:"
msg_error_unenroll: "There was an error unenrolling these students:"
msg_disallowed: "These students were removed from those who can enroll once they register:"
msg_notenrolled: "These students were not enrolled:"
}
msg_to_codes = {
msg_already_enrolled: ['user/ce/alreadyenrolled']
msg_enrolled: ['user/!ce/enrolled']
msg_error_enroll: ['user/!ce/rejected']
msg_allowed: ['!user/cea/allowed', '!user/!cea/allowed']
msg_willautoenroll: ['!user/cea/willautoenroll', '!user/!cea/willautoenroll']
msg_unenrolled: ['ce/unenrolled']
msg_error_unenroll: ['ce/rejected']
msg_disallowed: ['cea/disallowed']
msg_notenrolled: ['!ce/notenrolled']
}
for msg_symbol in message_ordering
# $task_response.text JSON.stringify(data)
msg_txt = msg_to_txt[msg_symbol]
# render populated result arrays
render_list = (label, emails) ->
log emails
task_res_section = $ '<div/>', class: 'request-res-section'
task_res_section.append $ '<h3/>', text: msg_txt
task_res_section.append $ '<h3/>', text: label
email_list = $ '<ul/>'
task_res_section.append email_list
will_attach = false
for code in msg_to_codes[msg_symbol]
emails = response_code_dict[code]
for email in emails
email_list.append $ '<li/>', text: email
if emails and emails.length
for email in emails
email_list.append $ '<li/>', text: email
will_attach = true
$task_response.append task_res_section
if will_attach
$task_response.append task_res_section
else
task_res_section.remove()
if errors.length
errors_label = do ->
if data_from_server.action is 'enroll'
"There was an error enrolling:"
else if data_from_server.action is 'unenroll'
"There was an error unenrolling:"
else
console.warn "unknown action from server '#{data_from_server.action}'"
"There was an error processing:"
for student_results in errors
console.log 'error with': student_results.email
if enrolled.length
render_list "Students Enrolled:", (sr.email for sr in enrolled)
if allowed.length
render_list "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:",
(sr.email for sr in autoenrolled)
if notenrolled.length
render_list "These students are now not enrolled:",
(sr.email for sr in notenrolled)
# manages a list of instructors or staff and the control of their access.
......
lms/static/coffee/src/instructor_dashboard/student_admin.coffee
......@@ -128,7 +128,6 @@ class StudentAdmin
send_data =
action: 'unenroll'
emails: @$field_student_select.val()
auto_enroll: false
$.ajax
dataType: 'json'
......
lms/urls.py
......@@ -346,8 +346,8 @@ if settings.COURSEWARE_ENABLED and settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR
'instructor.views.instructor_dashboard.instructor_dashboard_2', name="instructor_dashboard_2"),
# api endpoints for instructor
url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/instructor_dashboard/api/students_update_enrollment_email$',
'instructor.views.api.students_update_enrollment_email', name="students_update_enrollment_email"),
url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/instructor_dashboard/api/students_update_enrollment$',
'instructor.views.api.students_update_enrollment', name="students_update_enrollment"),
url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/instructor_dashboard/api/list_course_role_members$',
'instructor.views.api.list_course_role_members', name="list_course_role_members"),
url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/instructor_dashboard/api/access_allow_revoke$',
......
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