Merge pull request #1350 from MITx/feature/victor/cohorts

Feature/victor/cohorts

cms/envs/common.py

@@ -34,7 +34,7 @@ MITX_FEATURES = {
     'GITHUB_PUSH': False,
     'ENABLE_DISCUSSION_SERVICE': False,
     'AUTH_USE_MIT_CERTIFICATES' : False,
-    'STUB_VIDEO_FOR_TESTING': False, # do not display video when running automated acceptance tests    
+    'STUB_VIDEO_FOR_TESTING': False, # do not display video when running automated acceptance tests
 }
 ENABLE_JASMINE = False
 
@@ -281,7 +281,7 @@ INSTALLED_APPS = (
     'contentstore',
     'auth',
     'student',  # misleading name due to sharing with lms
-
+    'course_groups',  # not used in cms (yet), but tests run
     # For asset pipelining
     'pipeline',
     'staticfiles',

common/djangoapps/course_groups/cohorts.py

+"""
+This file contains the logic for cohort groups, as exposed internally to the
+forums, and to the cohort admin views.
+"""
+
+from django.contrib.auth.models import User
+from django.http import Http404
+import logging
+
+from courseware import courses
+from student.models import get_user_by_username_or_email
+from .models import CourseUserGroup
+
+log = logging.getLogger(__name__)
+
+def is_course_cohorted(course_id):
+    """
+    Given a course id, return a boolean for whether or not the course is
+    cohorted.
+
+    Raises:
+       Http404 if the course doesn't exist.
+    """
+    return courses.get_course_by_id(course_id).is_cohorted
+
+
+def get_cohort_id(user, course_id):
+    """
+    Given a course id and a user, return the id of the cohort that user is
+    assigned to in that course.  If they don't have a cohort, return None.
+    """
+    cohort = get_cohort(user, course_id)
+    return None if cohort is None else cohort.id
+
+
+def is_commentable_cohorted(course_id, commentable_id):
+    """
+    Given a course and a commentable id, return whether or not this commentable
+    is cohorted.
+
+    Raises:
+        Http404 if the course doesn't exist.
+    """
+    course = courses.get_course_by_id(course_id)
+
+    if not course.is_cohorted:
+        # this is the easy case :)
+        ans = False
+    elif commentable_id in course.top_level_discussion_topic_ids:
+        # top level discussions have to be manually configured as cohorted
+        # (default is not)
+        ans = commentable_id in course.cohorted_discussions()
+    else:
+        # inline discussions are cohorted by default
+        ans = True
+
+    log.debug("is_commentable_cohorted({0}, {1}) = {2}".format(course_id,
+                                                               commentable_id,
+                                                               ans))
+    return ans
+
+
+def get_cohort(user, course_id):
+    c = _get_cohort(user, course_id)
+    log.debug("get_cohort({0}, {1}) = {2}".format(
+        user, course_id,
+        c.id if c is not None else None))
+    return c
+
+
+def _get_cohort(user, course_id):
+    """
+    Given a django User and a course_id, return the user's cohort in that
+    cohort.
+
+    TODO: In classes with auto-cohorting, put the user in a cohort if they
+    aren't in one already.
+
+    Arguments:
+        user: a Django User object.
+        course_id: string in the format 'org/course/run'
+
+    Returns:
+        A CourseUserGroup object if the course is cohorted and the User has a
+        cohort, else None.
+
+    Raises:
+       ValueError if the course_id doesn't exist.
+    """
+    # First check whether the course is cohorted (users shouldn't be in a cohort
+    # in non-cohorted courses, but settings can change after )
+    try:
+        course = courses.get_course_by_id(course_id)
+    except Http404:
+        raise ValueError("Invalid course_id")
+
+    if not course.is_cohorted:
+        return None
+
+    try:
+        group = CourseUserGroup.objects.get(course_id=course_id,
+                                            group_type=CourseUserGroup.COHORT,
+                                            users__id=user.id)
+    except CourseUserGroup.DoesNotExist:
+        group = None
+
+    if group:
+        return group
+
+    # TODO: add auto-cohorting logic here once we know what that will be.
+    return None
+
+
+def get_course_cohorts(course_id):
+    """
+    Get a list of all the cohorts in the given course.
+
+    Arguments:
+        course_id: string in the format 'org/course/run'
+
+    Returns:
+        A list of CourseUserGroup objects.  Empty if there are no cohorts.
+    """
+    return list(CourseUserGroup.objects.filter(course_id=course_id,
+                                               group_type=CourseUserGroup.COHORT))
+
+### Helpers for cohort management views
+
+def get_cohort_by_name(course_id, name):
+    """
+    Return the CourseUserGroup object for the given cohort.  Raises DoesNotExist
+    it isn't present.
+    """
+    return CourseUserGroup.objects.get(course_id=course_id,
+                                       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
+    exists.
+    """
+    log.debug("Adding cohort %s to %s", name, course_id)
+    if CourseUserGroup.objects.filter(course_id=course_id,
+                                      group_type=CourseUserGroup.COHORT,
+                                      name=name).exists():
+        raise ValueError("Can't create two cohorts with the same name")
+
+    return CourseUserGroup.objects.create(course_id=course_id,
+                                          group_type=CourseUserGroup.COHORT,
+                                          name=name)
+
+class CohortConflict(Exception):
+    """
+    Raised when user to be added is already in another cohort in same course.
+    """
+    pass
+
+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 in this cohort.
+
+        CohortConflict if user already in another cohort.
+    """
+    user = get_user_by_username_or_email(username_or_email)
+
+    # If user in any cohorts in this course already, complain
+    course_cohorts = CourseUserGroup.objects.filter(
+        course_id=cohort.course_id,
+        users__id=user.id,
+        group_type=CourseUserGroup.COHORT)
+    if course_cohorts.exists():
+        if course_cohorts[0] == cohort:
+            raise ValueError("User {0} already present in cohort {1}".format(
+                user.username,
+                cohort.name))
+        else:
+            raise CohortConflict("User {0} is in another cohort {1} in course"
+                                 .format(user.username,
+                                         course_cohorts[0].name))
+
+    cohort.users.add(user)
+    return user
+
+
+def get_course_cohort_names(course_id):
+    """
+    Return a list of the cohort names in a course.
+    """
+    return [c.name for c in get_course_cohorts(course_id)]
+
+
+def delete_empty_cohort(course_id, name):
+    """
+    Remove an empty cohort.  Raise ValueError if cohort is not empty.
+    """
+    cohort = get_cohort_by_name(course_id, name)
+    if cohort.users.exists():
+        raise ValueError(
+            "Can't delete non-empty cohort {0} in course {1}".format(
+                name, course_id))
+
+    cohort.delete()
+

common/djangoapps/course_groups/models.py

+import logging
+
+from django.contrib.auth.models import User
+from django.db import models
+
+log = logging.getLogger(__name__)
+
+class CourseUserGroup(models.Model):
+    """
+    This model represents groups of users in a course.  Groups may have different types,
+    which may be treated specially.  For example, a user can be in at most one cohort per
+    course, and cohorts are used to split up the forums by group.
+    """
+    class Meta:
+        unique_together = (('name', 'course_id'), )
+
+    name = models.CharField(max_length=255,
+                            help_text=("What is the name of this group?  "
+                                       "Must be unique within a course."))
+    users = models.ManyToManyField(User, db_index=True, related_name='course_groups',
+                                   help_text="Who is in this group?")
+
+    # Note: groups associated with particular runs of a course.  E.g. Fall 2012 and Spring
+    # 2013 versions of 6.00x will have separate groups.
+    course_id = models.CharField(max_length=255, db_index=True,
+                                 help_text="Which course is this group associated with?")
+
+    # For now, only have group type 'cohort', but adding a type field to support
+    # things like 'question_discussion', 'friends', 'off-line-class', etc
+    COHORT = 'cohort'
+    GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'),)
+    group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES)
+
+

common/djangoapps/course_groups/tests/tests.py

+import django.test
+from django.contrib.auth.models import User
+from django.conf import settings
+
+from override_settings import override_settings
+
+from course_groups.models import CourseUserGroup
+from course_groups.cohorts import get_cohort, get_course_cohorts
+
+from xmodule.modulestore.django import modulestore
+
+class TestCohorts(django.test.TestCase):
+
+    def test_get_cohort(self):
+        # Need to fix this, but after we're testing on staging.  (Looks like
+        # problem is that when get_cohort internally tries to look up the
+        # course.id, it fails, even though we loaded it through the modulestore.
+
+        # Proper fix: give all tests a standard modulestore that uses the test
+        # dir.
+        course = modulestore().get_course("edX/toy/2012_Fall")
+        self.assertEqual(course.id, "edX/toy/2012_Fall")
+
+        user = User.objects.create(username="test", email="a@b.com")
+        other_user = User.objects.create(username="test2", email="a2@b.com")
+
+        self.assertIsNone(get_cohort(user, course.id), "No cohort created yet")
+
+        cohort = CourseUserGroup.objects.create(name="TestCohort",
+                                                course_id=course.id,
+                               group_type=CourseUserGroup.COHORT)
+
+        cohort.users.add(user)
+
+        self.assertIsNone(get_cohort(user, course.id),
+                          "Course isn't cohorted, so shouldn't have a cohort")
+
+        # Make the course cohorted...
+        course.metadata["cohort_config"] = {"cohorted": True}
+        
+        self.assertEquals(get_cohort(user, course.id).id, cohort.id,
+                          "Should find the right cohort")
+
+        self.assertEquals(get_cohort(other_user, course.id), None,
+                          "other_user shouldn't have a cohort")
+
+
+    def test_get_course_cohorts(self):
+        course1_id = 'a/b/c'
+        course2_id = 'e/f/g'
+
+        # add some cohorts to course 1
+        cohort = CourseUserGroup.objects.create(name="TestCohort",
+                                                course_id=course1_id,
+                                                group_type=CourseUserGroup.COHORT)
+
+        cohort = CourseUserGroup.objects.create(name="TestCohort2",
+                                                course_id=course1_id,
+                                                group_type=CourseUserGroup.COHORT)
+
+
+        # second course should have no cohorts
+        self.assertEqual(get_course_cohorts(course2_id), [])
+
+        cohorts = sorted([c.name for c in get_course_cohorts(course1_id)])
+        self.assertEqual(cohorts, ['TestCohort', 'TestCohort2'])
+

common/djangoapps/course_groups/views.py

+from django_future.csrf import ensure_csrf_cookie
+from django.contrib.auth.decorators import login_required
+from django.views.decorators.http import require_POST
+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
+import json
+import logging
+import re
+
+from courseware.courses import get_course_with_access
+from mitxmako.shortcuts import render_to_response, render_to_string
+
+from .models import CourseUserGroup
+from . import cohorts
+
+import track.views
+
+
+log = logging.getLogger(__name__)
+
+def json_http_response(data):
+    """
+    Return an HttpResponse with the data json-serialized and the right content
+    type header.
+    """
+    return HttpResponse(json.dumps(data), content_type="application/json")
+
+def split_by_comma_and_whitespace(s):
+    """
+    Split a string both by commas and whitespice.  Returns a list.
+    """
+    return re.split(r'[\s,]+', s)
+
+
+@ensure_csrf_cookie
+def list_cohorts(request, course_id):
+    """
+    Return json dump of dict:
+
+    {'success': True,
+     'cohorts': [{'name': name, 'id': id}, ...]}
+    """
+    get_course_with_access(request.user, course_id, 'staff')
+
+    all_cohorts = [{'name': c.name, 'id': c.id}
+               for c in cohorts.get_course_cohorts(course_id)]
+
+    return json_http_response({'success': True,
+                            'cohorts': all_cohorts})
+
+
+@ensure_csrf_cookie
+@require_POST
+def add_cohort(request, course_id):
+    """
+    Return json of dict:
+    {'success': True,
+     'cohort': {'id': id,
+                'name': name}}
+
+                or
+
+    {'success': False,
+     'msg': error_msg} if there's an error
+    """
+    get_course_with_access(request.user, course_id, 'staff')
+
+    name = request.POST.get("name")
+    if not name:
+        return json_http_response({'success': False,
+                                'msg': "No name specified"})
+
+    try:
+        cohort = cohorts.add_cohort(course_id, name)
+    except ValueError as err:
+        return json_http_response({'success': False,
+                                'msg': str(err)})
+
+    return json_http_response({'success': 'True',
+                            'cohort': {
+                                'id': cohort.id,
+                                'name': cohort.name
+                                }})
+
+
+@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')
+
+    # this will error if called with a non-int cohort_id.  That's ok--it
+    # shoudn't happen for valid clients.
+    cohort = cohorts.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 json_http_response({'success': True,
+                            'page': page,
+                            'num_pages': paginator.num_pages,
+                            'users': user_info})
+
+
+@ensure_csrf_cookie
+@require_POST
+def add_users_to_cohort(request, course_id, cohort_id):
+    """
+    Return json dict of:
+
+    {'success': True,
+     'added': [{'username': username,
+                'name': name,
+                'email': email}, ...],
+     'conflict': [{'username_or_email': ...,
+                    'msg': ...}],  # in another cohort
+     'present': [str1, str2, ...],    # already there
+     'unknown': [str1, str2, ...]}
+    """
+    get_course_with_access(request.user, course_id, 'staff')
+
+    cohort = cohorts.get_cohort_by_id(course_id, cohort_id)
+
+    users = request.POST.get('users', '')
+    added = []
+    present = []
+    conflict = []
+    unknown = []
+    for username_or_email in split_by_comma_and_whitespace(users):
+        try:
+            user = cohorts.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)
+        except cohorts.CohortConflict as err:
+            conflict.append({'username_or_email': username_or_email,
+                              'msg': str(err)})
+
+
+    return json_http_response({'success': True,
+                            'added': added,
+                            'present': present,
+                            'conflict': conflict,
+                            'unknown': unknown})
+
+@ensure_csrf_cookie
+@require_POST
+def remove_user_from_cohort(request, course_id, cohort_id):
+    """
+    Expects 'username': username in POST data.
+
+    Return json dict of:
+
+    {'success': True} or
+    {'success': False,
+     'msg': error_msg}
+    """
+    get_course_with_access(request.user, course_id, 'staff')
+
+    username = request.POST.get('username')
+    if username is None:
+        return json_http_response({'success': False,
+                                'msg': 'No username specified'})
+
+    cohort = cohorts.get_cohort_by_id(course_id, cohort_id)
+    try:
+        user = User.objects.get(username=username)
+        cohort.users.remove(user)
+        return json_http_response({'success': True})
+    except User.DoesNotExist:
+        log.debug('no user')
+        return json_http_response({'success': False,
+                                'msg': "No user '{0}'".format(username)})
+
+
+def debug_cohort_mgmt(request, course_id):
+    """
+    Debugging view for dev.
+    """
+    # 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)

common/djangoapps/student/tests.py

@@ -5,16 +5,11 @@ when you run "manage.py test".
 Replace this with more appropriate tests for your application.
 """
 import logging
-from datetime import datetime
-from hashlib import sha1
 
 from django.test import TestCase
-from mock import patch, Mock
-from nose.plugins.skip import SkipTest
+from mock import Mock
 
-from .models import (User, UserProfile, CourseEnrollment,
-                     replicate_user, USER_FIELDS_TO_COPY,
-                     unique_id_for_user)
+from .models import unique_id_for_user
 from .views import process_survey_link, _cert_info
 
 COURSE_1 = 'edX/toy/2012_Fall'
@@ -22,185 +17,6 @@ COURSE_2 = 'edx/full/6.002_Spring_2012'
 
 log = logging.getLogger(__name__)
 
-class ReplicationTest(TestCase):
-
-    multi_db = True
-
-    def test_user_replication(self):
-        """Test basic user replication."""
-        raise SkipTest()
-        portal_user = User.objects.create_user('rusty', 'rusty@edx.org', 'fakepass')
-        portal_user.first_name='Rusty'
-        portal_user.last_name='Skids'
-        portal_user.is_staff=True
-        portal_user.is_active=True
-        portal_user.is_superuser=True
-        portal_user.last_login=datetime(2012, 1, 1)
-        portal_user.date_joined=datetime(2011, 1, 1)
-        # This is an Askbot field and will break if askbot is not included
-
-        if hasattr(portal_user, 'seen_response_count'):
-            portal_user.seen_response_count = 10
-
-        portal_user.save(using='default')
-
-        # We replicate this user to Course 1, then pull the same user and verify
-        # that the fields copied over properly.
-        replicate_user(portal_user, COURSE_1)
-        course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-
-        # Make sure the fields we care about got copied over for this user.
-        for field in USER_FIELDS_TO_COPY:
-            self.assertEqual(getattr(portal_user, field),
-                             getattr(course_user, field),
-                             "{0} not copied from {1} to {2}".format(
-                                 field, portal_user, course_user
-                             ))
-
-        # This hasattr lameness is here because we don't want this test to be
-        # triggered when we're being run by CMS tests (Askbot doesn't exist
-        # there, so the test will fail).
-        #
-        # seen_response_count isn't a field we care about, so it shouldn't have
-        # been copied over.
-        if hasattr(portal_user, 'seen_response_count'):
-            portal_user.seen_response_count = 20
-            replicate_user(portal_user, COURSE_1)
-            course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-            self.assertEqual(portal_user.seen_response_count, 20)
-            self.assertEqual(course_user.seen_response_count, 0)
-
-        # Another replication should work for an email change however, since
-        # it's a field we care about.
-        portal_user.email = "clyde@edx.org"
-        replicate_user(portal_user, COURSE_1)
-        course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-        self.assertEqual(portal_user.email, course_user.email)
-
-        # During this entire time, the user data should never have made it over
-        # to COURSE_2
-        self.assertRaises(User.DoesNotExist,
-                          User.objects.using(COURSE_2).get,
-                          id=portal_user.id)
-
-
-    def test_enrollment_for_existing_user_info(self):
-        """Test the effect of Enrolling in a class if you've already got user
-        data to be copied over."""
-        raise SkipTest()
-        # Create our User
-        portal_user = User.objects.create_user('jack', 'jack@edx.org', 'fakepass')
-        portal_user.first_name = "Jack"
-        portal_user.save()
-
-        # Set up our UserProfile info
-        portal_user_profile = UserProfile.objects.create(
-                                  user=portal_user,
-                                  name="Jack Foo",
-                                  level_of_education=None,
-                                  gender='m',
-                                  mailing_address=None,
-                                  goals="World domination",
-                              )
-        portal_user_profile.save()
-
-        # Now let's see if creating a CourseEnrollment copies all the relevant
-        # data.
-        portal_enrollment = CourseEnrollment.objects.create(user=portal_user,
-                                                            course_id=COURSE_1)
-        portal_enrollment.save()
-
-        # Grab all the copies we expect
-        course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-        self.assertEquals(portal_user, course_user)
-        self.assertRaises(User.DoesNotExist,
-                          User.objects.using(COURSE_2).get,
-                          id=portal_user.id)
-
-        course_enrollment = CourseEnrollment.objects.using(COURSE_1).get(id=portal_enrollment.id)
-        self.assertEquals(portal_enrollment, course_enrollment)
-        self.assertRaises(CourseEnrollment.DoesNotExist,
-                          CourseEnrollment.objects.using(COURSE_2).get,
-                          id=portal_enrollment.id)
-
-        course_user_profile = UserProfile.objects.using(COURSE_1).get(id=portal_user_profile.id)
-        self.assertEquals(portal_user_profile, course_user_profile)
-        self.assertRaises(UserProfile.DoesNotExist,
-                          UserProfile.objects.using(COURSE_2).get,
-                          id=portal_user_profile.id)
-
-        log.debug("Make sure our seen_response_count is not replicated.")
-        if hasattr(portal_user, 'seen_response_count'):
-            portal_user.seen_response_count = 200
-            course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-            self.assertEqual(portal_user.seen_response_count, 200)
-            self.assertEqual(course_user.seen_response_count, 0)
-            portal_user.save()
-
-            course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-            self.assertEqual(portal_user.seen_response_count, 200)
-            self.assertEqual(course_user.seen_response_count, 0)
-
-            portal_user.email = 'jim@edx.org'
-            portal_user.save()
-            course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-            self.assertEqual(portal_user.email, 'jim@edx.org')
-            self.assertEqual(course_user.email, 'jim@edx.org')
-
-
-
-    def test_enrollment_for_user_info_after_enrollment(self):
-        """Test the effect of modifying User data after you've enrolled."""
-        raise SkipTest()
-
-        # Create our User
-        portal_user = User.objects.create_user('patty', 'patty@edx.org', 'fakepass')
-        portal_user.first_name = "Patty"
-        portal_user.save()
-
-        # Set up our UserProfile info
-        portal_user_profile = UserProfile.objects.create(
-                                  user=portal_user,
-                                  name="Patty Foo",
-                                  level_of_education=None,
-                                  gender='f',
-                                  mailing_address=None,
-                                  goals="World peace",
-                              )
-        portal_user_profile.save()
-
-        # Now let's see if creating a CourseEnrollment copies all the relevant
-        # data when things are saved.
-        portal_enrollment = CourseEnrollment.objects.create(user=portal_user,
-                                                            course_id=COURSE_1)
-        portal_enrollment.save()
-
-        portal_user.last_name = "Bar"
-        portal_user.save()
-        portal_user_profile.gender = 'm'
-        portal_user_profile.save()
-
-        # Grab all the copies we expect, and make sure it doesn't end up in
-        # places we don't expect.
-        course_user = User.objects.using(COURSE_1).get(id=portal_user.id)
-        self.assertEquals(portal_user, course_user)
-        self.assertRaises(User.DoesNotExist,
-                          User.objects.using(COURSE_2).get,
-                          id=portal_user.id)
-
-        course_enrollment = CourseEnrollment.objects.using(COURSE_1).get(id=portal_enrollment.id)
-        self.assertEquals(portal_enrollment, course_enrollment)
-        self.assertRaises(CourseEnrollment.DoesNotExist,
-                          CourseEnrollment.objects.using(COURSE_2).get,
-                          id=portal_enrollment.id)
-
-        course_user_profile = UserProfile.objects.using(COURSE_1).get(id=portal_user_profile.id)
-        self.assertEquals(portal_user_profile, course_user_profile)
-        self.assertRaises(UserProfile.DoesNotExist,
-                          UserProfile.objects.using(COURSE_2).get,
-                          id=portal_user_profile.id)
-
-
 class CourseEndingTest(TestCase):
     """Test things related to course endings: certificates, surveys, etc"""
 

common/lib/xmodule/xmodule/course_module.py

@@ -223,7 +223,7 @@ class CourseDescriptor(SequenceDescriptor):
 
         return policy_str
 
-    
+
     @classmethod
     def from_xml(cls, xml_data, system, org=None, course=None):
         instance = super(CourseDescriptor, cls).from_xml(xml_data, system, org, course)
@@ -248,7 +248,7 @@ class CourseDescriptor(SequenceDescriptor):
         except ValueError:
             system.error_tracker("Unable to decode grading policy as json")
             policy = None
-        
+
         # cdodge: import the grading policy information that is on disk and put into the
         # descriptor 'definition' bucket as a dictionary so that it is persisted in the DB
         instance.definition['data']['grading_policy'] = policy
@@ -303,28 +303,28 @@ class CourseDescriptor(SequenceDescriptor):
     @property
     def enrollment_start(self):
         return self._try_parse_time("enrollment_start")
-        
+
     @enrollment_start.setter
     def enrollment_start(self, value):
         if isinstance(value, time.struct_time):
             self.metadata['enrollment_start'] = stringify_time(value)
     @property
-    def enrollment_end(self):        
+    def enrollment_end(self):
         return self._try_parse_time("enrollment_end")
-        
+
     @enrollment_end.setter
     def enrollment_end(self, value):
         if isinstance(value, time.struct_time):
             self.metadata['enrollment_end'] = stringify_time(value)
-        
+
     @property
     def grader(self):
         return self._grading_policy['GRADER']
-    
+
     @property
     def raw_grader(self):
         return self._grading_policy['RAW_GRADER']
-    
+
     @raw_grader.setter
     def raw_grader(self, value):
         # NOTE WELL: this change will not update the processed graders. If we need that, this needs to call grader_from_conf
@@ -334,12 +334,12 @@ class CourseDescriptor(SequenceDescriptor):
     @property
     def grade_cutoffs(self):
         return self._grading_policy['GRADE_CUTOFFS']
-    
+
     @grade_cutoffs.setter
     def grade_cutoffs(self, value):
         self._grading_policy['GRADE_CUTOFFS'] = value
         self.definition['data'].setdefault('grading_policy',{})['GRADE_CUTOFFS'] = value
-    
+
 
     @property
     def lowest_passing_grade(self):
@@ -361,6 +361,41 @@ class CourseDescriptor(SequenceDescriptor):
         return self.metadata.get("show_calculator", None) == "Yes"
 
     @property
+    def is_cohorted(self):
+        """
+        Return whether the course is cohorted.
+        """
+        config = self.metadata.get("cohort_config")
+        if config is None:
+            return False
+
+        return bool(config.get("cohorted"))
+
+    @property
+    def top_level_discussion_topic_ids(self):
+        """
+        Return list of topic ids defined in course policy.
+        """
+        topics = self.metadata.get("discussion_topics", {})
+        return [d["id"] for d in topics.values()] 
+
+
+    @property
+    def cohorted_discussions(self):
+        """
+        Return the set of discussions that is explicitly cohorted.  It may be
+        the empty set.  Note that all inline discussions are automatically
+        cohorted based on the course's is_cohorted setting.
+        """
+        config = self.metadata.get("cohort_config")
+        if config is None:
+            return set()
+
+        return set(config.get("cohorted_discussions", []))
+
+
+
+    @property
     def is_new(self):
         """
         Returns if the course has been flagged as new in the metadata. If

common/lib/xmodule/xmodule/discussion_module.py

@@ -18,8 +18,10 @@ class DiscussionModule(XModule):
         }
         return self.system.render_template('discussion/_discussion_module.html', context)
 
-    def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs):
-        XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs)
+    def __init__(self, system, location, definition, descriptor,
+                 instance_state=None, shared_state=None, **kwargs):
+        XModule.__init__(self, system, location, definition, descriptor,
+                         instance_state, shared_state, **kwargs)
 
         if isinstance(instance_state, str):
             instance_state = json.loads(instance_state)

common/lib/xmodule/xmodule/tests/test_import.py

@@ -45,13 +45,24 @@ class DummySystem(ImportSystem):
             raise Exception("Shouldn't be called")
 
 
-class ImportTestCase(unittest.TestCase):
+class BaseCourseTestCase(unittest.TestCase):
     '''Make sure module imports work properly, including for malformed inputs'''
     @staticmethod
     def get_system(load_error_modules=True):
         '''Get a dummy system'''
         return DummySystem(load_error_modules)
 
+    def get_course(self, name):
+        """Get a test course by directory name.  If there's more than one, error."""
+        print "Importing {0}".format(name)
+
+        modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name])
+        courses = modulestore.get_courses()
+        self.assertEquals(len(courses), 1)
+        return courses[0]
+
+class ImportTestCase(BaseCourseTestCase):
+
     def test_fallback(self):
         '''Check that malformed xml loads as an ErrorDescriptor.'''
 
@@ -207,11 +218,7 @@ class ImportTestCase(unittest.TestCase):
         """Make sure that metadata is inherited properly"""
 
         print "Starting import"
-        initial_import = XMLModuleStore(DATA_DIR, course_dirs=['toy'])
-
-        courses = initial_import.get_courses()
-        self.assertEquals(len(courses), 1)
-        course = courses[0]
+        course = self.get_course('toy')
 
         def check_for_key(key, node):
             "recursive check for presence of key"
@@ -227,16 +234,8 @@ class ImportTestCase(unittest.TestCase):
         """Make sure that when two courses share content with the same
         org and course names, policy applies to the right one."""
 
-        def get_course(name):
-            print "Importing {0}".format(name)
-
-            modulestore = XMLModuleStore(DATA_DIR, course_dirs=[name])
-            courses = modulestore.get_courses()
-            self.assertEquals(len(courses), 1)
-            return courses[0]
-
-        toy = get_course('toy')
-        two_toys = get_course('two_toys')
+        toy = self.get_course('toy')
+        two_toys = self.get_course('two_toys')
 
         self.assertEqual(toy.url_name, "2012_Fall")
         self.assertEqual(two_toys.url_name, "TT_2012_Fall")
@@ -279,8 +278,8 @@ class ImportTestCase(unittest.TestCase):
         """Ensure that colons in url_names convert to file paths properly"""
 
         print "Starting import"
+        # Not using get_courses because we need the modulestore object too afterward
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy'])
-
         courses = modulestore.get_courses()
         self.assertEquals(len(courses), 1)
         course = courses[0]
@@ -317,7 +316,7 @@ class ImportTestCase(unittest.TestCase):
 
         toy_id = "edX/toy/2012_Fall"
 
-        course = modulestore.get_courses()[0]
+        course = modulestore.get_course(toy_id)
         chapters = course.get_children()
         ch1 = chapters[0]
         sections = ch1.get_children()
@@ -355,3 +354,30 @@ class ImportTestCase(unittest.TestCase):
         <slider var="a" style="width:400px;float:left;"/>\
 <plot style="margin-top:15px;margin-bottom:15px;"/>""".strip()
         self.assertEqual(gst_sample.definition['render'], render_string_from_sample_gst_xml)
+
+    def test_cohort_config(self):
+        """
+        Check that cohort config parsing works right.
+        """
+        modulestore = XMLModuleStore(DATA_DIR, course_dirs=['toy'])
+
+        toy_id = "edX/toy/2012_Fall"
+
+        course = modulestore.get_course(toy_id)
+
+        # No config -> False
+        self.assertFalse(course.is_cohorted)
+
+        # empty config -> False
+        course.metadata['cohort_config'] = {}
+        self.assertFalse(course.is_cohorted)
+
+        # false config -> False
+        course.metadata['cohort_config'] = {'cohorted': False}
+        self.assertFalse(course.is_cohorted)
+
+        # and finally...
+        course.metadata['cohort_config'] = {'cohorted': True}
+        self.assertTrue(course.is_cohorted)
+
+

common/static/js/course_groups/cohorts.js

+// structure stolen from http://briancray.com/posts/javascript-module-pattern
+
+var CohortManager = (function ($) {
+    // private variables and functions
+
+    // using jQuery
+    function getCookie(name) {
+        var cookieValue = null;
+        if (document.cookie && document.cookie != '') {
+            var cookies = document.cookie.split(';');
+            for (var i = 0; i < cookies.length; i++) {
+                var cookie = $.trim(cookies[i]);
+                // Does this cookie string begin with the name we want?
+                if (cookie.substring(0, name.length + 1) == (name + '=')) {
+                    cookieValue = decodeURIComponent(cookie.substring(name.length + 1));
+                    break;
+                }
+            }
+        }
+        return cookieValue;
+    }
+    var csrftoken = getCookie('csrftoken');
+    
+    function csrfSafeMethod(method) {
+        // these HTTP methods do not require CSRF protection
+        return (/^(GET|HEAD|OPTIONS|TRACE)$/.test(method));
+    }  
+    $.ajaxSetup({
+        crossDomain: false, // obviates need for sameOrigin test
+        beforeSend: function(xhr, settings) {
+            if (!csrfSafeMethod(settings.type)) {
+                xhr.setRequestHeader("X-CSRFToken", csrftoken);
+            }
+        }
+    });
+
+    // constructor
+    var module = function () {
+        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;
+
+        // 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_id = null;
+        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);
+            cohort_title = el.text();
+            detail_url = el.data('href');
+            cohort_id = el.data('id');
+            state = state_detail;
+            render();
+        }
+
+        function add_to_cohorts_list(item) {
+            var li = $('<li><a></a></li>');
+            $("a", li).text(item.name)
+                .data('href', url + '/' + item.id)
+                .addClass('link')
+                .click(show_cohort);
+            cohorts.append(li);
+        };
+
+        function log_error(msg) {
+            errors.empty();
+            errors.append($("<li />").text(msg).addClass("error"));
+        };
+        
+        function load_cohorts(response) {
+            cohorts.empty();
+            if (response && response.success) { 
+                response.cohorts.forEach(add_to_cohorts_list);
+            } else {
+                log_error(response.msg || "There was an error loading cohorts");
+            }
+            summary.show();
+        };
+
+
+        function added_cohort(response) {
+            if (response && response.success) {
+                add_to_cohorts_list(response.cohort);
+            } else {
+                log_error(response.msg || "There was an error adding a cohort");
+            }                
+        }
+
+        // *********** Detail view methods
+
+        function remove_user_from_cohort(username, cohort_id, row) {
+            var delete_url = detail_url + '/delete';
+            var data = {'username': username}
+            $.post(delete_url, data).done(function() {row.remove()})
+                .fail(function(jqXHR, status, error) {
+                    log_error('Error removing user ' + username + 
+                              ' from cohort. ' + status + ' ' + error);
+                });
+        }
+
+        function add_to_users_list(item) {
+            var tr = $('<tr><td class="name"></td><td class="username"></td>' + 
+                       '<td class="email"></td>' + 
+                       '<td class="remove"></td></tr>');
+            var current_cohort_id = cohort_id;
+
+            $(".name", tr).text(item.name);
+            $(".username", tr).text(item.username);
+            $(".email", tr).text(item.email);
+
+            $(".remove", tr).html('<a href="#">remove</a>')
+                .click(function() { 
+                    remove_user_from_cohort(item.username, current_cohort_id, tr);
+                });
+            
+            detail_users.append(tr);
+        };
+
+                
+        function show_users(response) {
+            detail_users.html("<tr><th>Name</th><th>Username</th><th>Email</th></tr>");
+            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></li>')
+                    if (typeof item === "object" && item.username) {
+                        li.text(note + ' ' + item.name + ', ' + item.username + ', ' + item.email);
+                    } else if (typeof item === "object" && item.msg) {
+                        li.text(note + ' ' + item.username_or_email + ', ' + item.msg);
+                    } else {
+                        // string
+                        li.text(note + ' ' + item);
+                    }
+                    li.css('color', color);
+                    op_results.append(li);
+                }
+            }
+            op_results.empty();
+            if (response && response.success) {
+                response.added.forEach(adder("Added", "green"));
+                response.present.forEach(adder("Already present:", "black"));
+                response.conflict.forEach(adder("In another cohort:", "purple"));
+                response.unknown.forEach(adder("Unknown user:", "red"));
+                users_area.val('')
+            } 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() { 
+            state = state_summary;
+            render();
+        });
+        
+        add_cohort_input.change(function() {
+            if (!$(this).val()) {
+                add_cohort_button.removeClass('button').addClass('button-disabled');
+            } else {
+                add_cohort_button.removeClass('button-disabled').addClass('button');
+            }
+        });
+
+        add_cohort_button.click(function() { 
+            var add_url = url + '/add';
+            data = {'name': add_cohort_input.val()}
+            $.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
+    module.prototype = {
+        constructor: module,
+    };
+
+    // return module
+    return module;
+})(jQuery);
+
+$(window).load(function () {
+    var my_module = new CohortManager();
+})
+

doc/xml-format.md

@@ -258,6 +258,11 @@ Supported fields at the course level:
 * "discussion_blackouts" -- An array of time intervals during which you want to disable a student's ability to create or edit posts in the forum. Moderators, Community TAs, and Admins are unaffected. You might use this during exam periods, but please be aware that the forum is often a very good place to catch mistakes and clarify points to students. The better long term solution would be to have better flagging/moderation mechanisms, but this is the hammer we have today. Format by example: [["2012-10-29T04:00", "2012-11-03T04:00"], ["2012-12-30T04:00", "2013-01-02T04:00"]]
 * "show_calculator" (value "Yes" if desired)
 * "days_early_for_beta" -- number of days (floating point ok) early that students in the beta-testers group get to see course content.  Can also be specified for any other course element, and overrides values set at higher levels.
+* "cohort_config" : dictionary with keys 
+    - "cohorted" : boolean.  Set to true if this course uses student cohorts.  If so, all inline discussions are automatically cohorted, and top-level discussion topics are configurable with an optional 'cohorted': bool parameter (with default value false).  
+    - "cohorted_discussions": list of discussions that should be cohorted.  
+    - ... more to come.  ('auto_cohort', how to auto cohort, etc)
+
 * TODO: there are others
 
 ### Grading policy file contents

lms/djangoapps/courseware/courses.py

@@ -83,7 +83,7 @@ def get_opt_course_with_access(user, course_id, action):
         return None
     return get_course_with_access(user, course_id, action)
 
-
+    
 def course_image_url(course):
     """Try to look up the image url for the course.  If it's not found,
     log an error and return the dead link"""

lms/djangoapps/courseware/views.py

@@ -39,6 +39,7 @@ log = logging.getLogger("mitx.courseware")
 template_imports = {'urllib': urllib}
 
 
+
 def user_groups(user):
     """
     TODO (vshnayder): This is not used. When we have a new plan for groups, adjust appropriately.

lms/djangoapps/django_comment_client/base/urls.py

@@ -24,9 +24,9 @@ urlpatterns = patterns('django_comment_client.base.views',
     url(r'comments/(?P<comment_id>[\w\-]+)/downvote$', 'vote_for_comment', {'value': 'down'}, name='downvote_comment'),
     url(r'comments/(?P<comment_id>[\w\-]+)/unvote$', 'undo_vote_for_comment', name='undo_vote_for_comment'),
     
-    url(r'(?P<commentable_id>[\w\-]+)/threads/create$', 'create_thread', name='create_thread'),
+    url(r'^(?P<commentable_id>[\w\-.]+)/threads/create$', 'create_thread', name='create_thread'),
     # TODO should we search within the board?
-    url(r'(?P<commentable_id>[\w\-]+)/threads/search_similar$', 'search_similar_threads', name='search_similar_threads'),
-    url(r'(?P<commentable_id>[\w\-]+)/follow$', 'follow_commentable', name='follow_commentable'),
-    url(r'(?P<commentable_id>[\w\-]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'),
+    url(r'^(?P<commentable_id>[\w\-.]+)/threads/search_similar$', 'search_similar_threads', name='search_similar_threads'),
+    url(r'^(?P<commentable_id>[\w\-.]+)/follow$', 'follow_commentable', name='follow_commentable'),
+    url(r'^(?P<commentable_id>[\w\-.]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'),
 )

lms/djangoapps/django_comment_client/base/views.py

@@ -21,12 +21,15 @@ from django.contrib.auth.models import User
 
 from mitxmako.shortcuts import render_to_response, render_to_string
 from courseware.courses import get_course_with_access
+from course_groups.cohorts import get_cohort_id, is_commentable_cohorted
 
 from django_comment_client.utils import JsonResponse, JsonError, extract, get_courseware_context
 
-from django_comment_client.permissions import check_permissions_by_view
+from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
 from django_comment_client.models import Role
 
+log = logging.getLogger(__name__)
+
 def permitted(fn):
     @functools.wraps(fn)
     def wrapper(request, *args, **kwargs):
@@ -58,10 +61,12 @@ def ajax_content_response(request, course_id, content, template_name):
         'annotated_content_info': annotated_content_info,
     })
 
+
 @require_POST
 @login_required
 @permitted
 def create_thread(request, course_id, commentable_id):
+    log.debug("Creating new thread in %r, id %r", course_id, commentable_id)
     course = get_course_with_access(request.user, course_id, 'load')
     post = request.POST
 
@@ -83,6 +88,23 @@ def create_thread(request, course_id, commentable_id):
         'course_id'          : course_id,
         'user_id'            : request.user.id,
     })
+
+
+    # Cohort the thread if the commentable is cohorted.
+    if is_commentable_cohorted(course_id, commentable_id):
+        user_group_id = get_cohort_id(request.user, course_id)
+        # TODO (vshnayder): once we have more than just cohorts, we'll want to
+        # change this to a single get_group_for_user_and_commentable function
+        # that can do different things depending on the commentable_id
+        if cached_has_permission(request.user, "see_all_cohorts", course_id):
+            # admins can optionally choose what group to post as
+            group_id = post.get('group_id', user_group_id)
+        else:
+            # regular users always post with their own id.
+            group_id = user_group_id
+
+        thread.update_attributes(group_id=group_id)
+
     thread.save()
     if post.get('auto_subscribe', 'false').lower() == 'true':
         user = cc.User.from_django_user(request.user)

lms/djangoapps/django_comment_client/forum/urls.py

@@ -4,7 +4,7 @@ import django_comment_client.forum.views
 urlpatterns = patterns('django_comment_client.forum.views',
     url(r'users/(?P<user_id>\w+)/followed$', 'followed_threads', name='followed_threads'),
     url(r'users/(?P<user_id>\w+)$', 'user_profile', name='user_profile'),
-    url(r'(?P<discussion_id>[\w\-]+)/threads/(?P<thread_id>\w+)$', 'single_thread', name='single_thread'),
-    url(r'(?P<discussion_id>[\w\-]+)/inline$', 'inline_discussion', name='inline_discussion'),
+    url(r'^(?P<discussion_id>[\w\-.]+)/threads/(?P<thread_id>\w+)$', 'single_thread', name='single_thread'),
+    url(r'^(?P<discussion_id>[\w\-.]+)/inline$', 'inline_discussion', name='inline_discussion'),
     url(r'', 'forum_form_discussion', name='forum_form_discussion'),
 )

lms/djangoapps/django_comment_client/forum/views.py

@@ -11,12 +11,14 @@ from django.contrib.auth.models import User
 
 from mitxmako.shortcuts import render_to_response, render_to_string
 from courseware.courses import get_course_with_access
+from course_groups.cohorts import get_cohort_id
 from courseware.access import has_access
 
 from urllib import urlencode
 from operator import methodcaller
 from django_comment_client.permissions import check_permissions_by_view
-from django_comment_client.utils import merge_dict, extract, strip_none, strip_blank, get_courseware_context
+from django_comment_client.utils import (merge_dict, extract, strip_none,
+                                         strip_blank, get_courseware_context)
 
 import django_comment_client.utils as utils
 import comment_client as cc
@@ -33,7 +35,6 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
     This may raise cc.utils.CommentClientError or
     cc.utils.CommentClientUnknownError if something goes wrong.
     """
-
     default_query_params = {
         'page': 1,
         'per_page': per_page,
@@ -58,8 +59,17 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
         user.default_sort_key = request.GET.get('sort_key')
         user.save()
 
+
+    #if the course-user is cohorted, then add the group id
+    group_id = get_cohort_id(user,course_id)
+    if group_id:
+        default_query_params["group_id"] = group_id
+
     query_params = merge_dict(default_query_params,
-                              strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags', 'commentable_ids'])))
+                              strip_none(extract(request.GET,
+                                                 ['page', 'sort_key',
+                                                  'sort_order', 'text',
+                                                  'tags', 'commentable_ids'])))
 
     threads, page, num_pages = cc.Thread.search(query_params)
 
@@ -218,7 +228,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
         #    course_id,
         #)
 
-        
+
         annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info)
 
         context = {

lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py

@@ -23,7 +23,7 @@ class Command(BaseCommand):
             student_role.add_permission(per)
 
         for per in ["edit_content", "delete_thread", "openclose_thread",
-                        "endorse_comment", "delete_comment"]:
+                        "endorse_comment", "delete_comment", "see_all_cohorts"]:
             moderator_role.add_permission(per)
 
         for per in ["manage_moderator"]:

lms/djangoapps/django_comment_client/tests.py

-from django.contrib.auth.models import User
+from django.contrib.auth.models import User, Group
+from django.core.urlresolvers import reverse
 from django.test import TestCase
-from student.models import CourseEnrollment, \
-                           replicate_enrollment_save, \
-                           replicate_enrollment_delete, \
-                           update_user_information, \
-                           replicate_user_save
-                           
+from django.test.client import RequestFactory
+from django.conf import settings
+
+from mock import Mock
+
+from override_settings import override_settings
+
+import xmodule.modulestore.django
+
+from student.models import CourseEnrollment
+
 from django.db.models.signals import m2m_changed, pre_delete, pre_save, post_delete, post_save
 from django.dispatch.dispatcher import _make_id
 import string
@@ -13,6 +19,57 @@ import random
 from .permissions import has_permission
 from .models import Role, Permission
 
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore import Location
+from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.modulestore.xml import XMLModuleStore
+
+import comment_client
+
+from courseware.tests.tests import PageLoader, TEST_DATA_XML_MODULESTORE
+
+#@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+#class TestCohorting(PageLoader):
+#    """Check that cohorting works properly"""
+#
+#    def setUp(self):
+#        xmodule.modulestore.django._MODULESTORES = {}
+#
+#        # Assume courses are there
+#        self.toy = modulestore().get_course("edX/toy/2012_Fall")
+#
+#        # Create two accounts
+#        self.student = 'view@test.com'
+#        self.student2 = 'view2@test.com'
+#        self.password = 'foo'
+#        self.create_account('u1', self.student, self.password)
+#        self.create_account('u2', self.student2, self.password)
+#        self.activate_user(self.student)
+#        self.activate_user(self.student2)
+#
+#    def test_create_thread(self):
+#        my_save = Mock()
+#        comment_client.perform_request = my_save
+#
+#        resp = self.client.post(
+#            reverse('django_comment_client.base.views.create_thread',
+#                    kwargs={'course_id': 'edX/toy/2012_Fall',
+#                            'commentable_id': 'General'}),
+#                                        {'some': "some",
+#                                         'data': 'data'})
+#        self.assertTrue(my_save.called)
+#
+#        #self.assertEqual(resp.status_code, 200)
+#        #self.assertEqual(my_save.something, "expected", "complaint if not true")
+#
+#        self.toy.metadata["cohort_config"] = {"cohorted": True}
+#
+#        # call the view again ...
+#
+#       # assert that different things happened
+
+
+
 class PermissionsTestCase(TestCase):
     def random_str(self, length=15, chars=string.ascii_uppercase + string.digits):
         return ''.join(random.choice(chars) for x in range(length))

lms/djangoapps/django_comment_client/utils.py

@@ -207,6 +207,9 @@ def initialize_discussion_info(course):
                                                       "sort_key": entry["sort_key"],
                                                       "start_date": entry["start_date"]}
 
+    # TODO.  BUG! : course location is not unique across multiple course runs!
+    # (I think Kevin already noticed this)  Need to send course_id with requests, store it
+    # in the backend.
     default_topics = {'General': {'id' :course.location.html_id()}}
     discussion_topics = course.metadata.get('discussion_topics', default_topics)
     for topic, entry in discussion_topics.items():

lms/djangoapps/instructor/views.py

@@ -24,6 +24,7 @@ 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,
@@ -31,7 +32,6 @@ from django_comment_client.models import (Role,
 from django_comment_client.utils import has_forum_access
 from psychometrics import psychoanalyze
 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
@@ -49,6 +49,9 @@ template_imports = {'urllib': urllib}
 FORUM_ROLE_ADD = 'add'
 FORUM_ROLE_REMOVE = 'remove'
 
+def split_by_comma_and_whitespace(s):
+    return re.split(r'[\s,]', s)
+
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 def instructor_dashboard(request, course_id):
@@ -193,8 +196,8 @@ def instructor_dashboard(request, course_id):
         # get the form data
         unique_student_identifier=request.POST.get('unique_student_identifier','')
         problem_to_reset=request.POST.get('problem_to_reset','')
-        
-        if problem_to_reset[-4:]==".xml": 
+
+        if problem_to_reset[-4:]==".xml":
             problem_to_reset=problem_to_reset[:-4]
 
         # try to uniquely id student by email address or username
@@ -213,8 +216,8 @@ def instructor_dashboard(request, course_id):
             try:
                 (org, course_name, run)=course_id.split("/")
                 module_state_key="i4x://"+org+"/"+course_name+"/problem/"+problem_to_reset
-                module_to_reset=StudentModule.objects.get(student_id=student_to_reset.id, 
-                                                          course_id=course_id, 
+                module_to_reset=StudentModule.objects.get(student_id=student_to_reset.id,
+                                                          course_id=course_id,
                                                           module_state_key=module_state_key)
                 msg+="Found module to reset.  "
             except Exception as e:
@@ -230,14 +233,14 @@ def instructor_dashboard(request, course_id):
             # save
             module_to_reset.state=json.dumps(problem_state)
             module_to_reset.save()
-            track.views.server_track(request, 
+            track.views.server_track(request,
                                     '{instructor} reset attempts from {old_attempts} to 0 for {student} on problem {problem} in {course}'.format(
                                         old_attempts=old_number_of_attempts,
                                         student=student_to_reset,
                                         problem=module_to_reset.module_state_key,
                                         instructor=request.user,
                                         course=course_id),
-                                    {}, 
+                                    {},
                                     page='idashboard')
             msg+="<font color='green'>Module state successfully reset!</font>"
         except:
@@ -252,12 +255,12 @@ def instructor_dashboard(request, course_id):
             else:
                 student_to_reset=User.objects.get(username=unique_student_identifier)
             progress_url=reverse('student_progress',kwargs={'course_id':course_id,'student_id': student_to_reset.id})
-            track.views.server_track(request, 
+            track.views.server_track(request,
                                     '{instructor} requested progress page for {student} in {course}'.format(
                                         student=student_to_reset,
                                         instructor=request.user,
                                         course=course_id),
-                                    {}, 
+                                    {},
                                     page='idashboard')
             msg+="<a href='{0}' target='_blank'> Progress page for username: {1} with email address: {2}</a>.".format(progress_url,student_to_reset.username,student_to_reset.email)
         except:
@@ -392,14 +395,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 += "<p>{0}</p>".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 += "<p>{0}</p>".format(
                 remove_user_from_group(request, username_or_email, group, 'beta testers', 'beta-tester'))
 
@@ -562,6 +565,7 @@ def instructor_dashboard(request, course_id):
                'djangopid' : os.getpid(),
                'mitx_version' : getattr(settings,'MITX_VERSION_STRING',''),
                'offline_grade_log' : offline_grades_available(course_id),
+               'cohorts_ajax_url' : reverse('cohorts', kwargs={'course_id': course_id}),
                }
 
     return render_to_response('courseware/instructor_dashboard.html', context)
@@ -870,21 +874,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]
 

lms/envs/common.py

@@ -577,6 +577,7 @@ INSTALLED_APPS = (
     'open_ended_grading',
     'psychometrics',
     'licenses',
+    'course_groups',
 
     #For the wiki
     'wiki', # The new django-wiki from benjaoming

lms/envs/test.py

@@ -17,7 +17,7 @@ from path import path
 MITX_FEATURES['DISABLE_START_DATES'] = True
 
 # Until we have discussion actually working in test mode, just turn it off
-MITX_FEATURES['ENABLE_DISCUSSION_SERVICE'] = False
+#MITX_FEATURES['ENABLE_DISCUSSION_SERVICE'] = True
 
 # Need wiki for courseware views to work. TODO (vshnayder): shouldn't need it.
 WIKI_ENABLED = True

lms/lib/comment_client/thread.py

@@ -10,12 +10,12 @@ class Thread(models.Model):
         'closed', 'tags', 'votes', 'commentable_id', 'username', 'user_id',
         'created_at', 'updated_at', 'comments_count', 'unread_comments_count',
         'at_position_list', 'children', 'type', 'highlighted_title',
-        'highlighted_body', 'endorsed', 'read'
+        'highlighted_body', 'endorsed', 'read', 'group_id'
     ]
 
     updatable_fields = [
         'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id',
-        'closed', 'tags', 'user_id', 'commentable_id',
+        'closed', 'tags', 'user_id', 'commentable_id', 'group_id'
     ]
 
     initializable_fields = updatable_fields

lms/templates/course_groups/cohort_management.html

+<section class="cohort_manager" data-ajax_url="${cohorts_ajax_url}">
+<h3>Cohort groups</h3>
+
+<div class="controls" style="padding-top:15px">
+  <a href="#" class="button show_cohorts">Show cohorts</a>
+</div>
+
+<ul class="errors">
+</ul>
+
+<div class="summary" style="display:none">
+  <h3>Cohorts in the course</h3>
+  <ul class="cohorts">
+  </ul>
+
+  <p>
+  <input class="cohort_name"/>
+  <a href="#" class="button add_cohort">Add cohort</a>
+  </p>
+  
+</div>
+
+<div class="detail" style="display:none">
+  <h3 class="header"></h3>
+  <table class="users">
+  </table>
+  <span class="page_num"></span>
+  
+  <p>
+  Add users by username or email.  One per line or comma-separated.
+  </p>
+  <textarea cols="50" row="30" class="users_area" style="height: 200px"></textarea>
+  <a href="#" class="button add_members">Add cohort members</a>
+
+  <ul class="op_results">
+  </ul>
+  
+</div>
+
+
+</section>

lms/templates/course_groups/debug.html

+<!DOCTYPE html>
+<html>
+<head>
+  <%block name="title"><title>edX</title></%block>
+
+  <script type="text/javascript" src="/static/js/vendor/jquery.min.js"></script>
+  <script type="text/javascript" src="/static/js/course_groups/cohorts.js"></script>
+
+</head>
+
+<body class="<%block name='bodyclass'/>">
+<%include file="/course_groups/cohort_management.html" />
+
+</body>
+</html>
+

lms/templates/courseware/instructor_dashboard.html

@@ -6,6 +6,8 @@
   <%static:css group='course'/>
   <script type="text/javascript" src="${static.url('js/vendor/flot/jquery.flot.js')}"></script>
   <script type="text/javascript" src="${static.url('js/vendor/flot/jquery.flot.axislabels.js')}"></script>
+  <script type="text/javascript" src="${static.url('js/course_groups/cohorts.js')}"></script>
+
 </%block>
 
 <%include file="/courseware/course_navigation.html" args="active_page='instructor'" />
@@ -282,16 +284,21 @@ function goto( mode)
     <input type="submit" name="action" value="Add beta testers">
     </p>
     <hr width="40%" style="align:left">
+
+    %if course.is_cohorted:
+    <%include file="/course_groups/cohort_management.html" />
+    %endif
+
   %endif
 %endif
 
     </form>
+##----------------------------------------------------------------------------- 
 
 %if msg:
     <p></p><p>${msg}</p>
 %endif
 ##----------------------------------------------------------------------------- 
-##----------------------------------------------------------------------------- 
 
 %if datatable and modeflag.get('Psychometrics') is None:
 

lms/urls.py

@@ -274,6 +274,26 @@ if settings.COURSEWARE_ENABLED:
             'open_ended_grading.peer_grading_service.save_grade', name='peer_grading_save_grade'),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/peer_grading/save_calibration_essay$',
             'open_ended_grading.peer_grading_service.save_calibration_essay', name='peer_grading_save_calibration_essay'),
+
+        # Cohorts management
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/cohorts$',
+            'course_groups.views.list_cohorts', name="cohorts"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/cohorts/add$',
+            'course_groups.views.add_cohort',
+            name="add_cohort"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/cohorts/(?P<cohort_id>[0-9]+)$',
+            'course_groups.views.users_in_cohort',
+            name="list_cohort"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/cohorts/(?P<cohort_id>[0-9]+)/add$',
+            'course_groups.views.add_users_to_cohort',
+            name="add_to_cohort"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/cohorts/(?P<cohort_id>[0-9]+)/delete$',
+            'course_groups.views.remove_user_from_cohort',
+            name="remove_from_cohort"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/cohorts/debug$',
+            'course_groups.views.debug_cohort_mgmt',
+            name="debug_cohort_mgmt"),
+
     )
 
     # discussion forums live within courseware, so courseware must be enabled first