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..a1d9cdda9b 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,26 @@ 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 + try: + return float(self.metadata['days_early_for_beta']) + except ValueError: + return None + + @property def own_metadata(self): """ @@ -715,7 +733,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..8138de4d7e 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 (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. * TODO: there are others ### Grading policy file contents diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 26f9fcdfd3..c7e09526c9 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,51 @@ 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. + + 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 + 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. + # (fun fact: datetime(*a_time_struct[:6]) is the beautiful syntax for + # converting time_structs into datetimes) + 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..eeb304b193 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""" 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..2a80b387d1 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -2,6 +2,7 @@ from collections import defaultdict import csv +import itertools import json import logging import os @@ -19,9 +20,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 +49,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 +109,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 @@ -237,11 +251,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: @@ -256,17 +266,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'] @@ -284,17 +285,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'] @@ -310,26 +302,50 @@ 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': + 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): + 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): + msg += "{0}
".format( + remove_user_from_group(request, username_or_email, group, 'beta testers', 'beta-tester')) + #---------------------------------------- # 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 +353,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 +434,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 +447,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 +464,7 @@ def instructor_dashboard(request, course_id): #---------------------------------------- # offline grades? - + if use_offline: msg += "