From 99220e096726fb227feed2c92eea49149a35ee5a Mon Sep 17 00:00:00 2001 From: Sid Verma Date: Fri, 24 Jul 2020 12:29:12 +0530 Subject: [PATCH] Add "Source from library" XBlock This lets the user import a block from a blockstore-based content library into a (modulestore based) course, by copying the block into the course. --- cms/djangoapps/contentstore/views/item.py | 3 + common/lib/xmodule/setup.py | 1 + .../public/js/library_source_block.js | 45 +++++++ .../xmodule/xmodule/library_content_module.py | 5 +- .../xmodule/xmodule/library_sourced_block.py | 127 ++++++++++++++++++ common/lib/xmodule/xmodule/library_tools.py | 120 ++++++++++++++++- .../split_mongo/caching_descriptor_system.py | 2 +- .../xmodule/modulestore/xml_importer.py | 6 +- .../library-sourced-block-author-view.html | 9 ++ .../xmodule/tests/test_library_content.py | 2 +- .../tests/test_library_sourced_block.py | 76 +++++++++++ .../xmodule/tests/test_library_tools.py | 72 +++++++++- lms/djangoapps/lms_xblock/runtime.py | 2 +- .../xblock/learning_context/manager.py | 7 +- .../completion_integration/test_services.py | 2 +- openedx/tests/settings.py | 4 + 16 files changed, 455 insertions(+), 28 deletions(-) create mode 100644 common/lib/xmodule/xmodule/assets/library_source_block/public/js/library_source_block.js create mode 100644 common/lib/xmodule/xmodule/library_sourced_block.py create mode 100644 common/lib/xmodule/xmodule/templates/library-sourced-block-author-view.html create mode 100644 common/lib/xmodule/xmodule/tests/test_library_sourced_block.py diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index f9244860e3..cd924868cc 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -67,6 +67,7 @@ from util.milestones_helpers import is_entrance_exams_enabled from xblock_config.models import CourseEditLTIFieldsEnabledFlag from xblock_django.user_service import DjangoXBlockUserService from xmodule.course_module import DEFAULT_START_DATE +from xmodule.library_tools import LibraryToolsService from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES @@ -319,6 +320,8 @@ class StudioEditModuleRuntime(object): return ConfigurationService(CourseEditLTIFieldsEnabledFlag) if service_name == "teams_configuration": return TeamsConfigurationService() + if service_name == "library_tools": + return LibraryToolsService(modulestore(), self._user.id) return None diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 76ac5f2dfe..1247955ac6 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -33,6 +33,7 @@ XBLOCKS = [ "course_info = xmodule.html_module:CourseInfoBlock", "html = xmodule.html_module:HtmlBlock", "library = xmodule.library_root_xblock:LibraryRoot", + "library_sourced = xmodule.library_sourced_block:LibrarySourcedBlock", "problem = xmodule.capa_module:ProblemBlock", "static_tab = xmodule.html_module:StaticTabBlock", "unit = xmodule.unit_block:UnitBlock", diff --git a/common/lib/xmodule/xmodule/assets/library_source_block/public/js/library_source_block.js b/common/lib/xmodule/xmodule/assets/library_source_block/public/js/library_source_block.js new file mode 100644 index 0000000000..cbbe6c0790 --- /dev/null +++ b/common/lib/xmodule/xmodule/assets/library_source_block/public/js/library_source_block.js @@ -0,0 +1,45 @@ +/* JavaScript for allowing editing options on LibrarySourceBlock's author view */ +window.LibrarySourceBlockAuthorView = function(runtime, element) { + 'use strict'; + var $element = $(element); + + $element.on('click', '.save-btn', function(e) { + var url = $(e.target).data('submit-url'); + var data = { + values: { + source_block_id: $element.find('input').val() + }, + defaults: ['display_name'] + }; + e.preventDefault(); + + runtime.notify('save', { + state: 'start', + message: gettext('Saving'), + element: element + }); + $.ajax({ + type: 'POST', + url: url, + data: JSON.stringify(data), + global: false // Disable error handling that conflicts with studio's notify('save') and notify('cancel') + }).done(function() { + runtime.notify('save', { + state: 'end', + element: element + }); + }).fail(function(jqXHR) { + var message = gettext('This may be happening because of an error with our server or your internet connection. Try refreshing the page or making sure you are online.'); // eslint-disable-line max-len + if (jqXHR.responseText) { // Is there a more specific error message we can show? + try { + message = JSON.parse(jqXHR.responseText).error; + if (typeof message === 'object' && message.messages) { + // e.g. {"error": {"messages": [{"text": "Unknown user 'bob'!", "type": "error"}, ...]}} etc. + message = $.map(message.messages, function(msg) { return msg.text; }).join(', '); + } + } catch (error) { message = jqXHR.responseText.substr(0, 300); } + } + runtime.notify('error', {title: gettext('Unable to update settings'), message: message}); + }); + }); +}; diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index a8bd13a4a8..9c204a0c9b 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -434,10 +434,9 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe this block is up to date or not. """ user_perms = self.runtime.service(self, 'studio_user_permissions') - user_id = self.get_user_id() if not self.tools: return Response("Library Tools unavailable in current runtime.", status=400) - self.tools.update_children(self, user_id, user_perms) + self.tools.update_children(self, user_perms) return Response() # Copy over any overridden settings the course author may have applied to the blocks. @@ -469,7 +468,7 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe user_perms = self.runtime.service(self, 'studio_user_permissions') if not self.tools: raise RuntimeError("Library tools unavailable, duplication will not be sane!") - self.tools.update_children(self, user_id, user_perms, version=self.source_library_version) + self.tools.update_children(self, user_perms, version=self.source_library_version) self._copy_overrides(store, user_id, source_block, self) diff --git a/common/lib/xmodule/xmodule/library_sourced_block.py b/common/lib/xmodule/xmodule/library_sourced_block.py new file mode 100644 index 0000000000..205bb7f4a0 --- /dev/null +++ b/common/lib/xmodule/xmodule/library_sourced_block.py @@ -0,0 +1,127 @@ +""" +Library Sourced Content XBlock +""" +import logging + +from copy import copy +from web_fragments.fragment import Fragment +from xblock.core import XBlock +from xblock.fields import Scope, String +from xblockutils.resources import ResourceLoader +from xblockutils.studio_editable import StudioEditableXBlockMixin +from webob import Response + +from cms.lib.xblock.runtime import handler_url +from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin +from xmodule.validation import StudioValidation, StudioValidationMessage + +log = logging.getLogger(__name__) +loader = ResourceLoader(__name__) + +# Make '_' a no-op so we can scrape strings. Using lambda instead of +# `django.utils.translation.ugettext_noop` because Django cannot be imported in this file +_ = lambda text: text + + +@XBlock.wants('library_tools') # Only needed in studio +class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlock): + """ + Library Sourced Content XBlock + + Allows copying specific XBlocks from a Blockstore-based content library into + a modulestore-based course. The selected blocks are copied and become + children of this block. + + When we implement support for Blockstore-based courses, it's expected we'll + use a different mechanism for importing library content into a course. + """ + display_name = String( + help=_("The display name for this component."), + default="Library Sourced Content", + display_name=_("Display Name"), + scope=Scope.content, + ) + source_block_id = String( + display_name=_("Library Block"), + help=_("Enter the IDs of the library XBlock that you wish to use."), + scope=Scope.content, + ) + editable_fields = ("display_name", "source_block_id") + has_children = True + has_author_view = True + resources_dir = 'assets/library_source_block' + + def __str__(self): + return "LibrarySourcedBlock: {}".format(self.display_name) + + def author_view(self, context): + """ + Renders the Studio preview view. + """ + fragment = Fragment() + root_xblock = context.get('root_xblock') + is_root = root_xblock and root_xblock.location == self.location # pylint: disable=no-member + # If block ID is not defined, ask user for the component ID in the author_view itself. + # We don't display the editor if is_root as that page should represent the student_view without any ambiguity + if not self.source_block_id and not is_root: + fragment.add_content( + loader.render_django_template('templates/library-sourced-block-author-view.html', { + 'save_url': handler_url(self, 'submit_studio_edits') + }) + ) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_source_block.js')) + fragment.initialize_js('LibrarySourceBlockAuthorView') + return fragment + + context = {} if not context else copy(context) # Isolate context - without this there are weird bugs in Studio + # EditableChildrenMixin.render_children will render HTML that allows instructors to make edits to the children + context['can_move'] = False + self.render_children(context, fragment, can_reorder=False, can_add=False) + return fragment + + def student_view(self, context): + """ + Renders the view that learners see. + """ + result = Fragment() + child_frags = self.runtime.render_children(self, context=context) + result.add_resources(child_frags) + result.add_content('
') + for frag in child_frags: + result.add_content(frag.content) + result.add_content('
') + return result + + def validate(self): + """ + Validates the state of this library_sourced_xblock Instance. This is the override of the general XBlock method, + and it will also ask its superclass to validate. + """ + validation = super().validate() + validation = StudioValidation.copy(validation) + + if not self.source_block_id: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.NOT_CONFIGURED, + _(u"No XBlock has been configured for this component. Enter the target ID below or in the editor"), + action_class='edit-button', + action_label=_(u"Open Editor") + ) + ) + return validation + + @XBlock.handler + def submit_studio_edits(self, data, suffix=''): + """ + Save changes to this block, applying edits made in Studio. + """ + response = super().submit_studio_edits(data, suffix) + # Replace our current children with the latest ones from the libraries. + lib_tools = self.runtime.service(self, 'library_tools') + try: + lib_tools.import_from_blockstore(self, self.source_block_id) + except Exception as err: # pylint: disable=broad-except + log.exception(err) + return Response(_(u"Importing Library Block failed - are the IDs valid and readable?"), status=400) + return response diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index cda300be0f..ef754be232 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -1,12 +1,19 @@ """ XBlock runtime services for LibraryContentModule """ - +import hashlib import six +from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied -from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator +from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator, LibraryUsageLocatorV2, BlockUsageLocator from search.search_engine_base import SearchEngine +from xblock.fields import Scope + +from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.xblock.api import load_block +from student.auth import has_studio_write_access from xmodule.capa_module import ProblemBlock from xmodule.library_content_module import ANY_CAPA_TYPE_VALUE from xmodule.modulestore import ModuleStoreEnum @@ -23,8 +30,9 @@ class LibraryToolsService(object): Service that allows LibraryContentModule to interact with libraries in the modulestore. """ - def __init__(self, modulestore): + def __init__(self, modulestore, user_id): self.store = modulestore + self.user_id = user_id def _get_library(self, library_key): """ @@ -127,7 +135,7 @@ class LibraryToolsService(object): """ return self.store.check_supports(block.location.course_key, 'copy_from_template') - def update_children(self, dest_block, user_id, user_perms=None, version=None): + def update_children(self, dest_block, user_perms=None, version=None): """ This method is to be used when the library that a LibraryContentModule references has been updated. It will re-fetch all matching blocks from @@ -163,11 +171,11 @@ class LibraryToolsService(object): source_blocks.extend(library.children) with self.store.bulk_operations(dest_block.location.course_key): - dest_block.source_library_version = six.text_type(library.location.library_key.version_guid) - self.store.update_item(dest_block, user_id) + dest_block.source_library_version = str(library.location.library_key.version_guid) + self.store.update_item(dest_block, self.user_id) head_validation = not version dest_block.children = self.store.copy_from_template( - source_blocks, dest_block.location, user_id, head_validation=head_validation + source_blocks, dest_block.location, self.user_id, head_validation=head_validation ) # ^-- copy_from_template updates the children in the DB # but we must also set .children here to avoid overwriting the DB again @@ -181,3 +189,101 @@ class LibraryToolsService(object): (lib.location.library_key.replace(version_guid=None, branch=None), lib.display_name) for lib in self.store.get_library_summaries() ] + + def import_from_blockstore(self, dest_block, blockstore_block_id): + """ + Imports a block from a blockstore-based learning context (usually a + content library) into modulestore, as a new child of dest_block. + Any existing children of dest_block are replaced. + + This is only used by LibrarySourcedBlock. It should verify first that + the number of block IDs is reasonable. + """ + dest_key = dest_block.scope_ids.usage_id + if not isinstance(dest_key, BlockUsageLocator): + raise TypeError("Destination {} should be a modulestore course.".format(dest_key)) + if self.user_id is None: + raise ValueError("Cannot check user permissions - LibraryTools user_id is None") + + dest_course_key = dest_key.context_key + user = User.objects.get(id=self.user_id) + if not has_studio_write_access(user, dest_course_key): + raise PermissionDenied() + + # Read the source block; this will also confirm that user has permission to read it. + orig_block = load_block(UsageKey.from_string(blockstore_block_id), user) + + with self.store.bulk_operations(dest_course_key): + new_block_id = self._import_block(orig_block, dest_key) + # Remove any existing children that are no longer used + for old_child_id in set(dest_block.children) - set([new_block_id]): + self.store.delete_item(old_child_id, self.user_id) + # If this was called from a handler, it will save dest_block at the end, so we must update + # dest_block.children to avoid it saving the old value of children and deleting the new ones. + dest_block.children = self.store.get_item(dest_key).children + + def _import_block(self, source_block, dest_parent_key): + """ + Recursively import a blockstore block and its children. See import_from_blockstore above. + """ + def generate_block_key(source_key, dest_parent_key): + """ + Deterministically generate an ID for the new block and return the key + """ + block_id = ( + dest_parent_key.block_id[:10] + + '-' + + hashlib.sha1(str(source_key).encode('utf-8')).hexdigest()[:10] + ) + return dest_parent_key.context_key.make_usage_key(source_key.block_type, block_id) + + source_key = source_block.scope_ids.usage_id + new_block_key = generate_block_key(source_key, dest_parent_key) + try: + new_block = self.store.get_item(new_block_key) + if new_block.parent != dest_parent_key: + raise ValueError( + "Expected existing block {} to be a child of {} but instead it's a child of {}".format( + new_block_key, dest_parent_key, new_block.parent, + ) + ) + except ItemNotFoundError: + new_block = self.store.create_child( + user_id=self.user_id, + parent_usage_key=dest_parent_key, + block_type=source_key.block_type, + block_id=new_block_key.block_id, + ) + + # Prepare a list of this block's static assets; any assets that are referenced as /static/{path} (the + # recommended way for referencing them) will stop working, and so we rewrite the url when importing. + # Copying assets not advised because modulestore doesn't namespace assets to each block like blockstore, which + # might cause conflicts when the same filename is used across imported blocks. + if isinstance(source_key, LibraryUsageLocatorV2): + all_assets = library_api.get_library_block_static_asset_files(source_key) + else: + all_assets = [] + + for field_name, field in source_block.fields.items(): + if field.scope not in (Scope.settings, Scope.content): + continue # Only copy authored field data + if field.is_set_on(source_block) or field.is_set_on(new_block): + field_value = getattr(source_block, field_name) + if isinstance(field_value, str): + # If string field (which may also be JSON/XML data), rewrite /static/... URLs to point to blockstore + for asset in all_assets: + field_value = field_value.replace('/static/{}'.format(asset.path), asset.url) + setattr(new_block, field_name, field_value) + new_block.save() + self.store.update_item(new_block, self.user_id) + + if new_block.has_children: + # Delete existing children in the new block, which can be reimported again if they still exist in the + # source library + for existing_child_key in new_block.children: + self.store.delete_item(existing_child_key, self.user_id) + # Now import the children + for child in source_block.get_children(): + self._import_block(child, new_block_key) + + return new_block_key diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 5f43fef5eb..e0a06cb082 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -85,7 +85,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): self.module_data = module_data self.default_class = default_class self.local_modules = {} - self._services['library_tools'] = LibraryToolsService(modulestore) + self._services['library_tools'] = LibraryToolsService(modulestore, user_id=None) @lazy @contract(returns="dict(BlockKey: BlockKey)") diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index ee7864e9d3..c52ef40b06 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -826,13 +826,11 @@ def _update_and_import_module( # if library exists, update source_library_version and children # according to this existing library and library content block. if store.get_library(block.source_library_key): - # Update library content block's children on draft branch with store.branch_setting(branch_setting=ModuleStoreEnum.Branch.draft_preferred): - LibraryToolsService(store).update_children( + LibraryToolsService(store, user_id).update_children( block, - user_id, - version=block.source_library_version + version=block.source_library_version, ) # Publish it if importing the course for branch setting published_only. diff --git a/common/lib/xmodule/xmodule/templates/library-sourced-block-author-view.html b/common/lib/xmodule/xmodule/templates/library-sourced-block-author-view.html new file mode 100644 index 0000000000..003d3b6181 --- /dev/null +++ b/common/lib/xmodule/xmodule/templates/library-sourced-block-author-view.html @@ -0,0 +1,9 @@ +
+

+ To display a component from a content library here, enter the component ID (XBlock ID) that you want to use: +

+
+ + +
+
\ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/test_library_content.py b/common/lib/xmodule/xmodule/tests/test_library_content.py index cfe4ce2b5d..d885d9129f 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_content.py +++ b/common/lib/xmodule/xmodule/tests/test_library_content.py @@ -33,7 +33,7 @@ class LibraryContentTest(MixedSplitTestCase): def setUp(self): super(LibraryContentTest, self).setUp() - self.tools = LibraryToolsService(self.store) + self.tools = LibraryToolsService(self.store, self.user_id) self.library = LibraryFactory.create(modulestore=self.store) self.lib_blocks = [ self.make_block("html", self.library, data="Hello world from block {}".format(i)) diff --git a/common/lib/xmodule/xmodule/tests/test_library_sourced_block.py b/common/lib/xmodule/xmodule/tests/test_library_sourced_block.py new file mode 100644 index 0000000000..c3b58a726d --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_library_sourced_block.py @@ -0,0 +1,76 @@ +""" +Tests for Source from Library XBlock +""" +from xblockutils.resources import ResourceLoader + +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest +from student.roles import CourseInstructorRole +from cms.lib.xblock.runtime import handler_url +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.tests import get_test_system +from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW + + +class LibrarySourcedBlockTestCase(ContentLibrariesRestApiTest): + """ + Tests for LibraryToolsService which interact with blockstore-based content libraries + """ + def setUp(self): + super().setUp() + self.store = modulestore() + + def test_block_views(self): + # Create a blockstore content library + library = self._create_library(slug="testlib1_preview", title="Test Library 1", description="Testing XBlocks") + # Add content to the library + html_block_id = self._add_block_to_library(library["id"], "html", "html_student_preview")["id"] + self._set_library_block_olx(html_block_id, 'Student Preview Test') + + # Create a modulestore course + course = CourseFactory.create(modulestore=self.store, user_id=self.user.id) + CourseInstructorRole(course.id).add_users(self.user) + # Add a "Source from Library" block to the course + source_block = ItemFactory.create( + category="library_sourced", + parent=course, + parent_location=course.location, + user_id=self.user.id, + modulestore=self.store + ) + + # Check if author_view for empty block renders using the editor template + html = source_block.render(AUTHOR_VIEW).content + loader = ResourceLoader('xmodule.library_sourced_block') + expected_html = loader.render_django_template('templates/library-sourced-block-author-view.html', { + 'save_url': handler_url(source_block, 'submit_studio_edits') + }) + self.assertEqual(expected_html, html) + + submit_studio_edits_url = '/xblock/{0}/handler/submit_studio_edits'.format(source_block.scope_ids.usage_id) + post_data = {"values": {"source_block_id": html_block_id}, "defaults": ["display_name"]} + # Import the html block from the library to the course + self.client.post(submit_studio_edits_url, data=post_data, format='json') + + # Check if author_view for a configured block renders the children correctly + # Use self.get_block_view for rendering these as mako templates are mocked to return repr of the template + # instead of the rendered html + res = self.get_block_view(source_block, AUTHOR_VIEW) + self.assertNotIn('library-sourced-block-author-view.html', res) + self.assertIn('studio_render_children_view.html', res) + self.assertIn('Student Preview Test', res) + + # Check if student_view renders the children correctly + res = self.get_block_view(source_block, STUDENT_VIEW) + self.assertIn('Student Preview Test', res) + + def get_block_view(self, block, view, context=None): + """ + Renders the specified view for a given XBlock + """ + context = context or {} + block = self.store.get_item(block.location) + module_system = get_test_system(block) + module_system.descriptor_runtime = block._runtime # pylint: disable=protected-access + block.bind_for_student(module_system, self.user.id) + return module_system.render(block, view, context).content diff --git a/common/lib/xmodule/xmodule/tests/test_library_tools.py b/common/lib/xmodule/xmodule/tests/test_library_tools.py index e9f3c925b0..188dbf180b 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_tools.py +++ b/common/lib/xmodule/xmodule/tests/test_library_tools.py @@ -1,12 +1,15 @@ """ Tests for library tools service. """ +from unittest.mock import patch - -from mock import patch - +from opaque_keys.edx.keys import UsageKey +from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest +from openedx.core.djangoapps.xblock.api import load_block +from student.roles import CourseInstructorRole from xmodule.library_tools import LibraryToolsService -from xmodule.modulestore.tests.factories import LibraryFactory +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase @@ -16,9 +19,8 @@ class LibraryToolsServiceTest(MixedSplitTestCase): """ def setUp(self): - super(LibraryToolsServiceTest, self).setUp() - - self.tools = LibraryToolsService(self.store) + super().setUp() + self.tools = LibraryToolsService(self.store, self.user_id) def test_list_available_libraries(self): """ @@ -36,3 +38,59 @@ class LibraryToolsServiceTest(MixedSplitTestCase): """ _ = self.tools.list_available_libraries() self.assertTrue(mock_get_library_summaries.called) + + +class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest): + """ + Tests for LibraryToolsService which interact with blockstore-based content libraries + """ + def setUp(self): + super().setUp() + self.tools = LibraryToolsService(self.store, self.user.id) + + def test_import_from_blockstore(self): + # Create a blockstore content library + library = self._create_library(slug="testlib1_import", title="A Test Library", description="Testing XBlocks") + # Create a unit block with an HTML block in it. + unit_block_id = self._add_block_to_library(library["id"], "unit", "unit1")["id"] + html_block_id = self._add_block_to_library(library["id"], "html", "html1", parent_block=unit_block_id)["id"] + html_block = load_block(UsageKey.from_string(html_block_id), self.user) + # Add assets and content to the HTML block + self._set_library_block_asset(html_block_id, "test.txt", b"data", expect_response=200) + self._set_library_block_olx(html_block_id, 'Hello world') + + # Create a modulestore course + course = CourseFactory.create(modulestore=self.store, user_id=self.user.id) + CourseInstructorRole(course.id).add_users(self.user) + # Add Source from library block to the course + sourced_block = self.make_block("library_sourced", course, user_id=self.user.id) + + # Import the unit block from the library to the course + self.tools.import_from_blockstore(sourced_block, unit_block_id) + + # Verify imported block with its children + self.assertEqual(len(sourced_block.children), 1) + self.assertEqual(sourced_block.children[0].category, 'unit') + + imported_unit_block = self.store.get_item(sourced_block.children[0]) + self.assertEqual(len(imported_unit_block.children), 1) + self.assertEqual(imported_unit_block.children[0].category, 'html') + + imported_html_block = self.store.get_item(imported_unit_block.children[0]) + self.assertIn('Hello world', imported_html_block.data) + + # Check that assets were imported and static paths were modified after importing + assets = library_api.get_library_block_static_asset_files(html_block.scope_ids.usage_id) + self.assertEqual(len(assets), 1) + self.assertIn(assets[0].url, imported_html_block.data) + + # Check that reimporting updates the target block + self._set_library_block_olx(html_block_id, 'Foo bar') + self.tools.import_from_blockstore(sourced_block, unit_block_id) + + self.assertEqual(len(sourced_block.children), 1) + imported_unit_block = self.store.get_item(sourced_block.children[0]) + self.assertEqual(len(imported_unit_block.children), 1) + imported_html_block = self.store.get_item(imported_unit_block.children[0]) + self.assertNotIn('Hello world', imported_html_block.data) + self.assertIn('Foo bar', imported_html_block.data) diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 91389800c2..d1f1b48a98 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -152,7 +152,7 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method services['completion'] = CompletionService(user=user, context_key=kwargs.get('course_id')) services['fs'] = xblock.reference.plugins.FSService() services['i18n'] = ModuleI18nService - services['library_tools'] = LibraryToolsService(store) + services['library_tools'] = LibraryToolsService(store, user_id=user.id if user else None) services['partitions'] = PartitionService( course_id=kwargs.get('course_id'), cache=request_cache_dict diff --git a/openedx/core/djangoapps/xblock/learning_context/manager.py b/openedx/core/djangoapps/xblock/learning_context/manager.py index 03a0837463..e1d687e72d 100644 --- a/openedx/core/djangoapps/xblock/learning_context/manager.py +++ b/openedx/core/djangoapps/xblock/learning_context/manager.py @@ -1,8 +1,7 @@ """ Helper methods for working with learning contexts """ - - +from opaque_keys import OpaqueKey from opaque_keys.edx.keys import LearningContextKey, UsageKeyV2 from openedx.core.djangoapps.xblock.apps import get_xblock_app_config @@ -38,9 +37,11 @@ def get_learning_context_impl(key): context_type = key.CANONICAL_NAMESPACE # e.g. 'lib' elif isinstance(key, UsageKeyV2): context_type = key.context_key.CANONICAL_NAMESPACE - else: + elif isinstance(key, OpaqueKey): # Maybe this is an older modulestore key etc. raise TypeError("Opaque key {} does not have a learning context.".format(key)) + else: + raise TypeError("key '{}' is not an opaque key. You probably forgot [KeyType].from_string(...)".format(key)) try: return _learning_context_cache[context_type] diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index 8e15f64f27..6dfa658afb 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -124,7 +124,7 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest """ module_system = get_test_system(course_id=module.location.course_key) module_system.descriptor_runtime = module.runtime._descriptor_system # pylint: disable=protected-access - module_system._services['library_tools'] = LibraryToolsService(self.store) # pylint: disable=protected-access + module_system._services['library_tools'] = LibraryToolsService(self.store, self.user.id) # pylint: disable=protected-access def get_module(descriptor): """Mocks module_system get_module function""" diff --git a/openedx/tests/settings.py b/openedx/tests/settings.py index 6907fb4da3..a976ce4d2d 100644 --- a/openedx/tests/settings.py +++ b/openedx/tests/settings.py @@ -80,6 +80,7 @@ INSTALLED_APPS = ( 'openedx.core.djangoapps.user_api', 'course_modes.apps.CourseModesConfig', 'lms.djangoapps.verify_student.apps.VerifyStudentConfig', + 'openedx.core.djangoapps.content_libraries', 'openedx.core.djangoapps.dark_lang', 'openedx.core.djangoapps.content.course_overviews.apps.CourseOverviewsConfig', 'openedx.core.djangoapps.content.block_structure.apps.BlockStructureConfig', @@ -103,6 +104,7 @@ INSTALLED_APPS = ( # Django 1.11 demands to have imported models supported by installed apps. 'completion', 'entitlements', + 'organizations', ) LMS_ROOT_URL = "http://localhost:8000" @@ -127,6 +129,8 @@ RETIRED_USERNAME_PREFIX = 'retired__user_' PROCTORING_SETTINGS = {} +ROOT_URLCONF = None +RUN_BLOCKSTORE_TESTS = False # Software Secure request retry settings # Time in seconds before a retry of the task should be 60 mints.