Merge pull request #480 from MITx/feature/victor/policy-dirs
Feature/victor/policy dirs
This commit is contained in:
@@ -140,7 +140,20 @@ def dashboard(request):
|
||||
if not user.is_active:
|
||||
message = render_to_string('registration/activate_account_notice.html', {'email': user.email})
|
||||
|
||||
context = {'courses': courses, 'message': message}
|
||||
|
||||
# Global staff can see what courses errored on their dashboard
|
||||
staff_access = False
|
||||
errored_courses = {}
|
||||
if has_access(user, 'global', 'staff'):
|
||||
# Show any courses that errored on load
|
||||
staff_access = True
|
||||
errored_courses = modulestore().get_errored_courses()
|
||||
|
||||
context = {'courses': courses,
|
||||
'message': message,
|
||||
'staff_access': staff_access,
|
||||
'errored_courses': errored_courses,}
|
||||
|
||||
return render_to_response('dashboard.html', context)
|
||||
|
||||
|
||||
|
||||
@@ -18,7 +18,7 @@ class CourseDescriptor(SequenceDescriptor):
|
||||
class Textbook:
|
||||
def __init__(self, title, book_url):
|
||||
self.title = title
|
||||
self.book_url = book_url
|
||||
self.book_url = book_url
|
||||
self.table_of_contents = self._get_toc_from_s3()
|
||||
|
||||
@classmethod
|
||||
@@ -30,11 +30,11 @@ class CourseDescriptor(SequenceDescriptor):
|
||||
return self.table_of_contents
|
||||
|
||||
def _get_toc_from_s3(self):
|
||||
'''
|
||||
"""
|
||||
Accesses the textbook's table of contents (default name "toc.xml") at the URL self.book_url
|
||||
|
||||
Returns XML tree representation of the table of contents
|
||||
'''
|
||||
"""
|
||||
toc_url = self.book_url + 'toc.xml'
|
||||
|
||||
# Get the table of contents from S3
|
||||
@@ -72,6 +72,22 @@ class CourseDescriptor(SequenceDescriptor):
|
||||
self.enrollment_start = self._try_parse_time("enrollment_start")
|
||||
self.enrollment_end = self._try_parse_time("enrollment_end")
|
||||
|
||||
# NOTE: relies on the modulestore to call set_grading_policy() right after
|
||||
# init. (Modulestore is in charge of figuring out where to load the policy from)
|
||||
|
||||
|
||||
def set_grading_policy(self, policy_str):
|
||||
"""Parse the policy specified in policy_str, and save it"""
|
||||
try:
|
||||
self._grading_policy = load_grading_policy(policy_str)
|
||||
except:
|
||||
self.system.error_tracker("Failed to load grading policy")
|
||||
# Setting this to an empty dictionary will lead to errors when
|
||||
# grading needs to happen, but should allow course staff to see
|
||||
# the error log.
|
||||
self._grading_policy = {}
|
||||
|
||||
|
||||
@classmethod
|
||||
def definition_from_xml(cls, xml_object, system):
|
||||
textbooks = []
|
||||
@@ -87,25 +103,11 @@ class CourseDescriptor(SequenceDescriptor):
|
||||
|
||||
@property
|
||||
def grader(self):
|
||||
return self.__grading_policy['GRADER']
|
||||
return self._grading_policy['GRADER']
|
||||
|
||||
@property
|
||||
def grade_cutoffs(self):
|
||||
return self.__grading_policy['GRADE_CUTOFFS']
|
||||
|
||||
@lazyproperty
|
||||
def __grading_policy(self):
|
||||
policy_string = ""
|
||||
|
||||
try:
|
||||
with self.system.resources_fs.open("grading_policy.json") as grading_policy_file:
|
||||
policy_string = grading_policy_file.read()
|
||||
except (IOError, ResourceNotFoundError):
|
||||
log.warning("Unable to load course settings file from grading_policy.json in course " + self.id)
|
||||
|
||||
grading_policy = load_grading_policy(policy_string)
|
||||
|
||||
return grading_policy
|
||||
return self._grading_policy['GRADE_CUTOFFS']
|
||||
|
||||
@lazyproperty
|
||||
def grading_context(self):
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import abc
|
||||
import json
|
||||
import logging
|
||||
import sys
|
||||
|
||||
from collections import namedtuple
|
||||
|
||||
@@ -14,11 +15,11 @@ def load_grading_policy(course_policy_string):
|
||||
"""
|
||||
This loads a grading policy from a string (usually read from a file),
|
||||
which can be a JSON object or an empty string.
|
||||
|
||||
|
||||
The JSON object can have the keys GRADER and GRADE_CUTOFFS. If either is
|
||||
missing, it reverts to the default.
|
||||
"""
|
||||
|
||||
|
||||
default_policy_string = """
|
||||
{
|
||||
"GRADER" : [
|
||||
@@ -56,7 +57,7 @@ def load_grading_policy(course_policy_string):
|
||||
}
|
||||
}
|
||||
"""
|
||||
|
||||
|
||||
# Load the global settings as a dictionary
|
||||
grading_policy = json.loads(default_policy_string)
|
||||
|
||||
@@ -64,15 +65,15 @@ def load_grading_policy(course_policy_string):
|
||||
course_policy = {}
|
||||
if course_policy_string:
|
||||
course_policy = json.loads(course_policy_string)
|
||||
|
||||
|
||||
# Override any global settings with the course settings
|
||||
grading_policy.update(course_policy)
|
||||
|
||||
# Here is where we should parse any configurations, so that we can fail early
|
||||
grading_policy['GRADER'] = grader_from_conf(grading_policy['GRADER'])
|
||||
|
||||
|
||||
return grading_policy
|
||||
|
||||
|
||||
|
||||
def aggregate_scores(scores, section_name="summary"):
|
||||
"""
|
||||
@@ -130,9 +131,11 @@ def grader_from_conf(conf):
|
||||
raise ValueError("Configuration has no appropriate grader class.")
|
||||
|
||||
except (TypeError, ValueError) as error:
|
||||
errorString = "Unable to parse grader configuration:\n " + str(subgraderconf) + "\n Error was:\n " + str(error)
|
||||
log.critical(errorString)
|
||||
raise ValueError(errorString)
|
||||
# Add info and re-raise
|
||||
msg = ("Unable to parse grader configuration:\n " +
|
||||
str(subgraderconf) +
|
||||
"\n Error was:\n " + str(error))
|
||||
raise ValueError(msg), None, sys.exc_info()[2]
|
||||
|
||||
return WeightedSubsectionsGrader(subgraders)
|
||||
|
||||
|
||||
@@ -4,16 +4,17 @@ import os
|
||||
import re
|
||||
|
||||
from collections import defaultdict
|
||||
from cStringIO import StringIO
|
||||
from fs.osfs import OSFS
|
||||
from importlib import import_module
|
||||
from lxml import etree
|
||||
from lxml.html import HtmlComment
|
||||
from path import path
|
||||
|
||||
from xmodule.errortracker import ErrorLog, make_error_tracker
|
||||
from xmodule.x_module import XModuleDescriptor, XMLParsingSystem
|
||||
from xmodule.course_module import CourseDescriptor
|
||||
from xmodule.mako_module import MakoDescriptorSystem
|
||||
from cStringIO import StringIO
|
||||
from xmodule.x_module import XModuleDescriptor, XMLParsingSystem
|
||||
|
||||
from . import ModuleStoreBase, Location
|
||||
from .exceptions import ItemNotFoundError
|
||||
@@ -134,12 +135,12 @@ class XMLModuleStore(ModuleStoreBase):
|
||||
self.data_dir = path(data_dir)
|
||||
self.modules = defaultdict(dict) # course_id -> dict(location -> XModuleDescriptor)
|
||||
self.courses = {} # course_dir -> XModuleDescriptor for the course
|
||||
self.errored_courses = {} # course_dir -> errorlog, for dirs that failed to load
|
||||
|
||||
if default_class is None:
|
||||
self.default_class = None
|
||||
else:
|
||||
module_path, _, class_name = default_class.rpartition('.')
|
||||
#log.debug('module_path = %s' % module_path)
|
||||
class_ = getattr(import_module(module_path), class_name)
|
||||
self.default_class = class_
|
||||
|
||||
@@ -162,18 +163,27 @@ class XMLModuleStore(ModuleStoreBase):
|
||||
'''
|
||||
Load a course, keeping track of errors as we go along.
|
||||
'''
|
||||
# Special-case code here, since we don't have a location for the
|
||||
# course before it loads.
|
||||
# So, make a tracker to track load-time errors, then put in the right
|
||||
# place after the course loads and we have its location
|
||||
errorlog = make_error_tracker()
|
||||
course_descriptor = None
|
||||
try:
|
||||
# Special-case code here, since we don't have a location for the
|
||||
# course before it loads.
|
||||
# So, make a tracker to track load-time errors, then put in the right
|
||||
# place after the course loads and we have its location
|
||||
errorlog = make_error_tracker()
|
||||
course_descriptor = self.load_course(course_dir, errorlog.tracker)
|
||||
except Exception as e:
|
||||
msg = "Failed to load course '{0}': {1}".format(course_dir, str(e))
|
||||
log.exception(msg)
|
||||
errorlog.tracker(msg)
|
||||
|
||||
if course_descriptor is not None:
|
||||
self.courses[course_dir] = course_descriptor
|
||||
self._location_errors[course_descriptor.location] = errorlog
|
||||
except:
|
||||
msg = "Failed to load course '%s'" % course_dir
|
||||
log.exception(msg)
|
||||
else:
|
||||
# Didn't load course. Instead, save the errors elsewhere.
|
||||
self.errored_courses[course_dir] = errorlog
|
||||
|
||||
|
||||
|
||||
def __unicode__(self):
|
||||
'''
|
||||
@@ -202,6 +212,28 @@ class XMLModuleStore(ModuleStoreBase):
|
||||
return {}
|
||||
|
||||
|
||||
def read_grading_policy(self, paths, tracker):
|
||||
"""Load a grading policy from the specified paths, in order, if it exists."""
|
||||
# Default to a blank policy
|
||||
policy_str = ""
|
||||
|
||||
for policy_path in paths:
|
||||
if not os.path.exists(policy_path):
|
||||
continue
|
||||
log.debug("Loading grading policy from {0}".format(policy_path))
|
||||
try:
|
||||
with open(policy_path) as grading_policy_file:
|
||||
policy_str = grading_policy_file.read()
|
||||
# if we successfully read the file, stop looking at backups
|
||||
break
|
||||
except (IOError):
|
||||
msg = "Unable to load course settings file from '{0}'".format(policy_path)
|
||||
tracker(msg)
|
||||
log.warning(msg)
|
||||
|
||||
return policy_str
|
||||
|
||||
|
||||
def load_course(self, course_dir, tracker):
|
||||
"""
|
||||
Load a course into this module store
|
||||
@@ -242,9 +274,16 @@ class XMLModuleStore(ModuleStoreBase):
|
||||
course = course_dir
|
||||
|
||||
url_name = course_data.get('url_name', course_data.get('slug'))
|
||||
policy_dir = None
|
||||
if url_name:
|
||||
policy_path = self.data_dir / course_dir / 'policies' / '{0}.json'.format(url_name)
|
||||
policy_dir = self.data_dir / course_dir / 'policies' / url_name
|
||||
policy_path = policy_dir / 'policy.json'
|
||||
policy = self.load_policy(policy_path, tracker)
|
||||
|
||||
# VS[compat]: remove once courses use the policy dirs.
|
||||
if policy == {}:
|
||||
old_policy_path = self.data_dir / course_dir / 'policies' / '{0}.json'.format(url_name)
|
||||
policy = self.load_policy(old_policy_path, tracker)
|
||||
else:
|
||||
policy = {}
|
||||
# VS[compat] : 'name' is deprecated, but support it for now...
|
||||
@@ -268,6 +307,15 @@ class XMLModuleStore(ModuleStoreBase):
|
||||
# after we have the course descriptor.
|
||||
XModuleDescriptor.compute_inherited_metadata(course_descriptor)
|
||||
|
||||
# Try to load grading policy
|
||||
paths = [self.data_dir / course_dir / 'grading_policy.json']
|
||||
if policy_dir:
|
||||
paths = [policy_dir / 'grading_policy.json'] + paths
|
||||
|
||||
policy_str = self.read_grading_policy(paths, tracker)
|
||||
course_descriptor.set_grading_policy(policy_str)
|
||||
|
||||
|
||||
log.debug('========> Done with course import from {0}'.format(course_dir))
|
||||
return course_descriptor
|
||||
|
||||
@@ -318,6 +366,12 @@ class XMLModuleStore(ModuleStoreBase):
|
||||
"""
|
||||
return self.courses.values()
|
||||
|
||||
def get_errored_courses(self):
|
||||
"""
|
||||
Return a dictionary of course_dir -> [(msg, exception_str)], for each
|
||||
course_dir where course loading failed.
|
||||
"""
|
||||
return dict( (k, self.errored_courses[k].errors) for k in self.errored_courses)
|
||||
|
||||
def create_item(self, location):
|
||||
raise NotImplementedError("XMLModuleStores are read-only")
|
||||
|
||||
@@ -233,6 +233,9 @@ class ImportTestCase(unittest.TestCase):
|
||||
self.assertEqual(toy_ch.display_name, "Overview")
|
||||
self.assertEqual(two_toys_ch.display_name, "Two Toy Overview")
|
||||
|
||||
# Also check that the grading policy loaded
|
||||
self.assertEqual(two_toys.grade_cutoffs['C'], 0.5999)
|
||||
|
||||
|
||||
def test_definition_loading(self):
|
||||
"""When two courses share the same org and course name and
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
{
|
||||
"GRADER" : [
|
||||
{
|
||||
"type" : "Homework",
|
||||
"min_count" : 12,
|
||||
"drop_count" : 2,
|
||||
"short_label" : "HW",
|
||||
"weight" : 0.15
|
||||
},
|
||||
{
|
||||
"type" : "Lab",
|
||||
"min_count" : 12,
|
||||
"drop_count" : 2,
|
||||
"category" : "Labs",
|
||||
"weight" : 0.15
|
||||
},
|
||||
{
|
||||
"type" : "Midterm",
|
||||
"name" : "Midterm Exam",
|
||||
"short_label" : "Midterm",
|
||||
"weight" : 0.3
|
||||
},
|
||||
{
|
||||
"type" : "Final",
|
||||
"name" : "Final Exam",
|
||||
"short_label" : "Final",
|
||||
"weight" : 0.4
|
||||
}
|
||||
],
|
||||
"GRADE_CUTOFFS" : {
|
||||
"A" : 0.87,
|
||||
"B" : 0.7,
|
||||
"C" : 0.5999
|
||||
}
|
||||
}
|
||||
@@ -63,6 +63,9 @@ def has_access(user, obj, action):
|
||||
if isinstance(obj, Location):
|
||||
return _has_access_location(user, obj, action)
|
||||
|
||||
if isinstance(obj, basestring):
|
||||
return _has_access_string(user, obj, action)
|
||||
|
||||
# Passing an unknown object here is a coding error, so rather than
|
||||
# returning a default, complain.
|
||||
raise TypeError("Unknown object type in has_access(): '{0}'"
|
||||
@@ -238,6 +241,30 @@ def _has_access_location(user, location, action):
|
||||
return _dispatch(checkers, action, user, location)
|
||||
|
||||
|
||||
def _has_access_string(user, perm, action):
|
||||
"""
|
||||
Check if user has certain special access, specified as string. Valid strings:
|
||||
|
||||
'global'
|
||||
|
||||
Valid actions:
|
||||
|
||||
'staff' -- global staff access.
|
||||
"""
|
||||
|
||||
def check_staff():
|
||||
if perm != 'global':
|
||||
debug("Deny: invalid permission '%s'", perm)
|
||||
return False
|
||||
return _has_global_staff_access(user)
|
||||
|
||||
checkers = {
|
||||
'staff': check_staff
|
||||
}
|
||||
|
||||
return _dispatch(checkers, action, user, perm)
|
||||
|
||||
|
||||
##### Internal helper methods below
|
||||
|
||||
def _dispatch(table, action, user, obj):
|
||||
@@ -266,6 +293,15 @@ def _course_staff_group_name(location):
|
||||
"""
|
||||
return 'staff_%s' % Location(location).course
|
||||
|
||||
def _has_global_staff_access(user):
|
||||
if user.is_staff:
|
||||
debug("Allow: user.is_staff")
|
||||
return True
|
||||
else:
|
||||
debug("Deny: not user.is_staff")
|
||||
return False
|
||||
|
||||
|
||||
def _has_staff_access_to_location(user, location):
|
||||
'''
|
||||
Returns True if the given user has staff access to a location. For now this
|
||||
|
||||
@@ -30,7 +30,6 @@ def get_course_by_id(course_id):
|
||||
raise Http404("Course not found.")
|
||||
|
||||
|
||||
|
||||
def get_course_with_access(user, course_id, action):
|
||||
"""
|
||||
Given a course_id, look up the corresponding course descriptor,
|
||||
|
||||
@@ -107,6 +107,21 @@
|
||||
</section>
|
||||
% endif
|
||||
|
||||
% if staff_access and len(errored_courses) > 0:
|
||||
<div id="course-errors">
|
||||
<h2>Course-loading errors</h2>
|
||||
|
||||
% for course_dir, errors in errored_courses.items():
|
||||
<h3>${course_dir | h}</h3>
|
||||
<ul>
|
||||
% for (msg, err) in errors:
|
||||
<li>${msg}
|
||||
<ul><li><pre>${err}</pre></li></ul>
|
||||
</li>
|
||||
% endfor
|
||||
</ul>
|
||||
% endfor
|
||||
% endif
|
||||
</section>
|
||||
</section>
|
||||
|
||||
|
||||
Reference in New Issue
Block a user