diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index ecdeca29e7..2e7bc5db83 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -22,25 +22,61 @@ from xmodule.modulestore.tests.factories import CourseFactory from models.settings.course_metadata import CourseMetadata from xmodule.modulestore.xml_importer import import_from_xml from xmodule.modulestore.django import modulestore +import time # YYYY-MM-DDThh:mm:ss.s+/-HH:MM class ConvertersTestCase(TestCase): @staticmethod def struct_to_datetime(struct_time): - return datetime.datetime(struct_time.tm_year, struct_time.tm_mon, struct_time.tm_mday, struct_time.tm_hour, - struct_time.tm_min, struct_time.tm_sec, tzinfo=UTC()) + return datetime.datetime(struct_time.tm_year, struct_time.tm_mon, + struct_time.tm_mday, struct_time.tm_hour, + struct_time.tm_min, struct_time.tm_sec, tzinfo=UTC()) def compare_dates(self, date1, date2, expected_delta): dt1 = ConvertersTestCase.struct_to_datetime(date1) dt2 = ConvertersTestCase.struct_to_datetime(date2) - self.assertEqual(dt1 - dt2, expected_delta, str(date1) + "-" + str(date2) + "!=" + str(expected_delta)) + self.assertEqual(dt1 - dt2, expected_delta, str(date1) + "-" + + str(date2) + "!=" + str(expected_delta)) def test_iso_to_struct(self): - self.compare_dates(converters.jsdate_to_time("2013-01-01"), converters.jsdate_to_time("2012-12-31"), datetime.timedelta(days=1)) - self.compare_dates(converters.jsdate_to_time("2013-01-01T00"), converters.jsdate_to_time("2012-12-31T23"), datetime.timedelta(hours=1)) - self.compare_dates(converters.jsdate_to_time("2013-01-01T00:00"), converters.jsdate_to_time("2012-12-31T23:59"), datetime.timedelta(minutes=1)) - self.compare_dates(converters.jsdate_to_time("2013-01-01T00:00:00"), converters.jsdate_to_time("2012-12-31T23:59:59"), datetime.timedelta(seconds=1)) + '''Test conversion from iso compatible date strings to struct_time''' + self.compare_dates(converters.jsdate_to_time("2013-01-01"), + converters.jsdate_to_time("2012-12-31"), + datetime.timedelta(days=1)) + self.compare_dates(converters.jsdate_to_time("2013-01-01T00"), + converters.jsdate_to_time("2012-12-31T23"), + datetime.timedelta(hours=1)) + self.compare_dates(converters.jsdate_to_time("2013-01-01T00:00"), + converters.jsdate_to_time("2012-12-31T23:59"), + datetime.timedelta(minutes=1)) + self.compare_dates(converters.jsdate_to_time("2013-01-01T00:00:00"), + converters.jsdate_to_time("2012-12-31T23:59:59"), + datetime.timedelta(seconds=1)) + self.compare_dates(converters.jsdate_to_time("2013-01-01T00:00:00Z"), + converters.jsdate_to_time("2012-12-31T23:59:59Z"), + datetime.timedelta(seconds=1)) + self.compare_dates( + converters.jsdate_to_time("2012-12-31T23:00:01-01:00"), + converters.jsdate_to_time("2013-01-01T00:00:00+01:00"), + datetime.timedelta(hours=1, seconds=1)) + + def test_struct_to_iso(self): + ''' + Test converting time reprs to iso dates + ''' + self.assertEqual( + converters.time_to_isodate( + time.strptime("2012-12-31T23:59:59Z", "%Y-%m-%dT%H:%M:%SZ")), + "2012-12-31T23:59:59Z") + self.assertEqual( + converters.time_to_isodate( + jsdate_to_time("2012-12-31T23:59:59Z")), + "2012-12-31T23:59:59Z") + self.assertEqual( + converters.time_to_isodate( + jsdate_to_time("2012-12-31T23:00:01-01:00")), + "2013-01-01T00:00:01Z") class CourseTestCase(ModuleStoreTestCase): @@ -104,7 +140,7 @@ class CourseDetailsTestCase(CourseTestCase): self.assertIsNone(jsondetails['effort'], "effort somehow initialized") def test_update_and_fetch(self): - ## NOTE: I couldn't figure out how to validly test time setting w/ all the conversions + # # NOTE: I couldn't figure out how to validly test time setting w/ all the conversions jsondetails = CourseDetails.fetch(self.course_location) jsondetails.syllabus = "bar" # encode - decode to convert date fields and other data which changes form @@ -182,7 +218,7 @@ class CourseDetailsViewTest(CourseTestCase): details_encoded = jsdate_to_time(details[field]) dt2 = ConvertersTestCase.struct_to_datetime(details_encoded) - expected_delta = datetime.timedelta(0) + expected_delta = datetime.timedelta(0) self.assertEqual(dt1 - dt2, expected_delta, str(dt1) + "!=" + str(dt2) + " at " + context) else: self.fail(field + " missing from encoded but in details at " + context) @@ -269,7 +305,7 @@ class CourseMetadataEditingTest(CourseTestCase): CourseTestCase.setUp(self) # add in the full class too import_from_xml(modulestore(), 'common/test/data/', ['full']) - self.fullcourse_location = Location(['i4x','edX','full','course','6.002_Spring_2012', None]) + self.fullcourse_location = Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]) def test_fetch_initial_fields(self): diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 63025d8abe..563dd16524 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -3,19 +3,24 @@ from contentstore.utils import get_modulestore from xmodule.x_module import XModuleDescriptor from xmodule.modulestore.inheritance import own_metadata from xblock.core import Scope +from xmodule.course_module import CourseDescriptor class CourseMetadata(object): ''' - For CRUD operations on metadata fields which do not have specific editors on the other pages including any user generated ones. - The objects have no predefined attrs but instead are obj encodings of the editable metadata. + For CRUD operations on metadata fields which do not have specific editors + on the other pages including any user generated ones. + The objects have no predefined attrs but instead are obj encodings of the + editable metadata. ''' - FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', 'checklists'] + FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', + 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', 'checklists'] @classmethod def fetch(cls, course_location): """ - Fetch the key:value editable course details for the given course from persistence and return a CourseMetadata model. + Fetch the key:value editable course details for the given course from + persistence and return a CourseMetadata model. """ if not isinstance(course_location, Location): course_location = Location(course_location) @@ -29,7 +34,7 @@ class CourseMetadata(object): continue if field.name not in cls.FILTERED_LIST: - course[field.name] = field.read_from(descriptor) + course[field.name] = field.read_json(descriptor) return course @@ -51,22 +56,26 @@ class CourseMetadata(object): if hasattr(descriptor, k) and getattr(descriptor, k) != v: dirty = True - setattr(descriptor, k, v) + value = getattr(CourseDescriptor, k).from_json(v) + setattr(descriptor, k, value) elif hasattr(descriptor.lms, k) and getattr(descriptor.lms, k) != k: dirty = True - setattr(descriptor.lms, k, v) + value = getattr(CourseDescriptor.lms, k).from_json(v) + setattr(descriptor.lms, k, value) if dirty: - get_modulestore(course_location).update_metadata(course_location, own_metadata(descriptor)) + get_modulestore(course_location).update_metadata(course_location, + own_metadata(descriptor)) - # Could just generate and return a course obj w/o doing any db reads, but I put the reads in as a means to confirm - # it persisted correctly + # Could just generate and return a course obj w/o doing any db reads, + # but I put the reads in as a means to confirm it persisted correctly return cls.fetch(course_location) @classmethod def delete_key(cls, course_location, payload): ''' - Remove the given metadata key(s) from the course. payload can be a single key or [key..] + Remove the given metadata key(s) from the course. payload can be a + single key or [key..] ''' descriptor = get_modulestore(course_location).get_item(course_location) @@ -76,6 +85,7 @@ class CourseMetadata(object): elif hasattr(descriptor.lms, key): delattr(descriptor.lms, key) - get_modulestore(course_location).update_metadata(course_location, own_metadata(descriptor)) + get_modulestore(course_location).update_metadata(course_location, + own_metadata(descriptor)) return cls.fetch(course_location) diff --git a/cms/urls.py b/cms/urls.py index a9c38765e5..e1eae3352a 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -42,36 +42,52 @@ urlpatterns = ('', 'contentstore.views.remove_user', name='remove_user'), url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)/remove_user$', 'contentstore.views.remove_user', name='remove_user'), - url(r'^(?P[^/]+)/(?P[^/]+)/info/(?P[^/]+)$', 'contentstore.views.course_info', name='course_info'), - url(r'^(?P[^/]+)/(?P[^/]+)/course_info/updates/(?P.*)$', 'contentstore.views.course_info_updates', name='course_info_json'), - url(r'^(?P[^/]+)/(?P[^/]+)/settings-details/(?P[^/]+)$', 'contentstore.views.get_course_settings', name='settings_details'), - url(r'^(?P[^/]+)/(?P[^/]+)/settings-grading/(?P[^/]+)$', 'contentstore.views.course_config_graders_page', name='settings_grading'), - url(r'^(?P[^/]+)/(?P[^/]+)/settings-details/(?P[^/]+)/section/(?P
[^/]+).*$', 'contentstore.views.course_settings_updates', name='course_settings'), - url(r'^(?P[^/]+)/(?P[^/]+)/settings-grading/(?P[^/]+)/(?P.*)$', 'contentstore.views.course_grader_updates', name='course_settings'), + url(r'^(?P[^/]+)/(?P[^/]+)/info/(?P[^/]+)$', + 'contentstore.views.course_info', name='course_info'), + url(r'^(?P[^/]+)/(?P[^/]+)/course_info/updates/(?P.*)$', + 'contentstore.views.course_info_updates', name='course_info_json'), + url(r'^(?P[^/]+)/(?P[^/]+)/settings-details/(?P[^/]+)$', + 'contentstore.views.get_course_settings', name='settings_details'), + url(r'^(?P[^/]+)/(?P[^/]+)/settings-grading/(?P[^/]+)$', + 'contentstore.views.course_config_graders_page', name='settings_grading'), + url(r'^(?P[^/]+)/(?P[^/]+)/settings-details/(?P[^/]+)/section/(?P
[^/]+).*$', + 'contentstore.views.course_settings_updates', name='course_settings'), + url(r'^(?P[^/]+)/(?P[^/]+)/settings-grading/(?P[^/]+)/(?P.*)$', + 'contentstore.views.course_grader_updates', name='course_settings'), # This is the URL to initially render the course advanced settings. - url(r'^(?P[^/]+)/(?P[^/]+)/settings-advanced/(?P[^/]+)$', 'contentstore.views.course_config_advanced_page', name='course_advanced_settings'), + url(r'^(?P[^/]+)/(?P[^/]+)/settings-advanced/(?P[^/]+)$', + 'contentstore.views.course_config_advanced_page', name='course_advanced_settings'), # This is the URL used by BackBone for updating and re-fetching the model. - url(r'^(?P[^/]+)/(?P[^/]+)/settings-advanced/(?P[^/]+)/update.*$', 'contentstore.views.course_advanced_updates', name='course_advanced_settings_updates'), + url(r'^(?P[^/]+)/(?P[^/]+)/settings-advanced/(?P[^/]+)/update.*$', + 'contentstore.views.course_advanced_updates', name='course_advanced_settings_updates'), - url(r'^(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/gradeas.*$', 'contentstore.views.assignment_type_update', name='assignment_type_update'), + url(r'^(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/gradeas.*$', + 'contentstore.views.assignment_type_update', name='assignment_type_update'), - url(r'^pages/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.static_pages', + url(r'^pages/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', + 'contentstore.views.static_pages', name='static_pages'), - url(r'^edit_static/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.edit_static', name='edit_static'), - url(r'^edit_tabs/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.edit_tabs', name='edit_tabs'), - url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)$', 'contentstore.views.asset_index', name='asset_index'), + url(r'^edit_static/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', + 'contentstore.views.edit_static', name='edit_static'), + url(r'^edit_tabs/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', + 'contentstore.views.edit_tabs', name='edit_tabs'), + url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)$', + 'contentstore.views.asset_index', name='asset_index'), # this is a generic method to return the data/metadata associated with a xmodule - url(r'^module_info/(?P.*)$', 'contentstore.views.module_info', name='module_info'), + url(r'^module_info/(?P.*)$', + 'contentstore.views.module_info', name='module_info'), # temporary landing page for a course - url(r'^edge/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', 'contentstore.views.landing', name='landing'), + url(r'^edge/(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', + 'contentstore.views.landing', name='landing'), url(r'^not_found$', 'contentstore.views.not_found', name='not_found'), url(r'^server_error$', 'contentstore.views.server_error', name='server_error'), - url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)$', 'contentstore.views.asset_index', name='asset_index'), + url(r'^(?P[^/]+)/(?P[^/]+)/assets/(?P[^/]+)$', + 'contentstore.views.asset_index', name='asset_index'), # temporary landing page for edge url(r'^edge$', 'contentstore.views.edge', name='edge'), diff --git a/common/djangoapps/util/converters.py b/common/djangoapps/util/converters.py index ec2d29ecfa..212cceb77d 100644 --- a/common/djangoapps/util/converters.py +++ b/common/djangoapps/util/converters.py @@ -1,17 +1,25 @@ import time import datetime -import re import calendar +import dateutil.parser def time_to_date(time_obj): """ - Convert a time.time_struct to a true universal time (can pass to js Date constructor) + Convert a time.time_struct to a true universal time (can pass to js Date + constructor) """ - # TODO change to using the isoformat() function on datetime. js date can parse those return calendar.timegm(time_obj) * 1000 +def time_to_isodate(source): + '''Convert to an iso date''' + if isinstance(source, time.struct_time): + return time.strftime('%Y-%m-%dT%H:%M:%SZ', source) + elif isinstance(source, datetime): + return source.isoformat() + 'Z' + + def jsdate_to_time(field): """ Convert a universal time (iso format) or msec since epoch to a time obj @@ -19,8 +27,7 @@ def jsdate_to_time(field): if field is None: return field elif isinstance(field, basestring): - # ISO format but ignores time zone assuming it's Z. - d = datetime.datetime(*map(int, re.split('[^\d]', field)[:6])) # stop after seconds. Debatable + d = dateutil.parser.parse(field) return d.utctimetuple() elif isinstance(field, (int, long, float)): return time.gmtime(field / 1000) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index d43dd54179..b1e5fa02c8 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1,6 +1,6 @@ import logging from cStringIO import StringIO -from math import exp, erf +from math import exp from lxml import etree from path import path # NOTE (THK): Only used for detecting presence of syllabus import requests @@ -12,14 +12,9 @@ from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.timeparse import parse_time from xmodule.util.decorators import lazyproperty from xmodule.graders import grader_from_conf -from datetime import datetime import json -import logging -import requests -import time -import copy -from xblock.core import Scope, ModelType, List, String, Object, Boolean +from xblock.core import Scope, List, String, Object, Boolean from .fields import Date @@ -29,30 +24,30 @@ log = logging.getLogger(__name__) class StringOrDate(Date): def from_json(self, value): """ - Parse an optional metadata key containing a time: if present, complain - if it doesn't parse. - Return None if not present or invalid. + Parse an optional metadata key containing a time or a string: + if present, assume it's a string if it doesn't parse. """ - if value is None: - return None - try: - return time.strptime(value, self.time_format) + result = super(StringOrDate, self).from_json(value) except ValueError: return value + if result is None: + return value + else: + return result def to_json(self, value): """ - Convert a time struct to a string + Convert a time struct or string to a string. """ - if value is None: - return None - try: - return time.strftime(self.time_format, value) - except (ValueError, TypeError): + result = super(StringOrDate, self).to_json(value) + except: return value - + if result is None: + return value + else: + return result edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, @@ -60,6 +55,7 @@ edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, _cached_toc = {} + class Textbook(object): def __init__(self, title, book_url): self.title = title @@ -179,7 +175,7 @@ class CourseFields(object): allow_anonymous_to_peers = Boolean(scope=Scope.settings, default=False) advanced_modules = List(help="Beta modules used in your course", scope=Scope.settings) has_children = True - checklists=List(scope=Scope.settings) + checklists = List(scope=Scope.settings) info_sidebar_name = String(scope=Scope.settings, default='Course Handouts') # An extra property is used rather than the wiki_slug/number because @@ -367,7 +363,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): textbooks.append((textbook.get('title'), textbook.get('book_url'))) xml_object.remove(textbook) - #Load the wiki tag if it exists + # Load the wiki tag if it exists wiki_slug = None wiki_tag = xml_object.find("wiki") if wiki_tag is not None: @@ -675,7 +671,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): # *end* of the same day, not the same time. It's going to be used as the # end of the exam overall, so we don't want the exam to disappear too soon. # It's also used optionally as the registration end date, so time matters there too. - self.last_eligible_appointment_date = self._try_parse_time('Last_Eligible_Appointment_Date') # or self.first_eligible_appointment_date + self.last_eligible_appointment_date = self._try_parse_time('Last_Eligible_Appointment_Date') # or self.first_eligible_appointment_date if self.last_eligible_appointment_date is None: raise ValueError("Last appointment date must be specified") self.registration_start_date = self._try_parse_time('Registration_Start_Date') or time.gmtime(0) diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index fb80752e56..99ead854ad 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -4,27 +4,35 @@ import re from datetime import timedelta from xblock.core import ModelType +import datetime +import dateutil.parser log = logging.getLogger(__name__) class Date(ModelType): - time_format = "%Y-%m-%dT%H:%M" - - def from_json(self, value): + ''' + Date fields know how to parse and produce json (iso) compatible formats. + ''' + # NB: these are copies of util.converters.* + def from_json(self, field): """ Parse an optional metadata key containing a time: if present, complain 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: - msg = "Field {0} has bad value '{1}': '{2}'".format( - self._name, value, e) + if field is None: + return field + elif isinstance(field, basestring): + d = dateutil.parser.parse(field) + return d.utctimetuple() + elif isinstance(field, (int, long, float)): + return time.gmtime(field / 1000) + elif isinstance(field, time.struct_time): + return field + else: + msg = "Field {0} has bad value '{1}'".format( + self._name, field) log.warning(msg) return None @@ -34,8 +42,11 @@ class Date(ModelType): """ if value is None: return None - - return time.strftime(self.time_format, value) + if isinstance(value, time.struct_time): + # struct_times are always utc + return time.strftime('%Y-%m-%dT%H:%M:%SZ', value) + elif isinstance(value, datetime.datetime): + return value.isoformat() + 'Z' TIMEDELTA_REGEX = re.compile(r'^((?P\d+?) day(?:s?))?(\s)?((?P\d+?) hour(?:s?))?(\s)?((?P\d+?) minute(?:s)?)?(\s)?((?P\d+?) second(?:s)?)?$') @@ -66,4 +77,4 @@ class Timedelta(ModelType): 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 + return ' '.join(values)