From fa75245e8a6f2358d24398c79eeb7327ee6668a0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 21 Dec 2012 13:22:53 -0500 Subject: [PATCH] WIP: Start cleaning up CMS to work with new field format --- .../contentstore/tests/factories.py | 12 ++-- cms/djangoapps/contentstore/views.py | 14 ++--- .../models/settings/course_details.py | 8 +-- .../models/settings/course_grading.py | 26 ++++++--- common/lib/xmodule/xmodule/capa_module.py | 56 +------------------ common/lib/xmodule/xmodule/fields.py | 39 +++++++++++++ common/lib/xmodule/xmodule/html_module.py | 4 +- .../xmodule/xmodule/self_assessment_module.py | 6 +- .../lib/xmodule/xmodule/tests/test_export.py | 15 +---- lms/xmodule_namespace.py | 3 +- 10 files changed, 79 insertions(+), 104 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/factories.py b/cms/djangoapps/contentstore/tests/factories.py index 24897bf795..1ae300daed 100644 --- a/cms/djangoapps/contentstore/tests/factories.py +++ b/cms/djangoapps/contentstore/tests/factories.py @@ -38,10 +38,9 @@ class XModuleCourseFactory(Factory): # This metadata code was copied from cms/djangoapps/contentstore/views.py if display_name is not None: - new_course.metadata['display_name'] = display_name + new_course.lms.display_name = display_name - new_course.metadata['data_dir'] = uuid4().hex - new_course.metadata['start'] = stringify_time(gmtime()) + new_course.start = gmtime() new_course.tabs = [{"type": "courseware"}, {"type": "course_info", "name": "Course Info"}, {"type": "discussion", "name": "Discussion"}, @@ -89,17 +88,14 @@ class XModuleItemFactory(Factory): new_item = store.clone_item(template, dest_location) - # TODO: This needs to be deleted when we have proper storage for static content - new_item.metadata['data_dir'] = parent.metadata['data_dir'] - # replace the display name with an optional parameter passed in from the caller if display_name is not None: - new_item.metadata['display_name'] = display_name + new_item.lms.display_name = display_name store.update_metadata(new_item.location.url(), own_metadata(new_item)) if new_item.location.category not in DETACHED_CATEGORIES: - store.update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) + store.update_children(parent_location, parent.children + [new_item.location.url()]) return new_item diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 22b46dc4be..809e43dea3 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -706,17 +706,14 @@ def clone_item(request): new_item = get_modulestore(template).clone_item(template, dest_location) - # TODO: This needs to be deleted when we have proper storage for static content - new_item.metadata['data_dir'] = parent.metadata['data_dir'] - # replace the display name with an optional parameter passed in from the caller if display_name is not None: - new_item.metadata['display_name'] = display_name + new_item.lms.display_name = display_name get_modulestore(template).update_metadata(new_item.location.url(), own_metadata(new_item)) if new_item.location.category not in DETACHED_CATEGORIES: - get_modulestore(parent.location).update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) + get_modulestore(parent.location).update_children(parent_location, parent.children + [new_item.location.url()]) return HttpResponse(json.dumps({'id': dest_location.url()})) @@ -1206,13 +1203,10 @@ def create_new_course(request): new_course = modulestore('direct').clone_item(template, dest_location) if display_name is not None: - new_course.metadata['display_name'] = display_name - - # we need a 'data_dir' for legacy reasons - new_course.metadata['data_dir'] = uuid4().hex + new_course.display_name = display_name # set a default start date to now - new_course.metadata['start'] = stringify_time(time.gmtime()) + new_course.start = time.gmtime() initialize_course_tabs(new_course) diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 59cadf6962..b1250862f6 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -44,25 +44,25 @@ class CourseDetails: temploc = course_location._replace(category='about', name='syllabus') try: - course.syllabus = get_modulestore(temploc).get_item(temploc).definition['data'] + course.syllabus = get_modulestore(temploc).get_item(temploc).data except ItemNotFoundError: pass temploc = temploc._replace(name='overview') try: - course.overview = get_modulestore(temploc).get_item(temploc).definition['data'] + course.overview = get_modulestore(temploc).get_item(temploc).data except ItemNotFoundError: pass temploc = temploc._replace(name='effort') try: - course.effort = get_modulestore(temploc).get_item(temploc).definition['data'] + course.effort = get_modulestore(temploc).get_item(temploc).data except ItemNotFoundError: pass temploc = temploc._replace(name='video') try: - raw_video = get_modulestore(temploc).get_item(temploc).definition['data'] + raw_video = get_modulestore(temploc).get_item(temploc).data course.intro_video = CourseDetails.parse_video_tag(raw_video) except ItemNotFoundError: pass diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index e0bab1f225..9ddbe87727 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -201,7 +201,7 @@ class CourseGradingModel: course_location = Location(course_location) descriptor = get_modulestore(course_location).get_item(course_location) - if 'graceperiod' in descriptor.metadata: del descriptor.metadata['graceperiod'] + if 'graceperiod' in descriptor.metadata: del descriptor.metadata['graceperiod'] get_modulestore(course_location).update_metadata(course_location, descriptor.metadata) @staticmethod @@ -226,20 +226,30 @@ class CourseGradingModel: descriptor.metadata['format'] = jsondict.get('graderType') descriptor.metadata['graded'] = True else: - if 'format' in descriptor.metadata: del descriptor.metadata['format'] - if 'graded' in descriptor.metadata: del descriptor.metadata['graded'] + if 'format' in descriptor.metadata: del descriptor.metadata['format'] + if 'graded' in descriptor.metadata: del descriptor.metadata['graded'] - get_modulestore(location).update_metadata(location, descriptor.metadata) + get_modulestore(location).update_metadata(location, descriptor.metadata) @staticmethod def convert_set_grace_period(descriptor): # 5 hours 59 minutes 59 seconds => converted to iso format - rawgrace = descriptor.metadata.get('graceperiod', None) + rawgrace = descriptor.lms.graceperiod if rawgrace: - parsedgrace = {str(key): val for (val, key) in re.findall('\s*(\d+)\s*(\w+)', rawgrace)} - return parsedgrace - else: return None + hours_from_day = rawgrace.days*24 + seconds = rawgrace.seconds + hours_from_seconds = int(seconds / 3600) + seconds -= hours_from_seconds * 3600 + minutes = int(seconds / 60) + seconds -= minutes * 60 + return { + 'hours': hourse_from_days + hours_from_seconds, + 'minutes': minutes_from_seconds, + 'seconds': seconds, + } + else: + return None @staticmethod def parse_grader(json_grader): diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 4ee648b241..f83c31fb5c 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -5,10 +5,8 @@ import dateutil.parser import json import logging import traceback -import re import sys -from datetime import timedelta from lxml import etree from pkg_resources import resource_string @@ -20,6 +18,9 @@ from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.exceptions import NotFoundError from .model import Int, Scope, ModuleScope, ModelType, String, Boolean, Object, Float +from .fields import Timedelta + +log = logging.getLogger("mitx.courseware") class StringyInt(Int): @@ -31,57 +32,6 @@ class StringyInt(Int): return int(value) return value -log = logging.getLogger("mitx.courseware") - -#----------------------------------------------------------------------------- -TIMEDELTA_REGEX = re.compile(r'^((?P\d+?) day(?:s?))?(\s)?((?P\d+?) hour(?:s?))?(\s)?((?P\d+?) minute(?:s)?)?(\s)?((?P\d+?) second(?:s)?)?$') - - -def only_one(lst, default="", process=lambda x: x): - """ - If lst is empty, returns default - - If lst has a single element, applies process to that element and returns it. - - Otherwise, raises an exception. - """ - if len(lst) == 0: - return default - elif len(lst) == 1: - return process(lst[0]) - else: - raise Exception('Malformed XML: expected at most one element in list.') - - -class Timedelta(ModelType): - def from_json(self, time_str): - """ - time_str: A string with the following components: - day[s] (optional) - hour[s] (optional) - minute[s] (optional) - second[s] (optional) - - Returns a datetime.timedelta parsed from the string - """ - parts = TIMEDELTA_REGEX.match(time_str) - if not parts: - return - parts = parts.groupdict() - time_params = {} - for (name, param) in parts.iteritems(): - if param: - time_params[name] = int(param) - return timedelta(**time_params) - - def to_json(self, value): - values = [] - for attr in ('days', 'hours', 'minutes', 'seconds'): - cur_value = getattr(value, attr, 0) - if cur_value > 0: - values.append("%d %s" % (cur_value, attr)) - return ' '.join(values) - class Randomization(String): def from_json(self, value): diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 715aea0c7c..21c360f914 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -1,6 +1,8 @@ import time import logging +import re +from datetime import timedelta from .model import ModelType log = logging.getLogger(__name__) @@ -15,6 +17,9 @@ class Date(ModelType): if it doesn't parse. Return None if not present or invalid. """ + if value is None: + return None + try: return time.strptime(value, self.time_format) except ValueError as e: @@ -27,4 +32,38 @@ class Date(ModelType): """ Convert a time struct to a string """ + if value is None: + return None + return time.strftime(self.time_format, value) + + +TIMEDELTA_REGEX = re.compile(r'^((?P\d+?) day(?:s?))?(\s)?((?P\d+?) hour(?:s?))?(\s)?((?P\d+?) minute(?:s)?)?(\s)?((?P\d+?) second(?:s)?)?$') +class Timedelta(ModelType): + def from_json(self, time_str): + """ + time_str: A string with the following components: + day[s] (optional) + hour[s] (optional) + minute[s] (optional) + second[s] (optional) + + Returns a datetime.timedelta parsed from the string + """ + parts = TIMEDELTA_REGEX.match(time_str) + if not parts: + return + parts = parts.groupdict() + time_params = {} + for (name, param) in parts.iteritems(): + if param: + time_params[name] = int(param) + return timedelta(**time_params) + + def to_json(self, value): + values = [] + for attr in ('days', 'hours', 'minutes', 'seconds'): + cur_value = getattr(value, attr, 0) + if cur_value > 0: + values.append("%d %s" % (cur_value, attr)) + return ' '.join(values) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 6ec1061451..55155810e9 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -149,7 +149,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): string to filename.html. ''' try: - return etree.fromstring(self.definition['data']) + return etree.fromstring(self.data) except etree.XMLSyntaxError: pass @@ -161,7 +161,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): resource_fs.makedir(os.path.dirname(filepath), recursive=True, allow_recreate=True) with resource_fs.open(filepath, 'w') as file: - file.write(self.definition['data']) + file.write(self.data) # write out the relative name relname = path(pathname).basename() diff --git a/common/lib/xmodule/xmodule/self_assessment_module.py b/common/lib/xmodule/xmodule/self_assessment_module.py index ca6eae9913..034ec01253 100644 --- a/common/lib/xmodule/xmodule/self_assessment_module.py +++ b/common/lib/xmodule/xmodule/self_assessment_module.py @@ -18,13 +18,11 @@ from progress import Progress from pkg_resources import resource_string -from .capa_module import only_one, ComplexEncoder +from .capa_module import ComplexEncoder from .editing_module import EditingDescriptor -from .html_checker import check_html from .stringify import stringify_children from .x_module import XModule from .xml_module import XmlDescriptor -from xmodule.modulestore import Location from .model import List, String, Scope, Int log = logging.getLogger("mitx.courseware") @@ -436,7 +434,7 @@ class SelfAssessmentDescriptor(XmlDescriptor, EditingDescriptor): elt = etree.Element('selfassessment') def add_child(k): - child_str = '<{tag}>{body}'.format(tag=k, body=self.definition[k]) + child_str = '<{tag}>{body}'.format(tag=k, body=getattr(self, k)) child_node = etree.fromstring(child_str) elt.append(child_node) diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py index aeebc6da6b..c6ea617d70 100644 --- a/common/lib/xmodule/xmodule/tests/test_export.py +++ b/common/lib/xmodule/xmodule/tests/test_export.py @@ -18,21 +18,12 @@ TEST_DIR = TEST_DIR / 'test' DATA_DIR = TEST_DIR / 'data' -def strip_metadata(descriptor, key): - """ - Recursively strips tag from all children. - """ - print "strip {key} from {desc}".format(key=key, desc=descriptor.location.url()) - descriptor.metadata.pop(key, None) - for d in descriptor.get_children(): - strip_metadata(d, key) - def strip_filenames(descriptor): """ Recursively strips 'filename' from all children's definitions. """ print "strip filename from {desc}".format(desc=descriptor.location.url()) - descriptor.definition.pop('filename', None) + descriptor._model_data.pop('filename', None) for d in descriptor.get_children(): strip_filenames(d) @@ -73,10 +64,6 @@ class RoundTripTestCase(unittest.TestCase): exported_course = courses2[0] print "Checking course equality" - # HACK: data_dir metadata tags break equality because they - # aren't real metadata, and depend on paths. Remove them. - strip_metadata(initial_course, 'data_dir') - strip_metadata(exported_course, 'data_dir') # HACK: filenames change when changing file formats # during imports from old-style courses. Ignore them. diff --git a/lms/xmodule_namespace.py b/lms/xmodule_namespace.py index 3a72a64dff..81a84edeaa 100644 --- a/lms/xmodule_namespace.py +++ b/lms/xmodule_namespace.py @@ -1,5 +1,5 @@ from xmodule.model import Namespace, Boolean, Scope, String, List -from xmodule.fields import Date +from xmodule.fields import Date, Timedelta class StringyBoolean(Boolean): @@ -39,3 +39,4 @@ class LmsNamespace(Namespace): giturl = String(help="DO NOT USE", scope=Scope.settings, default='https://github.com/MITx') xqa_key = String(help="DO NOT USE", scope=Scope.settings) ispublic = Boolean(help="Whether this course is open to the public, or only to admins", scope=Scope.settings) + graceperiod = Timedelta(help="Amount of time after the due date that submissions will be accepted", scope=Scope.settings)