From 219eeb6a6d3e7090d22812f647615151d6f3e13f Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 23:10:40 -0700 Subject: [PATCH 01/15] Library Content XModule --- cms/envs/common.py | 1 + common/lib/xmodule/setup.py | 1 + .../xmodule/xmodule/library_content_module.py | 441 ++++++++++++++++++ .../xmodule/public/js/library_content_edit.js | 24 + lms/djangoapps/courseware/models.py | 4 + lms/templates/library-block-author-view.html | 17 + lms/templates/staff_problem_info.html | 2 +- 7 files changed, 489 insertions(+), 1 deletion(-) create mode 100644 common/lib/xmodule/xmodule/library_content_module.py create mode 100644 common/lib/xmodule/xmodule/public/js/library_content_edit.js create mode 100644 lms/templates/library-block-author-view.html diff --git a/cms/envs/common.py b/cms/envs/common.py index 397620ef864b..c81a392134ce 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -743,6 +743,7 @@ 'word_cloud', 'graphical_slider_tool', 'lti', + 'library_content', # XBlocks from pmitros repos are prototypes. They should not be used # except for edX Learning Sciences experiments on edge.edx.org without # further work to make them robust, maintainable, finalize data formats, diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index f0721e91a47f..f2b548efe9c0 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -11,6 +11,7 @@ "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor", "html = xmodule.html_module:HtmlDescriptor", "image = xmodule.backcompat_module:TranslateCustomTagDescriptor", + "library_content = xmodule.library_content_module:LibraryContentDescriptor", "error = xmodule.error_module:ErrorDescriptor", "peergrading = xmodule.peer_grading_module:PeerGradingDescriptor", "poll_question = xmodule.poll_module:PollDescriptor", diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py new file mode 100644 index 000000000000..092bc2a93117 --- /dev/null +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -0,0 +1,441 @@ +""" +LibraryContent: The XBlock used to include blocks from a library in a course. +""" +from bson.objectid import ObjectId +from collections import namedtuple +from copy import copy +import hashlib +from .mako_module import MakoModuleDescriptor +from opaque_keys.edx.locator import LibraryLocator +import random +from webob import Response +from xblock.core import XBlock +from xblock.fields import Scope, String, List, Integer, Boolean +from xblock.fragment import Fragment +from xmodule.modulestore.exceptions import ItemNotFoundError +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 + +# Make '_' a no-op so we can scrape strings +_ = lambda text: text + + +def enum(**enums): + """ enum helper in lieu of enum34 """ + return type('Enum', (), enums) + + +class LibraryVersionReference(namedtuple("LibraryVersionReference", "library_id version")): + """ + A reference to a specific library, with an optional version. + The version is used to find out when the LibraryContentXBlock was last + updated with the latest content from the library. + + library_id is a LibraryLocator + version is an ObjectId or None + """ + def __new__(cls, library_id, version=None): + # pylint: disable=super-on-old-class + if not isinstance(library_id, LibraryLocator): + library_id = LibraryLocator.from_string(library_id) + if library_id.version_guid: + assert (version is None) or (version == library_id.version_guid) + if not version: + version = library_id.version_guid + library_id = library_id.for_version(None) + if version and not isinstance(version, ObjectId): + version = ObjectId(version) + return super(LibraryVersionReference, cls).__new__(cls, library_id, version) + + @staticmethod + def from_json(value): + """ + Implement from_json to convert from JSON + """ + return LibraryVersionReference(*value) + + def to_json(self): + """ + Implement to_json to convert value to JSON + """ + # TODO: Is there anyway for an xblock to *store* an ObjectId as + # part of the List() field value? + return [unicode(self.library_id), unicode(self.version) if self.version else None] # pylint: disable=no-member + + +class LibraryList(List): + """ + Special List class for listing references to content libraries. + Is simply a list of LibraryVersionReference tuples. + """ + def from_json(self, values): + """ + Implement from_json to convert from JSON. + + values might be a list of lists, or a list of strings + Normally the runtime gives us: + [[u'library-v1:ProblemX+PR0B', '5436ffec56c02c13806a4c1b'], ...] + But the studio editor gives us: + [u'library-v1:ProblemX+PR0B,5436ffec56c02c13806a4c1b', ...] + """ + def parse(val): + """ Convert this list entry from its JSON representation """ + if isinstance(val, basestring): + val = val.strip(' []') + parts = val.rsplit(',', 1) + val = [parts[0], parts[1] if len(parts) > 1 else None] + return LibraryVersionReference.from_json(val) + return [parse(v) for v in values] + + def to_json(self, values): + """ + Implement to_json to convert value to JSON + """ + return [lvr.to_json() for lvr in values] + + +class LibraryContentFields(object): + """ + Fields for the LibraryContentModule. + + Separated out for now because they need to be added to the module and the + descriptor. + """ + # Please note the display_name of each field below is used in + # common/test/acceptance/pages/studio/overview.py:StudioLibraryContentXBlockEditModal + # to locate input elements - keep synchronized + display_name = String( + display_name=_("Display Name"), + help=_("Display name for this module"), + default="Library Content", + scope=Scope.settings, + ) + source_libraries = LibraryList( + display_name=_("Libraries"), + help=_("Enter a library ID for each library from which you want to draw content."), + default=[], + scope=Scope.settings, + ) + mode = String( + help=_("Determines how content is drawn from the library"), + default="random", + values=[ + {"display_name": _("Choose n at random"), "value": "random"} + # Future addition: Choose a new random set of n every time the student refreshes the block, for self tests + # Future addition: manually selected blocks + ], + scope=Scope.settings, + ) + max_count = Integer( + display_name=_("Count"), + help=_("Enter the number of components to display to each student."), + default=1, + scope=Scope.settings, + ) + filters = String(default="") # TBD + has_score = Boolean( + display_name=_("Scored"), + help=_("Set this value to True if this module is either a graded assignment or a practice problem."), + default=False, + 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 + default=[], + scope=Scope.user_state, + ) + has_children = True + + +def _get_library(modulestore, library_key): + """ + Given a library key like "library-v1:ProblemX+PR0B", return the + 'library' XBlock with meta-information about the library. + + Returns None on error. + """ + if not isinstance(library_key, LibraryLocator): + library_key = LibraryLocator.from_string(library_key) + assert library_key.version_guid is None + + # TODO: Is this too tightly coupled to split? May need to abstract this into a service + # provided by the CMS runtime. + try: + library = modulestore.get_library(library_key, remove_version=False) + except ItemNotFoundError: + return None + # 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 + + +#pylint: disable=abstract-method +class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): + """ + An XBlock whose children are chosen dynamically from a content library. + Can be used to create randomized assessments among other things. + + Note: technically, all matching blocks from the content library are added + as children of this block, but only a subset of those children are shown to + any particular student. + """ + def selected_children(self): + """ + Returns a set() of block_ids indicating which of the possible children + have been selected to display to the current user. + + This reads and updates the "selected" field, which has user_state scope. + + Note: self.selected and the return value contain block_ids. To get + actual BlockUsageLocators, it is necessary to use self.children, + because the block_ids alone do not specify the block type. + """ + if hasattr(self, "_selected_set"): + # Already done: + return self._selected_set # pylint: disable=access-member-before-definition + # Determine which of our children we will show: + selected = set(tuple(k) for k in self.selected) # set of (block_type, block_id) tuples + valid_block_keys = set([(c.block_type, c.block_id) for c in self.children]) # pylint: disable=no-member + # Remove any selected blocks that are no longer valid: + selected -= (selected - valid_block_keys) + # If max_count has been decreased, we may have to drop some previously selected blocks: + while len(selected) > self.max_count: + selected.pop() + # Do we have enough blocks now? + num_to_add = self.max_count - len(selected) + if num_to_add > 0: + # We need to select [more] blocks to display to this user: + if self.mode == "random": + pool = valid_block_keys - selected + num_to_add = min(len(pool), num_to_add) + selected |= set(random.sample(pool, num_to_add)) + # We now have the correct n random children to show for this user. + else: + raise NotImplementedError("Unsupported mode.") + # Save our selections to the user state, to ensure consistency: + self.selected = list(selected) # TODO: this doesn't save from the LMS "Progress" page. + # Cache the results + self._selected_set = selected # pylint: disable=attribute-defined-outside-init + return selected + + def _get_selected_child_blocks(self): + """ + Generator returning XBlock instances of the children selected for the + current user. + """ + for block_type, block_id in self.selected_children(): + yield self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) + + def student_view(self, context): + fragment = Fragment() + contents = [] + child_context = {} if not context else copy(context) + + for child in self._get_selected_child_blocks(): + for displayable in child.displayable_items(): + rendered_child = displayable.render(STUDENT_VIEW, child_context) + fragment.add_frag_resources(rendered_child) + contents.append({ + 'id': displayable.location.to_deprecated_string(), + 'content': rendered_child.content + }) + + fragment.add_content(self.system.render_template('vert_module.html', { + 'items': contents, + 'xblock_context': context, + })) + return fragment + + def author_view(self, context): + """ + Renders the Studio views. + Normal studio view: displays library status and has an "Update" button. + Studio container view: displays a preview of all possible children. + """ + fragment = Fragment() + root_xblock = context.get('root_xblock') + is_root = root_xblock and root_xblock.location == self.location + + if is_root: + # User has clicked the "View" link. Show a preview of all possible children: + if self.children: # pylint: disable=no-member + self.render_children(context, fragment, can_reorder=False, can_add=False) + else: + fragment.add_content(u'

{}

'.format( + _('No matching content found in library, no library configured, or not yet loaded from library.') + )) + else: + # When shown on a unit page, don't show any sort of preview - just the status of this block. + LibraryStatus = enum( # pylint: disable=invalid-name + NONE=0, # no library configured + INVALID=1, # invalid configuration or library has been deleted/corrupted + OK=2, # library configured correctly and should be working fine + ) + UpdateStatus = enum( # pylint: disable=invalid-name + CANNOT=0, # Cannot update - library is not set, invalid, deleted, etc. + NEEDED=1, # An update is needed - prompt the user to update + UP_TO_DATE=2, # No update necessary - library is up to date + ) + library_names = [] + library_status = LibraryStatus.OK + update_status = UpdateStatus.UP_TO_DATE + if self.source_libraries: + for library_key, version in self.source_libraries: + library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) + if library is None: + library_status = LibraryStatus.INVALID + update_status = UpdateStatus.CANNOT + break + library_names.append(library.display_name) + latest_version = library.location.library_key.version_guid + if version is None or version != latest_version: + update_status = UpdateStatus.NEEDED + # else library is up to date. + else: + library_status = LibraryStatus.NONE + update_status = UpdateStatus.CANNOT + fragment.add_content(self.system.render_template('library-block-author-view.html', { + 'library_status': library_status, + 'LibraryStatus': LibraryStatus, + 'update_status': update_status, + 'UpdateStatus': UpdateStatus, + 'library_names': library_names, + 'max_count': self.max_count, + 'mode': self.mode, + 'num_children': len(self.children), # pylint: disable=no-member + })) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) + fragment.initialize_js('LibraryContentAuthorView') + return fragment + + def get_child_descriptors(self): + """ + Return only the subset of our children relevant to the current student. + """ + return list(self._get_selected_child_blocks()) + + +@XBlock.wants('user') +class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDescriptor, StudioEditableDescriptor): + """ + Descriptor class for LibraryContentModule XBlock. + """ + module_class = LibraryContentModule + mako_template = 'widgets/metadata-edit.html' + js = {'coffee': [resource_string(__name__, 'js/src/vertical/edit.coffee')]} + js_module_name = "VerticalDescriptor" + + @XBlock.handler + def refresh_children(self, request, suffix): # pylint: disable=unused-argument + """ + Refresh children: + This method is to be used when any of the libraries that this block + references have been updated. It will re-fetch all matching blocks from + the libraries, and copy them as children of this block. The children + will be given new block_ids, but the definition ID used should be the + exact same definition ID used in the library. + + This method will update this block's 'source_libraries' field to store + the version number of the libraries used, so we easily determine if + this block is up to date or not. + """ + user_id = self.runtime.service(self, 'user').user_id + root_children = [] + + store = self.system.modulestore + with store.bulk_operations(self.location.course_key): + # Currently, ALL children are essentially deleted and then re-added + # in a way that preserves their block_ids (and thus should preserve + # student data, grades, analytics, etc.) + # Once course-level field overrides are implemented, this will + # change to a more conservative implementation. + + # First, delete all our existing children to avoid block_id conflicts when we add them: + for child in self.children: # pylint: disable=access-member-before-definition + store.delete_item(child, user_id) + + # Now add all matching children, and record the library version we use: + new_libraries = [] + for library_key, old_version in self.source_libraries: # pylint: disable=unused-variable + library = _get_library(self.system.modulestore, library_key) # pylint: disable=protected-access + + def copy_children_recursively(from_block): + """ + Internal method to copy blocks from the library recursively + """ + new_children = [] + for child_key in from_block.children: + child = store.get_item(child_key, depth=9) + # 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 + # we compute block_id as a hash of three pieces of data: + unique_data = "{}:{}:{}".format( + self.location.block_id, # Must not clash with other usages of the same library in this course + unicode(library_key.for_version(None)).encode("utf-8"), # The block ID below is only unique within a library, so we need this too + child_key.block_id, # Child block ID. Should not change even if the block is edited. + ) + child_block_id = hashlib.sha1(unique_data).hexdigest()[:20] + fields = {} + for field in child.fields.itervalues(): + if field.scope == Scope.settings and field.is_set_on(child): + fields[field.name] = field.read_from(child) + if child.has_children: + fields['children'] = copy_children_recursively(from_block=child) + new_child_info = store.create_item( + user_id, + self.location.course_key, + child_key.block_type, + block_id=child_block_id, + definition_locator=child.definition_locator, + runtime=self.system, + fields=fields, + ) + new_children.append(new_child_info.location) + return new_children + root_children.extend(copy_children_recursively(from_block=library)) + new_libraries.append(LibraryVersionReference(library_key, library.location.library_key.version_guid)) + self.source_libraries = new_libraries + self.children = root_children # pylint: disable=attribute-defined-outside-init + self.system.modulestore.update_item(self, user_id) + return Response() + + def has_dynamic_children(self): + """ + Inform the runtime that our children vary per-user. + See get_child_descriptors() above + """ + return True + + def get_content_titles(self): + """ + Returns list of friendly titles for our selected children only; without + thi, all possible children's titles would be seen in the sequence bar in + the LMS. + + This overwrites the get_content_titles method included in x_module by default. + """ + titles = [] + for child in self._xmodule.get_child_descriptors(): + titles.extend(child.get_content_titles()) + return titles + + @classmethod + def definition_from_xml(cls, xml_object, system): + """ XML support not yet implemented. """ + raise NotImplementedError + + def definition_to_xml(self, resource_fs): + """ XML support not yet implemented. """ + raise NotImplementedError + + @classmethod + def from_xml(cls, xml_data, system, id_generator): + """ XML support not yet implemented. """ + raise NotImplementedError + + def export_to_xml(self, resource_fs): + """ XML support not yet implemented. """ + raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/public/js/library_content_edit.js b/common/lib/xmodule/xmodule/public/js/library_content_edit.js new file mode 100644 index 000000000000..9a84a214049c --- /dev/null +++ b/common/lib/xmodule/xmodule/public/js/library_content_edit.js @@ -0,0 +1,24 @@ +/* JavaScript for editing operations that can be done on LibraryContentXBlock */ +window.LibraryContentAuthorView = function (runtime, element) { + $(element).find('.library-update-btn').on('click', function(e) { + e.preventDefault(); + // Update the XBlock with the latest matching content from the library: + runtime.notify('save', { + state: 'start', + element: element, + message: gettext('Updating with latest library content') + }); + $.post(runtime.handlerUrl(element, 'refresh_children')).done(function() { + runtime.notify('save', { + state: 'end', + element: element + }); + // runtime.refreshXBlock(element); + // The above does not work, because this XBlock's runtime has no reference + // to the page (XBlockContainerPage). Only the Vertical XBlock's runtime has + // a reference to the page, and we have no way of getting a reference to it. + // So instead we: + location.reload(); + }); + }); +}; diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 56818d4e2eea..d1f1f45b89ed 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -32,6 +32,10 @@ class StudentModule(models.Model): MODULE_TYPES = (('problem', 'problem'), ('video', 'video'), ('html', 'html'), + ('course', 'course'), + ('chapter', 'Section'), + ('sequential', 'Subsection'), + ('library_content', 'Library Content'), ) ## These three are the key for the object module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True) diff --git a/lms/templates/library-block-author-view.html b/lms/templates/library-block-author-view.html new file mode 100644 index 000000000000..521946a903db --- /dev/null +++ b/lms/templates/library-block-author-view.html @@ -0,0 +1,17 @@ +<%! +from django.utils.translation import ugettext as _ +%> +
+ % if library_status == LibraryStatus.OK: +

${_('This component will be replaced by {mode} {max_count} components from the {num_children} matching components from {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

+ % if update_status == UpdateStatus.NEEDED: +

${_('This component is out of date.')} ↻ ${_('Update now with latest components from the library')}

+ % elif update_status == UpdateStatus.UP_TO_DATE: +

${_(u'✓ Up to date.')}

+ % endif + % elif library_status == LibraryStatus.NONE: +

${_('No library or filters configured. Press "Edit" to configure.')}

+ % else: +

${_('Library is invalid, corrupt, or has been deleted.')}

+ % endif +
diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index 75d2789d7c6e..f486bfc6f65f 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -4,7 +4,7 @@ ## The JS for this is defined in xqa_interface.html ${block_content} -%if location.category in ['problem','video','html','combinedopenended','graphical_slider_tool']: +%if location.category in ['problem','video','html','combinedopenended','graphical_slider_tool', 'library_content']: % if edit_link:
Edit From d2ae624ce752666937e6c29bb8da9a2119c577e8 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Sat, 1 Nov 2014 19:47:28 -0700 Subject: [PATCH 02/15] Unit and integration tests of content libraries --- .../contentstore/tests/test_libraries.py | 256 ++++++++++++++++++ .../xmodule/tests/test_library_content.py | 142 ++++++++++ 2 files changed, 398 insertions(+) create mode 100644 cms/djangoapps/contentstore/tests/test_libraries.py create mode 100644 common/lib/xmodule/xmodule/tests/test_library_content.py diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py new file mode 100644 index 000000000000..b6c6119ed1b0 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -0,0 +1,256 @@ +""" +Content library unit tests that require the CMS runtime. +""" +from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from contentstore.utils import reverse_usage_url +from contentstore.views.preview import _load_preview_module +from contentstore.views.tests.test_library import LIBRARY_REST_URL +import ddt +from xmodule.library_content_module import LibraryVersionReference +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.tests import get_test_system +from mock import Mock +from opaque_keys.edx.locator import CourseKey, LibraryLocator + + +@ddt.ddt +class TestLibraries(ModuleStoreTestCase): + """ + High-level tests for libraries + """ + def setUp(self): + user_password = super(TestLibraries, self).setUp() + + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password=user_password) + + self.lib_key = self._create_library() + self.library = modulestore().get_library(self.lib_key) + + def _create_library(self, org="org", library="lib", display_name="Test Library"): + """ + Helper method used to create a library. Uses the REST API. + """ + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': org, + 'library': library, + 'display_name': display_name, + }) + self.assertEqual(response.status_code, 200) + lib_info = parse_json(response) + lib_key = CourseKey.from_string(lib_info['library_key']) + self.assertIsInstance(lib_key, LibraryLocator) + return lib_key + + def _add_library_content_block(self, course, library_key, other_settings=None): + """ + Helper method to add a LibraryContent block to a course. + The block will be configured to select content from the library + specified by library_key. + other_settings can be a dict of Scope.settings fields to set on the block. + """ + return ItemFactory.create( + category='library_content', + parent_location=course.location, + user_id=self.user.id, + publish_item=False, + source_libraries=[LibraryVersionReference(library_key)], + **(other_settings or {}) + ) + + def _refresh_children(self, lib_content_block): + """ + Helper method: Uses the REST API to call the 'refresh_children' handler + of a LibraryContent block + """ + 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'}) + response = self.client.ajax_post(handler_url) + self.assertEqual(response.status_code, 200) + return modulestore().get_item(lib_content_block.location) + + @ddt.data( + (2, 1, 1), + (2, 2, 2), + (2, 20, 2), + ) + @ddt.unpack + def test_max_items(self, num_to_create, num_to_select, num_expected): + """ + 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) + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': num_to_select}) + self.assertEqual(len(lc_block.children), 0) + lc_block = self._refresh_children(lc_block) + + # Now, we want to make sure that .children has the total # of potential + # children, and that get_child_descriptors() returns the actual children + # chosen for a given student. + # In order to be able to call get_child_descriptors(), we must first + # call bind_for_student: + lc_block.bind_for_student(get_test_system(), lc_block._field_data) # pylint: disable=protected-access + self.assertEqual(len(lc_block.children), num_to_create) + self.assertEqual(len(lc_block.get_child_descriptors()), num_expected) + + def test_consistent_children(self): + """ + Test that the same student will always see the same selected child block + """ + session_data = {} + + def bind_module(descriptor): + """ + Helper to use the CMS's module system so we can access student-specific fields. + """ + request = Mock(user=self.user, session=session_data) + return _load_preview_module(request, descriptor) # pylint: disable=protected-access + + # Create many blocks in the library and add them to a course: + for num in range(0, 8): + ItemFactory.create( + data="This is #{}".format(num + 1), + category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False + ) + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': 1}) + lc_block_key = lc_block.location + lc_block = self._refresh_children(lc_block) + + def get_child_of_lc_block(block): + """ + Fetch the child shown to the current user. + """ + children = block.get_child_descriptors() + self.assertEqual(len(children), 1) + return children[0] + + # Check which child a student will see: + bind_module(lc_block) + chosen_child = get_child_of_lc_block(lc_block) + chosen_child_defn_id = chosen_child.definition_locator.definition_id + lc_block.save() + + modulestore().update_item(lc_block, self.user.id) + + # Now re-load the block and try again: + def check(): + """ + Confirm that chosen_child is still the child seen by the test student + """ + for _ in range(0, 6): # Repeat many times b/c blocks are randomized + lc_block = modulestore().get_item(lc_block_key) # Reload block from the database + bind_module(lc_block) + current_child = get_child_of_lc_block(lc_block) + self.assertEqual(current_child.location, chosen_child.location) + self.assertEqual(current_child.data, chosen_child.data) + self.assertEqual(current_child.definition_locator.definition_id, chosen_child_defn_id) + + check() + # Refresh the children: + lc_block = self._refresh_children(lc_block) + # Now re-load the block and try yet again, in case refreshing the children changed anything: + check() + + def test_definition_shared_with_library(self): + """ + 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) + 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) + def_id2 = block2.definition_locator.definition_id + self.assertNotEqual(def_id1, def_id2) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + for child_key in lc_block.children: + child = modulestore().get_item(child_key) + def_id = child.definition_locator.definition_id + self.assertIn(def_id, (def_id1, def_id2)) + + def test_fields(self): + """ + Test that blocks used from a library have the same field values as + defined by the library author. + """ + data_value = "A Scope.content value" + name_value = "A Scope.settings value" + lib_block = ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name_value, + data=data_value, + ) + self.assertEqual(lib_block.data, data_value) + self.assertEqual(lib_block.display_name, name_value) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + course_block = modulestore().get_item(lc_block.children[0]) + + self.assertEqual(course_block.data, data_value) + self.assertEqual(course_block.display_name, name_value) + + def test_block_with_children(self): + """ + Test that blocks used from a library can have children. + """ + data_value = "A Scope.content value" + name_value = "A Scope.settings value" + # In the library, create a vertical block with a child: + vert_block = ItemFactory.create( + category="vertical", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + ) + child_block = ItemFactory.create( + category="html", + parent_location=vert_block.location, + user_id=self.user.id, + publish_item=False, + display_name=name_value, + data=data_value, + ) + self.assertEqual(child_block.data, data_value) + self.assertEqual(child_block.display_name, name_value) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 1) + course_vert_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(len(course_vert_block.children), 1) + course_child_block = modulestore().get_item(course_vert_block.children[0]) + + self.assertEqual(course_child_block.data, data_value) + self.assertEqual(course_child_block.display_name, name_value) diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py new file mode 100644 index 000000000000..2b52386e3740 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -0,0 +1,142 @@ +# -*- coding: utf-8 -*- +""" +Basic unit tests for LibraryContentModule + +Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. +""" +import ddt +from xmodule.library_content_module import LibraryVersionReference +from xmodule.modulestore.tests.factories import LibraryFactory, CourseFactory, ItemFactory +from xmodule.modulestore.tests.utils import MixedSplitTestCase +from xmodule.tests import get_test_system +from xmodule.validation import StudioValidationMessage + + +@ddt.ddt +class TestLibraries(MixedSplitTestCase): + """ + Basic unit tests for LibraryContentModule (library_content_module.py) + """ + def setUp(self): + super(TestLibraries, self).setUp() + + self.library = LibraryFactory.create(modulestore=self.store) + self.lib_blocks = [ + ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user_id, + publish_item=False, + metadata={"data": "Hello world from block {}".format(i), }, + modulestore=self.store, + ) + for i in range(1, 5) + ] + self.course = CourseFactory.create(modulestore=self.store) + self.chapter = ItemFactory.create( + category="chapter", + parent_location=self.course.location, + user_id=self.user_id, + modulestore=self.store, + ) + self.sequential = ItemFactory.create( + category="sequential", + parent_location=self.chapter.location, + user_id=self.user_id, + modulestore=self.store, + ) + self.vertical = ItemFactory.create( + category="vertical", + parent_location=self.sequential.location, + user_id=self.user_id, + modulestore=self.store, + ) + self.lc_block = ItemFactory.create( + category="library_content", + parent_location=self.vertical.location, + user_id=self.user_id, + modulestore=self.store, + metadata={ + 'max_count': 1, + 'source_libraries': [LibraryVersionReference(self.library.location.library_key)] + } + ) + + def _bind_course_module(self, module): + """ + Bind a module (part of self.course) so we can access student-specific data. + """ + module_system = get_test_system(course_id=self.course.location.course_key) + module_system.descriptor_runtime = module.runtime + + def get_module(descriptor): + """Mocks module_system get_module function""" + sub_module_system = get_test_system(course_id=self.course.location.course_key) + sub_module_system.get_module = get_module + sub_module_system.descriptor_runtime = descriptor.runtime + descriptor.bind_for_student(sub_module_system, descriptor._field_data) # pylint: disable=protected-access + return descriptor + + module_system.get_module = get_module + module.xmodule_runtime = module_system + + def test_lib_content_block(self): + """ + Test that blocks from a library are copied and added as children + """ + # Check that the LibraryContent block has no children initially + # Normally the children get added when the "source_libraries" setting + # is updated, but the way we do it through a factory doesn't do that. + self.assertEqual(len(self.lc_block.children), 0) + # Update the LibraryContent module: + self.lc_block.refresh_children(None, None) + # Check that all blocks from the library are now children of the block: + self.assertEqual(len(self.lc_block.children), len(self.lib_blocks)) + + def test_children_seen_by_a_user(self): + """ + Test that each student sees only one block as a child of the LibraryContent block. + """ + self.lc_block.refresh_children(None, None) + self.lc_block = self.store.get_item(self.lc_block.location) + self._bind_course_module(self.lc_block) + # Make sure the runtime knows that the block's children vary per-user: + self.assertTrue(self.lc_block.has_dynamic_children()) + + self.assertEqual(len(self.lc_block.children), len(self.lib_blocks)) + + # Check how many children each user will see: + self.assertEqual(len(self.lc_block.get_child_descriptors()), 1) + # 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): + """ + Test that the validation method of LibraryContent blocks is working. + """ + # When source_libraries is blank, the validation summary should say this block needs to be configured: + self.lc_block.source_libraries = [] + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.NOT_CONFIGURED, result.summary.type) + + # When source_libraries references a non-existent library, we should get an error: + self.lc_block.source_libraries = [LibraryVersionReference("library-v1:BAD+WOLF")] + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.ERROR, result.summary.type) + self.assertIn("invalid", result.summary.text) + + # When source_libraries is set but the block needs to be updated, the summary should say so: + self.lc_block.source_libraries = [LibraryVersionReference(self.library.location.library_key)] + result = self.lc_block.validate() + self.assertFalse(result) # Validation fails due to at least one warning/message + self.assertTrue(result.summary) + self.assertEqual(StudioValidationMessage.WARNING, result.summary.type) + 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.assertTrue(self.lc_block.validate()) From d1681d05b4b596140d01c7ef635817f22644be56 Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 8 Dec 2014 15:16:42 +0700 Subject: [PATCH 03/15] LibraryContent bok choy acceptance tests --- .../xmodule/xmodule/library_content_module.py | 1 + common/test/acceptance/fixtures/course.py | 2 - common/test/acceptance/fixtures/library.py | 1 + common/test/acceptance/pages/lms/library.py | 37 ++++ .../test/acceptance/pages/studio/library.py | 179 +++++++++++++++++- .../test/acceptance/tests/lms/test_library.py | 169 +++++++++++++++++ .../tests/studio/base_studio_test.py | 6 +- .../tests/studio/test_studio_library.py | 4 +- .../studio/test_studio_library_container.py | 133 +++++++++++++ 9 files changed, 521 insertions(+), 11 deletions(-) create mode 100644 common/test/acceptance/pages/lms/library.py create mode 100644 common/test/acceptance/tests/lms/test_library.py create mode 100644 common/test/acceptance/tests/studio/test_studio_library_container.py diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 092bc2a93117..2d1e386847b3 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -119,6 +119,7 @@ class LibraryContentFields(object): scope=Scope.settings, ) mode = String( + display_name=_("Mode"), help=_("Determines how content is drawn from the library"), default="random", values=[ diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index 1e5bca8a337f..656a12a9658a 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -375,5 +375,3 @@ def _create_xblock_children(self, parent_loc, xblock_descriptions): """ super(CourseFixture, self)._create_xblock_children(parent_loc, xblock_descriptions) self._publish_xblock(parent_loc) - - diff --git a/common/test/acceptance/fixtures/library.py b/common/test/acceptance/fixtures/library.py index f97b8e9fc222..5692c078dbd5 100644 --- a/common/test/acceptance/fixtures/library.py +++ b/common/test/acceptance/fixtures/library.py @@ -27,6 +27,7 @@ def __init__(self, org, number, display_name): 'display_name': display_name } + self.display_name = display_name self._library_key = None super(LibraryFixture, self).__init__() diff --git a/common/test/acceptance/pages/lms/library.py b/common/test/acceptance/pages/lms/library.py new file mode 100644 index 000000000000..8655fae79f55 --- /dev/null +++ b/common/test/acceptance/pages/lms/library.py @@ -0,0 +1,37 @@ +""" +Library Content XBlock Wrapper +""" +from bok_choy.page_object import PageObject + + +class LibraryContentXBlockWrapper(PageObject): + """ + A PageObject representing a wrapper around a LibraryContent block seen in the LMS + """ + url = None + BODY_SELECTOR = '.xblock-student_view div' + + def __init__(self, browser, locator): + super(LibraryContentXBlockWrapper, self).__init__(browser) + self.locator = locator + + def is_browser_on_page(self): + return self.q(css='{}[data-id="{}"]'.format(self.BODY_SELECTOR, self.locator)).present + + def _bounded_selector(self, selector): + """ + Return `selector`, but limited to this particular block's context + """ + return '{}[data-id="{}"] {}'.format( + self.BODY_SELECTOR, + self.locator, + selector + ) + + @property + def children_contents(self): + """ + Gets contents of all child XBlocks as list of strings + """ + child_blocks = self.q(css=self._bounded_selector("div[data-id]")) + return frozenset(child.text for child in child_blocks) diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index 64f93f21167e..3151324cd079 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,8 +3,12 @@ """ from bok_choy.page_object import PageObject -from ...pages.studio.pagination import PaginatedMixin +from bok_choy.promise import EmptyPromise +from selenium.webdriver.common.keys import Keys +from selenium.webdriver.support.select import Select +from .overview import CourseOutlineModal from .container import XBlockWrapper +from ...pages.studio.pagination import PaginatedMixin from ...tests.helpers import disable_animations from .utils import confirm_prompt, wait_for_notification from . import BASE_URL @@ -48,7 +52,10 @@ def wait_until_ready(self): for improved test reliability. """ self.wait_for_ajax() - self.wait_for_element_invisibility('.ui-loading', 'Wait for the page to complete its initial loading of XBlocks via AJAX') + self.wait_for_element_invisibility( + '.ui-loading', + 'Wait for the page to complete its initial loading of XBlocks via AJAX' + ) disable_animations(self) @property @@ -80,14 +87,18 @@ def _get_xblocks(self): Create an XBlockWrapper for each XBlock div found on the page. """ prefix = '.wrapper-xblock.level-page ' - return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map(lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator'))).results + return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map( + lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator')) + ).results def _div_for_xblock_id(self, xblock_id): """ Given an XBlock's usage locator as a string, return the WebElement for that block's wrapper div. """ - return self.q(css='.wrapper-xblock.level-page .studio-xblock-wrapper').filter(lambda el: el.get_attribute('data-locator') == xblock_id) + return self.q(css='.wrapper-xblock.level-page .studio-xblock-wrapper').filter( + lambda el: el.get_attribute('data-locator') == xblock_id + ) def _action_btn_for_xblock_id(self, xblock_id, action): """ @@ -95,4 +106,162 @@ def _action_btn_for_xblock_id(self, xblock_id, action): buttons. action is 'edit', 'duplicate', or 'delete' """ - return self._div_for_xblock_id(xblock_id)[0].find_element_by_css_selector('.header-actions .{action}-button.action-button'.format(action=action)) + return self._div_for_xblock_id(xblock_id)[0].find_element_by_css_selector( + '.header-actions .{action}-button.action-button'.format(action=action) + ) + + +class StudioLibraryContentXBlockEditModal(CourseOutlineModal, PageObject): + """ + Library Content XBlock Modal edit window + """ + url = None + MODAL_SELECTOR = ".wrapper-modal-window-edit-xblock" + + # Labels used to identify the fields on the edit modal: + LIBRARY_LABEL = "Libraries" + COUNT_LABEL = "Count" + SCORED_LABEL = "Scored" + + def is_browser_on_page(self): + """ + Check that we are on the right page in the browser. + """ + return self.is_shown() + + @property + def library_key(self): + """ + Gets value of first library key input + """ + library_key_input = self.get_metadata_input(self.LIBRARY_LABEL) + if library_key_input is not None: + return library_key_input.get_attribute('value').strip(',') + return None + + @library_key.setter + def library_key(self, library_key): + """ + Sets value of first library key input, creating it if necessary + """ + library_key_input = self.get_metadata_input(self.LIBRARY_LABEL) + if library_key_input is None: + library_key_input = self._add_library_key() + if library_key is not None: + # can't use lib_text.clear() here as input get deleted by client side script + library_key_input.send_keys(Keys.HOME) + library_key_input.send_keys(Keys.SHIFT, Keys.END) + library_key_input.send_keys(library_key) + else: + library_key_input.clear() + EmptyPromise(lambda: self.library_key == library_key, "library_key is updated in modal.").fulfill() + + @property + def count(self): + """ + Gets value of children count input + """ + return int(self.get_metadata_input(self.COUNT_LABEL).get_attribute('value')) + + @count.setter + def count(self, count): + """ + Sets value of children count input + """ + count_text = self.get_metadata_input(self.COUNT_LABEL) + count_text.clear() + count_text.send_keys(count) + EmptyPromise(lambda: self.count == count, "count is updated in modal.").fulfill() + + @property + def scored(self): + """ + Gets value of scored select + """ + value = self.get_metadata_input(self.SCORED_LABEL).get_attribute('value') + if value == 'True': + return True + elif value == 'False': + return False + raise ValueError("Unknown value {value} set for {label}".format(value=value, label=self.SCORED_LABEL)) + + @scored.setter + def scored(self, scored): + """ + Sets value of scored select + """ + select_element = self.get_metadata_input(self.SCORED_LABEL) + select_element.click() + scored_select = Select(select_element) + scored_select.select_by_value(str(scored)) + EmptyPromise(lambda: self.scored == scored, "scored is updated in modal.").fulfill() + + def _add_library_key(self): + """ + Adds library key input + """ + wrapper = self._get_metadata_element(self.LIBRARY_LABEL) + add_button = wrapper.find_element_by_xpath(".//a[contains(@class, 'create-action')]") + add_button.click() + return self._get_list_inputs(wrapper)[0] + + def _get_list_inputs(self, list_wrapper): + """ + Finds nested input elements (useful for List and Dict fields) + """ + return list_wrapper.find_elements_by_xpath(".//input[@type='text']") + + def _get_metadata_element(self, metadata_key): + """ + Gets metadata input element (a wrapper div for List and Dict fields) + """ + metadata_inputs = self.find_css(".metadata_entry .wrapper-comp-setting label.setting-label") + target_label = [elem for elem in metadata_inputs if elem.text == metadata_key][0] + label_for = target_label.get_attribute('for') + return self.find_css("#" + label_for)[0] + + def get_metadata_input(self, metadata_key): + """ + Gets input/select element for given field + """ + element = self._get_metadata_element(metadata_key) + if element.tag_name == 'div': + # List or Dict field - return first input + # TODO support multiple values + inputs = self._get_list_inputs(element) + element = inputs[0] if inputs else None + return element + + +class StudioLibraryContainerXBlockWrapper(XBlockWrapper): + """ + Wraps :class:`.container.XBlockWrapper` for use with LibraryContent blocks + """ + url = None + + @classmethod + def from_xblock_wrapper(cls, xblock_wrapper): + """ + Factory method: creates :class:`.StudioLibraryContainerXBlockWrapper` from :class:`.container.XBlockWrapper` + """ + return cls(xblock_wrapper.browser, xblock_wrapper.locator) + + @property + def header_text(self): + """ + Gets library content text + """ + return self.get_body_paragraphs().first.text[0] + + def get_body_paragraphs(self): + """ + Gets library content body paragraphs + """ + return self.q(css=self._bounded_selector(".xblock-message-area p")) + + def refresh_children(self): + """ + Click "Update now..." button + """ + refresh_button = self.q(css=self._bounded_selector(".library-update-btn")) + refresh_button.click() diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py new file mode 100644 index 000000000000..78d699faa6fd --- /dev/null +++ b/common/test/acceptance/tests/lms/test_library.py @@ -0,0 +1,169 @@ +# -*- coding: utf-8 -*- +""" +End-to-end tests for LibraryContent block in LMS +""" +import ddt + +from ..helpers import UniqueCourseTest +from ...pages.studio.auto_auth import AutoAuthPage +from ...pages.studio.overview import CourseOutlinePage +from ...pages.studio.library import StudioLibraryContentXBlockEditModal, StudioLibraryContainerXBlockWrapper +from ...pages.lms.courseware import CoursewarePage +from ...pages.lms.library import LibraryContentXBlockWrapper +from ...pages.common.logout import LogoutPage +from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from ...fixtures.library import LibraryFixture + +SECTION_NAME = 'Test Section' +SUBSECTION_NAME = 'Test Subsection' +UNIT_NAME = 'Test Unit' + + +@ddt.ddt +class LibraryContentTest(UniqueCourseTest): + """ + Test courseware. + """ + USERNAME = "STUDENT_TESTER" + EMAIL = "student101@example.com" + + STAFF_USERNAME = "STAFF_TESTER" + STAFF_EMAIL = "staff101@example.com" + + def setUp(self): + """ + Set up library, course and library content XBlock + """ + super(LibraryContentTest, self).setUp() + + self.courseware_page = CoursewarePage(self.browser, self.course_id) + + self.course_outline = CourseOutlinePage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + self.library_fixture = LibraryFixture('test_org', self.unique_id, 'Test Library {}'.format(self.unique_id)) + self.library_fixture.add_children( + XBlockFixtureDesc("html", "Html1", data='html1'), + XBlockFixtureDesc("html", "Html2", data='html2'), + XBlockFixtureDesc("html", "Html3", data='html3'), + ) + + self.library_fixture.install() + self.library_info = self.library_fixture.library_info + self.library_key = self.library_fixture.library_key + + # Install a course with library content xblock + self.course_fixture = CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'] + ) + + library_content_metadata = { + 'source_libraries': [self.library_key], + 'mode': 'random', + 'max_count': 1, + 'has_score': False + } + + self.lib_block = XBlockFixtureDesc('library_content', "Library Content", metadata=library_content_metadata) + + self.course_fixture.add_children( + XBlockFixtureDesc('chapter', SECTION_NAME).add_children( + XBlockFixtureDesc('sequential', SUBSECTION_NAME).add_children( + XBlockFixtureDesc('vertical', UNIT_NAME).add_children( + self.lib_block + ) + ) + ) + ) + + self.course_fixture.install() + + def _refresh_library_content_children(self, count=1): + """ + Performs library block refresh in Studio, configuring it to show {count} children + """ + unit_page = self._go_to_unit_page(True) + library_container_block = StudioLibraryContainerXBlockWrapper.from_xblock_wrapper(unit_page.xblocks[0]) + modal = StudioLibraryContentXBlockEditModal(library_container_block.edit()) + modal.count = count + library_container_block.save_settings() + library_container_block.refresh_children() + self._go_to_unit_page(change_login=False) + unit_page.wait_for_page() + unit_page.publish_action.click() + unit_page.wait_for_ajax() + self.assertIn("Published and Live", unit_page.publish_title) + + @property + def library_xblocks_texts(self): + """ + Gets texts of all xblocks in library + """ + return frozenset(child.data for child in self.library_fixture.children) + + def _go_to_unit_page(self, change_login=True): + """ + Open unit page in Studio + """ + if change_login: + LogoutPage(self.browser).visit() + self._auto_auth(self.STAFF_USERNAME, self.STAFF_EMAIL, True) + self.course_outline.visit() + subsection = self.course_outline.section(SECTION_NAME).subsection(SUBSECTION_NAME) + return subsection.toggle_expand().unit(UNIT_NAME).go_to() + + def _goto_library_block_page(self, block_id=None): + """ + Open library page in LMS + """ + self.courseware_page.visit() + block_id = block_id if block_id is not None else self.lib_block.locator + #pylint: disable=attribute-defined-outside-init + self.library_content_page = LibraryContentXBlockWrapper(self.browser, block_id) + + def _auto_auth(self, username, email, staff): + """ + Logout and login with given credentials. + """ + AutoAuthPage(self.browser, username=username, email=email, + course_id=self.course_id, staff=staff).visit() + + @ddt.data(1, 2, 3) + def test_shows_random_xblocks_from_configured(self, count): + """ + Scenario: Ensures that library content shows {count} random xblocks from library in LMS + Given I have a library, a course and a LibraryContent block in that course + When I go to studio unit page for library content xblock as staff + And I set library content xblock to display {count} random children + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see {count} random xblocks from the library + """ + self._refresh_library_content_children(count=count) + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_contents = self.library_content_page.children_contents + self.assertEqual(len(children_contents), count) + self.assertLessEqual(children_contents, self.library_xblocks_texts) + + def test_shows_all_if_max_set_to_greater_value(self): + """ + Scenario: Ensures that library content shows {count} random xblocks from library in LMS + Given I have a library, a course and a LibraryContent block in that course + When I go to studio unit page for library content xblock as staff + And I set library content xblock to display more children than library have + And I refresh library content xblock and pulbish unit + When I go to LMS courseware page for library content xblock as student + Then I can see all xblocks from the library + """ + self._refresh_library_content_children(count=10) + self._auto_auth(self.USERNAME, self.EMAIL, False) + self._goto_library_block_page() + children_contents = self.library_content_page.children_contents + self.assertEqual(len(children_contents), 3) + self.assertEqual(children_contents, self.library_xblocks_texts) diff --git a/common/test/acceptance/tests/studio/base_studio_test.py b/common/test/acceptance/tests/studio/base_studio_test.py index ec94f7f058c5..02fdcbe99849 100644 --- a/common/test/acceptance/tests/studio/base_studio_test.py +++ b/common/test/acceptance/tests/studio/base_studio_test.py @@ -109,8 +109,9 @@ class StudioLibraryTest(WebAppTest): """ Base class for all Studio library tests. """ + as_staff = True - def setUp(self, is_staff=False): # pylint: disable=arguments-differ + def setUp(self): # pylint: disable=arguments-differ """ Install a library with no content using a fixture. """ @@ -122,10 +123,11 @@ def setUp(self, is_staff=False): # pylint: disable=arguments-differ ) self.populate_library_fixture(fixture) fixture.install() + self.library_fixture = fixture self.library_info = fixture.library_info self.library_key = fixture.library_key self.user = fixture.user - self.log_in(self.user, is_staff) + self.log_in(self.user, self.as_staff) def populate_library_fixture(self, library_fixture): """ diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index 491c9093d0fc..b0d6cffb1aed 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -18,7 +18,7 @@ def setUp(self): # pylint: disable=arguments-differ """ Ensure a library exists and navigate to the library edit page. """ - super(LibraryEditPageTest, self).setUp(is_staff=True) + super(LibraryEditPageTest, self).setUp() self.lib_page = LibraryPage(self.browser, self.library_key) self.lib_page.visit() self.lib_page.wait_until_ready() @@ -156,7 +156,7 @@ def setUp(self): # pylint: disable=arguments-differ """ Ensure a library exists and navigate to the library edit page. """ - super(LibraryNavigationTest, self).setUp(is_staff=True) + super(LibraryNavigationTest, self).setUp() self.lib_page = LibraryPage(self.browser, self.library_key) self.lib_page.visit() self.lib_page.wait_until_ready() diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py new file mode 100644 index 000000000000..7bb712c779b5 --- /dev/null +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -0,0 +1,133 @@ +""" +Acceptance tests for Library Content in LMS +""" +import ddt +from .base_studio_test import StudioLibraryTest, ContainerBase +from ...pages.studio.library import StudioLibraryContentXBlockEditModal, StudioLibraryContainerXBlockWrapper +from ...fixtures.course import XBlockFixtureDesc + +SECTION_NAME = 'Test Section' +SUBSECTION_NAME = 'Test Subsection' +UNIT_NAME = 'Test Unit' + + +@ddt.ddt +class StudioLibraryContainerTest(ContainerBase, StudioLibraryTest): + """ + Test Library Content block in LMS + """ + def setUp(self): + """ + Install library with some content and a course using fixtures + """ + super(StudioLibraryContainerTest, self).setUp() + self.outline.visit() + subsection = self.outline.section(SECTION_NAME).subsection(SUBSECTION_NAME) + self.unit_page = subsection.toggle_expand().unit(UNIT_NAME).go_to() + + def populate_library_fixture(self, library_fixture): + """ + Populate the children of the test course fixture. + """ + library_fixture.add_children( + XBlockFixtureDesc("html", "Html1"), + XBlockFixtureDesc("html", "Html2"), + XBlockFixtureDesc("html", "Html3"), + ) + + def populate_course_fixture(self, course_fixture): + """ Install a course with sections/problems, tabs, updates, and handouts """ + library_content_metadata = { + 'source_libraries': [self.library_key], + 'mode': 'random', + 'max_count': 1, + 'has_score': False + } + + course_fixture.add_children( + XBlockFixtureDesc('chapter', SECTION_NAME).add_children( + XBlockFixtureDesc('sequential', SUBSECTION_NAME).add_children( + XBlockFixtureDesc('vertical', UNIT_NAME).add_children( + XBlockFixtureDesc('library_content', "Library Content", metadata=library_content_metadata) + ) + ) + ) + ) + + def _get_library_xblock_wrapper(self, xblock): + """ + Wraps xblock into :class:`...pages.studio.library.StudioLibraryContainerXBlockWrapper` + """ + return StudioLibraryContainerXBlockWrapper.from_xblock_wrapper(xblock) + + @ddt.data( + ('library-v1:111+111', 1, True), + ('library-v1:edX+L104', 2, False), + ('library-v1:OtherX+IDDQD', 3, True), + ) + @ddt.unpack + def test_can_edit_metadata(self, library_key, max_count, scored): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I edit library content metadata and save it + Then I can ensure that data is persisted + """ + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.library_key = library_key + edit_modal.count = max_count + edit_modal.scored = scored + + library_container.save_settings() # saving settings + + # open edit window again to verify changes are persistent + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + self.assertEqual(edit_modal.library_key, library_key) + self.assertEqual(edit_modal.count, max_count) + self.assertEqual(edit_modal.scored, scored) + + def test_no_library_shows_library_not_configured(self): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I edit set library key to none + Then I can see that library content block is misconfigured + """ + expected_text = 'No library or filters configured. Press "Edit" to configure.' + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + # precondition check - assert library is configured before we remove it + self.assertNotIn(expected_text, library_container.header_text) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.library_key = None + + library_container.save_settings() + + self.assertIn(expected_text, library_container.header_text) + + @ddt.data( + 'library-v1:111+111', + 'library-v1:edX+L104', + ) + def test_set_missing_library_shows_correct_label(self, library_key): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + And I edit set library key to non-existent library + Then I can see that library content block is misconfigured + """ + expected_text = "Library is invalid, corrupt, or has been deleted." + + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + # precondition check - assert library is configured before we remove it + self.assertNotIn(expected_text, library_container.header_text) + + edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) + edit_modal.library_key = library_key + + library_container.save_settings() + + self.assertIn(expected_text, library_container.header_text) From e312c15a8d91a1550df9ea970e7f23df008e17ba Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Dec 2014 13:02:38 -0800 Subject: [PATCH 04/15] Friendly error message when library key is invalid --- cms/djangoapps/contentstore/views/item.py | 5 ++-- .../contentstore/views/tests/test_item.py | 23 +++++++++++++++++++ .../xmodule/xmodule/library_content_module.py | 17 +++++++++++--- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index c1f627009cd0..7b893486eedd 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -427,8 +427,9 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, else: try: value = field.from_json(value) - except ValueError: - return JsonResponse({"error": "Invalid data"}, 400) + except ValueError as verr: + reason = _("Invalid data ({details})").format(details=verr.message) if verr.message else _("Invalid data") + return JsonResponse({"error": reason}, 400) field.write_to(xblock, value) # update the xblock and call any xblock callbacks diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index d6d913a59489..6b595ae00755 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -894,6 +894,29 @@ def test_publish_states_of_nested_xblocks(self): self._verify_published_with_draft(unit_usage_key) self._verify_published_with_draft(html_usage_key) + def test_field_value_errors(self): + """ + Test that if the user's input causes a ValueError on an XBlock field, + we provide a friendly error message back to the user. + """ + response = self.create_xblock(parent_usage_key=self.seq_usage_key, category='video') + video_usage_key = self.response_usage_key(response) + update_url = reverse_usage_url('xblock_handler', video_usage_key) + + response = self.client.ajax_post( + update_url, + data={ + 'id': unicode(video_usage_key), + 'metadata': { + 'saved_video_position': "Not a valid relative time", + }, + } + ) + self.assertEqual(response.status_code, 400) + parsed = json.loads(response.content) + self.assertIn("error", parsed) + self.assertIn("Incorrect RelativeTime value", parsed["error"]) # See xmodule/fields.py + class TestEditSplitModule(ItemTest): """ diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 2d1e386847b3..4233429e0b2f 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -1,11 +1,12 @@ """ LibraryContent: The XBlock used to include blocks from a library in a course. """ -from bson.objectid import ObjectId +from bson.objectid import ObjectId, InvalidId from collections import namedtuple from copy import copy import hashlib from .mako_module import MakoModuleDescriptor +from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import LibraryLocator import random from webob import Response @@ -46,7 +47,10 @@ def __new__(cls, library_id, version=None): version = library_id.version_guid library_id = library_id.for_version(None) if version and not isinstance(version, ObjectId): - version = ObjectId(version) + try: + version = ObjectId(version) + except InvalidId: + raise ValueError(version) return super(LibraryVersionReference, cls).__new__(cls, library_id, version) @staticmethod @@ -86,7 +90,14 @@ def parse(val): val = val.strip(' []') parts = val.rsplit(',', 1) val = [parts[0], parts[1] if len(parts) > 1 else None] - return LibraryVersionReference.from_json(val) + try: + return LibraryVersionReference.from_json(val) + except InvalidKeyError: + try: + friendly_val = val[0] # Just get the library key part, not the version + except IndexError: + friendly_val = unicode(val) + raise ValueError(_('"{value}" is not a valid library ID.').format(value=friendly_val)) return [parse(v) for v in values] def to_json(self, values): From eecc6f1032ab55d249d7c4289f9c5cf64caaa91e Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Wed, 26 Nov 2014 21:48:08 +0000 Subject: [PATCH 05/15] Added explanation to container view of Library Block. --- common/lib/xmodule/xmodule/library_content_module.py | 7 ++++++- lms/templates/library-block-author-preview-header.html | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 lms/templates/library-block-author-preview-header.html diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 4233429e0b2f..753d1938556a 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -251,7 +251,7 @@ def student_view(self, context): fragment.add_frag_resources(rendered_child) contents.append({ 'id': displayable.location.to_deprecated_string(), - 'content': rendered_child.content + 'content': rendered_child.content, }) fragment.add_content(self.system.render_template('vert_module.html', { @@ -273,6 +273,11 @@ def author_view(self, context): if is_root: # User has clicked the "View" link. Show a preview of all possible children: if self.children: # pylint: disable=no-member + fragment.add_content(self.system.render_template("library-block-author-preview-header.html", { + 'max_count': self.max_count, + 'display_name': self.display_name or self.url_name, + 'mode': self.mode, + })) self.render_children(context, fragment, can_reorder=False, can_add=False) else: fragment.add_content(u'

{}

'.format( diff --git a/lms/templates/library-block-author-preview-header.html b/lms/templates/library-block-author-preview-header.html new file mode 100644 index 000000000000..4596281b6702 --- /dev/null +++ b/lms/templates/library-block-author-preview-header.html @@ -0,0 +1,10 @@ +<%! from django.utils.translation import ugettext as _ %> +
+
+

+ + ${_('Showing all matching content eligible to be added into {display_name}. Each student will be assigned {mode} {max_count} components from this list.').format(max_count=max_count, display_name=display_name, mode=mode)} + +

+
+
From a7b577d1737daca865f29daff94578b7f058a0a2 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Wed, 26 Nov 2014 23:57:16 +0000 Subject: [PATCH 06/15] Made errors on Library blocks use validate functionality. --- .../xmodule/xmodule/library_content_module.py | 77 ++++++++++++------- lms/templates/library-block-author-view.html | 6 +- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 753d1938556a..476c97b0112d 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -14,6 +14,7 @@ from xblock.fields import Scope, String, List, Integer, Boolean from xblock.fragment import Fragment from xmodule.modulestore.exceptions import ItemNotFoundError +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 @@ -260,6 +261,40 @@ def student_view(self, context): })) return fragment + def validate(self): + """ + Validates the state of this Library Content Module Instance. This + is the override of the general XBlock method, and it will also ask + its superclass to validate. + """ + validation = super(LibraryContentModule, self).validate() + if not isinstance(validation, StudioValidation): + validation = StudioValidation.copy(validation) + if not self.source_libraries: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.NOT_CONFIGURED, + _(u"A library has not yet been selected."), + action_class='edit-button', + action_label=_(u"Select a Library") + ) + ) + return validation + for library_key, version in self.source_libraries: # pylint: disable=unused-variable + library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) + if library is None: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.ERROR, + _(u'Library is invalid, corrupt, or has been deleted.'), + action_class='edit-button', + action_label=_(u"Edit Library List") + ) + ) + break + + return validation + def author_view(self, context): """ Renders the Studio views. @@ -284,41 +319,31 @@ def author_view(self, context): _('No matching content found in library, no library configured, or not yet loaded from library.') )) else: - # When shown on a unit page, don't show any sort of preview - just the status of this block. - LibraryStatus = enum( # pylint: disable=invalid-name - NONE=0, # no library configured - INVALID=1, # invalid configuration or library has been deleted/corrupted - OK=2, # library configured correctly and should be working fine - ) UpdateStatus = enum( # pylint: disable=invalid-name CANNOT=0, # Cannot update - library is not set, invalid, deleted, etc. NEEDED=1, # An update is needed - prompt the user to update UP_TO_DATE=2, # No update necessary - library is up to date ) + # When shown on a unit page, don't show any sort of preview - just the status of this block. + library_ok = bool(self.source_libraries) # True if at least one source library is defined library_names = [] - library_status = LibraryStatus.OK update_status = UpdateStatus.UP_TO_DATE - if self.source_libraries: - for library_key, version in self.source_libraries: - library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) - if library is None: - library_status = LibraryStatus.INVALID - update_status = UpdateStatus.CANNOT - break - library_names.append(library.display_name) - latest_version = library.location.library_key.version_guid - if version is None or version != latest_version: - update_status = UpdateStatus.NEEDED - # else library is up to date. - else: - library_status = LibraryStatus.NONE - update_status = UpdateStatus.CANNOT + for library_key, version in self.source_libraries: + library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) + if library is None: + update_status = UpdateStatus.CANNOT + library_ok = False + break + library_names.append(library.display_name) + latest_version = library.location.library_key.version_guid + if version is None or version != latest_version: + update_status = UpdateStatus.NEEDED + fragment.add_content(self.system.render_template('library-block-author-view.html', { - 'library_status': library_status, - 'LibraryStatus': LibraryStatus, - 'update_status': update_status, - 'UpdateStatus': UpdateStatus, 'library_names': library_names, + 'library_ok': library_ok, + 'UpdateStatus': UpdateStatus, + 'update_status': update_status, 'max_count': self.max_count, 'mode': self.mode, 'num_children': len(self.children), # pylint: disable=no-member diff --git a/lms/templates/library-block-author-view.html b/lms/templates/library-block-author-view.html index 521946a903db..ce1542cc5888 100644 --- a/lms/templates/library-block-author-view.html +++ b/lms/templates/library-block-author-view.html @@ -2,16 +2,12 @@ from django.utils.translation import ugettext as _ %>
- % if library_status == LibraryStatus.OK: + % if library_ok:

${_('This component will be replaced by {mode} {max_count} components from the {num_children} matching components from {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

% if update_status == UpdateStatus.NEEDED:

${_('This component is out of date.')} ↻ ${_('Update now with latest components from the library')}

% elif update_status == UpdateStatus.UP_TO_DATE:

${_(u'✓ Up to date.')}

% endif - % elif library_status == LibraryStatus.NONE: -

${_('No library or filters configured. Press "Edit" to configure.')}

- % else: -

${_('Library is invalid, corrupt, or has been deleted.')}

% endif
From dfd0d70ab8209a57b88cf5c3155fbac25d1c4c6d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Dec 2014 14:08:51 -0800 Subject: [PATCH 07/15] Move update link to the validation area --- .../xmodule/xmodule/library_content_module.py | 65 +++++++++---------- .../xmodule/public/js/library_content_edit.js | 12 +++- .../test/acceptance/pages/studio/container.py | 39 +++++++++++ .../test/acceptance/pages/studio/library.py | 11 +--- .../studio/test_studio_library_container.py | 47 ++++++++++---- .../library-block-author-preview-header.html | 2 +- lms/templates/library-block-author-view.html | 9 +-- 7 files changed, 117 insertions(+), 68 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 476c97b0112d..d9e28e93fd09 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """ LibraryContent: The XBlock used to include blocks from a library in a course. """ @@ -280,9 +281,21 @@ def validate(self): ) ) return validation - for library_key, version in self.source_libraries: # pylint: disable=unused-variable + for library_key, version in self.source_libraries: library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) - if library is None: + if library is not None: + latest_version = library.location.library_key.version_guid + if version is None or version != latest_version: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + _(u'This component is out of date. The library has new content.'), + action_class='library-update-btn', # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + action_label=_(u"↻ Update now") + ) + ) + break + else: validation.set_summary( StudioValidationMessage( StudioValidationMessage.ERROR, @@ -298,7 +311,7 @@ def validate(self): def author_view(self, context): """ Renders the Studio views. - Normal studio view: displays library status and has an "Update" button. + Normal studio view: If block is properly configured, displays library status summary Studio container view: displays a preview of all possible children. """ fragment = Fragment() @@ -311,45 +324,25 @@ def author_view(self, context): fragment.add_content(self.system.render_template("library-block-author-preview-header.html", { 'max_count': self.max_count, 'display_name': self.display_name or self.url_name, - 'mode': self.mode, })) self.render_children(context, fragment, can_reorder=False, can_add=False) - else: - fragment.add_content(u'

{}

'.format( - _('No matching content found in library, no library configured, or not yet loaded from library.') - )) else: - UpdateStatus = enum( # pylint: disable=invalid-name - CANNOT=0, # Cannot update - library is not set, invalid, deleted, etc. - NEEDED=1, # An update is needed - prompt the user to update - UP_TO_DATE=2, # No update necessary - library is up to date - ) # When shown on a unit page, don't show any sort of preview - just the status of this block. - library_ok = bool(self.source_libraries) # True if at least one source library is defined library_names = [] - update_status = UpdateStatus.UP_TO_DATE - for library_key, version in self.source_libraries: + for library_key, version in self.source_libraries: # pylint: disable=unused-variable library = _get_library(self.runtime.descriptor_runtime.modulestore, library_key) - if library is None: - update_status = UpdateStatus.CANNOT - library_ok = False - break - library_names.append(library.display_name) - latest_version = library.location.library_key.version_guid - if version is None or version != latest_version: - update_status = UpdateStatus.NEEDED - - fragment.add_content(self.system.render_template('library-block-author-view.html', { - 'library_names': library_names, - 'library_ok': library_ok, - 'UpdateStatus': UpdateStatus, - 'update_status': update_status, - 'max_count': self.max_count, - 'mode': self.mode, - 'num_children': len(self.children), # pylint: disable=no-member - })) - fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit.js')) - fragment.initialize_js('LibraryContentAuthorView') + if library is not None: + library_names.append(library.display_name) + + if library_names: + fragment.add_content(self.system.render_template('library-block-author-view.html', { + 'library_names': library_names, + 'max_count': self.max_count, + 'num_children': len(self.children), # pylint: disable=no-member + })) + # 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')) + fragment.initialize_js('LibraryContentAuthorView') return fragment def get_child_descriptors(self): diff --git a/common/lib/xmodule/xmodule/public/js/library_content_edit.js b/common/lib/xmodule/xmodule/public/js/library_content_edit.js index 9a84a214049c..2db019feddf2 100644 --- a/common/lib/xmodule/xmodule/public/js/library_content_edit.js +++ b/common/lib/xmodule/xmodule/public/js/library_content_edit.js @@ -1,6 +1,14 @@ -/* JavaScript for editing operations that can be done on LibraryContentXBlock */ +/* JavaScript for special editing operations that can be done on LibraryContentXBlock */ window.LibraryContentAuthorView = function (runtime, element) { - $(element).find('.library-update-btn').on('click', function(e) { + "use strict"; + var usage_id = $(element).data('usage-id'); + // The "Update Now" button is not a child of 'element', as it is in the validation message area + // But it is still inside this xblock's wrapper element, which we can easily find: + var $wrapper = $(element).parents('*[data-locator="'+usage_id+'"]'); + + // We can't bind to the button itself because in the bok choy test environment, + // it may not yet exist at this point in time... not sure why. + $wrapper.on('click', '.library-update-btn', function(e) { e.preventDefault(); // Update the XBlock with the latest matching content from the library: runtime.notify('save', { diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 20cf140ed117..dc93be67f561 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -333,6 +333,45 @@ def children(self): grand_locators = [grandkid.locator for grandkid in grandkids] return [descendant for descendant in descendants if descendant.locator not in grand_locators] + @property + def has_validation_message(self): + """ Is a validation warning/error/message shown? """ + return self.q(css=self._bounded_selector('.xblock-message.validation')).present + + def _validation_paragraph(self, css_class): + """ Helper method to return the

element of a validation warning """ + return self.q(css=self._bounded_selector('.xblock-message.validation p.{}'.format(css_class))) + + @property + def has_validation_warning(self): + """ Is a validation warning shown? """ + return self._validation_paragraph('warning').present + + @property + def has_validation_error(self): + """ Is a validation error shown? """ + return self._validation_paragraph('error').present + + @property + def has_validation_not_configured_warning(self): + """ Is a validation "not configured" message shown? """ + return self._validation_paragraph('not-configured').present + + @property + def validation_warning_text(self): + """ Get the text of the validation warning. """ + return self._validation_paragraph('warning').text[0] + + @property + def validation_error_text(self): + """ Get the text of the validation error. """ + return self._validation_paragraph('error').text[0] + + @property + def validation_not_configured_warning_text(self): + """ Get the text of the validation "not configured" message. """ + return self._validation_paragraph('not-configured').text[0] + @property def preview_selector(self): return self._bounded_selector('.xblock-student_view,.xblock-author_view') diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index 3151324cd079..ea7f2299f961 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -246,13 +246,6 @@ def from_xblock_wrapper(cls, xblock_wrapper): """ return cls(xblock_wrapper.browser, xblock_wrapper.locator) - @property - def header_text(self): - """ - Gets library content text - """ - return self.get_body_paragraphs().first.text[0] - def get_body_paragraphs(self): """ Gets library content body paragraphs @@ -263,5 +256,7 @@ def refresh_children(self): """ Click "Update now..." button """ - refresh_button = self.q(css=self._bounded_selector(".library-update-btn")) + btn_selector = self._bounded_selector(".library-update-btn") + refresh_button = self.q(css=btn_selector) refresh_button.click() + self.wait_for_element_absence(btn_selector, 'Wait for the XBlock to reload') 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 7bb712c779b5..ba66bdd7b1ec 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -94,40 +94,61 @@ def test_no_library_shows_library_not_configured(self): And I edit set library key to none Then I can see that library content block is misconfigured """ - expected_text = 'No library or filters configured. Press "Edit" to configure.' + expected_text = 'A library has not yet been selected.' + expected_action = 'Select a Library' library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) - # precondition check - assert library is configured before we remove it - self.assertNotIn(expected_text, library_container.header_text) + # precondition check - the library block should be configured before we remove the library setting + self.assertFalse(library_container.has_validation_not_configured_warning) edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) edit_modal.library_key = None - library_container.save_settings() - self.assertIn(expected_text, library_container.header_text) + self.assertTrue(library_container.has_validation_not_configured_warning) + self.assertIn(expected_text, library_container.validation_not_configured_warning_text) + self.assertIn(expected_action, library_container.validation_not_configured_warning_text) - @ddt.data( - 'library-v1:111+111', - 'library-v1:edX+L104', - ) - def test_set_missing_library_shows_correct_label(self, library_key): + def test_set_missing_library_shows_correct_label(self): """ Scenario: Given I have a library, a course and library content xblock in a course When I go to studio unit page for library content block And I edit set library key to non-existent library Then I can see that library content block is misconfigured """ + nonexistent_lib_key = 'library-v1:111+111' expected_text = "Library is invalid, corrupt, or has been deleted." library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) # precondition check - assert library is configured before we remove it - self.assertNotIn(expected_text, library_container.header_text) + self.assertFalse(library_container.has_validation_error) edit_modal = StudioLibraryContentXBlockEditModal(library_container.edit()) - edit_modal.library_key = library_key + edit_modal.library_key = nonexistent_lib_key library_container.save_settings() - self.assertIn(expected_text, library_container.header_text) + self.assertTrue(library_container.has_validation_error) + self.assertIn(expected_text, library_container.validation_error_text) + + def test_out_of_date_message(self): + """ + Scenario: Given I have a library, a course and library content xblock in a course + When I go to studio unit page for library content block + Then I can see that library content block needs to be updated + When I click on the update link + Then I can see that the content no longer needs to be updated + """ + expected_text = "This component is out of date. The library has new content." + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + self.assertTrue(library_container.has_validation_warning) + self.assertIn(expected_text, library_container.validation_warning_text) + + library_container.refresh_children() + + self.unit_page.wait_for_page() # Wait for the page to reload + library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + + self.assertFalse(library_container.has_validation_message) diff --git a/lms/templates/library-block-author-preview-header.html b/lms/templates/library-block-author-preview-header.html index 4596281b6702..ad76623deb6a 100644 --- a/lms/templates/library-block-author-preview-header.html +++ b/lms/templates/library-block-author-preview-header.html @@ -3,7 +3,7 @@

- ${_('Showing all matching content eligible to be added into {display_name}. Each student will be assigned {mode} {max_count} components from this list.').format(max_count=max_count, display_name=display_name, mode=mode)} + ${_('Showing all matching content eligible to be added into {display_name}. Each student will be assigned {max_count} component[s] drawn randomly from this list.').format(max_count=max_count, display_name=display_name)}

diff --git a/lms/templates/library-block-author-view.html b/lms/templates/library-block-author-view.html index ce1542cc5888..46202aa2a9c6 100644 --- a/lms/templates/library-block-author-view.html +++ b/lms/templates/library-block-author-view.html @@ -2,12 +2,5 @@ from django.utils.translation import ugettext as _ %>
- % if library_ok: -

${_('This component will be replaced by {mode} {max_count} components from the {num_children} matching components from {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

- % if update_status == UpdateStatus.NEEDED: -

${_('This component is out of date.')} ↻ ${_('Update now with latest components from the library')}

- % elif update_status == UpdateStatus.UP_TO_DATE: -

${_(u'✓ Up to date.')}

- % endif - % endif +

${_('This component will be replaced by {max_count} component[s] randomly chosen from the {num_children} matching components in {lib_names}.').format(mode=mode, max_count=max_count, num_children=num_children, lib_names=', '.join(library_names))}

From e20b904b6b64ef65ac1393bc92709832ec71f644 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Dec 2014 20:59:49 -0800 Subject: [PATCH 08/15] Fix: don't need to reload the whole page to refresh_children from the container view --- .../xmodule/public/js/library_content_edit.js | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/public/js/library_content_edit.js b/common/lib/xmodule/xmodule/public/js/library_content_edit.js index 2db019feddf2..89011789b99b 100644 --- a/common/lib/xmodule/xmodule/public/js/library_content_edit.js +++ b/common/lib/xmodule/xmodule/public/js/library_content_edit.js @@ -1,10 +1,11 @@ /* JavaScript for special editing operations that can be done on LibraryContentXBlock */ window.LibraryContentAuthorView = function (runtime, element) { "use strict"; - var usage_id = $(element).data('usage-id'); + var $element = $(element); + var usage_id = $element.data('usage-id'); // The "Update Now" button is not a child of 'element', as it is in the validation message area // But it is still inside this xblock's wrapper element, which we can easily find: - var $wrapper = $(element).parents('*[data-locator="'+usage_id+'"]'); + var $wrapper = $element.parents('*[data-locator="'+usage_id+'"]'); // We can't bind to the button itself because in the bok choy test environment, // it may not yet exist at this point in time... not sure why. @@ -21,12 +22,15 @@ window.LibraryContentAuthorView = function (runtime, element) { state: 'end', element: element }); - // runtime.refreshXBlock(element); - // The above does not work, because this XBlock's runtime has no reference - // to the page (XBlockContainerPage). Only the Vertical XBlock's runtime has - // a reference to the page, and we have no way of getting a reference to it. - // So instead we: - location.reload(); + if ($element.closest('.wrapper-xblock').is(':not(.level-page)')) { + // We are on a course unit page. The notify('save') should refresh this block, + // but that is only working on the container page view of this block. + // Why? On the unit page, this XBlock's runtime has no reference to the + // XBlockContainerPage - only the top-level XBlock (a vertical) runtime does. + // But unfortunately there is no way to get a reference to our parent block's + // JS 'runtime' object. So instead we must refresh the whole page: + location.reload(); + } }); }); }; From 66f5f8f25799817d60d9bc3a9a207ec304ce233d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 11 Dec 2014 00:40:08 -0800 Subject: [PATCH 09/15] Refresh children automatically when library setting is changed --- .../xmodule/xmodule/library_content_module.py | 24 ++++++++++++++++--- .../test/acceptance/pages/studio/container.py | 8 +++++++ .../test/acceptance/tests/lms/test_library.py | 1 - .../studio/test_studio_library_container.py | 22 ++++++++++++----- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index d9e28e93fd09..f89f147330dd 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -363,7 +363,7 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe js_module_name = "VerticalDescriptor" @XBlock.handler - def refresh_children(self, request, suffix): # pylint: disable=unused-argument + def refresh_children(self, request, suffix, update_db=True): # pylint: disable=unused-argument """ Refresh children: This method is to be used when any of the libraries that this block @@ -375,8 +375,12 @@ def refresh_children(self, request, suffix): # pylint: disable=unused-argument This method will update this block's 'source_libraries' field to store the version number of the libraries used, so we easily determine if this block is up to date or not. + + If update_db is True (default), this will explicitly persist the changes + to the modulestore by calling update_item() """ - user_id = self.runtime.service(self, 'user').user_id + user_service = self.runtime.service(self, 'user') + user_id = user_service.user_id if user_service else None # May be None when creating bok choy test fixtures root_children = [] store = self.system.modulestore @@ -395,6 +399,8 @@ def refresh_children(self, request, suffix): # pylint: disable=unused-argument new_libraries = [] for library_key, old_version in self.source_libraries: # pylint: disable=unused-variable library = _get_library(self.system.modulestore, library_key) # pylint: disable=protected-access + if library is None: + raise ValueError("Required library not found.") def copy_children_recursively(from_block): """ @@ -434,9 +440,21 @@ def copy_children_recursively(from_block): new_libraries.append(LibraryVersionReference(library_key, library.location.library_key.version_guid)) self.source_libraries = new_libraries self.children = root_children # pylint: disable=attribute-defined-outside-init - self.system.modulestore.update_item(self, user_id) + if update_db: + self.system.modulestore.update_item(self, user_id) return Response() + def editor_saved(self, user, old_metadata, old_content): + """ + If source_libraries has been edited, refresh_children automatically. + """ + old_source_libraries = LibraryList().from_json(old_metadata.get('source_libraries', [])) + if set(old_source_libraries) != set(self.source_libraries): + try: + self.refresh_children(None, None, update_db=False) # update_db=False since update_item() is about to be called anyways + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + def has_dynamic_children(self): """ Inform the runtime that our children vary per-user. diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index dc93be67f561..d8a760cac972 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -309,6 +309,14 @@ def student_content(self): """ return self.q(css=self._bounded_selector('.xblock-student_view'))[0].text + @property + def author_content(self): + """ + Returns the text content of the xblock as displayed on the container page. + (For blocks which implement a distinct author_view). + """ + return self.q(css=self._bounded_selector('.xblock-author_view'))[0].text + @property def name(self): titles = self.q(css=self._bounded_selector(self.NAME_SELECTOR)).text diff --git a/common/test/acceptance/tests/lms/test_library.py b/common/test/acceptance/tests/lms/test_library.py index 78d699faa6fd..f83e6b94e9d5 100644 --- a/common/test/acceptance/tests/lms/test_library.py +++ b/common/test/acceptance/tests/lms/test_library.py @@ -92,7 +92,6 @@ def _refresh_library_content_children(self, count=1): modal = StudioLibraryContentXBlockEditModal(library_container_block.edit()) modal.count = count library_container_block.save_settings() - library_container_block.refresh_children() self._go_to_unit_page(change_login=False) unit_page.wait_for_page() unit_page.publish_action.click() 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 ba66bdd7b1ec..6e8fddeb8efb 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -136,19 +136,29 @@ def test_out_of_date_message(self): """ Scenario: Given I have a library, a course and library content xblock in a course When I go to studio unit page for library content block + Then I update the library being used + Then I refresh the page Then I can see that library content block needs to be updated When I click on the update link Then I can see that the content no longer needs to be updated """ expected_text = "This component is out of date. The library has new content." - library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + 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) + + self.library_fixture.create_xblock(self.library_fixture.library_location, XBlockFixtureDesc("html", "Html4")) - self.assertTrue(library_container.has_validation_warning) - self.assertIn(expected_text, library_container.validation_warning_text) + self.unit_page.visit() # Reload the page - library_container.refresh_children() + self.assertTrue(library_block.has_validation_warning) + self.assertIn(expected_text, library_block.validation_warning_text) + + library_block.refresh_children() self.unit_page.wait_for_page() # Wait for the page to reload - library_container = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) + library_block = self._get_library_xblock_wrapper(self.unit_page.xblocks[0]) - self.assertFalse(library_container.has_validation_message) + self.assertFalse(library_block.has_validation_message) + self.assertIn("4 matching components", library_block.author_content) From c2f757ffe0dfe52f2f42ee39a403e538e0fa344b Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 11 Dec 2014 15:10:38 -0800 Subject: [PATCH 10/15] Fix greedy intrusion of split_test documentation --- cms/templates/container.html | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cms/templates/container.html b/cms/templates/container.html index de395457f24b..e36914506b1b 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -103,7 +103,7 @@

${_("Page Actions")}