diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 701cce0e6c..dd46e5a055 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -80,6 +80,15 @@ def get_cohort_by_name(course_id, name): group_type=CourseUserGroup.COHORT, name=name) +def get_cohort_by_id(course_id, cohort_id): + """ + Return the CourseUserGroup object for the given cohort. Raises DoesNotExist + it isn't present. Uses the course_id for extra validation... + """ + return CourseUserGroup.objects.get(course_id=course_id, + group_type=CourseUserGroup.COHORT, + id=cohort_id) + def add_cohort(course_id, name): """ Add a cohort to a course. Raises ValueError if a cohort of the same name already @@ -95,6 +104,34 @@ def add_cohort(course_id, name): group_type=CourseUserGroup.COHORT, name=name) +def add_user_to_cohort(cohort, username_or_email): + """ + Look up the given user, and if successful, add them to the specified cohort. + + Arguments: + cohort: CourseUserGroup + username_or_email: string. Treated as email if has '@' + + Returns: + User object. + + Raises: + User.DoesNotExist if can't find user. + + ValueError if user already present. + """ + if '@' in username_or_email: + user = User.objects.get(email=username_or_email) + else: + user = User.objects.get(username=username_or_email) + + if cohort.users.filter(id=user.id).exists(): + raise ValueError("User {0} already present".format(user.username)) + + cohort.users.add(user) + return user + + def get_course_cohort_names(course_id): """ Return a list of the cohort names in a course. diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py index 9ee9935c3e..f02bff2d00 100644 --- a/common/djangoapps/course_groups/views.py +++ b/common/djangoapps/course_groups/views.py @@ -1,7 +1,9 @@ import json from django_future.csrf import ensure_csrf_cookie from django.contrib.auth.decorators import login_required +from django.contrib.auth.models import User from django.core.context_processors import csrf +from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.urlresolvers import reverse from django.http import HttpResponse, HttpResponseForbidden, Http404 from django.shortcuts import redirect @@ -9,6 +11,8 @@ import logging from courseware.courses import get_course_with_access from mitxmako.shortcuts import render_to_response, render_to_string +from string_util import split_by_comma_and_whitespace + from .models import CourseUserGroup from . import models @@ -81,19 +85,84 @@ def add_cohort(request, course_id): @ensure_csrf_cookie def users_in_cohort(request, course_id, cohort_id): """ + Return users in the cohort. Show up to 100 per page, and page + using the 'page' GET attribute in the call. Format: + + Returns: + Json dump of dictionary in the following format: + {'success': True, + 'page': page, + 'num_pages': paginator.num_pages, + 'users': [{'username': ..., 'email': ..., 'name': ...}] + } """ get_course_with_access(request.user, course_id, 'staff') - return JsonHttpReponse({'error': 'Not implemented'}) + cohort = models.get_cohort_by_id(course_id, int(cohort_id)) + + paginator = Paginator(cohort.users.all(), 100) + page = request.GET.get('page') + try: + users = paginator.page(page) + except PageNotAnInteger: + # return the first page + page = 1 + users = paginator.page(page) + except EmptyPage: + # Page is out of range. Return last page + page = paginator.num_pages + contacts = paginator.page(page) + + user_info = [{'username': u.username, + 'email': u.email, + 'name': '{0} {1}'.format(u.first_name, u.last_name)} + for u in users] + + return JsonHttpReponse({'success': True, + 'page': page, + 'num_pages': paginator.num_pages, + 'users': user_info}) @ensure_csrf_cookie -def add_users_to_cohort(request, course_id): +def add_users_to_cohort(request, course_id, cohort_id): """ + Return json dict of: + + {'success': True, + 'added': [{'username': username, + 'name': name, + 'email': email}, ...], + 'present': [str1, str2, ...], # already there + 'unknown': [str1, str2, ...]} """ get_course_with_access(request.user, course_id, 'staff') - return JsonHttpReponse({'error': 'Not implemented'}) + if request.method != "POST": + raise Http404("Must POST to add users to cohorts") + + cohort = models.get_cohort_by_id(course_id, cohort_id) + + users = request.POST.get('users', '') + added = [] + present = [] + unknown = [] + for username_or_email in split_by_comma_and_whitespace(users): + try: + user = models.add_user_to_cohort(cohort, username_or_email) + added.append({'username': user.username, + 'name': "{0} {1}".format(user.first_name, user.last_name), + 'email': user.email, + }) + except ValueError: + present.append(username_or_email) + except User.DoesNotExist: + unknown.append(username_or_email) + + return JsonHttpReponse({'success': True, + 'added': added, + 'present': present, + 'unknown': unknown}) def debug_cohort_mgmt(request, course_id): @@ -102,7 +171,7 @@ def debug_cohort_mgmt(request, course_id): """ # add staff check to make sure it's safe if it's accidentally deployed. get_course_with_access(request.user, course_id, 'staff') - + context = {'cohorts_ajax_url': reverse('cohorts', kwargs={'course_id': course_id})} return render_to_response('/course_groups/debug.html', context) diff --git a/common/lib/string_util.py b/common/lib/string_util.py new file mode 100644 index 0000000000..0db385f2d6 --- /dev/null +++ b/common/lib/string_util.py @@ -0,0 +1,11 @@ +import itertools + +def split_by_comma_and_whitespace(s): + """ + Split a string both by on commas and whitespice. + """ + # Note: split() with no args removes empty strings from output + lists = [x.split() for x in s.split(',')] + # return all of them + return itertools.chain(*lists) + diff --git a/common/static/js/course_groups/cohorts.js b/common/static/js/course_groups/cohorts.js index 7b1793dcf8..531ce51923 100644 --- a/common/static/js/course_groups/cohorts.js +++ b/common/static/js/course_groups/cohorts.js @@ -36,19 +36,51 @@ var CohortManager = (function ($) { // constructor var module = function () { - var url = $(".cohort_manager").data('ajax_url'); + var el = $(".cohort_manager"); + // localized jquery + var $$ = function (selector) { + return $(selector, el) + } + var state_init = "init"; + var state_summary = "summary"; + var state_detail = "detail"; + var state = state_init; + + var url = el.data('ajax_url'); var self = this; - var error_list = $(".cohort_errors"); - var cohort_list = $(".cohort_list"); - var cohorts_display = $(".cohorts_display"); - var show_cohorts_button = $(".cohort_controls .show_cohorts"); - var add_cohort_input = $("#cohort-name"); - var add_cohort_button = $(".add_cohort"); + + // Pull out the relevant parts of the html + // global stuff + var errors = $$(".errors"); + + // cohort summary display + var summary = $$(".summary"); + var cohorts = $$(".cohorts"); + var show_cohorts_button = $$(".controls .show_cohorts"); + var add_cohort_input = $$(".cohort_name"); + var add_cohort_button = $$(".add_cohort"); + + // single cohort user display + var detail = $$(".detail"); + var detail_header = $(".header", detail); + var detail_users = $$(".users"); + var detail_page_num = $$(".page_num"); + var users_area = $$(".users_area"); + var add_members_button = $$(".add_members"); + var op_results = $$("op_results"); + var cohort_title = null; + var detail_url = null; + var page = null; + + // *********** Summary view methods function show_cohort(item) { // item is a li that has a data-href link to the cohort base url var el = $(this); - alert("would show you data about " + el.text() + " from " + el.data('href')); + cohort_title = el.text(); + detail_url = el.data('href'); + state = state_detail; + render(); } function add_to_cohorts_list(item) { @@ -57,24 +89,25 @@ var CohortManager = (function ($) { .data('href', url + '/' + item.id) .addClass('link') .click(show_cohort); - cohort_list.append(li); + cohorts.append(li); }; function log_error(msg) { - error_list.empty(); - error_list.append($("
  • ").text(msg).addClass("error")); + errors.empty(); + errors.append($("
  • ").text(msg).addClass("error")); }; function load_cohorts(response) { - cohort_list.empty(); + cohorts.empty(); if (response && response.success) { response.cohorts.forEach(add_to_cohorts_list); } else { log_error(response.msg || "There was an error loading cohorts"); } - cohorts_display.show(); + summary.show(); }; + function added_cohort(response) { if (response && response.success) { add_to_cohorts_list(response.cohort); @@ -83,8 +116,75 @@ var CohortManager = (function ($) { } } + // *********** Detail view methods + + function add_to_users_list(item) { + var tr = $('' + + ''); + $(".name", tr).text(item.name); + $(".username", tr).text(item.username); + $(".email", tr).text(item.email); + detail_users.append(tr); + }; + + + function show_users(response) { + detail_users.html("NameUsernameEmail"); + if (response && response.success) { + response.users.forEach(add_to_users_list); + detail_page_num.text("Page " + response.page + " of " + response.num_pages); + } else { + log_error(response.msg || + "There was an error loading users for " + cohort.title); + } + detail.show(); + } + + + function added_users(response) { + function adder(note, color) { + return function(item) { + var li = $('
  • ') + li.text(note + ' ' + item.name + ', ' + item.username + ', ' + item.email); + li.css('color', color); + op_results.append(li); + } + } + if (response && response.success) { + response.added.forEach(adder("Added", "green")); + response.present.forEach(adder("Already present:", "black")); + response.unknown.forEach(adder("Already present:", "red")); + } else { + log_error(response.msg || "There was an error adding users"); + } + } + + // ******* Rendering + + + function render() { + // Load and render the right thing based on the state + + // start with both divs hidden + summary.hide(); + detail.hide(); + // and clear out the errors + errors.empty(); + if (state == state_summary) { + $.ajax(url).done(load_cohorts).fail(function() { + log_error("Error trying to load cohorts"); + }); + } else if (state == state_detail) { + detail_header.text("Members of " + cohort_title); + $.ajax(detail_url).done(show_users).fail(function() { + log_error("Error trying to load users in cohort"); + }); + } + } + show_cohorts_button.click(function() { - $.ajax(url).done(load_cohorts); + state = state_summary; + render(); }); add_cohort_input.change(function() { @@ -101,6 +201,13 @@ var CohortManager = (function ($) { $.post(add_url, data).done(added_cohort); }); + add_members_button.click(function() { + var add_url = detail_url + '/add'; + data = {'users': users_area.val()} + $.post(add_url, data).done(added_users); + }); + + }; // prototype diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 8069d3f184..1b51698834 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -24,14 +24,15 @@ from courseware import grades from courseware.access import (has_access, get_access_group_name, course_beta_test_group_name) from courseware.courses import get_course_with_access +from courseware.models import StudentModule from django_comment_client.models import (Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA) from django_comment_client.utils import has_forum_access from psychometrics import psychoanalyze +from string_util import split_by_comma_and_whitespace from student.models import CourseEnrollment, CourseEnrollmentAllowed -from courseware.models import StudentModule from xmodule.course_module import CourseDescriptor from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore @@ -392,14 +393,14 @@ def instructor_dashboard(request, course_id): users = request.POST['betausers'] log.debug("users: {0!r}".format(users)) group = get_beta_group(course) - for username_or_email in _split_by_comma_and_whitespace(users): + for username_or_email in split_by_comma_and_whitespace(users): msg += "

    {0}

    ".format( add_user_to_group(request, username_or_email, group, 'beta testers', 'beta-tester')) elif action == 'Remove beta testers': users = request.POST['betausers'] group = get_beta_group(course) - for username_or_email in _split_by_comma_and_whitespace(users): + for username_or_email in split_by_comma_and_whitespace(users): msg += "

    {0}

    ".format( remove_user_from_group(request, username_or_email, group, 'beta testers', 'beta-tester')) @@ -871,21 +872,11 @@ def grade_summary(request, course_id): #----------------------------------------------------------------------------- # enrollment - -def _split_by_comma_and_whitespace(s): - """ - Split a string both by on commas and whitespice. - """ - # Note: split() with no args removes empty strings from output - lists = [x.split() for x in s.split(',')] - # return all of them - return itertools.chain(*lists) - def _do_enroll_students(course, course_id, students, overload=False): """Do the actual work of enrolling multiple students, presented as a string of emails separated by commas or returns""" - new_students = _split_by_comma_and_whitespace(students) + new_students = split_by_comma_and_whitespace(students) new_students = [str(s.strip()) for s in new_students] new_students_lc = [x.lower() for x in new_students] diff --git a/lms/templates/course_groups/cohort_management.html b/lms/templates/course_groups/cohort_management.html index 1512d09689..962d4de645 100644 --- a/lms/templates/course_groups/cohort_management.html +++ b/lms/templates/course_groups/cohort_management.html @@ -1,24 +1,40 @@

    Cohort groups

    -
    +
    Show cohorts
    -