Skip to content
Snippets Groups Projects
Commit 605f080b authored by Tomasz Gargas's avatar Tomasz Gargas
Browse files

Simplify the content enrollment tab in the CCX Dashboard. 


Signed-off-by: default avatarTomasz Gargas <tomasz@opencraft.com>
parent 841ddacc
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 72 additions and 202 deletions
common/test/acceptance/pages/lms/ccx_dashboard_page.py
+ 1
− 1
View file @ 605f080b
......@@ -23,7 +23,7 @@ class CoachDashboardPage(CoursePage):
"""
check if enrollment page in ccx dashboard is open.
"""
return self.q(css='div.batch-enrollment').present
return self.q(css='div.batch-enrollment-ccx').present
def fill_ccx_name_text_box(self, ccx_name):
"""
......
lms/djangoapps/ccx/tests/test_views.py
+ 15
− 91
View file @ 605f080b
......@@ -594,10 +594,8 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
self.assertEqual(response.status_code, 200)
@ddt.data(
('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Enroll')),
('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll')),
('ccx_manage_student', True, 1, 'student-id', ('student-action', 'add')),
('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add')),
('ccx-manage-students', True, 1, 'student-ids', ('enrollment-button', 'Enroll')),
('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Enroll')),
)
@ddt.unpack
def test_enroll_member_student(self, view_name, send_email, outbox_count, student_form_input_name, button_tuple):
......@@ -658,7 +656,7 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
]
url = reverse(
'ccx_invite',
'ccx-manage-students',
kwargs={'course_id': ccx_course_key}
)
data = {
......@@ -689,81 +687,11 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[5]).exists()
)
def test_manage_student_enrollment_limit(self):
"""
Enroll students up to the enrollment limit.
This test is specific to one of the enrollment views: the reason is because
the view used in this test cannot perform bulk enrollments.
"""
students_limit = 1
self.make_coach()
staff = self.make_staff()
ccx = self.make_ccx(max_students_allowed=students_limit)
ccx_course_key = CCXLocator.from_course_locator(self.course.id, ccx.id)
students = [
UserFactory.create(is_staff=False) for _ in range(2)
]
url = reverse(
'ccx_manage_student',
kwargs={'course_id': CCXLocator.from_course_locator(self.course.id, ccx.id)}
)
# enroll the first student
data = {
'student-action': 'add',
'student-id': students[0].email,
}
response = self.client.post(url, data=data, follow=True)
self.assertEqual(response.status_code, 200)
# a CcxMembership exists for this student
self.assertTrue(
CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[0]).exists()
)
# try to enroll the second student without success
# enroll the first student
data = {
'student-action': 'add',
'student-id': students[1].email,
}
response = self.client.post(url, data=data, follow=True)
self.assertEqual(response.status_code, 200)
# a CcxMembership does not exist for this student
self.assertFalse(
CourseEnrollment.objects.filter(course_id=ccx_course_key, user=students[1]).exists()
)
error_message = 'The course is full: the limit is {students_limit}'.format(
students_limit=students_limit
)
self.assertContains(response, error_message, status_code=200)
# try to enroll the 3rd student which is staff
data = {
'student-action': 'add',
'student-id': staff.email,
}
response = self.client.post(url, data=data, follow=True)
self.assertEqual(response.status_code, 200)
# staff gets enroll
self.assertTrue(
CourseEnrollment.objects.filter(course_id=ccx_course_key, user=staff).exists()
)
self.assertEqual(CourseEnrollment.objects.num_enrolled_in_exclude_admins(ccx_course_key), 1)
# asert that number of enroll is still 0 because staff and instructor do not count.
CourseEnrollment.enroll(staff, self.course.id)
self.assertEqual(CourseEnrollment.objects.num_enrolled_in_exclude_admins(self.course.id), 0)
# assert that handles wrong ccx id code
ccx_course_key_fake = CCXLocator.from_course_locator(self.course.id, 55)
self.assertEqual(CourseEnrollment.objects.num_enrolled_in_exclude_admins(ccx_course_key_fake), 0)
@ddt.data(
('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Unenroll')),
('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll')),
('ccx_manage_student', True, 1, 'student-id', ('student-action', 'revoke')),
('ccx_manage_student', False, 0, 'student-id', ('student-action', 'revoke')),
('ccx-manage-students', True, 1, 'student-ids', ('enrollment-button', 'Unenroll')),
('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Unenroll')),
('ccx-manage-students', True, 1, 'student-id', ('student-action', 'revoke')),
('ccx-manage-students', False, 0, 'student-id', ('student-action', 'revoke')),
)
@ddt.unpack
def test_unenroll_member_student(self, view_name, send_email, outbox_count, student_form_input_name, button_tuple):
......@@ -805,14 +733,10 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
)
@ddt.data(
('ccx_invite', True, 1, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody@nowhere.com'),
('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody@nowhere.com'),
('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
('ccx_manage_student', True, 0, 'student-id', ('student-action', 'add'), 'dummy_student_id'),
('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add'), 'dummy_student_id'),
('ccx_manage_student', True, 1, 'student-id', ('student-action', 'add'), 'xyz@gmail.com'),
('ccx_manage_student', False, 0, 'student-id', ('student-action', 'add'), 'xyz@gmail.com'),
('ccx-manage-students', True, 1, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody@nowhere.com'),
('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody@nowhere.com'),
('ccx-manage-students', True, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Enroll'), 'nobody'),
)
@ddt.unpack
def test_enroll_non_user_student(
......@@ -862,10 +786,10 @@ class TestCoachDashboard(CcxTestCase, LoginEnrollmentTestCase):
)
@ddt.data(
('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody@nowhere.com'),
('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody@nowhere.com'),
('ccx_invite', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
('ccx_invite', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
('ccx-manage-students', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody@nowhere.com'),
('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody@nowhere.com'),
('ccx-manage-students', True, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
('ccx-manage-students', False, 0, 'student-ids', ('enrollment-button', 'Unenroll'), 'nobody'),
)
@ddt.unpack
def test_unenroll_non_user_student(
......
lms/djangoapps/ccx/urls.py
+ 1
− 2
View file @ 605f080b
......@@ -9,9 +9,8 @@ urlpatterns = [
url(r'^ccx_coach$', ccx.views.dashboard, name='ccx_coach_dashboard'),
url(r'^create_ccx$', ccx.views.create_ccx, name='create_ccx'),
url(r'^save_ccx$', ccx.views.save_ccx, name='save_ccx'),
url(r'^ccx_invite$', ccx.views.ccx_invite, name='ccx_invite'),
url(r'^ccx_schedule$', ccx.views.ccx_schedule, name='ccx_schedule'),
url(r'^ccx_manage_student$', ccx.views.ccx_student_management, name='ccx_manage_student'),
url(r'^ccx-manage-students$', ccx.views.ccx_students_management, name='ccx-manage-students'),
# Grade book
url(r'^ccx_gradebook$', ccx.views.ccx_gradebook, name='ccx_gradebook'),
......
lms/djangoapps/ccx/utils.py
+ 26
− 9
View file @ 605f080b
......@@ -21,12 +21,14 @@ from lms.djangoapps.ccx.models import CustomCourseForEdX
from lms.djangoapps.ccx.overrides import get_override_for_ccx
from lms.djangoapps.instructor.access import allow_access, list_with_level, revoke_access
from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params, unenroll_email
from lms.djangoapps.instructor.views.api import _split_input_list
from lms.djangoapps.instructor.views.tools import get_student_from_identifier
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.content.course_structures.models import CourseStructure
from student.models import CourseEnrollment, CourseEnrollmentException
from student.roles import CourseCcxCoachRole, CourseInstructorRole, CourseStaffRole
log = logging.getLogger("edx.ccx")
......@@ -96,6 +98,27 @@ def get_date(ccx, node, date_type=None, parent_node=None):
return date
def get_enrollment_action_and_identifiers(request):
"""
Extracts action type and student identifiers from the request
on Enrollment tab of CCX Dashboard.
"""
action, identifiers = None, None
student_action = request.POST.get('student-action', None)
batch_action = request.POST.get('enrollment-button', None)
if student_action:
action = student_action
student_id = request.POST.get('student-id', '')
identifiers = [student_id]
elif batch_action:
action = batch_action
identifiers_raw = request.POST.get('student-ids')
identifiers = _split_input_list(identifiers_raw)
return action, identifiers
def validate_date(year, month, day, hour, minute):
"""
avoid corrupting db if bad dates come in
......@@ -208,16 +231,10 @@ def get_valid_student_with_email(identifier):
def ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, coach):
"""
Function to enroll/add or unenroll/revoke students.
This function exists for backwards compatibility: in CCX there are
two different views to manage students that used to implement
a different logic. Now the logic has been reconciled at the point that
this function can be used by both.
The two different views can be merged after some UI refactoring.
Function to enroll or unenroll/revoke students.
Arguments:
action (str): type of action to perform (add, Enroll, revoke, Unenroll)
action (str): type of action to perform (Enroll, Unenroll/revoke)
identifiers (list): list of students username/email
email_students (bool): Flag to send an email to students
course_key (CCXLocator): a CCX course key
......@@ -229,7 +246,7 @@ def ccx_students_enrolling_center(action, identifiers, email_students, course_ke
"""
errors = []
if action == 'Enroll' or action == 'add':
if action == 'Enroll':
ccx_course_overview = CourseOverview.get_from_id(course_key)
course_locator = course_key.to_course_locator()
staff = CourseStaffRole(course_locator).users_with_role()
......
lms/djangoapps/ccx/views.py
+ 4
− 29
View file @ 605f080b
......@@ -45,11 +45,11 @@ from lms.djangoapps.ccx.utils import (
get_ccx_creation_dict,
get_ccx_for_coach,
get_date,
get_enrollment_action_and_identifiers,
parse_date,
)
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params
from lms.djangoapps.instructor.views.api import _split_input_list
from lms.djangoapps.instructor.views.gradebook_api import get_grade_book_page
from student.models import CourseEnrollment
from student.roles import CourseCcxCoachRole
......@@ -459,43 +459,18 @@ def ccx_schedule(request, course, ccx=None): # pylint: disable=unused-argument
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@coach_dashboard
def ccx_invite(request, course, ccx=None):
def ccx_students_management(request, course, ccx=None):
"""
Invite users to new ccx
Manage the enrollment of the students in a CCX
"""
if not ccx:
raise Http404
action = request.POST.get('enrollment-button')
identifiers_raw = request.POST.get('student-ids')
identifiers = _split_input_list(identifiers_raw)
action, identifiers = get_enrollment_action_and_identifiers(request)
email_students = 'email-students' in request.POST
course_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name)
ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, ccx.coach)
url = reverse('ccx_coach_dashboard', kwargs={'course_id': course_key})
return redirect(url)
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@coach_dashboard
def ccx_student_management(request, course, ccx=None):
"""
Manage the enrollment of individual students in a CCX
"""
if not ccx:
raise Http404
action = request.POST.get('student-action', None)
student_id = request.POST.get('student-id', '')
email_students = 'email-students' in request.POST
identifiers = [student_id]
course_key = CCXLocator.from_course_locator(course.id, unicode(ccx.id))
email_params = get_email_params(course, auto_enroll=True, course_key=course_key, display_name=ccx.display_name)
errors = ccx_students_enrolling_center(action, identifiers, email_students, course_key, email_params, ccx.coach)
for error_message in errors:
......
lms/static/sass/course/instructor/_instructor_2.scss
+ 16
− 0
View file @ 605f080b
......@@ -635,6 +635,16 @@
}
}
}
.batch-enrollment-ccx{
@extend .batch-enrollment;
float: left;
width: 100%;
textarea {
width: 100%;
}
}
// Auto Enroll Csv Section
.auto_enroll_csv {
.enrollment_signup_button {
......@@ -806,6 +816,12 @@
}
}
}
.member-lists-management-ccx {
@extend .member-lists-management;
float: left;
width: 100%;
}
}
......
lms/templates/ccx/coach_dashboard.html
+ 2
− 24
View file @ 605f080b
......@@ -112,34 +112,12 @@ from openedx.core.djangolib.js_utils import (
function setup_management_form() {
$(".member-lists-management form").on("submit", function (event) {
var target, action;
target = $(event.target);
if (target.serialize().indexOf('student-action') < 0) {
action = $('<input />', {
type: 'hidden',
name: 'student-action',
value: 'add'
});
target.append(action);
}
});
$(".member-lists-management form .add, .member-lists-management form .revoke").on("click", function(event) {
$(".member-lists-management-ccx form .revoke").on("click", function(event) {
var target, form, action, studentId, selectedStudent;
event.preventDefault();
target = $(event.target);
form = target.parents('form').first();
if (target.hasClass('add')) {
// adding a new student, add the student-action input and submit
action = $('<input />', {
type: 'hidden',
name: 'student-action',
// this is untenable, tied to a translated value. Fix it.
value: 'add'
});
form.append(action).submit();
} else if (target.hasClass('revoke')) {
if (target.hasClass('revoke')) {
// revoking access for a student, get set form values and submit
// get the email address of the student, since they might not be 'enrolled' yet.
selectedStudent = target.parent('td').siblings().last().text();
......
lms/templates/ccx/enrollment.html
+ 7
− 46
View file @ 605f080b
......@@ -4,14 +4,14 @@ from django.utils.translation import ugettext as _
from openedx.core.djangolib.markup import HTML, Text
%>
<h2 class="hd hd-2">${_("Batch Enrollment")}</h2>
<div class="batch-enrollment" style="float:left;width:50%">
<form method="POST" action="ccx_invite">
<h2 class="hd hd-2">${_("Enrollment")}</h2>
<div class="batch-enrollment-ccx">
<form method="POST" action="ccx-manage-students">
<input type="hidden" name="csrfmiddlewaretoken" value="${ csrf_token }">
<label for="student-ids" class="sr">${_("Email Addresses/Usernames")}</label>
<p id="label_student_ids" class="text-helper">
${_("Enter email addresses and/or usernames separated by new lines or commas.")}
${_("You will not get notification for emails that bounce, so please double-check spelling.")}
${_("Enter one or more email addresses or usernames separated by new lines or commas.")}
${_("Make sure you enter the information carefully. You will not receive notification for invalid usernames or email addresses.")}
</p>
<textarea rows="6" name="student-ids" id="student-ids" aria-describedby="label_student_ids" placeholder="${_("Email Addresses/Usernames")}" spellcheck="false"></textarea>
......@@ -61,8 +61,8 @@ from openedx.core.djangolib.markup import HTML, Text
</form>
</div>
<div class="member-lists-management" style="float:left;width:50%">
<form method="POST" action="ccx_manage_student" class="ccx-manage-student-form">
<div class="member-lists-management-ccx">
<form method="POST" action="ccx-manage-students" class="ccx-manage-student-form">
<input type="hidden" name="csrfmiddlewaretoken" value="${ csrf_token }">
<div class="auth-list-container active">
<div class="member-list-widget">
......@@ -95,45 +95,6 @@ from openedx.core.djangolib.markup import HTML, Text
</tbody>
</table>
</div>
<div class="bottom-bar">
<label for="student-id" class="sr">${_("Enter username or email")}</label>
<input name="student-id" id="student-id" class="add-field" placeholder="${_("Enter username or email")}" type="text">
<input name="student-action" class="add" value="${_("Add Student")}" type="button">
<div class="enroll-option">
<input type="checkbox" name="auto-enroll" id="auto-enroll" value="Auto-Enroll" checked="yes" aria-describedby="auto-enroll-helper" disabled>
<label style="display:inline" for="auto-enroll">${_("Auto Enroll")}</label>
<div class="hint auto-enroll-hint">
<span class="hint-caret"></span>
<p class="text-helper" id="auto-enroll-helper">
${Text(_("If this option is {em_start}checked{em_end}, users who have not yet registered for {platform_name} will be automatically enrolled.")).format(
em_start=HTML('<em>'),
em_end=HTML('</em>'),
platform_name=settings.PLATFORM_NAME,
)}
${Text(_("If this option is left {em_start}unchecked{em_end}, users who have not yet registered for {platform_name} will not be enrolled, but will be allowed to enroll once they make an account.")).format(
em_start=HTML('<em>'),
em_end=HTML('</em>'),
platform_name=settings.PLATFORM_NAME,
)}
<br /><br />
${_("Checking this box has no effect if 'Revoke' is clicked.")}
</p>
</div>
</div>
<div class="enroll-option ccx-notify-user">
<input type="checkbox" name="email-students" id="email-students" value="Notify-students-by-email" checked="yes" aria-describedby="email-students-helper">
<label style="display:inline" for="email-students">${_("Notify users by email")}</label>
<div class="hint email-students-hint">
<span class="hint-caret"></span>
<p class="text-helper" id="email-students-helper">
${Text(_("If this option is {em_start}checked{em_end}, users will receive an email notification.")).format(
em_start=HTML('<em>'),
em_end=HTML('</em>'),
)}
</p>
</div>
</div>
</div>
</div>
</div>
</form>
......
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