Merge pull request #421 from edx/dhm/template_acceptance

Fix due/start date setting in edit_subsection

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))
+

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):

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) {

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<days>\d+?) day(?:s?))?(\s)?((?P<hours>\d+?) hour(?:s?))?(\s)?((?P<minutes>\d+?) minute(?:s)?)?(\s)?((?P<seconds>\d+?) second(?:s)?)?$')
 

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):