Merge pull request #15 from edx/dcs/inheritance

Changed FieldData inheritance to match other FieldData subclasses' behavior

edx_when/__init__.py

@@ -4,6 +4,6 @@
 
 from __future__ import absolute_import, unicode_literals
 
-__version__ = '0.1.6'
+__version__ = '0.2'
 
 default_app_config = 'edx_when.apps.EdxWhenConfig'  # pylint: disable=invalid-name

edx_when/field_data.py

@@ -8,11 +8,28 @@
 
 from . import api
 
+try:
+    from xmodule.modulestore.inheritance import InheritanceMixin
+    INHERITABLE_FIELDS = set(InheritanceMixin.fields.keys())
+except ImportError:
+    INHERITABLE_FIELDS = set(('due', 'start', 'end'))
+
+
 log = logging.getLogger('edx-when')
 
 NOT_FOUND = object()
 
 
+def _lineage(block):
+    """
+    Return an iterator over all ancestors of the given block.
+    """
+    parent = block.get_parent()
+    while parent:
+        yield parent
+        parent = parent.get_parent()
+
+
 class DateLookupFieldData(FieldData):
     """
     FieldData instance that looks up date fields in django models.
@@ -47,33 +64,49 @@ def has(self, block, name):
         """
         Return whether the field exists in the block.
         """
-        try:
-            return bool(self.get(block, name))
-        except KeyError:
-            return False
+        val = self._get(block, name)
+        if val is NOT_FOUND:
+            if name in INHERITABLE_FIELDS:
+                for ancestor in _lineage(block):
+                    if self._get(ancestor, name) is not NOT_FOUND:
+                        return False
 
-    def get(self, block, name):
+        return val is not NOT_FOUND or self._defaults.has(block, name)
+
+    def _get(self, block, name):
         """
-        Return field value for given block and field name.
+        Return the value, if it's in edx-when.
+
+        Otherwise, return NOT_FOUND
         """
         if not isinstance(name, text_type):
             name = text_type(name)
         if name in api.FIELDS_TO_EXTRACT:
             val = self._course_dates.get((text_type(block.location), name), NOT_FOUND)
         else:
             val = NOT_FOUND
-        if val is NOT_FOUND:
-            try:
-                val = self._defaults.get(block, name)
-            except KeyError:
-                parent = block.get_parent()
-                if parent:
-                    val = self.get(parent, name)
-                else:
-                    raise
-            log.debug("NOT FOUND %r %r", (block.location, name), self._course_dates)
         return val
 
+    def get(self, block, name):
+        """
+        Return field value for given block and field name.
+        """
+        val = self._get(block, name)
+        if val is not NOT_FOUND:
+            return val
+        return self._defaults.get(block, name)
+
+    def default(self, block, name):
+        """
+        Return the default for the field.
+        """
+        if name in INHERITABLE_FIELDS:
+            for ancestor in _lineage(block):
+                value = self._get(ancestor, name)
+                if value is not NOT_FOUND:
+                    return value
+        return self._defaults.default(block, name)
+
     def set(self, block, name, value):
         """
         Set the value in the default FieldData.

tests/test_xblock_services.py

@@ -62,17 +62,30 @@ def test_field_data_get(self):
         assert defaults.get.called_once_with(block.location, 'foo')
 
         # non-existent value
-        defaults.get.side_effect = KeyError()
+        defaults.has.return_value = False
         badblock = MockBlock('foo')
         assert dfd.has(badblock, 'foo') is False
 
         # block with parent with date
         child = MockBlock('child', block)
         date = dfd.get(child, 'due')
-        assert date == self.items[0][1]['due']
+        assert dfd.get(child, 'due') is defaults.get(child, 'due')
+        assert dfd.default(child, 'due') == self.items[0][1]['due']
+
+        assert dfd.get(badblock, 'due') is defaults.get(badblock, 'due')
 
-        with self.assertRaises(KeyError):
-            dfd.get(badblock, 'due')
+    def test_field_data_has(self):
+        defaults = mock.MagicMock()
+        dfd = field_data.DateLookupFieldData(defaults, course_id=self.course_id, use_cached=False)
+        block = MockBlock(self.items[0][0])
+
+        assert dfd.has(block, 'due') is True
+        assert dfd.has(block, 'foo') is defaults.has(block, 'foo')
+        child = MockBlock('child', block)
+        # import pdb;pdb.set_trace()
+        assert dfd.has(child, 'due') is False
+        assert dfd.default(child, 'due') == self.items[0][1]['due']
+        assert dfd.default(child, 'foo') is defaults.default(child, 'foo')
 
     def test_field_data_set_delete(self):
         defaults = mock.MagicMock()