prioritize Library's defaults over inherited ones.
This commit is contained in:
@@ -283,6 +283,16 @@ class InheritingFieldData(KvsFieldData):
|
||||
super(InheritingFieldData, self).__init__(**kwargs)
|
||||
self.inheritable_names = set(inheritable_names)
|
||||
|
||||
def has_default_value(self, name):
|
||||
"""
|
||||
Return whether or not the field `name` has a default value
|
||||
"""
|
||||
has_default_value = getattr(self._kvs, 'has_default_value', False)
|
||||
if callable(has_default_value):
|
||||
return has_default_value(name)
|
||||
|
||||
return has_default_value
|
||||
|
||||
def default(self, block, name):
|
||||
"""
|
||||
The default for an inheritable name is found on a parent.
|
||||
@@ -294,6 +304,15 @@ class InheritingFieldData(KvsFieldData):
|
||||
# node of the tree, the block's default will be used.
|
||||
field = block.fields[name]
|
||||
ancestor = block.get_parent()
|
||||
# In case, if block's parent is of type 'library_content',
|
||||
# bypass inheritance and use kvs' default instead of reusing
|
||||
# from parent as '_copy_from_templates' puts fields into
|
||||
# defaults.
|
||||
if ancestor and \
|
||||
ancestor.location.category == 'library_content' and \
|
||||
self.has_default_value(name):
|
||||
return super(InheritingFieldData, self).default(block, name)
|
||||
|
||||
while ancestor is not None:
|
||||
if field.is_set_on(ancestor):
|
||||
return field.read_json(ancestor)
|
||||
|
||||
@@ -2517,7 +2517,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
del new_block_info.defaults['markdown']
|
||||
# </workaround>
|
||||
|
||||
new_block_info.fields = existing_block_info.fields # Preserve any existing overrides
|
||||
# Preserve any existing overrides
|
||||
new_block_info.fields = existing_block_info.fields
|
||||
|
||||
if 'children' in new_block_info.defaults:
|
||||
del new_block_info.defaults['children'] # Will be set later
|
||||
|
||||
|
||||
@@ -136,7 +136,7 @@ class SplitMongoKVS(InheritanceKeyValueStore):
|
||||
|
||||
def has(self, key):
|
||||
"""
|
||||
Is the given field explicitly set in this kvs (not inherited nor default)
|
||||
Is the given field explicitly set in this kvs (neither inherited nor default)
|
||||
"""
|
||||
# handle any special cases
|
||||
if key.scope not in self.VALID_SCOPES:
|
||||
@@ -158,6 +158,12 @@ class SplitMongoKVS(InheritanceKeyValueStore):
|
||||
# if someone changes it so that they do, then change any tests of field.name in xx._field_data
|
||||
return key.field_name in self._fields
|
||||
|
||||
def has_default_value(self, field_name):
|
||||
"""
|
||||
Is the given field has default value in this kvs
|
||||
"""
|
||||
return field_name in self._defaults
|
||||
|
||||
def default(self, key):
|
||||
"""
|
||||
Check to see if the default should be from the template's defaults (if any)
|
||||
|
||||
@@ -5,6 +5,7 @@ import unittest
|
||||
|
||||
from mock import Mock
|
||||
from nose.tools import assert_equals, assert_not_equals, assert_true, assert_false, assert_in, assert_not_in # pylint: disable=no-name-in-module
|
||||
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
|
||||
|
||||
from xblock.field_data import DictFieldData
|
||||
from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
|
||||
@@ -12,6 +13,7 @@ from xblock.runtime import KvsFieldData, DictKeyValueStore
|
||||
|
||||
from xmodule.fields import Date, Timedelta, RelativeTime
|
||||
from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin, InheritingFieldData
|
||||
from xmodule.modulestore.split_mongo.split_mongo_kvs import SplitMongoKVS
|
||||
from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
|
||||
from xmodule.course_module import CourseDescriptor
|
||||
from xmodule.seq_module import SequenceDescriptor
|
||||
@@ -56,15 +58,20 @@ class TestFields(object):
|
||||
|
||||
|
||||
class InheritingFieldDataTest(unittest.TestCase):
|
||||
"""Tests of InheritingFieldData."""
|
||||
"""
|
||||
Tests of InheritingFieldData.
|
||||
"""
|
||||
|
||||
class TestableInheritingXBlock(XmlDescriptor):
|
||||
"""An XBlock we can use in these tests."""
|
||||
"""
|
||||
An XBlock we can use in these tests.
|
||||
"""
|
||||
inherited = String(scope=Scope.settings, default="the default")
|
||||
not_inherited = String(scope=Scope.settings, default="nothing")
|
||||
|
||||
def setUp(self):
|
||||
super(InheritingFieldDataTest, self).setUp()
|
||||
self.dummy_course_key = CourseLocator('test_org', 'test_123', 'test_run')
|
||||
self.system = get_test_descriptor_system()
|
||||
self.all_blocks = {}
|
||||
self.system.get_block = self.all_blocks.get
|
||||
@@ -73,11 +80,34 @@ class InheritingFieldDataTest(unittest.TestCase):
|
||||
kvs=DictKeyValueStore({}),
|
||||
)
|
||||
|
||||
def get_block_using_split_kvs(self, block_type, block_id, fields, defaults):
|
||||
"""
|
||||
Construct an Xblock with split mongo kvs.
|
||||
"""
|
||||
kvs = SplitMongoKVS(
|
||||
definition=Mock(),
|
||||
initial_values=fields,
|
||||
default_values=defaults,
|
||||
parent=None
|
||||
)
|
||||
self.field_data = InheritingFieldData(
|
||||
inheritable_names=['inherited'],
|
||||
kvs=kvs,
|
||||
)
|
||||
block = self.get_a_block(
|
||||
usage_id=self.get_usage_id(block_type, block_id)
|
||||
)
|
||||
|
||||
return block
|
||||
|
||||
def get_a_block(self, usage_id=None):
|
||||
"""Construct an XBlock for testing with."""
|
||||
"""
|
||||
Construct an XBlock for testing with.
|
||||
"""
|
||||
scope_ids = Mock()
|
||||
if usage_id is None:
|
||||
usage_id = "_auto%d" % len(self.all_blocks)
|
||||
block_id = "_auto{id}".format(id=len(self.all_blocks))
|
||||
usage_id = self.get_usage_id("course", block_id)
|
||||
scope_ids.usage_id = usage_id
|
||||
block = self.system.construct_xblock_from_class(
|
||||
self.TestableInheritingXBlock,
|
||||
@@ -87,14 +117,24 @@ class InheritingFieldDataTest(unittest.TestCase):
|
||||
self.all_blocks[usage_id] = block
|
||||
return block
|
||||
|
||||
def get_usage_id(self, block_type, block_id):
|
||||
"""
|
||||
Constructs usage id using 'block_type' and 'block_id'
|
||||
"""
|
||||
return BlockUsageLocator(self.dummy_course_key, block_type=block_type, block_id=block_id)
|
||||
|
||||
def test_default_value(self):
|
||||
# Blocks with nothing set with return the fields' defaults.
|
||||
"""
|
||||
Test that the Blocks with nothing set with return the fields' defaults.
|
||||
"""
|
||||
block = self.get_a_block()
|
||||
self.assertEqual(block.inherited, "the default")
|
||||
self.assertEqual(block.not_inherited, "nothing")
|
||||
|
||||
def test_set_value(self):
|
||||
# If you set a value, that's what you get back.
|
||||
"""
|
||||
Test that If you set a value, that's what you get back.
|
||||
"""
|
||||
block = self.get_a_block()
|
||||
block.inherited = "Changed!"
|
||||
block.not_inherited = "New Value!"
|
||||
@@ -102,36 +142,86 @@ class InheritingFieldDataTest(unittest.TestCase):
|
||||
self.assertEqual(block.not_inherited, "New Value!")
|
||||
|
||||
def test_inherited(self):
|
||||
# A child with get a value inherited from the parent.
|
||||
parent = self.get_a_block(usage_id="parent")
|
||||
parent.inherited = "Changed!"
|
||||
self.assertEqual(parent.inherited, "Changed!")
|
||||
"""
|
||||
Test that a child with get a value inherited from the parent.
|
||||
"""
|
||||
parent_block = self.get_a_block(usage_id=self.get_usage_id("course", "parent"))
|
||||
parent_block.inherited = "Changed!"
|
||||
self.assertEqual(parent_block.inherited, "Changed!")
|
||||
|
||||
child = self.get_a_block(usage_id="child")
|
||||
child.parent = "parent"
|
||||
child = self.get_a_block(usage_id=self.get_usage_id("vertical", "child"))
|
||||
child.parent = parent_block.location
|
||||
self.assertEqual(child.inherited, "Changed!")
|
||||
|
||||
def test_inherited_across_generations(self):
|
||||
# A child with get a value inherited from a great-grandparent.
|
||||
parent = self.get_a_block(usage_id="parent")
|
||||
"""
|
||||
Test that a child with get a value inherited from a great-grandparent.
|
||||
"""
|
||||
parent = self.get_a_block(usage_id=self.get_usage_id("course", "parent"))
|
||||
parent.inherited = "Changed!"
|
||||
self.assertEqual(parent.inherited, "Changed!")
|
||||
for child_num in range(10):
|
||||
usage_id = "child_{}".format(child_num)
|
||||
usage_id = self.get_usage_id("vertical", "child_{}".format(child_num))
|
||||
child = self.get_a_block(usage_id=usage_id)
|
||||
child.parent = "parent"
|
||||
child.parent = parent.location
|
||||
self.assertEqual(child.inherited, "Changed!")
|
||||
|
||||
def test_not_inherited(self):
|
||||
# Fields not in the inherited_names list won't be inherited.
|
||||
parent = self.get_a_block(usage_id="parent")
|
||||
"""
|
||||
Test that the fields not in the inherited_names list won't be inherited.
|
||||
"""
|
||||
parent = self.get_a_block(usage_id=self.get_usage_id("course", "parent"))
|
||||
parent.not_inherited = "Changed!"
|
||||
self.assertEqual(parent.not_inherited, "Changed!")
|
||||
|
||||
child = self.get_a_block(usage_id="child")
|
||||
child.parent = "parent"
|
||||
child = self.get_a_block(usage_id=self.get_usage_id("vertical", "child"))
|
||||
child.parent = parent.location
|
||||
self.assertEqual(child.not_inherited, "nothing")
|
||||
|
||||
def test_non_defaults_inherited_across_lib(self):
|
||||
"""
|
||||
Test that a child inheriting from library_content block, inherits fields
|
||||
from parent if these fields are not in its defaults.
|
||||
"""
|
||||
parent_block = self.get_block_using_split_kvs(
|
||||
block_type="library_content",
|
||||
block_id="parent",
|
||||
fields=dict(inherited="changed!"),
|
||||
defaults=dict(inherited="parent's default"),
|
||||
)
|
||||
self.assertEqual(parent_block.inherited, "changed!")
|
||||
|
||||
child = self.get_block_using_split_kvs(
|
||||
block_type="problem",
|
||||
block_id="child",
|
||||
fields={},
|
||||
defaults={},
|
||||
)
|
||||
child.parent = parent_block.location
|
||||
self.assertEqual(child.inherited, "changed!")
|
||||
|
||||
def test_defaults_not_inherited_across_lib(self):
|
||||
"""
|
||||
Test that a child inheriting from library_content block, does not inherit
|
||||
fields from parent if these fields are in its defaults already.
|
||||
"""
|
||||
parent_block = self.get_block_using_split_kvs(
|
||||
block_type="library_content",
|
||||
block_id="parent",
|
||||
fields=dict(inherited="changed!"),
|
||||
defaults=dict(inherited="parent's default"),
|
||||
)
|
||||
self.assertEqual(parent_block.inherited, "changed!")
|
||||
|
||||
child = self.get_block_using_split_kvs(
|
||||
block_type="library_content",
|
||||
block_id="parent",
|
||||
fields={},
|
||||
defaults=dict(inherited="child's default"),
|
||||
)
|
||||
child.parent = parent_block.location
|
||||
self.assertEqual(child.inherited, "child's default")
|
||||
|
||||
|
||||
class EditableMetadataFieldsTest(unittest.TestCase):
|
||||
def test_display_name_field(self):
|
||||
|
||||
Reference in New Issue
Block a user