diff --git a/cms/envs/common.py b/cms/envs/common.py index 98a5fbf26d..f2d47dfdc6 100644 --- a/cms/envs/common.py +++ b/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', diff --git a/common/djangoapps/course_groups/__init__.py b/common/djangoapps/course_groups/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py new file mode 100644 index 0000000000..df8b22ef26 --- /dev/null +++ b/common/djangoapps/course_groups/cohorts.py @@ -0,0 +1,225 @@ +""" +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() + diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py new file mode 100644 index 0000000000..957d230d92 --- /dev/null +++ b/common/djangoapps/course_groups/models.py @@ -0,0 +1,34 @@ +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) + + diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py new file mode 100644 index 0000000000..0303eaaa55 --- /dev/null +++ b/common/djangoapps/course_groups/tests/tests.py @@ -0,0 +1,67 @@ +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']) + diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py new file mode 100644 index 0000000000..d591c44356 --- /dev/null +++ b/common/djangoapps/course_groups/views.py @@ -0,0 +1,219 @@ +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) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 0d8a643ecb..44b947c045 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1,30 +1,5 @@ """ -Models for Student Information - -Replication Notes - -TODO: Update this to be consistent with reality (no portal servers, no more askbot) - -In our live deployment, we intend to run in a scenario where there is a pool of -Portal servers that hold the canoncial user information and that user -information is replicated to slave Course server pools. Each Course has a set of -servers that serves only its content and has users that are relevant only to it. - -We replicate the following tables into the Course DBs where the user is -enrolled. Only the Portal servers should ever write to these models. -* UserProfile -* CourseEnrollment - -We do a partial replication of: -* User -- Askbot extends this and uses the extra fields, so we replicate only - the stuff that comes with basic django_auth and ignore the rest.) - -There are a couple different scenarios: - -1. There's an update of User or UserProfile -- replicate it to all Course DBs - that the user is enrolled in (found via CourseEnrollment). -2. There's a change in CourseEnrollment. We need to push copies of UserProfile, - CourseEnrollment, and the base fields in User +Models for User Information (students, staff, etc) Migration Notes @@ -146,8 +121,8 @@ class TestCenterUser(models.Model): The field names and lengths are modeled on the conventions and constraints of Pearson's data import system, including oddities such as suffix having a limit of 255 while last_name only gets 50. - - Also storing here the confirmation information received from Pearson (if any) + + Also storing here the confirmation information received from Pearson (if any) as to the success or failure of the upload. (VCDC file) """ # Our own record keeping... @@ -197,7 +172,7 @@ class TestCenterUser(models.Model): uploaded_at = models.DateTimeField(null=True, blank=True, db_index=True) # confirmation back from the test center, as well as timestamps - # on when they processed the request, and when we received + # on when they processed the request, and when we received # confirmation back. processed_at = models.DateTimeField(null=True, db_index=True) upload_status = models.CharField(max_length=20, blank=True, db_index=True) # 'Error' or 'Accepted' @@ -211,52 +186,52 @@ class TestCenterUser(models.Model): @property def needs_uploading(self): return self.uploaded_at is None or self.uploaded_at < self.user_updated_at - + @staticmethod def user_provided_fields(): - return [ 'first_name', 'middle_name', 'last_name', 'suffix', 'salutation', - 'address_1', 'address_2', 'address_3', 'city', 'state', 'postal_code', 'country', + return [ 'first_name', 'middle_name', 'last_name', 'suffix', 'salutation', + 'address_1', 'address_2', 'address_3', 'city', 'state', 'postal_code', 'country', 'phone', 'extension', 'phone_country_code', 'fax', 'fax_country_code', 'company_name'] - + @property def email(self): return self.user.email - + def needs_update(self, fields): for fieldname in TestCenterUser.user_provided_fields(): if fieldname in fields and getattr(self, fieldname) != fields[fieldname]: return True - - return False - + + return False + @staticmethod def _generate_edx_id(prefix): NUM_DIGITS = 12 return u"{}{:012}".format(prefix, randint(1, 10**NUM_DIGITS-1)) - + @staticmethod def _generate_candidate_id(): return TestCenterUser._generate_edx_id("edX") - + @classmethod def create(cls, user): testcenter_user = cls(user=user) - # testcenter_user.candidate_id remains unset + # testcenter_user.candidate_id remains unset # assign an ID of our own: cand_id = cls._generate_candidate_id() while TestCenterUser.objects.filter(client_candidate_id=cand_id).exists(): cand_id = cls._generate_candidate_id() - testcenter_user.client_candidate_id = cand_id + testcenter_user.client_candidate_id = cand_id return testcenter_user @property def is_accepted(self): return self.upload_status == TEST_CENTER_STATUS_ACCEPTED - + @property def is_rejected(self): return self.upload_status == TEST_CENTER_STATUS_ERROR - + @property def is_pending(self): return not self.is_accepted and not self.is_rejected @@ -264,26 +239,26 @@ class TestCenterUser(models.Model): class TestCenterUserForm(ModelForm): class Meta: model = TestCenterUser - fields = ( 'first_name', 'middle_name', 'last_name', 'suffix', 'salutation', - 'address_1', 'address_2', 'address_3', 'city', 'state', 'postal_code', 'country', + fields = ( 'first_name', 'middle_name', 'last_name', 'suffix', 'salutation', + 'address_1', 'address_2', 'address_3', 'city', 'state', 'postal_code', 'country', 'phone', 'extension', 'phone_country_code', 'fax', 'fax_country_code', 'company_name') - + def update_and_save(self): new_user = self.save(commit=False) # create additional values here: new_user.user_updated_at = datetime.utcnow() new_user.upload_status = '' new_user.save() - log.info("Updated demographic information for user's test center exam registration: username \"{}\" ".format(new_user.user.username)) - + log.info("Updated demographic information for user's test center exam registration: username \"{}\" ".format(new_user.user.username)) + # add validation: - + def clean_country(self): code = self.cleaned_data['country'] if code and len(code) != 3: raise forms.ValidationError(u'Must be three characters (ISO 3166-1): e.g. USA, CAN, MNG') return code - + def clean(self): def _can_encode_as_latin(fieldvalue): try: @@ -291,40 +266,40 @@ class TestCenterUserForm(ModelForm): except UnicodeEncodeError: return False return True - + cleaned_data = super(TestCenterUserForm, self).clean() - + # check for interactions between fields: if 'country' in cleaned_data: country = cleaned_data.get('country') if country == 'USA' or country == 'CAN': if 'state' in cleaned_data and len(cleaned_data['state']) == 0: - self._errors['state'] = self.error_class([u'Required if country is USA or CAN.']) + self._errors['state'] = self.error_class([u'Required if country is USA or CAN.']) del cleaned_data['state'] if 'postal_code' in cleaned_data and len(cleaned_data['postal_code']) == 0: - self._errors['postal_code'] = self.error_class([u'Required if country is USA or CAN.']) + self._errors['postal_code'] = self.error_class([u'Required if country is USA or CAN.']) del cleaned_data['postal_code'] - + if 'fax' in cleaned_data and len(cleaned_data['fax']) > 0 and 'fax_country_code' in cleaned_data and len(cleaned_data['fax_country_code']) == 0: - self._errors['fax_country_code'] = self.error_class([u'Required if fax is specified.']) + self._errors['fax_country_code'] = self.error_class([u'Required if fax is specified.']) del cleaned_data['fax_country_code'] # check encoding for all fields: cleaned_data_fields = [fieldname for fieldname in cleaned_data] for fieldname in cleaned_data_fields: if not _can_encode_as_latin(cleaned_data[fieldname]): - self._errors[fieldname] = self.error_class([u'Must only use characters in Latin-1 (iso-8859-1) encoding']) + self._errors[fieldname] = self.error_class([u'Must only use characters in Latin-1 (iso-8859-1) encoding']) del cleaned_data[fieldname] # Always return the full collection of cleaned data. return cleaned_data - -# our own code to indicate that a request has been rejected. -ACCOMMODATION_REJECTED_CODE = 'NONE' - + +# our own code to indicate that a request has been rejected. +ACCOMMODATION_REJECTED_CODE = 'NONE' + ACCOMMODATION_CODES = ( - (ACCOMMODATION_REJECTED_CODE, 'No Accommodation Granted'), + (ACCOMMODATION_REJECTED_CODE, 'No Accommodation Granted'), ('EQPMNT', 'Equipment'), ('ET12ET', 'Extra Time - 1/2 Exam Time'), ('ET30MN', 'Extra Time - 30 Minutes'), @@ -334,11 +309,11 @@ ACCOMMODATION_CODES = ( ('SRRERC', 'Separate Room and Reader/Recorder'), ('SRRECR', 'Separate Room and Recorder'), ('SRSEAN', 'Separate Room and Service Animal'), - ('SRSGNR', 'Separate Room and Sign Language Interpreter'), + ('SRSGNR', 'Separate Room and Sign Language Interpreter'), ) ACCOMMODATION_CODE_DICT = { code : name for (code, name) in ACCOMMODATION_CODES } - + class TestCenterRegistration(models.Model): """ This is our representation of a user's registration for in-person testing, @@ -353,20 +328,20 @@ class TestCenterRegistration(models.Model): of Pearson's data import system. """ # to find an exam registration, we key off of the user and course_id. - # If multiple exams per course are possible, we would also need to add the + # If multiple exams per course are possible, we would also need to add the # exam_series_code. testcenter_user = models.ForeignKey(TestCenterUser, default=None) course_id = models.CharField(max_length=128, db_index=True) - + created_at = models.DateTimeField(auto_now_add=True, db_index=True) updated_at = models.DateTimeField(auto_now=True, db_index=True) # user_updated_at happens only when the user makes a change to their data, # and is something Pearson needs to know to manage updates. Unlike # updated_at, this will not get incremented when we do a batch data import. - # The appointment dates, the exam count, and the accommodation codes can be updated, + # The appointment dates, the exam count, and the accommodation codes can be updated, # but hopefully this won't happen often. user_updated_at = models.DateTimeField(db_index=True) - # "client_authorization_id" is our unique identifier for the authorization. + # "client_authorization_id" is our unique identifier for the authorization. # This must be present for an update or delete to be sent to Pearson. client_authorization_id = models.CharField(max_length=20, unique=True, db_index=True) @@ -376,10 +351,10 @@ class TestCenterRegistration(models.Model): eligibility_appointment_date_last = models.DateField(db_index=True) # this is really a list of codes, using an '*' as a delimiter. - # So it's not a choice list. We use the special value of ACCOMMODATION_REJECTED_CODE + # So it's not a choice list. We use the special value of ACCOMMODATION_REJECTED_CODE # to indicate the rejection of an accommodation request. accommodation_code = models.CharField(max_length=64, blank=True) - + # store the original text of the accommodation request. accommodation_request = models.CharField(max_length=1024, blank=True, db_index=True) @@ -387,7 +362,7 @@ class TestCenterRegistration(models.Model): uploaded_at = models.DateTimeField(null=True, db_index=True) # confirmation back from the test center, as well as timestamps - # on when they processed the request, and when we received + # on when they processed the request, and when we received # confirmation back. processed_at = models.DateTimeField(null=True, db_index=True) upload_status = models.CharField(max_length=20, blank=True, db_index=True) # 'Error' or 'Accepted' @@ -397,11 +372,11 @@ class TestCenterRegistration(models.Model): # (However, it may never be set if we are always initiating such candidate creation.) authorization_id = models.IntegerField(null=True, db_index=True) confirmed_at = models.DateTimeField(null=True, db_index=True) - + @property def candidate_id(self): return self.testcenter_user.candidate_id - + @property def client_candidate_id(self): return self.testcenter_user.client_candidate_id @@ -414,15 +389,15 @@ class TestCenterRegistration(models.Model): return 'Add' else: # TODO: decide what to send when we have uploaded an initial version, - # but have not received confirmation back from that upload. If the + # but have not received confirmation back from that upload. If the # registration here has been changed, then we don't know if this changed - # registration should be submitted as an 'add' or an 'update'. + # registration should be submitted as an 'add' or an 'update'. # - # If the first registration were lost or in error (e.g. bad code), + # If the first registration were lost or in error (e.g. bad code), # the second should be an "Add". If the first were processed successfully, # then the second should be an "Update". We just don't know.... return 'Update' - + @property def exam_authorization_count(self): # TODO: figure out if this should really go in the database (with a default value). @@ -447,7 +422,7 @@ class TestCenterRegistration(models.Model): @staticmethod def _generate_authorization_id(): return TestCenterUser._generate_edx_id("edXexam") - + @staticmethod def _create_client_authorization_id(): """ @@ -459,8 +434,8 @@ class TestCenterRegistration(models.Model): while TestCenterRegistration.objects.filter(client_authorization_id=auth_id).exists(): auth_id = TestCenterRegistration._generate_authorization_id() return auth_id - - # methods for providing registration status details on registration page: + + # methods for providing registration status details on registration page: @property def demographics_is_accepted(self): return self.testcenter_user.is_accepted @@ -468,7 +443,7 @@ class TestCenterRegistration(models.Model): @property def demographics_is_rejected(self): return self.testcenter_user.is_rejected - + @property def demographics_is_pending(self): return self.testcenter_user.is_pending @@ -480,7 +455,7 @@ class TestCenterRegistration(models.Model): @property def accommodation_is_rejected(self): return len(self.accommodation_request) > 0 and self.accommodation_code == ACCOMMODATION_REJECTED_CODE - + @property def accommodation_is_pending(self): return len(self.accommodation_request) > 0 and len(self.accommodation_code) == 0 @@ -492,20 +467,20 @@ class TestCenterRegistration(models.Model): @property def registration_is_accepted(self): return self.upload_status == TEST_CENTER_STATUS_ACCEPTED - + @property def registration_is_rejected(self): return self.upload_status == TEST_CENTER_STATUS_ERROR - + @property def registration_is_pending(self): return not self.registration_is_accepted and not self.registration_is_rejected - # methods for providing registration status summary on dashboard page: + # methods for providing registration status summary on dashboard page: @property def is_accepted(self): return self.registration_is_accepted and self.demographics_is_accepted - + @property def is_rejected(self): return self.registration_is_rejected or self.demographics_is_rejected @@ -513,17 +488,17 @@ class TestCenterRegistration(models.Model): @property def is_pending(self): return not self.is_accepted and not self.is_rejected - + def get_accommodation_codes(self): return self.accommodation_code.split('*') def get_accommodation_names(self): - return [ ACCOMMODATION_CODE_DICT.get(code, "Unknown code " + code) for code in self.get_accommodation_codes() ] + return [ ACCOMMODATION_CODE_DICT.get(code, "Unknown code " + code) for code in self.get_accommodation_codes() ] @property def registration_signup_url(self): return settings.PEARSONVUE_SIGNINPAGE_URL - + class TestCenterRegistrationForm(ModelForm): class Meta: model = TestCenterRegistration @@ -534,19 +509,19 @@ class TestCenterRegistrationForm(ModelForm): if code and len(code) > 0: return code.strip() return code - + def update_and_save(self): registration = self.save(commit=False) # create additional values here: registration.user_updated_at = datetime.utcnow() registration.upload_status = '' registration.save() - log.info("Updated registration information for user's test center exam registration: username \"{}\" course \"{}\", examcode \"{}\"".format(registration.testcenter_user.user.username, registration.course_id, registration.exam_series_code)) + log.info("Updated registration information for user's test center exam registration: username \"{}\" course \"{}\", examcode \"{}\"".format(registration.testcenter_user.user.username, registration.course_id, registration.exam_series_code)) # TODO: add validation code for values added to accommodation_code field. - - - + + + def get_testcenter_registration(user, course_id, exam_series_code): try: tcu = TestCenterUser.objects.get(user=user) @@ -564,7 +539,7 @@ def unique_id_for_user(user): e.g. personalized survey links. """ # include the secret key as a salt, and to make the ids unique across - # different LMS installs. + # different LMS installs. h = hashlib.md5() h.update(settings.SECRET_KEY) h.update(str(user.id)) @@ -646,7 +621,20 @@ class CourseEnrollmentAllowed(models.Model): #cache_relation(User.profile) -#### Helper methods for use from python manage.py shell. +#### Helper methods for use from python manage.py shell and other classes. + +def get_user_by_username_or_email(username_or_email): + """ + Return a User object, looking up by email if username_or_email contains a + '@', otherwise by username. + + Raises: + User.DoesNotExist is lookup fails. + """ + if '@' in username_or_email: + return User.objects.get(email=username_or_email) + else: + return User.objects.get(username=username_or_email) def get_user(email): @@ -737,167 +725,3 @@ def update_user_information(sender, instance, created, **kwargs): log.error(unicode(e)) log.error("update user info to discussion failed for user with id: " + str(instance.id)) - -########################## REPLICATION SIGNALS ################################# -# @receiver(post_save, sender=User) -def replicate_user_save(sender, **kwargs): - user_obj = kwargs['instance'] - if not should_replicate(user_obj): - return - for course_db_name in db_names_to_replicate_to(user_obj.id): - replicate_user(user_obj, course_db_name) - - -# @receiver(post_save, sender=CourseEnrollment) -def replicate_enrollment_save(sender, **kwargs): - """This is called when a Student enrolls in a course. It has to do the - following: - - 1. Make sure the User is copied into the Course DB. It may already exist - (someone deleting and re-adding a course). This has to happen first or - the foreign key constraint breaks. - 2. Replicate the CourseEnrollment. - 3. Replicate the UserProfile. - """ - if not is_portal(): - return - - enrollment_obj = kwargs['instance'] - log.debug("Replicating user because of new enrollment") - for course_db_name in db_names_to_replicate_to(enrollment_obj.user.id): - replicate_user(enrollment_obj.user, course_db_name) - - log.debug("Replicating enrollment because of new enrollment") - replicate_model(CourseEnrollment.save, enrollment_obj, enrollment_obj.user_id) - - log.debug("Replicating user profile because of new enrollment") - user_profile = UserProfile.objects.get(user_id=enrollment_obj.user_id) - replicate_model(UserProfile.save, user_profile, enrollment_obj.user_id) - - -# @receiver(post_delete, sender=CourseEnrollment) -def replicate_enrollment_delete(sender, **kwargs): - enrollment_obj = kwargs['instance'] - return replicate_model(CourseEnrollment.delete, enrollment_obj, enrollment_obj.user_id) - - -# @receiver(post_save, sender=UserProfile) -def replicate_userprofile_save(sender, **kwargs): - """We just updated the UserProfile (say an update to the name), so push that - change to all Course DBs that we're enrolled in.""" - user_profile_obj = kwargs['instance'] - return replicate_model(UserProfile.save, user_profile_obj, user_profile_obj.user_id) - - -######### Replication functions ######### -USER_FIELDS_TO_COPY = ["id", "username", "first_name", "last_name", "email", - "password", "is_staff", "is_active", "is_superuser", - "last_login", "date_joined"] - - -def replicate_user(portal_user, course_db_name): - """Replicate a User to the correct Course DB. This is more complicated than - it should be because Askbot extends the auth_user table and adds its own - fields. So we need to only push changes to the standard fields and leave - the rest alone so that Askbot changes at the Course DB level don't get - overridden. - """ - try: - course_user = User.objects.using(course_db_name).get(id=portal_user.id) - log.debug("User {0} found in Course DB, replicating fields to {1}" - .format(course_user, course_db_name)) - except User.DoesNotExist: - log.debug("User {0} not found in Course DB, creating copy in {1}" - .format(portal_user, course_db_name)) - course_user = User() - - for field in USER_FIELDS_TO_COPY: - setattr(course_user, field, getattr(portal_user, field)) - - mark_handled(course_user) - course_user.save(using=course_db_name) - unmark(course_user) - - -def replicate_model(model_method, instance, user_id): - """ - model_method is the model action that we want replicated. For instance, - UserProfile.save - """ - if not should_replicate(instance): - return - - course_db_names = db_names_to_replicate_to(user_id) - log.debug("Replicating {0} for user {1} to DBs: {2}" - .format(model_method, user_id, course_db_names)) - - mark_handled(instance) - for db_name in course_db_names: - model_method(instance, using=db_name) - unmark(instance) - - -######### Replication Helpers ######### - - -def is_valid_course_id(course_id): - """Right now, the only database that's not a course database is 'default'. - I had nicer checking in here originally -- it would scan the courses that - were in the system and only let you choose that. But it was annoying to run - tests with, since we don't have course data for some for our course test - databases. Hence the lazy version. - """ - return course_id != 'default' - - -def is_portal(): - """Are we in the portal pool? Only Portal servers are allowed to replicate - their changes. For now, only Portal servers see multiple DBs, so we use - that to decide.""" - return len(settings.DATABASES) > 1 - - -def db_names_to_replicate_to(user_id): - """Return a list of DB names that this user_id is enrolled in.""" - return [c.course_id - for c in CourseEnrollment.objects.filter(user_id=user_id) - if is_valid_course_id(c.course_id)] - - -def marked_handled(instance): - """Have we marked this instance as being handled to avoid infinite loops - caused by saving models in post_save hooks for the same models?""" - return hasattr(instance, '_do_not_copy_to_course_db') and instance._do_not_copy_to_course_db - - -def mark_handled(instance): - """You have to mark your instance with this function or else we'll go into - an infinite loop since we're putting listeners on Model saves/deletes and - the act of replication requires us to call the same model method. - - We create a _replicated attribute to differentiate the first save of this - model vs. the duplicate save we force on to the course database. Kind of - a hack -- suggestions welcome. - """ - instance._do_not_copy_to_course_db = True - - -def unmark(instance): - """If we don't unmark a model after we do replication, then consecutive - save() calls won't be properly replicated.""" - instance._do_not_copy_to_course_db = False - - -def should_replicate(instance): - """Should this instance be replicated? We need to be a Portal server and - the instance has to not have been marked_handled.""" - if marked_handled(instance): - # Basically, avoid an infinite loop. You should - log.debug("{0} should not be replicated because it's been marked" - .format(instance)) - return False - if not is_portal(): - log.debug("{0} should not be replicated because we're not a portal." - .format(instance)) - return False - return True diff --git a/common/djangoapps/student/tests.py b/common/djangoapps/student/tests.py index 4c7c9e2592..8ce407bcd1 100644 --- a/common/djangoapps/student/tests.py +++ b/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""" diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 7f97ca69dc..6e3e2cfa39 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/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): @@ -360,6 +360,41 @@ class CourseDescriptor(SequenceDescriptor): def show_calculator(self): 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): """ diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 1deceac5d0..57d7780d95 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 554e89ac74..7cd91223e3 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/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): \ """.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) + + diff --git a/common/static/js/course_groups/cohorts.js b/common/static/js/course_groups/cohorts.js new file mode 100644 index 0000000000..aa3ce34b5b --- /dev/null +++ b/common/static/js/course_groups/cohorts.js @@ -0,0 +1,256 @@ +// 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 = $('
    • '); + $("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($("
  • ").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 = $('' + + '' + + ''); + var current_cohort_id = cohort_id; + + $(".name", tr).text(item.name); + $(".username", tr).text(item.username); + $(".email", tr).text(item.email); + + $(".remove", tr).html('remove') + .click(function() { + remove_user_from_cohort(item.username, current_cohort_id, tr); + }); + + 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 = $('
  • ') + 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(); +}) + diff --git a/doc/xml-format.md b/doc/xml-format.md index 8138de4d7e..f4fd1054cb 100644 --- a/doc/xml-format.md +++ b/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 diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index f81a281511..89a1496eca 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/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""" diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index c00aa25dae..760ccb1d05 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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. diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index f2cb4ccb15..23f2afa037 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -24,9 +24,9 @@ urlpatterns = patterns('django_comment_client.base.views', url(r'comments/(?P[\w\-]+)/downvote$', 'vote_for_comment', {'value': 'down'}, name='downvote_comment'), url(r'comments/(?P[\w\-]+)/unvote$', 'undo_vote_for_comment', name='undo_vote_for_comment'), - url(r'(?P[\w\-]+)/threads/create$', 'create_thread', name='create_thread'), + url(r'^(?P[\w\-.]+)/threads/create$', 'create_thread', name='create_thread'), # TODO should we search within the board? - url(r'(?P[\w\-]+)/threads/search_similar$', 'search_similar_threads', name='search_similar_threads'), - url(r'(?P[\w\-]+)/follow$', 'follow_commentable', name='follow_commentable'), - url(r'(?P[\w\-]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'), + url(r'^(?P[\w\-.]+)/threads/search_similar$', 'search_similar_threads', name='search_similar_threads'), + url(r'^(?P[\w\-.]+)/follow$', 'follow_commentable', name='follow_commentable'), + url(r'^(?P[\w\-.]+)/unfollow$', 'unfollow_commentable', name='unfollow_commentable'), ) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 63d69427c9..777c7bafce 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/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) diff --git a/lms/djangoapps/django_comment_client/forum/urls.py b/lms/djangoapps/django_comment_client/forum/urls.py index 526ae3e582..1e676dee87 100644 --- a/lms/djangoapps/django_comment_client/forum/urls.py +++ b/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\w+)/followed$', 'followed_threads', name='followed_threads'), url(r'users/(?P\w+)$', 'user_profile', name='user_profile'), - url(r'(?P[\w\-]+)/threads/(?P\w+)$', 'single_thread', name='single_thread'), - url(r'(?P[\w\-]+)/inline$', 'inline_discussion', name='inline_discussion'), + url(r'^(?P[\w\-.]+)/threads/(?P\w+)$', 'single_thread', name='single_thread'), + url(r'^(?P[\w\-.]+)/inline$', 'inline_discussion', name='inline_discussion'), url(r'', 'forum_form_discussion', name='forum_form_discussion'), ) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 35e7fd6618..6a182665a8 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/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 = { diff --git a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py index 3faa846033..958b67cdb3 100644 --- a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py +++ b/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"]: diff --git a/lms/djangoapps/django_comment_client/tests.py b/lms/djangoapps/django_comment_client/tests.py index 2f3126be2c..ac059a1e3f 100644 --- a/lms/djangoapps/django_comment_client/tests.py +++ b/lms/djangoapps/django_comment_client/tests.py @@ -1,11 +1,17 @@ -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)) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 3094367491..3c9669ac37 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/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(): diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 2cf3bbb0a9..f0a0ef324a 100644 --- a/lms/djangoapps/instructor/views.py +++ b/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+="Module state successfully reset!" 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+=" Progress page for username: {1} with email address: {2}.".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 += "

    {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')) @@ -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] diff --git a/lms/envs/common.py b/lms/envs/common.py index 5e5ea86a4e..16472795e0 100644 --- a/lms/envs/common.py +++ b/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 diff --git a/lms/envs/test.py b/lms/envs/test.py index 8b546549eb..a045baa669 100644 --- a/lms/envs/test.py +++ b/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 diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 424250033e..6fd31b0823 100644 --- a/lms/lib/comment_client/thread.py +++ b/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 diff --git a/lms/templates/course_groups/cohort_management.html b/lms/templates/course_groups/cohort_management.html new file mode 100644 index 0000000000..239863beeb --- /dev/null +++ b/lms/templates/course_groups/cohort_management.html @@ -0,0 +1,41 @@ +
    +

    Cohort groups

    + +
    + Show cohorts +
    + +
      +
    + + + + + + +
    diff --git a/lms/templates/course_groups/debug.html b/lms/templates/course_groups/debug.html new file mode 100644 index 0000000000..d8bbc324de --- /dev/null +++ b/lms/templates/course_groups/debug.html @@ -0,0 +1,16 @@ + + + + <%block name="title">edX + + + + + + + +<%include file="/course_groups/cohort_management.html" /> + + + + diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 4d46505705..7b177c6b6c 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -6,6 +6,8 @@ <%static:css group='course'/> + + <%include file="/courseware/course_navigation.html" args="active_page='instructor'" /> @@ -282,16 +284,21 @@ function goto( mode)

    + + %if course.is_cohorted: + <%include file="/course_groups/cohort_management.html" /> + %endif + %endif %endif +##----------------------------------------------------------------------------- %if msg:

    ${msg}

    %endif ##----------------------------------------------------------------------------- -##----------------------------------------------------------------------------- %if datatable and modeflag.get('Psychometrics') is None: diff --git a/lms/urls.py b/lms/urls.py index 9d590387e4..3353c8a6ba 100644 --- a/lms/urls.py +++ b/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[^/]+/[^/]+/[^/]+)/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[^/]+/[^/]+/[^/]+)/cohorts$', + 'course_groups.views.list_cohorts', name="cohorts"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/add$', + 'course_groups.views.add_cohort', + name="add_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/(?P[0-9]+)$', + 'course_groups.views.users_in_cohort', + name="list_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/(?P[0-9]+)/add$', + 'course_groups.views.add_users_to_cohort', + name="add_to_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/(?P[0-9]+)/delete$', + 'course_groups.views.remove_user_from_cohort', + name="remove_from_cohort"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/cohorts/debug$', + 'course_groups.views.debug_cohort_mgmt', + name="debug_cohort_mgmt"), + ) # discussion forums live within courseware, so courseware must be enabled first