diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index cd203e6af7..578b82b3cf 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/cms/djangoapps/contentstore/tests/test_item.py @@ -4,6 +4,8 @@ from django.core.urlresolvers import reverse from xmodule.capa_module import CapaDescriptor import json from xmodule.modulestore.django import modulestore +import datetime +from pytz import UTC class DeleteItem(CourseTestCase): @@ -151,16 +153,16 @@ class TestEditItem(CourseTestCase): reverse('create_item'), json.dumps( {'parent_location': chap_location, - 'category': 'vertical' + 'category': 'sequential' }), content_type="application/json" ) - vert_location = self.response_id(resp) + self.seq_location = self.response_id(resp) # create problem w/ boilerplate template_id = 'multiplechoice.yaml' resp = self.client.post( reverse('create_item'), - json.dumps({'parent_location': vert_location, + json.dumps({'parent_location': self.seq_location, 'category': 'problem', 'boilerplate': template_id }), @@ -210,3 +212,32 @@ class TestEditItem(CourseTestCase): ) problem = modulestore('draft').get_item(self.problems[0]) self.assertIsNone(problem.markdown) + + def test_date_fields(self): + """ + Test setting due & start dates on sequential + """ + sequential = modulestore().get_item(self.seq_location) + self.assertIsNone(sequential.lms.due) + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.seq_location, + 'metadata': {'due': '2010-11-22T04:00Z'} + }), + content_type="application/json" + ) + sequential = modulestore().get_item(self.seq_location) + self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) + self.client.post( + reverse('save_item'), + json.dumps({ + 'id': self.seq_location, + 'metadata': {'start': '2010-09-12T14:00Z'} + }), + content_type="application/json" + ) + sequential = modulestore().get_item(self.seq_location) + self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC)) + self.assertEqual(sequential.lms.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC)) + diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 9a5e40e967..efebded9b9 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -59,17 +59,21 @@ def save_item(request): # 'apply' the submitted metadata, so we don't end up deleting system metadata existing_item = modulestore().get_item(item_location) for metadata_key in request.POST.get('nullout', []): - setattr(existing_item, metadata_key, None) + # [dhm] see comment on _get_xblock_field + _get_xblock_field(existing_item, metadata_key).write_to(existing_item, None) # update existing metadata with submitted metadata (which can be partial) # IMPORTANT NOTE: if the client passed 'null' (None) for a piece of metadata that means 'remove it'. If # the intent is to make it None, use the nullout field for metadata_key, value in request.POST.get('metadata', {}).items(): + # [dhm] see comment on _get_xblock_field + field = _get_xblock_field(existing_item, metadata_key) if value is None: - delattr(existing_item, metadata_key) + field.delete_from(existing_item) else: - setattr(existing_item, metadata_key, value) + value = field.from_json(value) + field.write_to(existing_item, value) # Save the data that we've just changed to the underlying # MongoKeyValueStore before we update the mongo datastore. existing_item.save() @@ -79,6 +83,33 @@ def save_item(request): return HttpResponse() +# [DHM] A hack until we implement a permanent soln. Proposed perm solution is to make namespace fields also top level +# fields in xblocks rather than requiring dereference through namespace but we'll need to consider whether there are +# plausible use cases for distinct fields w/ same name in different namespaces on the same blocks. +# The idea is that consumers of the xblock, and particularly the web client, shouldn't know about our internal +# representation (namespaces as means of decorating all modules). +# Given top-level access, the calls can simply be setattr(existing_item, field, value) ... +# Really, this method should be elsewhere (e.g., xblock). We also need methods for has_value (v is_default)... +def _get_xblock_field(xblock, field_name): + """ + A temporary function to get the xblock field either from the xblock or one of its namespaces by name. + :param xblock: + :param field_name: + """ + def find_field(fields): + for field in fields: + if field.name == field_name: + return field + + found = find_field(xblock.fields) + if found: + return found + for namespace in xblock.namespaces: + found = find_field(getattr(xblock, namespace).fields) + if found: + return found + + @login_required @expect_json def create_item(request): diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 3d8cd7684e..329624ef46 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -253,17 +253,13 @@ function syncReleaseDate(e) { } function getEdxTimeFromDateTimeVals(date_val, time_val) { - var edxTimeStr = null; - if (date_val != '') { if (time_val == '') time_val = '00:00'; - // Note, we are using date.js utility which has better parsing abilities than the built in JS date parsing - var date = Date.parse(date_val + " " + time_val); - edxTimeStr = date.toString('yyyy-MM-ddTHH:mm'); + return new Date(date_val + " " + time_val + "Z"); } - return edxTimeStr; + else return null; } function getEdxTimeFromDateTimeInputs(date_id, time_id) { diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index a9b4be4fcd..465993a51f 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -58,8 +58,7 @@ class Date(ModelType): else: msg = "Field {0} has bad value '{1}'".format( self._name, field) - log.warning(msg) - return None + raise TypeError(msg) def to_json(self, value): """ @@ -76,6 +75,8 @@ class Date(ModelType): return value.strftime('%Y-%m-%dT%H:%M:%SZ') else: return value.isoformat() + else: + raise TypeError("Cannot convert {} to json".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)?)?$') diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index f0eb082dcf..8453adaa20 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -44,7 +44,8 @@ class DateTest(unittest.TestCase): def test_return_None(self): self.assertIsNone(DateTest.date.from_json("")) self.assertIsNone(DateTest.date.from_json(None)) - self.assertIsNone(DateTest.date.from_json(['unknown value'])) + with self.assertRaises(TypeError): + DateTest.date.from_json(['unknown value']) def test_old_due_date_format(self): current = datetime.datetime.today() @@ -83,6 +84,8 @@ class DateTest(unittest.TestCase): DateTest.date.to_json( DateTest.date.from_json("2012-12-31T23:00:01-01:00")), "2012-12-31T23:00:01-01:00") + with self.assertRaises(TypeError): + DateTest.date.to_json('2012-12-31T23:00:01-01:00') class TimedeltaTest(unittest.TestCase):