pylint fixes

This commit is contained in:
E. Kolpakov
2015-01-12 18:09:03 +03:00
committed by Braden MacDonald
parent 134a75b367
commit 195d5b57bc
26 changed files with 183 additions and 77 deletions

View File

@@ -177,7 +177,7 @@ class CapaDescriptor(CapaFields, RawDescriptor):
@property
def problem_types(self):
""" Low-level problem type introspection for content libraries filtering by problem type """
tree = etree.XML(self.data)
tree = etree.XML(self.data) # pylint: disable=no-member
registered_tags = responsetypes.registry.registered_tags()
return set([node.tag for node in tree.iter() if node.tag in registered_tags])

View File

@@ -20,7 +20,7 @@ from xmodule.validation import StudioValidationMessage, StudioValidation
from xmodule.x_module import XModule, STUDENT_VIEW
from xmodule.studio_editable import StudioEditableModule, StudioEditableDescriptor
from .xml_module import XmlDescriptor
from pkg_resources import resource_string
from pkg_resources import resource_string # pylint: disable=no-name-in-module
# Make '_' a no-op so we can scrape strings
@@ -187,7 +187,8 @@ class LibraryContentFields(object):
scope=Scope.settings,
)
selected = List(
# This is a list of (block_type, block_id) tuples used to record which random/first set of matching blocks was selected per user
# This is a list of (block_type, block_id) tuples used to record
# which random/first set of matching blocks was selected per user
default=[],
scope=Scope.user_state,
)
@@ -296,7 +297,8 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule):
'display_name': self.display_name or self.url_name,
}))
self.render_children(context, fragment, can_reorder=False, can_add=False)
# else: When shown on a unit page, don't show any sort of preview - just the status of this block in the validation area.
# else: When shown on a unit page, don't show any sort of preview -
# just the status of this block in the validation area.
# The following JS is used to make the "Update now" button work on the unit page and the container view:
fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js'))
@@ -412,7 +414,8 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe
if not self._validate_library_version(validation, lib_tools, version, library_key):
break
# Note: we assume refresh_children() has been called since the last time fields like source_libraries or capa_types were changed.
# 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(

View File

@@ -93,4 +93,5 @@ class LibraryToolsService(object):
dest_block.source_libraries = new_libraries
self.store.update_item(dest_block, user_id)
dest_block.children = self.store.copy_from_template(source_blocks, dest_block.location, user_id)
# ^-- copy_from_template updates the children in the DB but we must also set .children here to avoid overwriting the DB again
# ^-- copy_from_template updates the children in the DB
# but we must also set .children here to avoid overwriting the DB again

View File

@@ -673,7 +673,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
new_module_data = {}
for block_id in base_block_ids:
new_module_data = self.descendants(
copy.deepcopy(system.course_entry.structure['blocks']), # copy or our changes like setting 'definition_loaded' will affect the active bulk operation data
# copy or our changes like setting 'definition_loaded' will affect the active bulk operation data
copy.deepcopy(system.course_entry.structure['blocks']),
block_id,
depth,
new_module_data
@@ -2125,8 +2126,11 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
# Set of all descendent block IDs of dest_usage that are to be replaced:
block_key = BlockKey(dest_usage.block_type, dest_usage.block_id)
orig_descendants = set(self.descendants(dest_structure['blocks'], block_key, depth=None, descendent_map={}))
orig_descendants.remove(block_key) # The descendants() method used above adds the block itself, which we don't consider a descendant.
new_descendants = self._copy_from_template(source_structures, source_keys, dest_structure, block_key, user_id)
# The descendants() method used above adds the block itself, which we don't consider a descendant.
orig_descendants.remove(block_key)
new_descendants = self._copy_from_template(
source_structures, source_keys, dest_structure, block_key, user_id
)
# Update the edit info:
dest_info = dest_structure['blocks'][block_key]
@@ -2144,7 +2148,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
self.update_structure(destination_course, dest_structure)
self._update_head(destination_course, index_entry, destination_course.branch, dest_structure['_id'])
# Return usage locators for all the new children:
return [destination_course.make_usage_key(*k) for k in dest_structure['blocks'][block_key]['fields']['children']]
return [
destination_course.make_usage_key(*k)
for k in dest_structure['blocks'][block_key]['fields']['children']
]
def _copy_from_template(self, source_structures, source_keys, dest_structure, new_parent_block_key, user_id):
"""
@@ -2184,10 +2191,13 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
new_block_info['defaults'] = new_block_info['fields']
# <workaround>
# CAPA modules store their 'markdown' value (an alternate representation of their content) in Scope.settings rather than Scope.content :-/
# CAPA modules store their 'markdown' value (an alternate representation of their content)
# in Scope.settings rather than Scope.content :-/
# markdown is a field that really should not be overridable - it fundamentally changes the content.
# capa modules also use a custom editor that always saves their markdown field to the metadata, even if it hasn't changed, which breaks our override system.
# So until capa modules are fixed, we special-case them and remove their markdown fields, forcing the inherited version to use XML only.
# capa modules also use a custom editor that always saves their markdown field to the metadata,
# even if it hasn't changed, which breaks our override system.
# So until capa modules are fixed, we special-case them and remove their markdown fields,
# forcing the inherited version to use XML only.
if usage_key.block_type == 'problem' and 'markdown' in new_block_info['defaults']:
del new_block_info['defaults']['markdown']
# </workaround>
@@ -2199,7 +2209,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
new_block_info['edit_info'] = existing_block_info.get('edit_info', {})
new_block_info['edit_info']['previous_version'] = new_block_info['edit_info'].get('update_version', None)
new_block_info['edit_info']['update_version'] = dest_structure['_id']
# Note we do not set 'source_version' - it's only used for copying identical blocks from draft to published as part of publishing workflow.
# Note we do not set 'source_version' - it's only used for copying identical blocks
# from draft to published as part of publishing workflow.
# Setting it to the source_block_info structure version here breaks split_draft's has_changes() method.
new_block_info['edit_info']['edited_by'] = user_id
new_block_info['edit_info']['edited_on'] = datetime.datetime.now(UTC)
@@ -2208,7 +2219,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
children = source_block_info['fields'].get('children')
if children:
children = [src_course_key.make_usage_key(child.type, child.id) for child in children]
new_blocks |= self._copy_from_template(source_structures, children, dest_structure, new_block_key, user_id)
new_blocks |= self._copy_from_template(
source_structures, children, dest_structure, new_block_key, user_id
)
new_blocks.add(new_block_key)
# And add new_block_key to the list of new_parent_block_key's new children:

View File

@@ -110,7 +110,8 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
if usage_key.category in DIRECT_ONLY_CATEGORIES:
self.publish(usage_key.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)
children = getattr(self.get_item(usage_key, **kwargs), "children", [])
keys_to_check.extend(children) # e.g. if usage_key is a chapter, it may have an auto-publish sequential child
# e.g. if usage_key is a chapter, it may have an auto-publish sequential child
keys_to_check.extend(children)
return new_keys
def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs):
@@ -431,6 +432,7 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
pass
def _get_head(self, xblock, branch):
""" Gets block at the head of specified branch """
try:
course_structure = self._lookup_course(xblock.location.course_key.for_branch(branch)).structure
except ItemNotFoundError:

View File

@@ -28,12 +28,18 @@ class TestSplitCopyTemplate(MixedSplitTestCase):
# Add a vertical with a capa child to the source library/course:
vertical_block = self.make_block("vertical", source_container)
problem_library_display_name = "Problem Library Display Name"
problem_block = self.make_block("problem", vertical_block, display_name=problem_library_display_name, markdown="Problem markdown here")
problem_block = self.make_block(
"problem", vertical_block, display_name=problem_library_display_name, markdown="Problem markdown here"
)
if source_type == LibraryFactory:
source_container = self.store.get_library(source_container.location.library_key, remove_version=False, remove_branch=False)
source_container = self.store.get_library(
source_container.location.library_key, remove_version=False, remove_branch=False
)
else:
source_container = self.store.get_course(source_container.location.course_key, remove_version=False, remove_branch=False)
source_container = self.store.get_course(
source_container.location.course_key, remove_version=False, remove_branch=False
)
# Inherit the vertical and the problem from the library into the course:
source_keys = [source_container.children[0]]
@@ -48,7 +54,8 @@ class TestSplitCopyTemplate(MixedSplitTestCase):
problem_block_course = self.store.get_item(vertical_block_course.children[0])
self.assertEqual(problem_block_course.display_name, problem_library_display_name)
# Check that when capa modules are copied, their "markdown" fields (Scope.settings) are removed. (See note in split.py:copy_from_template())
# Check that when capa modules are copied, their "markdown" fields (Scope.settings) are removed.
# (See note in split.py:copy_from_template())
self.assertIsNotNone(problem_block.markdown)
self.assertIsNone(problem_block_course.markdown)
@@ -59,7 +66,8 @@ class TestSplitCopyTemplate(MixedSplitTestCase):
problem_block_course.weight = new_weight
self.store.update_item(problem_block_course, self.user_id)
# Test that "Any previously existing children of `dest_usage` that haven't been replaced/updated by this copy_from_template operation will be deleted."
# Test that "Any previously existing children of `dest_usage`
# that haven't been replaced/updated by this copy_from_template operation will be deleted."
extra_block = self.make_block("html", vertical_block_course)
# Repeat the copy_from_template():
@@ -86,19 +94,26 @@ class TestSplitCopyTemplate(MixedSplitTestCase):
display_name_expected = "CUSTOM Library Display Name"
self.make_block("problem", source_library, display_name=display_name_expected)
# Reload source_library since we need its branch and version to use copy_from_template:
source_library = self.store.get_library(source_library.location.library_key, remove_version=False, remove_branch=False)
source_library = self.store.get_library(
source_library.location.library_key, remove_version=False, remove_branch=False
)
# And a course with a vertical:
course = CourseFactory.create(modulestore=self.store)
self.make_block("vertical", course)
problem_key_in_course = self.store.copy_from_template(source_library.children, dest_key=course.location, user_id=self.user_id)[0]
problem_key_in_course = self.store.copy_from_template(
source_library.children, dest_key=course.location, user_id=self.user_id
)[0]
# We do the following twice because different methods get used inside split modulestore on first vs. subsequent publish
# We do the following twice because different methods get used inside
# split modulestore on first vs. subsequent publish
for __ in range(0, 2):
# Publish:
self.store.publish(problem_key_in_course, self.user_id)
# Test that the defaults values are there.
problem_published = self.store.get_item(problem_key_in_course.for_branch(ModuleStoreEnum.BranchName.published))
problem_published = self.store.get_item(
problem_key_in_course.for_branch(ModuleStoreEnum.BranchName.published)
)
self.assertEqual(problem_published.display_name, display_name_expected)
def test_copy_from_template_auto_publish(self):
@@ -119,7 +134,9 @@ class TestSplitCopyTemplate(MixedSplitTestCase):
html = self.make_block("html", source_course)
# Reload source_course since we need its branch and version to use copy_from_template:
source_course = self.store.get_course(source_course.location.course_key, remove_version=False, remove_branch=False)
source_course = self.store.get_course(
source_course.location.course_key, remove_version=False, remove_branch=False
)
# Inherit the vertical and the problem from the library into the course:
source_keys = [block.location for block in [about, chapter, html]]
@@ -146,10 +163,13 @@ class TestSplitCopyTemplate(MixedSplitTestCase):
# Check that the auto-publish blocks have been published:
self.assertFalse(self.store.has_changes(new_blocks["about"]))
self.assertTrue(published_version_exists(new_blocks["chapter"])) # We can't use has_changes because it includes descendants
# We can't use has_changes because it includes descendants
self.assertTrue(published_version_exists(new_blocks["chapter"]))
self.assertTrue(published_version_exists(new_blocks["sequential"])) # Ditto
# Check that non-auto-publish blocks and blocks with non-auto-publish descendants show changes:
self.assertTrue(self.store.has_changes(new_blocks["html"]))
self.assertTrue(self.store.has_changes(new_blocks["problem"]))
self.assertTrue(self.store.has_changes(new_blocks["chapter"])) # Will have changes since a child block has changes.
self.assertFalse(published_version_exists(new_blocks["vertical"])) # Verify that our published_version_exists works
# Will have changes since a child block has changes.
self.assertTrue(self.store.has_changes(new_blocks["chapter"]))
# Verify that our published_version_exists works
self.assertFalse(published_version_exists(new_blocks["vertical"]))

View File

@@ -195,7 +195,8 @@ class TestLibraryContentModule(LibraryContentTest):
"""
# 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
# In the normal studio editing process, editor_saved() calls refresh_children at this point
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)
@@ -269,7 +270,9 @@ class TestLibraryContentModule(LibraryContentTest):
self.assertNotIn(LibraryContentDescriptor.display_name, non_editable_metadata_fields)
@patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render)
@patch(
'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render
)
@patch('xmodule.html_module.HtmlModule.author_view', dummy_render, create=True)
@patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: [])
class TestLibraryContentRender(LibraryContentTest):

View File

@@ -14,7 +14,9 @@ from xmodule.modulestore.tests.utils import MixedSplitTestCase
dummy_render = lambda block, _: Fragment(block.data) # pylint: disable=invalid-name
@patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render)
@patch(
'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render
)
@patch('xmodule.html_module.HtmlDescriptor.author_view', dummy_render, create=True)
@patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: [])
class TestLibraryRoot(MixedSplitTestCase):