From dba39e243174f28a1245103721194ef33a7300ca Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 18 Jan 2013 12:50:25 -0500 Subject: [PATCH 01/37] add group support to lms --- lms/djangoapps/courseware/courses.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 7c0d30ebd8..90ebbd33da 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -58,6 +58,29 @@ def get_opt_course_with_access(user, course_id, action): return get_course_with_access(user, course_id, action) + + +def is_course_cohorted(course_id): + """ + given a course id, return a boolean for whether or not the course 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 + and if the course is not cohorted or the user is an instructor, return None + + """ + +def is_commentable_cohorted(course_id,commentable_id) + """ + given a course and a commentable id, return whether or not this commentable is cohorted + + """ + + + 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""" From b8e3cfcf96a143294df5482f904d8e45c10becf9 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 18 Jan 2013 13:50:05 -0500 Subject: [PATCH 02/37] added get_cohort_ids --- lms/djangoapps/courseware/courses.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 90ebbd33da..8fd4497baf 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -78,8 +78,12 @@ def is_commentable_cohorted(course_id,commentable_id) given a course and a commentable id, return whether or not this commentable is cohorted """ + - +def get_cohort_ids(course_id): + """ + given a course id, return an array of all cohort ids for that course (needed for UI + """ def course_image_url(course): """Try to look up the image url for the course. If it's not found, From b26c59bda0292c801400d104abb0ba5097ce18cf Mon Sep 17 00:00:00 2001 From: Kevin Chugh Date: Sun, 20 Jan 2013 01:53:25 -0500 Subject: [PATCH 03/37] fix syntax error --- lms/djangoapps/courseware/courses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 8fd4497baf..551e459492 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -73,7 +73,7 @@ def get_cohort_id(user, course_id): """ -def is_commentable_cohorted(course_id,commentable_id) +def is_commentable_cohorted(course_id,commentable_id): """ given a course and a commentable id, return whether or not this commentable is cohorted From 59b3dc61a286dbac4a66d90cea1a98b43dab07a6 Mon Sep 17 00:00:00 2001 From: Kevin Chugh Date: Wed, 23 Jan 2013 16:25:26 -0500 Subject: [PATCH 04/37] lms producing grouped content --- lms/djangoapps/courseware/courses.py | 1 + .../django_comment_client/base/views.py | 19 +++++++++++++++++++ .../django_comment_client/forum/views.py | 7 +++++++ lms/lib/comment_client/thread.py | 4 ++-- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 551e459492..057ef42a58 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -72,6 +72,7 @@ def get_cohort_id(user, course_id): and if the course is not cohorted or the user is an instructor, return None """ + return 101 def is_commentable_cohorted(course_id,commentable_id): """ diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 63d69427c9..d1948c3fc7 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -21,6 +21,7 @@ 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 courseware.courses import get_cohort_id from django_comment_client.utils import JsonResponse, JsonError, extract, get_courseware_context @@ -83,6 +84,24 @@ def create_thread(request, course_id, commentable_id): 'course_id' : course_id, 'user_id' : request.user.id, }) + + #now cohort id + #if the group id came in from the form, set it there, otherwise, + #see if the user and the commentable are cohorted + print post + + group_id = None + + if 'group_id' in post: + group_id = post['group_id'] + + + if group_id is None: + group_id = get_cohort_id(request.user, course_id) + + if group_id is not None: + 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/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 35e7fd6618..0e8a044097 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -11,6 +11,7 @@ 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 courseware.courses import get_cohort_id from courseware.access import has_access from urllib import urlencode @@ -58,6 +59,12 @@ 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']))) 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 From def2d8adf22eea0e941deb615b6543e1c531ab5d Mon Sep 17 00:00:00 2001 From: Kevin Chugh Date: Thu, 24 Jan 2013 14:20:18 -0500 Subject: [PATCH 05/37] fix cohorts in create, and read, and update regular expressions to fix courses with periods not working in General commentable --- lms/djangoapps/courseware/courses.py | 10 +---- lms/djangoapps/courseware/views.py | 1 + .../django_comment_client/base/urls.py | 8 ++-- .../django_comment_client/base/views.py | 40 ++++++++++++------- .../django_comment_client/forum/urls.py | 4 +- .../django_comment_client/forum/views.py | 3 +- .../commands/seed_permissions_roles.py | 2 +- 7 files changed, 37 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index b4e8da2633..74f5e4c54f 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -84,21 +84,13 @@ def get_opt_course_with_access(user, course_id, action): return get_course_with_access(user, course_id, action) - - -def is_course_cohorted(course_id): - """ - given a course id, return a boolean for whether or not the course 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 and if the course is not cohorted or the user is an instructor, return None """ - return 101 + return 127 def is_commentable_cohorted(course_id,commentable_id): """ diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b1c4a5e9a9..f170f3fb86 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 d1948c3fc7..c1e188ff1a 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -21,11 +21,11 @@ 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 courseware.courses import get_cohort_id +from courseware.courses 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 def permitted(fn): @@ -59,10 +59,17 @@ def ajax_content_response(request, course_id, content, template_name): 'annotated_content_info': annotated_content_info, }) + + +def is_moderator(user, course_id): + cached_has_permission(user, "see_all_cohorts", course_id) + @require_POST @login_required @permitted def create_thread(request, course_id, commentable_id): + print "\n\n\n\n\n*******************" + print commentable_id course = get_course_with_access(request.user, course_id, 'load') post = request.POST @@ -85,23 +92,28 @@ def create_thread(request, course_id, commentable_id): 'user_id' : request.user.id, }) - #now cohort id + + #now cohort the thread if the commentable is cohorted #if the group id came in from the form, set it there, otherwise, #see if the user and the commentable are cohorted - print post + if is_commentable_cohorted(course_id,commentable_id): + if 'group_id' in post: #if a group id was submitted in the form + posted_group_id = post['group_id'] + else: + post_group_id = None - group_id = None - - if 'group_id' in post: - group_id = post['group_id'] - - - if group_id is None: - group_id = get_cohort_id(request.user, course_id) + user_group_id = get_cohort_id(request.user, course_id) - if group_id is not None: + if is_moderator(request.user,course_id): + if post_group_id is None: + group_id = user_group_id + else: + group_id = post_group_id + else: + 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 0e8a044097..443329ec1f 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -34,7 +34,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, @@ -64,6 +63,8 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG group_id = get_cohort_id(user,course_id); if group_id: default_query_params["group_id"] = group_id; + print("\n\n\n\n\n****************GROUP ID IS ") + print group_id query_params = merge_dict(default_query_params, strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags', 'commentable_ids']))) 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"]: From c655f62f7a8f0690efa910989aacc09b32ce02f2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 12:08:31 -0500 Subject: [PATCH 06/37] Remove out-of-date text about user replication, askbot --- common/djangoapps/student/models.py | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index f13a691215..00819d7dc4 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 From 0e78eaaf80f21bbc5bdb76f019d4e33396c62a21 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 13:20:34 -0500 Subject: [PATCH 07/37] Add a course_groups djangoapp with a CourseUserGroup model. --- common/djangoapps/course_groups/__init__.py | 0 common/djangoapps/course_groups/models.py | 27 +++++++++++++++++++++ lms/envs/common.py | 1 + 3 files changed, 28 insertions(+) create mode 100644 common/djangoapps/course_groups/__init__.py create mode 100644 common/djangoapps/course_groups/models.py 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/models.py b/common/djangoapps/course_groups/models.py new file mode 100644 index 0000000000..5423b6ec16 --- /dev/null +++ b/common/djangoapps/course_groups/models.py @@ -0,0 +1,27 @@ +from django.contrib.auth.models import User +from django.db import models + +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 + GROUP_TYPE_CHOICES = (('cohort', 'Cohort'),) + group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES) 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 From 1b9a3557eb60b8be8539e6adcfb7e4a067b0cc37 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 13:40:13 -0500 Subject: [PATCH 08/37] get_cohort() function and test --- common/djangoapps/course_groups/models.py | 29 ++++++++++++++++++- .../djangoapps/course_groups/tests/tests.py | 21 ++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 common/djangoapps/course_groups/tests/tests.py diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 5423b6ec16..36c9927e18 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -23,5 +23,32 @@ class CourseUserGroup(models.Model): # For now, only have group type 'cohort', but adding a type field to support # things like 'question_discussion', 'friends', 'off-line-class', etc - GROUP_TYPE_CHOICES = (('cohort', 'Cohort'),) + COHORT = 'cohort' + GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'),) group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES) + + +def get_cohort(user, course_id): + """ + Given a django User and a course_id, return the user's cohort. 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 User has a cohort, or None. + """ + group_type = CourseUserGroup.COHORT + try: + group = CourseUserGroup.objects.get(course_id=course_id, group_type=group_type, + users__id=user.id) + except CourseUserGroup.DoesNotExist: + group = None + + if group: + return group + + # TODO: add auto-cohorting logic here + return None diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py new file mode 100644 index 0000000000..89c77c5b65 --- /dev/null +++ b/common/djangoapps/course_groups/tests/tests.py @@ -0,0 +1,21 @@ +from django.contrib.auth.models import User +from nose.tools import assert_equals + +from course_groups.models import CourseUserGroup, get_cohort + +def test_get_cohort(): + course_id = "a/b/c" + cohort = CourseUserGroup.objects.create(name="TestCohort", course_id=course_id, + group_type=CourseUserGroup.COHORT) + + user = User.objects.create(username="test", email="a@b.com") + other_user = User.objects.create(username="test2", email="a2@b.com") + + cohort.users.add(user) + + got = get_cohort(user, course_id) + assert_equals(got.id, cohort.id, "Should find the right cohort") + + got = get_cohort(other_user, course_id) + assert_equals(got, None, "other_user shouldn't have a cohort") + From fa17913a91e4906e1fb53563cb3a4dfac69f041c Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:17:35 -0500 Subject: [PATCH 09/37] is_cohorted course descriptor property, docs --- common/lib/xmodule/xmodule/course_module.py | 11 +++++++++++ doc/xml-format.md | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 5416dae583..c0b3000258 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -360,6 +360,17 @@ 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 is_new(self): """ diff --git a/doc/xml-format.md b/doc/xml-format.md index 8138de4d7e..8f9e512ac1 100644 --- a/doc/xml-format.md +++ b/doc/xml-format.md @@ -258,6 +258,10 @@ 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). + - ... more to come. ('auto-cohort', how to auto cohort, etc) + * TODO: there are others ### Grading policy file contents From f005b70f3b8b5b4c68519c22f85fa153a173a212 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:17:58 -0500 Subject: [PATCH 10/37] Minor test cleanups --- .../lib/xmodule/xmodule/tests/test_import.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 554e89ac74..8fc8916dc3 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -52,6 +52,16 @@ class ImportTestCase(unittest.TestCase): '''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] + + def test_fallback(self): '''Check that malformed xml loads as an ErrorDescriptor.''' @@ -207,11 +217,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 +233,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 +277,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 +315,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() From 90a5703419b9322eb3d5c8130af6a655b938cb3e Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:18:06 -0500 Subject: [PATCH 11/37] Test for cohort config --- .../lib/xmodule/xmodule/tests/test_import.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 8fc8916dc3..8a5eda3882 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -353,3 +353,28 @@ 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) From 3488083e6bccbfbfcb3833350d8c7f4e2c2a2962 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 14:58:46 -0500 Subject: [PATCH 12/37] Add a get_course_cohorts function and test --- common/djangoapps/course_groups/models.py | 21 ++++++-- .../djangoapps/course_groups/tests/tests.py | 49 ++++++++++++++----- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 36c9927e18..46859a900e 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -40,15 +40,28 @@ def get_cohort(user, course_id): Returns: A CourseUserGroup object if the User has a cohort, or None. """ - group_type = CourseUserGroup.COHORT try: - group = CourseUserGroup.objects.get(course_id=course_id, group_type=group_type, - users__id=user.id) + 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 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)) diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py index 89c77c5b65..676643567d 100644 --- a/common/djangoapps/course_groups/tests/tests.py +++ b/common/djangoapps/course_groups/tests/tests.py @@ -1,21 +1,44 @@ +import django.test from django.contrib.auth.models import User -from nose.tools import assert_equals -from course_groups.models import CourseUserGroup, get_cohort +from course_groups.models import CourseUserGroup, get_cohort, get_course_cohorts -def test_get_cohort(): - course_id = "a/b/c" - cohort = CourseUserGroup.objects.create(name="TestCohort", course_id=course_id, - group_type=CourseUserGroup.COHORT) +class TestCohorts(django.test.TestCase): - user = User.objects.create(username="test", email="a@b.com") - other_user = User.objects.create(username="test2", email="a2@b.com") + def test_get_cohort(self): + course_id = "a/b/c" + cohort = CourseUserGroup.objects.create(name="TestCohort", course_id=course_id, + group_type=CourseUserGroup.COHORT) - cohort.users.add(user) + user = User.objects.create(username="test", email="a@b.com") + other_user = User.objects.create(username="test2", email="a2@b.com") - got = get_cohort(user, course_id) - assert_equals(got.id, cohort.id, "Should find the right cohort") + cohort.users.add(user) - got = get_cohort(other_user, course_id) - assert_equals(got, None, "other_user shouldn't have a cohort") + got = get_cohort(user, course_id) + self.assertEquals(got.id, cohort.id, "Should find the right cohort") + + got = get_cohort(other_user, course_id) + self.assertEquals(got, 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']) From 0d41a04490a20deac30e1216e63982e8940d88e2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 15:00:43 -0500 Subject: [PATCH 13/37] fix line length --- common/lib/xmodule/xmodule/discussion_module.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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) From ac8c59126d40e4fd7d70fc4113634885bcbd67c0 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 19 Jan 2013 15:05:41 -0500 Subject: [PATCH 14/37] Add note about non-unique topic id across course runs bug --- lms/djangoapps/django_comment_client/utils.py | 3 +++ 1 file changed, 3 insertions(+) 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(): From bab3b0c39e94b6e060b80e6e31a41a090bb3f972 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 21 Jan 2013 15:48:58 -0500 Subject: [PATCH 15/37] Add initial set of views for managing cohorts - lets user see and add cohorts - needs styling - needs to allow them to actually manage membership! --- common/djangoapps/course_groups/models.py | 51 +++++++- common/djangoapps/course_groups/views.py | 108 ++++++++++++++++ common/static/js/course_groups/cohorts.js | 118 ++++++++++++++++++ lms/djangoapps/instructor/views.py | 1 + .../course_groups/cohort_management.html | 25 ++++ lms/templates/course_groups/debug.html | 16 +++ .../courseware/instructor_dashboard.html | 7 +- lms/urls.py | 17 +++ 8 files changed, 341 insertions(+), 2 deletions(-) create mode 100644 common/djangoapps/course_groups/views.py create mode 100644 common/static/js/course_groups/cohorts.js create mode 100644 lms/templates/course_groups/cohort_management.html create mode 100644 lms/templates/course_groups/debug.html diff --git a/common/djangoapps/course_groups/models.py b/common/djangoapps/course_groups/models.py index 46859a900e..701cce0e6c 100644 --- a/common/djangoapps/course_groups/models.py +++ b/common/djangoapps/course_groups/models.py @@ -1,6 +1,10 @@ +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, @@ -27,7 +31,6 @@ class CourseUserGroup(models.Model): GROUP_TYPE_CHOICES = ((COHORT, 'Cohort'),) group_type = models.CharField(max_length=20, choices=GROUP_TYPE_CHOICES) - def get_cohort(user, course_id): """ Given a django User and a course_id, return the user's cohort. In classes with @@ -65,3 +68,49 @@ def get_course_cohorts(course_id): """ return list(CourseUserGroup.objects.filter(course_id=course_id, group_type=CourseUserGroup.COHORT)) + +### Helpers for cohor 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 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) + +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/views.py b/common/djangoapps/course_groups/views.py new file mode 100644 index 0000000000..9ee9935c3e --- /dev/null +++ b/common/djangoapps/course_groups/views.py @@ -0,0 +1,108 @@ +import json +from django_future.csrf import ensure_csrf_cookie +from django.contrib.auth.decorators import login_required +from django.core.context_processors import csrf +from django.core.urlresolvers import reverse +from django.http import HttpResponse, HttpResponseForbidden, Http404 +from django.shortcuts import redirect +import logging + +from courseware.courses import get_course_with_access +from mitxmako.shortcuts import render_to_response, render_to_string +from .models import CourseUserGroup +from . import models + +import track.views + + +log = logging.getLogger(__name__) + +def JsonHttpReponse(data): + """ + Return an HttpResponse with the data json-serialized and the right content type + header. Named to look like a class. + """ + return HttpResponse(json.dumps(data), content_type="application/json") + +@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') + + cohorts = [{'name': c.name, 'id': c.id} + for c in models.get_course_cohorts(course_id)] + + return JsonHttpReponse({'success': True, + 'cohorts': cohorts}) + + +@ensure_csrf_cookie +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') + + + if request.method != "POST": + raise Http404("Must POST to add cohorts") + + name = request.POST.get("name") + if not name: + return JsonHttpReponse({'success': False, + 'msg': "No name specified"}) + + try: + cohort = models.add_cohort(course_id, name) + except ValueError as err: + return JsonHttpReponse({'success': False, + 'msg': str(err)}) + + return JsonHttpReponse({'success': 'True', + 'cohort': { + 'id': cohort.id, + 'name': cohort.name + }}) + + +@ensure_csrf_cookie +def users_in_cohort(request, course_id, cohort_id): + """ + """ + get_course_with_access(request.user, course_id, 'staff') + + return JsonHttpReponse({'error': 'Not implemented'}) + + +@ensure_csrf_cookie +def add_users_to_cohort(request, course_id): + """ + """ + get_course_with_access(request.user, course_id, 'staff') + + return JsonHttpReponse({'error': 'Not implemented'}) + + +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/static/js/course_groups/cohorts.js b/common/static/js/course_groups/cohorts.js new file mode 100644 index 0000000000..7b1793dcf8 --- /dev/null +++ b/common/static/js/course_groups/cohorts.js @@ -0,0 +1,118 @@ +// 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 url = $(".cohort_manager").data('ajax_url'); + var self = this; + var error_list = $(".cohort_errors"); + var cohort_list = $(".cohort_list"); + var cohorts_display = $(".cohorts_display"); + var show_cohorts_button = $(".cohort_controls .show_cohorts"); + var add_cohort_input = $("#cohort-name"); + var add_cohort_button = $(".add_cohort"); + + function show_cohort(item) { + // item is a li that has a data-href link to the cohort base url + var el = $(this); + alert("would show you data about " + el.text() + " from " + el.data('href')); + } + + function add_to_cohorts_list(item) { + var li = $('
    • '); + $("a", li).text(item.name) + .data('href', url + '/' + item.id) + .addClass('link') + .click(show_cohort); + cohort_list.append(li); + }; + + function log_error(msg) { + error_list.empty(); + error_list.append($("
  • ").text(msg).addClass("error")); + }; + + function load_cohorts(response) { + cohort_list.empty(); + if (response && response.success) { + response.cohorts.forEach(add_to_cohorts_list); + } else { + log_error(response.msg || "There was an error loading cohorts"); + } + cohorts_display.show(); + }; + + 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"); + } + } + + show_cohorts_button.click(function() { + $.ajax(url).done(load_cohorts); + }); + + 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); + }); + + }; + + // prototype + module.prototype = { + constructor: module, + }; + + // return module + return module; +})(jQuery); + +$(window).load(function () { + var my_module = new CohortManager(); +}) + diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 2cf3bbb0a9..f8872082da 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -562,6 +562,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) diff --git a/lms/templates/course_groups/cohort_management.html b/lms/templates/course_groups/cohort_management.html new file mode 100644 index 0000000000..1512d09689 --- /dev/null +++ b/lms/templates/course_groups/cohort_management.html @@ -0,0 +1,25 @@ +
    +

    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..e9b9ae0354 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,19 @@ function goto( mode)

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

    {0}

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

    {0}

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

    Cohort groups

    -
    +
    Show cohorts
    -
      +
      -