Skip to content

E
edx-platform
Commit 2be036fe by Braden MacDonald Committed by E. Kolpakov
Options

Simplifications and changes to reduce conflicts with PR 6399

parent d82da9e7
Showing with 48 additions and 52 deletions
common/lib/xmodule/xmodule/library_content_module.py
......@@ -323,7 +323,7 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe
js_module_name = "VerticalDescriptor"
@XBlock.handler
def refresh_children(self, request, suffix, update_db=True): # pylint: disable=unused-argument
def refresh_children(self, request=None, suffix=None, update_db=True): # pylint: disable=unused-argument
"""
Refresh children:
This method is to be used when any of the libraries that this block
......@@ -402,17 +402,12 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe
)
return validation
lib_tools = self.runtime.service(self, 'library_tools')
matching_children_count = 0
for library_key, version in self.source_libraries:
if not self._validate_library_version(validation, lib_tools, version, library_key):
break
library = lib_tools.get_library(library_key)
children_matching_filter = lib_tools.get_filtered_children(library, self.capa_type)
# get_filtered_children returns generator, so can't use len.
# And we don't actually need those children, so no point of constructing a list
matching_children_count += sum(1 for child in children_matching_filter)
# Note: we assume refresh_children() has been called since the last time fields like source_libraries or capa_types were changed.
matching_children_count = len(self.children) # pylint: disable=no-member
if matching_children_count == 0:
self._set_validation_error_if_empty(
validation,
......
common/lib/xmodule/xmodule/library_tools.py
......@@ -18,7 +18,7 @@ class LibraryToolsService(object):
def __init__(self, modulestore):
self.store = modulestore
def get_library(self, library_key):
def _get_library(self, library_key):
"""
Given a library key like "library-v1:ProblemX+PR0B", return the
'library' XBlock with meta-information about the library.
......@@ -39,39 +39,26 @@ class LibraryToolsService(object):
Get the version (an ObjectID) of the given library.
Returns None if the library does not exist.
"""
library = self.get_library(lib_key)
library = self._get_library(lib_key)
if library:
# We need to know the library's version so ensure it's set in library.location.library_key.version_guid
assert library.location.library_key.version_guid is not None
return library.location.library_key.version_guid
return None
def _filter_child(self, capa_type, child_descriptor):
def _filter_child(self, usage_key, capa_type):
"""
Filters children by CAPA problem type, if configured
"""
if capa_type == ANY_CAPA_TYPE_VALUE:
return True
if not isinstance(child_descriptor, CapaDescriptor):
if usage_key.block_type != "problem":
return False
return capa_type in child_descriptor.problem_types
def get_filtered_children(self, from_block, capa_type=ANY_CAPA_TYPE_VALUE):
"""
Filters children of `from_block` that satisfy filter criteria
Returns generator containing (child_key, child) for all children matching filter criteria
"""
children = (
(child_key, self.store.get_item(child_key, depth=None))
for child_key in from_block.children
)
return (
(child_key, child)
for child_key, child in children
if self._filter_child(capa_type, child)
)
descriptor = self.store.get_item(usage_key, depth=0)
assert isinstance(descriptor, CapaDescriptor)
return capa_type in descriptor.problem_types
def update_children(self, dest_block, user_id, user_perms=None, update_db=True):
"""
......@@ -104,7 +91,7 @@ class LibraryToolsService(object):
# First, load and validate the source_libraries:
libraries = []
for library_key, old_version in dest_block.source_libraries: # pylint: disable=unused-variable
library = self.get_library(library_key)
library = self._get_library(library_key)
if library is None:
raise ValueError("Required library not found.")
if user_perms and not user_perms.can_read(library_key):
......@@ -124,9 +111,12 @@ class LibraryToolsService(object):
Internal method to copy blocks from the library recursively
"""
new_children = []
target_capa_type = dest_block.capa_type if filter_problem_type else ANY_CAPA_TYPE_VALUE
filtered_children = self.get_filtered_children(from_block, target_capa_type)
for child_key, child in filtered_children:
if filter_problem_type:
filtered_children = [key for key in from_block.children if self._filter_child(key, dest_block.capa_type)]
else:
filtered_children = from_block.children
for child_key in filtered_children:
child = self.store.get_item(child_key, depth=None)
# We compute a block_id for each matching child block found in the library.
# block_ids are unique within any branch, but are not unique per-course or globally.
# We need our block_ids to be consistent when content in the library is updated, so
......
common/lib/xmodule/xmodule/tests/test_library_content.py
......@@ -138,9 +138,10 @@ class TestLibraries(MixedSplitTestCase):
# Check that get_content_titles() doesn't return titles for hidden/unused children
self.assertEqual(len(self.lc_block.get_content_titles()), 1)
def test_validation(self):
def test_validation_of_course_libraries(self):
"""
Test that the validation method of LibraryContent blocks is working.
Test that the validation method of LibraryContent blocks can validate
the source_libraries setting.
"""
# When source_libraries is blank, the validation summary should say this block needs to be configured:
self.lc_block.source_libraries = []
......@@ -166,11 +167,17 @@ class TestLibraries(MixedSplitTestCase):
self.assertIn("out of date", result.summary.text)
# Now if we update the block, all validation should pass:
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertTrue(self.lc_block.validate())
def test_validation_of_matching_blocks(self):
"""
Test that the validation method of LibraryContent blocks can warn
the user about problems with other settings (max_count and capa_type).
"""
# Set max_count to higher value than exists in library
self.lc_block.max_count = 50
self.lc_block.refresh_children() # In the normal studio editing process, editor_saved() calls refresh_children at this point
result = self.lc_block.validate()
self.assertFalse(result) # Validation fails due to at least one warning/message
self.assertTrue(result.summary)
......@@ -180,17 +187,19 @@ class TestLibraries(MixedSplitTestCase):
# Add some capa problems so we can check problem type validation messages
self.lc_block.max_count = 1
self._create_capa_problems()
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertTrue(self.lc_block.validate())
# Existing problem type should pass validation
self.lc_block.max_count = 1
self.lc_block.capa_type = 'multiplechoiceresponse'
self.lc_block.refresh_children()
self.assertTrue(self.lc_block.validate())
# ... unless requested more blocks than exists in library
self.lc_block.max_count = 10
self.lc_block.capa_type = 'multiplechoiceresponse'
self.lc_block.refresh_children()
result = self.lc_block.validate()
self.assertFalse(result) # Validation fails due to at least one warning/message
self.assertTrue(result.summary)
......@@ -200,6 +209,7 @@ class TestLibraries(MixedSplitTestCase):
# Missing problem type should always fail validation
self.lc_block.max_count = 1
self.lc_block.capa_type = 'customresponse'
self.lc_block.refresh_children()
result = self.lc_block.validate()
self.assertFalse(result) # Validation fails due to at least one warning/message
self.assertTrue(result.summary)
......@@ -213,21 +223,21 @@ class TestLibraries(MixedSplitTestCase):
self._create_capa_problems()
self.assertEqual(len(self.lc_block.children), 0) # precondition check
self.lc_block.capa_type = "multiplechoiceresponse"
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertEqual(len(self.lc_block.children), 1)
self.lc_block.capa_type = "optionresponse"
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertEqual(len(self.lc_block.children), 3)
self.lc_block.capa_type = "coderesponse"
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertEqual(len(self.lc_block.children), 2)
self.lc_block.capa_type = "customresponse"
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertEqual(len(self.lc_block.children), 0)
self.lc_block.capa_type = ANY_CAPA_TYPE_VALUE
self.lc_block.refresh_children(None, None)
self.lc_block.refresh_children()
self.assertEqual(len(self.lc_block.children), len(self.lib_blocks) + 4)
common/test/acceptance/tests/studio/test_studio_library_container.py
......@@ -43,16 +43,6 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest):
XBlockFixtureDesc("html", "Html1"),
XBlockFixtureDesc("html", "Html2"),
XBlockFixtureDesc("html", "Html3"),
XBlockFixtureDesc(
"problem", "Dropdown",
data=textwrap.dedent("""
<problem>
<p>Dropdown</p>
<optionresponse><optioninput label="Dropdown" options="('1', '2')" correct="'2'"></optioninput></optionresponse>
</problem>
""")
)
)
def populate_course_fixture(self, course_fixture):
......@@ -189,9 +179,20 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest):
When I go to studio unit page for library content block
And I set Problem Type selector so that no libraries have matching content
Then I can see that "No matching content" warning is shown
When I set Problem Type selector so that there are matching content
When I set Problem Type selector so that there is matching content
Then I can see that warning messages are not shown
"""
# Add a single "Dropdown" type problem to the library (which otherwise has only HTML blocks):
self.library_fixture.create_xblock(self.library_fixture.library_location, XBlockFixtureDesc(
"problem", "Dropdown",
data=textwrap.dedent("""
<problem>
<p>Dropdown</p>
<optionresponse><optioninput label="Dropdown" options="('1', '2')" correct="'2'"></optioninput></optionresponse>
</problem>
""")
))
expected_text = 'There are no matching problem types in the specified libraries. Select another problem type'
library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0])
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment