diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index a9f29b9a13..fb0721454c 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -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,15 @@ 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""" + self._grading_policy = load_grading_policy(policy_str) + + @classmethod def definition_from_xml(cls, xml_object, system): textbooks = [] @@ -87,25 +96,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): diff --git a/common/lib/xmodule/xmodule/graders.py b/common/lib/xmodule/xmodule/graders.py index fca862aa9f..82dc37bf57 100644 --- a/common/lib/xmodule/xmodule/graders.py +++ b/common/lib/xmodule/xmodule/graders.py @@ -14,11 +14,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 +56,7 @@ def load_grading_policy(course_policy_string): } } """ - + # Load the global settings as a dictionary grading_policy = json.loads(default_policy_string) @@ -64,15 +64,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,7 +130,9 @@ 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) + errorString = ("Unable to parse grader configuration:\n " + + str(subgraderconf) + + "\n Error was:\n " + str(error)) log.critical(errorString) raise ValueError(errorString) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 769d110518..30610479b9 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -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 @@ -202,6 +203,27 @@ 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 + 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 +264,11 @@ class XMLModuleStore(ModuleStoreBase): course = course_dir url_name = course_data.get('url_name', course_data.get('slug')) + policy_dir = None if url_name: - old_policy_path = self.data_dir / course_dir / 'policies' / url_name / 'policy.json' - policy = self.load_policy(old_policy_path, tracker) + 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 == {}: @@ -273,6 +297,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 / '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 diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 34e4767a62..3454366c1a 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -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 diff --git a/common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json b/common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json new file mode 100644 index 0000000000..13942c1715 --- /dev/null +++ b/common/test/data/two_toys/policies/TT_2012_Fall/grading_policy.json @@ -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 + } +}