From 3e5a2f9bded5eac1f3f1c867437a00fe648c28ba Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 12 Jan 2013 09:48:44 -0500 Subject: [PATCH 1/9] Add beta test group support * add a days_early_for_beta inherited policy option * students in the beta_testers_{COURSE} group get to see content that many days early * Clean up some docstrings related to time formats * Add basic test --- common/lib/xmodule/xmodule/timeparse.py | 7 ++- common/lib/xmodule/xmodule/x_module.py | 22 +++++++-- doc/xml-format.md | 1 + lms/djangoapps/courseware/access.py | 58 ++++++++++++++++++++++-- lms/djangoapps/courseware/tests/tests.py | 46 +++++++++++++++++-- 5 files changed, 120 insertions(+), 14 deletions(-) diff --git a/common/lib/xmodule/xmodule/timeparse.py b/common/lib/xmodule/xmodule/timeparse.py index 117105d085..36c0f725e5 100644 --- a/common/lib/xmodule/xmodule/timeparse.py +++ b/common/lib/xmodule/xmodule/timeparse.py @@ -7,8 +7,11 @@ TIME_FORMAT = "%Y-%m-%dT%H:%M" def parse_time(time_str): """ - Takes a time string in TIME_FORMAT, returns - it as a time_struct. Raises ValueError if the string is not in the right format. + Takes a time string in TIME_FORMAT + + Returns it as a time_struct. + + Raises ValueError if the string is not in the right format. """ return time.strptime(time_str, TIME_FORMAT) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 19a592191e..12de947a5e 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -414,7 +414,11 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): 'xqa_key', # TODO: This is used by the XMLModuleStore to provide for locations for # static files, and will need to be removed when that code is removed - 'data_dir' + 'data_dir', + # How many days early to show a course element to beta testers (float) + # intended to be set per-course, but can be overridden in for specific + # elements. Can be a float. + 'days_early_for_beta' ) # cdodge: this is a list of metadata names which are 'system' metadata @@ -497,12 +501,23 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): @property def start(self): """ - If self.metadata contains start, return it. Else return None. + If self.metadata contains a valid start time, return it as a time struct. + Else return None. """ if 'start' not in self.metadata: return None return self._try_parse_time('start') + @property + def days_early_for_beta(self): + """ + If self.metadata contains start, return the number, as a float. Else return None. + """ + if 'days_early_for_beta' not in self.metadata: + return None + return float(self.metadata['days_early_for_beta']) + + @property def own_metadata(self): """ @@ -715,7 +730,8 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): """ Parse an optional metadata key containing a time: if present, complain if it doesn't parse. - Return None if not present or invalid. + + Returns a time_struct, or None if metadata key is not present or is invalid. """ if key in self.metadata: try: diff --git a/doc/xml-format.md b/doc/xml-format.md index d7c5027a79..e4bbc0d4db 100644 --- a/doc/xml-format.md +++ b/doc/xml-format.md @@ -257,6 +257,7 @@ Supported fields at the course level: * "tabs" -- have custom tabs in the courseware. See below for details on config. * "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 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. * TODO: there are others ### Grading policy file contents diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 26f9fcdfd3..097d14b81e 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -4,13 +4,13 @@ like DISABLE_START_DATES""" import logging import time +from datetime import datetime, timedelta from django.conf import settings from xmodule.course_module import CourseDescriptor from xmodule.error_module import ErrorDescriptor from xmodule.modulestore import Location -from xmodule.timeparse import parse_time from xmodule.x_module import XModule, XModuleDescriptor from student.models import CourseEnrollmentAllowed @@ -73,7 +73,7 @@ def has_access(user, obj, action): raise TypeError("Unknown object type in has_access(): '{0}'" .format(type(obj))) -def get_access_group_name(obj,action): +def get_access_group_name(obj, action): ''' Returns group name for user group which has "action" access to the given object. @@ -226,9 +226,10 @@ def _has_access_descriptor(user, descriptor, action): # Check start date if descriptor.start is not None: now = time.gmtime() - if now > descriptor.start: + effective_start = _adjust_start_date_for_beta_testers(user, descriptor) + if now > effective_start: # after start date, everyone can see it - debug("Allow: now > start date") + debug("Allow: now > effective start date") return True # otherwise, need staff access return _has_staff_access_to_descriptor(user, descriptor) @@ -328,6 +329,15 @@ def _course_staff_group_name(location): """ return 'staff_%s' % Location(location).course +def _course_beta_test_group_name(location): + """ + Get the name of the beta tester group for a location. Right now, that's + beta_testers_COURSE. + + location: something that can passed to Location. + """ + return 'beta_testers_{0}'.format(Location(location).course) + def _course_instructor_group_name(location): """ @@ -348,6 +358,46 @@ def _has_global_staff_access(user): return False +def _adjust_start_date_for_beta_testers(user, descriptor): + """ + If user is in a beta test group, adjust the start date by the appropriate number of + days. + + Arguments: + user: A django user. May be anonymous. + descriptor: the XModuleDescriptor the user is trying to get access to, with a + non-None start date. + + Returns: + A time, in the same format as returned by time.gmtime(). Either the same as + start, or earlier for beta testers. + + NOTE: number of days to adjust should be cached to avoid looking it up thousands of + times per query. + + NOTE: For now, this function assumes that the descriptor's location is in the course + the user is looking at. Once we have proper usages and definitions per the XBlock + design, this should use the course the usage is in. + """ + if descriptor.days_early_for_beta is None: + # bail early if no beta testing is set up + return descriptor.start + + user_groups = [g.name for g in user.groups.all()] + + beta_group = _course_beta_test_group_name(descriptor.location) + if beta_group in user_groups: + debug("Adjust start time: user in group %s", beta_group) + # time_structs don't support subtraction, so convert to datetimes, + # subtract, convert back. + start_as_datetime = datetime(*descriptor.start[:6]) + delta = timedelta(descriptor.days_early_for_beta) + effective = start_as_datetime - delta + # ...and back to time_struct + return effective.timetuple() + + return descriptor.start + def _has_instructor_access_to_location(user, location): return _has_access_to_location(user, location, 'instructor') diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index b6ec6aff02..6972ded307 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -17,7 +17,8 @@ import xmodule.modulestore.django # Need access to internal func to put users in the right group from courseware import grades -from courseware.access import _course_staff_group_name +from courseware.access import (has_access, _course_staff_group_name, + _course_beta_test_group_name) from courseware.models import StudentModuleCache from student.models import Registration @@ -238,7 +239,7 @@ class PageLoader(ActivateLoginTestCase): n = 0 num_bad = 0 all_ok = True - for descriptor in module_store.modules[course_id].itervalues(): + for descriptor in module_store.modules[course_id].itervalues(): n += 1 print "Checking ", descriptor.location.url() #print descriptor.__class__, descriptor.location @@ -259,11 +260,11 @@ class PageLoader(ActivateLoginTestCase): # check content to make sure there were no rendering failures content = resp.content if content.find("this module is temporarily unavailable")>=0: - msg = "ERROR unavailable module " + msg = "ERROR unavailable module " all_ok = False num_bad += 1 elif isinstance(descriptor, ErrorDescriptor): - msg = "ERROR error descriptor loaded: " + msg = "ERROR error descriptor loaded: " msg = msg + descriptor.definition['data']['error_msg'] all_ok = False num_bad += 1 @@ -286,7 +287,7 @@ class TestCoursesLoadTestCase(PageLoader): # xmodule.modulestore.django.modulestore().collection.drop() # store = xmodule.modulestore.django.modulestore() # is there a way to empty the store? - + def test_toy_course_loads(self): self.check_pages_load('toy', TEST_DATA_DIR, modulestore()) @@ -453,6 +454,9 @@ class TestViewAuth(PageLoader): """Check that enrollment periods work""" self.run_wrapped(self._do_test_enrollment_period) + def test_beta_period(self): + """Check that beta-test access works""" + self.run_wrapped(self._do_test_beta_period) def _do_test_dark_launch(self): """Actually do the test, relying on settings to be right.""" @@ -618,6 +622,38 @@ class TestViewAuth(PageLoader): self.unenroll(self.toy) self.assertTrue(self.try_enroll(self.toy)) + def _do_test_beta_period(self): + """Actually test beta periods, relying on settings to be right.""" + + # trust, but verify :) + self.assertFalse(settings.MITX_FEATURES['DISABLE_START_DATES']) + + # Make courses start in the future + tomorrow = time.time() + 24 * 3600 + nextday = tomorrow + 24 * 3600 + yesterday = time.time() - 24 * 3600 + + # toy course's hasn't started + self.toy.metadata['start'] = stringify_time(time.gmtime(tomorrow)) + self.assertFalse(self.toy.has_started()) + + # but should be accessible for beta testers + self.toy.metadata['days_early_for_beta'] = '2' + + # student user shouldn't see it + student_user = user(self.student) + self.assertFalse(has_access(student_user, self.toy, 'load')) + + # now add the student to the beta test group + group_name = _course_beta_test_group_name(self.toy.location) + g = Group.objects.create(name=group_name) + g.user_set.add(student_user) + + # now the student should see it + self.assertTrue(has_access(student_user, self.toy, 'load')) + + + @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestCourseGrader(PageLoader): """Check that a course gets graded properly""" From 58a9fdeddc952d4be9ccc248ad15aba6f53b52cf Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 12 Jan 2013 10:10:01 -0500 Subject: [PATCH 2/9] add note about settings needed for manual testing --- lms/djangoapps/courseware/access.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 097d14b81e..f4e4a95e07 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -378,6 +378,9 @@ def _adjust_start_date_for_beta_testers(user, descriptor): NOTE: For now, this function assumes that the descriptor's location is in the course the user is looking at. Once we have proper usages and definitions per the XBlock design, this should use the course the usage is in. + + NOTE: If testing manually, make sure MITX_FEATURES['DISABLE_START_DATES'] = False + in envs/dev.py! """ if descriptor.days_early_for_beta is None: # bail early if no beta testing is set up From 1a5fc065ad67249d2b8740a4b1f959e744e818e0 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 14 Jan 2013 08:08:31 -0500 Subject: [PATCH 3/9] Add beta test group management view to instructor dashboard * still needs to support uploading a list * also want to clean up and refactor the code a bit --- lms/djangoapps/courseware/access.py | 4 +- lms/djangoapps/courseware/tests/tests.py | 4 +- lms/djangoapps/instructor/tests.py | 2 +- lms/djangoapps/instructor/views.py | 148 +++++++++++++----- .../courseware/instructor_dashboard.html | 20 ++- 5 files changed, 133 insertions(+), 45 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index f4e4a95e07..64bef8fa1a 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -329,7 +329,7 @@ def _course_staff_group_name(location): """ return 'staff_%s' % Location(location).course -def _course_beta_test_group_name(location): +def course_beta_test_group_name(location): """ Get the name of the beta tester group for a location. Right now, that's beta_testers_COURSE. @@ -388,7 +388,7 @@ def _adjust_start_date_for_beta_testers(user, descriptor): user_groups = [g.name for g in user.groups.all()] - beta_group = _course_beta_test_group_name(descriptor.location) + beta_group = course_beta_test_group_name(descriptor.location) if beta_group in user_groups: debug("Adjust start time: user in group %s", beta_group) # time_structs don't support subtraction, so convert to datetimes, diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 6972ded307..eeb304b193 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -18,7 +18,7 @@ import xmodule.modulestore.django # Need access to internal func to put users in the right group from courseware import grades from courseware.access import (has_access, _course_staff_group_name, - _course_beta_test_group_name) + course_beta_test_group_name) from courseware.models import StudentModuleCache from student.models import Registration @@ -645,7 +645,7 @@ class TestViewAuth(PageLoader): self.assertFalse(has_access(student_user, self.toy, 'load')) # now add the student to the beta test group - group_name = _course_beta_test_group_name(self.toy.location) + group_name = course_beta_test_group_name(self.toy.location) g = Group.objects.create(name=group_name) g.user_set.add(student_user) diff --git a/lms/djangoapps/instructor/tests.py b/lms/djangoapps/instructor/tests.py index 2d17cee47d..57c0436921 100644 --- a/lms/djangoapps/instructor/tests.py +++ b/lms/djangoapps/instructor/tests.py @@ -179,7 +179,7 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader): self.assertTrue(response.content.find('Removed "{0}" from "{1}" forum role = "{2}"'.format(username, course.id, rolename))>=0) self.assertFalse(has_forum_access(username, course.id, rolename)) - def test_add_and_readd_forum_admin_users(self): + def test_add_and_read_forum_admin_users(self): course = self.toy self.initialize_roles(course.id) url = reverse('instructor_dashboard', kwargs={'course_id': course.id}) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 2d58799efe..1519d876bb 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -19,9 +19,13 @@ from mitxmako.shortcuts import render_to_response from django.core.urlresolvers import reverse from courseware import grades -from courseware.access import has_access, get_access_group_name -from courseware.courses import get_course_with_access -from django_comment_client.models import Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA +from courseware.access import (has_access, get_access_group_name, + course_beta_test_group_name) +from courseware.courses import get_course_with_access +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 student.models import CourseEnrollment, CourseEnrollmentAllowed @@ -44,13 +48,12 @@ FORUM_ROLE_REMOVE = 'remove' @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) - def instructor_dashboard(request, course_id): """Display the instructor dashboard for a course.""" course = get_course_with_access(request.user, course_id, 'staff') instructor_access = has_access(request.user, course, 'instructor') # an instructor can manage staff lists - + forum_admin_access = has_forum_access(request.user, course_id, FORUM_ROLE_ADMINISTRATOR) msg = '' @@ -105,6 +108,16 @@ def instructor_dashboard(request, course_id): except Group.DoesNotExist: group = Group(name=grpname) # create the group group.save() + + def get_beta_group(course): + """ + Get the group for beta testers of course. + """ + # Not using get_group because there is no access control action called + # 'beta', so adding it to get_access_group_name doesn't really make + # sense. + name = course_beta_test_group_name(course.location) + (group, created) = Group.objects.get_or_create(name=name) return group # process actions from form POST @@ -310,26 +323,64 @@ def instructor_dashboard(request, course_id): user.groups.remove(group) track.views.server_track(request, 'remove-instructor {0}'.format(user), {}, page='idashboard') + #---------------------------------------- + # Group management + + elif 'List beta testers' in action: + group = get_beta_group(course) + msg += 'Beta test group = {0}'.format(group.name) + datatable = _group_members_table(group, "List of beta_testers", course_id) + track.views.server_track(request, 'list-beta-testers', {}, page='idashboard') + + elif action == 'Add beta testers': + uname = request.POST['betausers'] + try: + user = User.objects.get(username=uname) + except User.DoesNotExist: + msg += 'Error: unknown username "{0}"'.format(uname) + user = None + if user is not None: + group = get_beta_group(course) + msg += 'Added {0} to beta testers group = {1}'.format(user, group.name) + log.debug('staffgrp={0}'.format(group.name)) + user.groups.add(group) + track.views.server_track(request, 'add-beta-tester {0}'.format(user), {}, page='idashboard') + + elif action == 'Remove beta testers': + uname = request.POST['betausers'] + try: + user = User.objects.get(username=uname) + except User.DoesNotExist: + msg += 'Error: unknown username "{0}"'.format(uname) + user = None + if user is not None: + group = get_beta_group(course) + msg += 'Removed {0} from beta tester group = {1}'.format(user, group.name) + log.debug('staffgrp={0}'.format(group.name)) + user.groups.remove(group) + track.views.server_track(request, 'remove-beta-tester {0}'.format(user), {}, page='idashboard') + + #---------------------------------------- # forum administration - + elif action == 'List course forum admins': rolename = FORUM_ROLE_ADMINISTRATOR datatable = {} msg += _list_course_forum_members(course_id, rolename, datatable) track.views.server_track(request, 'list-{0}'.format(rolename), {}, page='idashboard') - - + + elif action == 'Remove forum admin': uname = request.POST['forumadmin'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_REMOVE) - track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_REMOVE, uname, FORUM_ROLE_ADMINISTRATOR, course_id), + track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_REMOVE, uname, FORUM_ROLE_ADMINISTRATOR, course_id), {}, page='idashboard') elif action == 'Add forum admin': uname = request.POST['forumadmin'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_ADD) - track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_ADD, uname, FORUM_ROLE_ADMINISTRATOR, course_id), + track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_ADD, uname, FORUM_ROLE_ADMINISTRATOR, course_id), {}, page='idashboard') elif action == 'List course forum moderators': @@ -337,35 +388,35 @@ def instructor_dashboard(request, course_id): datatable = {} msg += _list_course_forum_members(course_id, rolename, datatable) track.views.server_track(request, 'list-{0}'.format(rolename), {}, page='idashboard') - + elif action == 'Remove forum moderator': uname = request.POST['forummoderator'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_MODERATOR, FORUM_ROLE_REMOVE) - track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_REMOVE, uname, FORUM_ROLE_MODERATOR, course_id), + track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_REMOVE, uname, FORUM_ROLE_MODERATOR, course_id), {}, page='idashboard') - + elif action == 'Add forum moderator': uname = request.POST['forummoderator'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_MODERATOR, FORUM_ROLE_ADD) - track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_ADD, uname, FORUM_ROLE_MODERATOR, course_id), + track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_ADD, uname, FORUM_ROLE_MODERATOR, course_id), {}, page='idashboard') - + elif action == 'List course forum community TAs': rolename = FORUM_ROLE_COMMUNITY_TA datatable = {} msg += _list_course_forum_members(course_id, rolename, datatable) track.views.server_track(request, 'list-{0}'.format(rolename), {}, page='idashboard') - + elif action == 'Remove forum community TA': uname = request.POST['forummoderator'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_REMOVE) - track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_REMOVE, uname, FORUM_ROLE_COMMUNITY_TA, course_id), + track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_REMOVE, uname, FORUM_ROLE_COMMUNITY_TA, course_id), {}, page='idashboard') - + elif action == 'Add forum community TA': uname = request.POST['forummoderator'] msg += _update_forum_role_membership(uname, course, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_ADD) - track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_ADD, uname, FORUM_ROLE_COMMUNITY_TA, course_id), + track.views.server_track(request, '{0} {1} as {2} for {3}'.format(FORUM_ROLE_ADD, uname, FORUM_ROLE_COMMUNITY_TA, course_id), {}, page='idashboard') #---------------------------------------- @@ -418,7 +469,7 @@ def instructor_dashboard(request, course_id): msg2, datatable = _do_remote_gradebook(request.user, course, 'get-sections') msg += msg2 - elif action in ['List students in section in remote gradebook', + elif action in ['List students in section in remote gradebook', 'Overload enrollment list using remote gradebook', 'Merge enrollment list with remote gradebook']: @@ -431,7 +482,7 @@ def instructor_dashboard(request, course_id): overload = 'Overload' in action ret = _do_enroll_students(course, course_id, students, overload=overload) datatable = ret['datatable'] - + #---------------------------------------- # psychometrics @@ -448,7 +499,7 @@ def instructor_dashboard(request, course_id): #---------------------------------------- # offline grades? - + if use_offline: msg += "
Grades from %s" % offline_grades_available(course_id) @@ -482,17 +533,17 @@ def _do_remote_gradebook(user, course, action, args=None, files=None): if not rg: msg = "No remote gradebook defined in course metadata" return msg, {} - + rgurl = settings.MITX_FEATURES.get('REMOTE_GRADEBOOK_URL','') if not rgurl: msg = "No remote gradebook url defined in settings.MITX_FEATURES" return msg, {} - + rgname = rg.get('name','') if not rgname: msg = "No gradebook name defined in course remote_gradebook metadata" return msg, {} - + if args is None: args = {} data = dict(submit=action, gradebook=rgname, user=user.email) @@ -522,15 +573,15 @@ def _do_remote_gradebook(user, course, action, args=None, files=None): return msg, datatable def _list_course_forum_members(course_id, rolename, datatable): - ''' + """ Fills in datatable with forum membership information, for a given role, so that it will be displayed on instructor dashboard. - + course_ID = the ID string for a course rolename = one of "Administrator", "Moderator", "Community TA" - + Returns message status string to append to displayed message, if role is unknown. - ''' + """ # make sure datatable is set up properly for display first, before checking for errors datatable['header'] = ['Username', 'Full name', 'Roles'] datatable['title'] = 'List of Forum {0}s in course {1}'.format(rolename, course_id) @@ -549,13 +600,13 @@ def _list_course_forum_members(course_id, rolename, datatable): def _update_forum_role_membership(uname, course, rolename, add_or_remove): ''' Supports adding a user to a course's forum role - + uname = username string for user - course = course object + course = course object rolename = one of "Administrator", "Moderator", "Community TA" add_or_remove = one of "add" or "remove" - - Returns message status string to append to displayed message, Status is returned if user + + Returns message status string to append to displayed message, Status is returned if user or role is unknown, or if entry already exists when adding, or if entry doesn't exist when removing. ''' # check that username and rolename are valid: @@ -575,21 +626,42 @@ def _update_forum_role_membership(uname, course, rolename, add_or_remove): if add_or_remove == FORUM_ROLE_REMOVE: if not alreadyexists: msg ='Error: user "{0}" does not have rolename "{1}", cannot remove'.format(uname, rolename) - else: + else: user.roles.remove(role) msg = 'Removed "{0}" from "{1}" forum role = "{2}"'.format(user, course.id, rolename) else: if alreadyexists: msg = 'Error: user "{0}" already has rolename "{1}", cannot add'.format(uname, rolename) - else: - if (rolename == FORUM_ROLE_ADMINISTRATOR and not has_access(user, course, 'staff')): + else: + if (rolename == FORUM_ROLE_ADMINISTRATOR and not has_access(user, course, 'staff')): msg = 'Error: user "{0}" should first be added as staff before adding as a forum administrator, cannot add'.format(uname) else: user.roles.add(role) msg = 'Added "{0}" to "{1}" forum role = "{2}"'.format(user, course.id, rolename) return msg - + +def _group_members_table(group, title, course_id): + """ + Return a data table of usernames and names of users in group_name. + + Arguments: + group -- a django group. + title -- a descriptive title to show the user + + Returns: + a dictionary with keys + 'header': ['Username', 'Full name'], + 'data': [[username, name] for all users] + 'title': "{title} in course {course}" + """ + uset = group.user_set.all() + datatable = {'header': ['Username', 'Full name']} + datatable['data'] = [[x.username, x.profile.name] for x in uset] + datatable['title'] = '{0} in course {1}'.format(title, course_id) + return datatable + + def get_student_grade_summary_data(request, course, course_id, get_grades=True, get_raw_scores=False, use_offline=False): ''' @@ -750,7 +822,7 @@ def _do_enroll_students(course, course_id, students, overload=False): def sf(stat): return [x for x in status if status[x]==stat] - data = dict(added=sf('added'), rejected=sf('rejected')+sf('exists'), + data = dict(added=sf('added'), rejected=sf('rejected')+sf('exists'), deleted=sf('deleted'), datatable=datatable) return data diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 7f1912cd45..04248336e5 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -59,7 +59,7 @@ function goto( mode) Admin | Forum Admin | Enrollment - ] + Manage Groups ]
${djangopid} @@ -168,7 +168,8 @@ function goto( mode)

- + +

%endif @@ -258,6 +259,21 @@ function goto( mode) ##----------------------------------------------------------------------------- +%if modeflag.get('Manage Groups'): + %if instructor_access: +
+

+ +

+ Enter usernames or emails for students who should be beta-testers. They will get to see course materials early, as configured via the days_early_for_beta option in the course policy. +

+ + + +
+ %endif +%endif + ##----------------------------------------------------------------------------- From 3f654af7cd48b309cb3ce3e6e0d6c539284bde85 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 14 Jan 2013 09:22:57 -0500 Subject: [PATCH 4/9] Refactor common code, fix view a bit remaining: * support putting multiple names / emails in the big box we now have --- lms/djangoapps/instructor/views.py | 120 ++++++++++-------- .../courseware/instructor_dashboard.html | 9 +- 2 files changed, 77 insertions(+), 52 deletions(-) diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 1519d876bb..99d96cad95 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -250,11 +250,7 @@ def instructor_dashboard(request, course_id): elif 'List course staff' in action: group = get_staff_group(course) msg += 'Staff group = {0}'.format(group.name) - log.debug('staffgrp={0}'.format(group.name)) - uset = group.user_set.all() - datatable = {'header': ['Username', 'Full name']} - datatable['data'] = [[x.username, x.profile.name] for x in uset] - datatable['title'] = 'List of Staff in course {0}'.format(course_id) + datatable = _group_members_table(group, "List of Staff", course_id) track.views.server_track(request, 'list-staff', {}, page='idashboard') elif 'List course instructors' in action and request.user.is_staff: @@ -269,17 +265,8 @@ def instructor_dashboard(request, course_id): elif action == 'Add course staff': uname = request.POST['staffuser'] - try: - user = User.objects.get(username=uname) - except User.DoesNotExist: - msg += 'Error: unknown username "{0}"'.format(uname) - user = None - if user is not None: - group = get_staff_group(course) - msg += 'Added {0} to staff group = {1}'.format(user, group.name) - log.debug('staffgrp={0}'.format(group.name)) - user.groups.add(group) - track.views.server_track(request, 'add-staff {0}'.format(user), {}, page='idashboard') + group = get_staff_group(course) + msg += add_user_to_group(request, uname, group, 'staff', 'staff') elif action == 'Add instructor' and request.user.is_staff: uname = request.POST['instructor'] @@ -297,17 +284,8 @@ def instructor_dashboard(request, course_id): elif action == 'Remove course staff': uname = request.POST['staffuser'] - try: - user = User.objects.get(username=uname) - except User.DoesNotExist: - msg += 'Error: unknown username "{0}"'.format(uname) - user = None - if user is not None: - group = get_staff_group(course) - msg += 'Removed {0} from staff group = {1}'.format(user, group.name) - log.debug('staffgrp={0}'.format(group.name)) - user.groups.remove(group) - track.views.server_track(request, 'remove-staff {0}'.format(user), {}, page='idashboard') + group = get_staff_group(course) + msg += remove_user_from_group(request, uname, group, 'staff', 'staff') elif action == 'Remove instructor' and request.user.is_staff: uname = request.POST['instructor'] @@ -334,32 +312,13 @@ def instructor_dashboard(request, course_id): elif action == 'Add beta testers': uname = request.POST['betausers'] - try: - user = User.objects.get(username=uname) - except User.DoesNotExist: - msg += 'Error: unknown username "{0}"'.format(uname) - user = None - if user is not None: - group = get_beta_group(course) - msg += 'Added {0} to beta testers group = {1}'.format(user, group.name) - log.debug('staffgrp={0}'.format(group.name)) - user.groups.add(group) - track.views.server_track(request, 'add-beta-tester {0}'.format(user), {}, page='idashboard') + group = get_beta_group(course) + msg += add_user_to_group(request, uname, group, 'beta testers', 'beta-tester') elif action == 'Remove beta testers': uname = request.POST['betausers'] - try: - user = User.objects.get(username=uname) - except User.DoesNotExist: - msg += 'Error: unknown username "{0}"'.format(uname) - user = None - if user is not None: - group = get_beta_group(course) - msg += 'Removed {0} from beta tester group = {1}'.format(user, group.name) - log.debug('staffgrp={0}'.format(group.name)) - user.groups.remove(group) - track.views.server_track(request, 'remove-beta-tester {0}'.format(user), {}, page='idashboard') - + group = get_beta_group(course) + msg += remove_user_from_group(request, uname, group, 'beta testers', 'beta-tester') #---------------------------------------- # forum administration @@ -661,6 +620,67 @@ def _group_members_table(group, title, course_id): datatable['title'] = '{0} in course {1}'.format(title, course_id) return datatable +def add_user_to_group(request, username_or_email, group, group_title, event_name): + """ + Look up the given user by username (if no '@') or email (otherwise), and add them to group. + + Arguments: + request: django request--used for tracking log + username_or_email: who to add. Decide if it's an email by presense of an '@' + group: django group object + group_title: what to call this group in messages to user--e.g. "beta-testers". + event_name: what to call this event when logging to tracking logs. + + Returns: + html to insert in the message field + """ + user = None + try: + if '@' in username_or_email: + user = User.objects.get(email=username_or_email) + else: + user = User.objects.get(username=username_or_email) + except User.DoesNotExist: + msg = 'Error: unknown username "{0}"'.format(uname) + user = None + + if user is not None: + msg = 'Added {0} to {1} group = {2}'.format(user, group_title, group.name) + user.groups.add(group) + track.views.server_track(request, 'add-{0} {1}'.format(event_name, user), {}, page='idashboard') + + return msg + +def remove_user_from_group(request, username_or_email, group, group_title, event_name): + """ + Look up the given user by username (if no '@') or email (otherwise), and remove them from group. + + Arguments: + request: django request--used for tracking log + username_or_email: who to remove. Decide if it's an email by presense of an '@' + group: django group object + group_title: what to call this group in messages to user--e.g. "beta-testers". + event_name: what to call this event when logging to tracking logs. + + Returns: + html to insert in the message field + """ + user = None + try: + if '@' in username_or_email: + user = User.objects.get(email=username_or_email) + else: + user = User.objects.get(username=username_or_email) + except User.DoesNotExist: + msg = 'Error: unknown username "{0}"'.format(uname) + user = None + + if user is not None: + msg = 'Removed {0} from {1} group = {2}'.format(user, group_title, group.name) + user.groups.remove(group) + track.views.server_track(request, 'remove-{0} {1}'.format(event_name, user), {}, page='idashboard') + + return msg def get_student_grade_summary_data(request, course, course_id, get_grades=True, get_raw_scores=False, use_offline=False): diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 04248336e5..b12d17dced 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -36,6 +36,9 @@ table.stat_table td { a.selectedmode { background-color: yellow; } +textarea { + height: 200px; +}