diff --git a/cms/djangoapps/contentstore/features/courses.py b/cms/djangoapps/contentstore/features/courses.py index 2640b5c233..cb7b85f076 100644 --- a/cms/djangoapps/contentstore/features/courses.py +++ b/cms/djangoapps/contentstore/features/courses.py @@ -1,5 +1,6 @@ # pylint: disable=missing-docstring # pylint: disable=redefined-outer-name +# pylint: disable=unused-argument from lettuce import world, step from common import * @@ -33,7 +34,7 @@ def i_create_a_course(step): create_a_course() -# pylint disable=unused-argument, invalid-name +# pylint: disable=invalid-name @step('I click the course link in Studio Home$') def i_click_the_course_link_in_studio_home(step): course_css = 'a.course-link' @@ -42,7 +43,11 @@ def i_click_the_course_link_in_studio_home(step): @step('I see an error about the length of the org/course/run tuple') def i_see_error_about_length(step): - assert world.css_has_text('#course_creation_error', 'The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + assert world.css_has_text( + '#course_creation_error', + 'The combined length of the organization, course number, ' + 'and course run fields cannot be more than 65 characters.' + ) ############ ASSERTIONS ################### @@ -54,7 +59,6 @@ def courseware_page_has_loaded_in_studio(step): @step('I see the course listed in Studio Home$') -# pylint disable=unused-argument def i_see_the_course_in_studio_home(step): course_css = 'h3.class-title' assert world.css_has_text(course_css, world.scenario_dict['COURSE'].display_name) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index b2e748fd55..dc6d8b976d 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -67,6 +67,13 @@ class LibraryTestCase(ModuleStoreTestCase): **(other_settings or {}) ) + def _add_simple_content_block(self): + """ Adds simple HTML block to library """ + return ItemFactory.create( + category="html", parent_location=self.library.location, + user_id=self.user.id, publish_item=False + ) + def _refresh_children(self, lib_content_block, status_code_expected=200): """ Helper method: Uses the REST API to call the 'refresh_children' handler @@ -74,7 +81,11 @@ class LibraryTestCase(ModuleStoreTestCase): """ if 'user' not in lib_content_block.runtime._services: # pylint: disable=protected-access lib_content_block.runtime._services['user'] = Mock(user_id=self.user.id) # pylint: disable=protected-access - handler_url = reverse_usage_url('component_handler', lib_content_block.location, kwargs={'handler': 'refresh_children'}) + handler_url = reverse_usage_url( + 'component_handler', + lib_content_block.location, + kwargs={'handler': 'refresh_children'} + ) response = self.client.ajax_post(handler_url) self.assertEqual(response.status_code, status_code_expected) return modulestore().get_item(lib_content_block.location) @@ -128,7 +139,7 @@ class TestLibraries(LibraryTestCase): Test the 'max_count' property of LibraryContent blocks. """ for _ in range(0, num_to_create): - ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + self._add_simple_content_block() with modulestore().default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() @@ -203,9 +214,9 @@ class TestLibraries(LibraryTestCase): """ Test that the same block definition is used for the library and course[s] """ - block1 = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + block1 = self._add_simple_content_block() def_id1 = block1.definition_locator.definition_id - block2 = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + block2 = self._add_simple_content_block() def_id2 = block2.definition_locator.definition_id self.assertNotEqual(def_id1, def_id2) @@ -461,7 +472,10 @@ class TestLibraryAccess(LibraryTestCase): def _assert_cannot_create_library(self, org="org", library="libfail", expected_code=403): """ Ensure the current user is not able to create a library. """ self.assertTrue(expected_code >= 300) - response = self.client.ajax_post(LIBRARY_REST_URL, {'org': org, 'library': library, 'display_name': "Irrelevant"}) + response = self.client.ajax_post( + LIBRARY_REST_URL, + {'org': org, 'library': library, 'display_name': "Irrelevant"} + ) self.assertEqual(response.status_code, expected_code) key = LibraryLocator(org=org, library=library) self.assertEqual(modulestore().get_library(key), None) @@ -574,7 +588,7 @@ class TestLibraryAccess(LibraryTestCase): Test the read-only role (LibraryUserRole and its org-level equivalent) """ # As staff user, add a block to self.library: - block = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + block = self._add_simple_content_block() # Login as a non_staff_user: self._login_as_non_staff_user() @@ -650,7 +664,7 @@ class TestLibraryAccess(LibraryTestCase): from a library with (write, read, or no) access to a course with (write or no) access. """ # As staff user, add a block to self.library: - block = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + block = self._add_simple_content_block() # And create a course: with modulestore().default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() @@ -687,7 +701,7 @@ class TestLibraryAccess(LibraryTestCase): access to a course with (write or no) access. """ # As staff user, add a block to self.library: - ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + self._add_simple_content_block() # And create a course: with modulestore().default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() @@ -702,7 +716,8 @@ class TestLibraryAccess(LibraryTestCase): # Try updating our library content block: lc_block = self._add_library_content_block(course, self.lib_key) - self._bind_module(lc_block, user=self.non_staff_user) # We must use the CMS's module system in order to get permissions checks. + # We must use the CMS's module system in order to get permissions checks. + self._bind_module(lc_block, user=self.non_staff_user) lc_block = self._refresh_children(lc_block, status_code_expected=200 if expected_result else 403) self.assertEqual(len(lc_block.children), 1 if expected_result else 0) @@ -822,7 +837,7 @@ class TestOverrides(LibraryTestCase): # Change the settings in the library version: self.problem.display_name = "X" self.problem.weight = 99 - new_data_value = "

We change the data as well to check that non-overriden fields do get updated.

" + new_data_value = "

Changed data to check that non-overriden fields *do* get updated.

" self.problem.data = new_data_value modulestore().update_item(self.problem, self.user.id) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 57f1d59ca9..e390811096 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -169,7 +169,10 @@ def xblock_handler(request, usage_key_string): source_course = duplicate_source_usage_key.course_key dest_course = parent_usage_key.course_key - if not has_studio_write_access(request.user, dest_course) or not has_studio_read_access(request.user, source_course): + if ( + not has_studio_write_access(request.user, dest_course) or + not has_studio_read_access(request.user, source_course) + ): raise PermissionDenied() dest_usage_key = _duplicate_item( @@ -252,6 +255,7 @@ def xblock_view_handler(request, usage_key_string, view_name): 'page_size': int(request.REQUEST.get('page_size', 0)), } except ValueError: + # pylint: disable=too-many-format-args return HttpResponse( content="Couldn't parse paging parameters: enable_paging: " "%s, page_number: %s, page_size: %s".format( @@ -435,7 +439,9 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, try: value = field.from_json(value) except ValueError as verr: - reason = _("Invalid data ({details})").format(details=verr.message) if verr.message else _("Invalid data") + reason = _("Invalid data") + if verr.message: + reason = _("Invalid data ({details})").format(details=verr.message) return JsonResponse({"error": reason}, 400) field.write_to(xblock, value) @@ -544,7 +550,9 @@ def _create_item(request): ) store.update_item(course, request.user.id) - return JsonResponse({"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)}) + return JsonResponse( + {"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)} + ) def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_name=None): @@ -559,9 +567,11 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ category = dest_usage_key.block_type # Update the display name to indicate this is a duplicate (unless display name provided). - duplicate_metadata = {} # Can't use own_metadata(), b/c it converts data for JSON serialization - not suitable for setting metadata of the new block + # Can't use own_metadata(), b/c it converts data for JSON serialization - + # not suitable for setting metadata of the new block + duplicate_metadata = {} for field in source_item.fields.values(): - if (field.scope == Scope.settings and field.is_set_on(source_item)): + if field.scope == Scope.settings and field.is_set_on(source_item): duplicate_metadata[field.name] = field.read_from(source_item) if display_name is not None: duplicate_metadata['display_name'] = display_name @@ -746,7 +756,9 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F is_library_block = isinstance(xblock.location, LibraryUsageLocator) is_xblock_unit = is_unit(xblock, parent_xblock) # this should not be calculated for Sections and Subsections on Unit page or for library blocks - has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) and not is_library_block else None + has_changes = None + if (is_xblock_unit or course_outline) and not is_library_block: + has_changes = modulestore().has_changes(xblock) if graders is None: if not is_library_block: diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 9aeb9c9b13..2bd433c83f 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -26,7 +26,9 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from .component import get_component_templates, CONTAINER_TEMPATES -from student.auth import STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access +from student.auth import ( + STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access +) from student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole, LibraryUserRole from student import auth from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest @@ -71,7 +73,10 @@ def _display_library(library_key_string, request): log.exception("Non-library key passed to content libraries API.") # Should never happen due to url regex raise Http404 # This is not a library if not has_studio_read_access(request.user, library_key): - log.exception(u"User %s tried to access library %s without permission", request.user.username, unicode(library_key)) + log.exception( + u"User %s tried to access library %s without permission", + request.user.username, unicode(library_key) + ) raise PermissionDenied() library = modulestore().get_library(library_key) @@ -80,7 +85,10 @@ def _display_library(library_key_string, request): raise Http404 response_format = 'html' - if request.REQUEST.get('format', 'html') == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html'): + if ( + request.REQUEST.get('format', 'html') == 'json' or + 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html') + ): response_format = 'json' return library_blocks_view(library, request.user, response_format) @@ -134,8 +142,8 @@ def _create_library(request): except InvalidKeyError as error: log.exception("Unable to create library - invalid key.") return JsonResponseBadRequest({ - "ErrMsg": _("Unable to create library '{name}'.\n\n{err}").format(name=display_name, err=error.message)} - ) + "ErrMsg": _("Unable to create library '{name}'.\n\n{err}").format(name=display_name, err=error.message) + }) except DuplicateCourseError: log.exception("Unable to create library - one already exists with the same key.") return JsonResponseBadRequest({ @@ -203,7 +211,7 @@ def manage_library_users(request, library_key_string): if not isinstance(library_key, LibraryLocator): raise Http404 # This is not a library user_perms = get_user_permissions(request.user, library_key) - if not (user_perms & STUDIO_VIEW_USERS): + if not user_perms & STUDIO_VIEW_USERS: raise PermissionDenied() library = modulestore().get_library(library_key) if library is None: diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 69898bfbe2..c322233319 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1508,7 +1508,8 @@ class TestLibraryXBlockInfo(ModuleStoreTestCase): parent_location=self.library.location, category='vertical', user_id=user_id, publish_item=False ) self.child_html = ItemFactory.create( - parent_location=self.vertical.location, category='html', display_name='Test HTML Child Block', user_id=user_id, publish_item=False + parent_location=self.vertical.location, category='html', display_name='Test HTML Child Block', + user_id=user_id, publish_item=False ) def test_lib_xblock_info(self): diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 0b0abe2e3b..7289022e8f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -215,7 +215,10 @@ class UnitTestLibraries(ModuleStoreTestCase): self.assertNotIn(extra_user.username, response.content) # Now add extra_user to the library: - user_details_url = reverse_course_url('course_team_handler', library.location.library_key, kwargs={'email': extra_user.email}) + user_details_url = reverse_course_url( + 'course_team_handler', + library.location.library_key, kwargs={'email': extra_user.email} + ) edit_response = self.client.ajax_post(user_details_url, {"role": LibraryUserRole.ROLE}) self.assertIn(edit_response.status_code, (200, 204)) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 812890a451..93a7c66a77 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -67,7 +67,7 @@ def _manage_users(request, course_key): """ # check that logged in user has permissions to this item user_perms = get_user_permissions(request.user, course_key) - if not (user_perms & STUDIO_VIEW_USERS): + if not user_perms & STUDIO_VIEW_USERS: raise PermissionDenied() course_module = modulestore().get_course(course_key) @@ -156,7 +156,8 @@ def _course_team_user(request, course_key, email): role = role_type(course_key) if role_type.ROLE == new_role: if (requester_perms & STUDIO_EDIT_ROLES) or (user.id == request.user.id and old_roles): - # User has STUDIO_EDIT_ROLES permission or is currently a member of a higher role, and is thus demoting themself + # User has STUDIO_EDIT_ROLES permission or + # is currently a member of a higher role, and is thus demoting themself auth.add_users(request.user, role, user) role_added = True else: diff --git a/cms/urls.py b/cms/urls.py index 81077120fc..ce46e77908 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -6,7 +6,9 @@ from ratelimitbackend import admin admin.autodiscover() # Pattern to match a course key or a library key -COURSELIKE_KEY_PATTERN = r'(?P({}|{}))'.format(r'[^/]+/[^/]+/[^/]+', r'[^/:]+:[^/+]+\+[^/+]+(\+[^/]+)?') +COURSELIKE_KEY_PATTERN = r'(?P({}|{}))'.format( + r'[^/]+/[^/]+/[^/]+', r'[^/:]+:[^/+]+\+[^/+]+(\+[^/]+)?' +) # Pattern to match a library key only LIBRARY_KEY_PATTERN = r'(?Plibrary-v1:[^/+]+\+[^/+]+)' diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 90b311e77e..a927bb5097 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -17,7 +17,7 @@ STUDIO_EDIT_ROLES = 8 STUDIO_VIEW_USERS = 4 STUDIO_EDIT_CONTENT = 2 STUDIO_VIEW_CONTENT = 1 -# In addition to the above, one is always allowed to "demote" oneself to a lower role within a course, or remove oneself. +# In addition to the above, one is always allowed to "demote" oneself to a lower role within a course, or remove oneself def has_access(user, role): @@ -70,7 +70,7 @@ def get_user_permissions(user, course_key, org=None): if OrgStaffRole(org=org).has_user(user) or (course_key and has_access(user, CourseStaffRole(course_key))): return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT # Otherwise, for libraries, users can view only: - if (course_key and isinstance(course_key, LibraryLocator)): + if course_key and isinstance(course_key, LibraryLocator): if OrgLibraryUserRole(org=org).has_user(user) or has_access(user, LibraryUserRole(course_key)): return STUDIO_VIEW_USERS | STUDIO_VIEW_CONTENT return 0 diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py index f13811467a..2b5248a1ab 100644 --- a/common/djangoapps/terrain/ui_helpers.py +++ b/common/djangoapps/terrain/ui_helpers.py @@ -48,7 +48,7 @@ REQUIREJS_WAIT = { "js/base", "js/models/course", "js/models/location", "js/models/section"], # Dashboard - # pylint disable=anomalous-backslash-in-string + # pylint: disable=anomalous-backslash-in-string re.compile('^Studio Home \|'): [ "js/sock", "gettext", "js/base", "jquery.ui", "coffee/src/main", "underscore"], diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 47583d9706..b748b359f1 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -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]) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 972919e2fa..e4c3252b37 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -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( diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index 4bdf51bdce..8f3c6d0cee 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 6c375d2dfc..c4d5d638e8 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -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'] # - # 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'] # @@ -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: diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index c651512c82..30fe251823 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -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: diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_copy_from_template.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_copy_from_template.py index 3d73178644..bfdd7d0fea 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_copy_from_template.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_copy_from_template.py @@ -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"])) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index 5f32dab130..0a60895f0a 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -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): diff --git a/common/lib/xmodule/xmodule/tests/test_library_root.py b/common/lib/xmodule/xmodule/tests/test_library_root.py index 8774e7bb32..574bcf4152 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_root.py +++ b/common/lib/xmodule/xmodule/tests/test_library_root.py @@ -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): diff --git a/common/test/acceptance/fixtures/library.py b/common/test/acceptance/fixtures/library.py index 5692c078db..a7a55d207d 100644 --- a/common/test/acceptance/fixtures/library.py +++ b/common/test/acceptance/fixtures/library.py @@ -82,9 +82,9 @@ class LibraryFixture(XBlockContainerFixture): err_msg = response.json().get('ErrMsg') except ValueError: err_msg = "Unknown Error" - raise FixtureError( - "Could not create library {}. Status was {}, error was: {}".format(self.library_info, response.status_code, err_msg) - ) + raise FixtureError("Could not create library {}. Status was {}, error was: {}".format( + self.library_info, response.status_code, err_msg + )) def create_xblock(self, parent_loc, xblock_desc): # Disable publishing for library XBlocks: diff --git a/common/test/acceptance/pages/studio/auto_auth.py b/common/test/acceptance/pages/studio/auto_auth.py index 2e2cffd677..d3759b1136 100644 --- a/common/test/acceptance/pages/studio/auto_auth.py +++ b/common/test/acceptance/pages/studio/auto_auth.py @@ -15,7 +15,8 @@ class AutoAuthPage(PageObject): this url will create a user and log them in. """ - def __init__(self, browser, username=None, email=None, password=None, staff=None, course_id=None, roles=None, no_login=None): + def __init__(self, browser, username=None, email=None, password=None, + staff=None, course_id=None, roles=None, no_login=None): """ Auto-auth is an end-point for HTTP GET requests. By default, it will create accounts with random user credentials, diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 8f4ccb8590..88a05f9e02 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -365,6 +365,7 @@ class XBlockWrapper(PageObject): return self._validation_paragraph('error').present @property + # pylint: disable=invalid-name def has_validation_not_configured_warning(self): """ Is a validation "not configured" message shown? """ return self._validation_paragraph('not-configured').present @@ -380,6 +381,7 @@ class XBlockWrapper(PageObject): return self._validation_paragraph('error').text[0] @property + # pylint: disable=invalid-name def validation_not_configured_warning_text(self): """ Get the text of the validation "not configured" message. """ return self._validation_paragraph('not-configured').text[0] diff --git a/common/test/acceptance/pages/studio/index.py b/common/test/acceptance/pages/studio/index.py index 2a69b43343..d6d6a25603 100644 --- a/common/test/acceptance/pages/studio/index.py +++ b/common/test/acceptance/pages/studio/index.py @@ -88,7 +88,8 @@ class DashboardPage(PageObject): """ List all the libraries found on the page's list of libraries. """ - self.q(css='#course-index-tabs .libraries-tab a').click() # Workaround Selenium/Firefox bug: `.text` property is broken on invisible elements + # Workaround Selenium/Firefox bug: `.text` property is broken on invisible elements + self.q(css='#course-index-tabs .libraries-tab a').click() div2info = lambda element: { 'name': element.find_element_by_css_selector('.course-title').text, 'org': element.find_element_by_css_selector('.course-org .value').text, diff --git a/common/test/acceptance/pages/studio/users.py b/common/test/acceptance/pages/studio/users.py index c1a2427d56..5c0216f893 100644 --- a/common/test/acceptance/pages/studio/users.py +++ b/common/test/acceptance/pages/studio/users.py @@ -47,7 +47,9 @@ class UsersPage(PageObject): """ Return a list of users listed on this page. """ - return self.q(css='.user-list .user-item').map(lambda el: UserWrapper(self.browser, el.get_attribute('data-email'))).results + return self.q(css='.user-list .user-item').map( + lambda el: UserWrapper(self.browser, el.get_attribute('data-email')) + ).results @property def has_add_button(self): diff --git a/common/test/acceptance/pages/studio/utils.py b/common/test/acceptance/pages/studio/utils.py index dd8ec091a3..df86d5deca 100644 --- a/common/test/acceptance/pages/studio/utils.py +++ b/common/test/acceptance/pages/studio/utils.py @@ -118,7 +118,10 @@ def add_component(page, item_type, specific_type): if multiple_templates: sub_template_menu_div_selector = '.new-component-{}'.format(item_type) page.wait_for_element_visibility(sub_template_menu_div_selector, 'Wait for the templates sub-menu to appear') - page.wait_for_element_invisibility('.add-xblock-component .new-component', 'Wait for the add component menu to disappear') + page.wait_for_element_invisibility( + '.add-xblock-component .new-component', + 'Wait for the add component menu to disappear' + ) all_options = page.q(css='.new-component-{} ul.new-component-template li a span'.format(item_type)) chosen_option = all_options.filter(lambda el: el.text == specific_type).first diff --git a/common/test/acceptance/tests/studio/test_studio_general.py b/common/test/acceptance/tests/studio/test_studio_general.py index c0b409b66b..adaa46227d 100644 --- a/common/test/acceptance/tests/studio/test_studio_general.py +++ b/common/test/acceptance/tests/studio/test_studio_general.py @@ -93,7 +93,7 @@ class CoursePagesTest(StudioCourseTest): /course/ is the base URL for all courses, but by itself, it should redirect to /home/. """ - self.dashboard_page = DashboardPage(self.browser) + self.dashboard_page = DashboardPage(self.browser) # pylint: disable=attribute-defined-outside-init self.dashboard_page.visit() self.assertEqual(self.browser.current_url.strip('/').rsplit('/')[-1], 'home') diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index 593243e628..175ee08539 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -26,10 +26,15 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest): """ super(StudioLibraryContainerTest, self).setUp() # Also create a course: - self.course_fixture = CourseFixture(self.course_info['org'], self.course_info['number'], self.course_info['run'], self.course_info['display_name']) + self.course_fixture = CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'] + ) self.populate_course_fixture(self.course_fixture) self.course_fixture.install() - self.outline = CourseOutlinePage(self.browser, self.course_info['org'], self.course_info['number'], self.course_info['run']) + self.outline = CourseOutlinePage( + self.browser, self.course_info['org'], self.course_info['number'], self.course_info['run'] + ) self.outline.visit() subsection = self.outline.section(SECTION_NAME).subsection(SUBSECTION_NAME) @@ -156,7 +161,8 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest): library_block = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) self.assertFalse(library_block.has_validation_warning) - #self.assertIn("3 matching components", library_block.author_content) # Removed this assert until a summary message is added back to the author view (SOL-192) + # Removed this assert until a summary message is added back to the author view (SOL-192) + #self.assertIn("3 matching components", library_block.author_content) self.library_fixture.create_xblock(self.library_fixture.library_location, XBlockFixtureDesc("html", "Html4")) @@ -171,7 +177,8 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest): library_block = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) self.assertFalse(library_block.has_validation_message) - #self.assertIn("4 matching components", library_block.author_content) # Removed this assert until a summary message is added back to the author view (SOL-192) + # Removed this assert until a summary message is added back to the author view (SOL-192) + #self.assertIn("4 matching components", library_block.author_content) def test_no_content_message(self): """