Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: implement V2 libraries usage for library content block (#30615)

Browse Source 
YT: https://youtrack.raccoongang.com/issue/EDX_BLND_CLI-87

- V2 libraries are available for selection in the Random Block edit modal;
- selected V2 library blocks are copied to the modulestore and saved as children of the Random Block;
- V2 library version validation works the same as for the V1 libraries (with possibility to update block with the latest version);
- filtering by problem type can't be done for V2 the same as for V1 because the v2 library problems are not divided by types;
- the problem type field is hidden for v2 libraries in the edit mode;
- unit tests added/updated.
This commit is contained in:
Eugene Dyudyunov
2022-08-16 15:25:26 +03:00
committed by GitHub
parent f6e886b027
commit dcf1cc14b6
6 changed files with 351 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
xmodule/assets/library_content/public/js/library_content_edit_helpers.js Normal file
View File

@@ -0,0 +1,54 @@
/* JavaScript for special editing operations that can be done on LibraryContentXBlock */
// This is a temporary UI improvements that will be removed when V2 content libraries became
// fully functional
/**
* Toggle the "Problem Type" settings section depending on selected library type.
* As for now, the V2 libraries don't support different problem types, so they can't be
* filtered by it. We're hiding the Problem Type field for them.
*/
function checkProblemTypeShouldBeVisible(editor) {
var libraries = editor.find('.wrapper-comp-settings.metadata_edit.is-active')
.data().metadata.source_library_id.options;
var selectedIndex = $("select[name='Library']", editor)[0].selectedIndex;
var libraryKey = libraries[selectedIndex].value;
var url = URI('/xblock')
.segment(editor.find('.xblock.xblock-studio_view.xblock-studio_view-library_content.xblock-initialized')
.data('usage-id'))
.segment('handler')
.segment('is_v2_library');
$.ajax({
type: 'POST',
url: url,
data: JSON.stringify({'library_key': libraryKey}),
success: function(data) {
var problemTypeSelect = editor.find("select[name='Problem Type']")
.parents("li.field.comp-setting-entry.metadata_entry");
data.is_v2 ? problemTypeSelect.hide() : problemTypeSelect.show();
}
});
}
/**
* Waits untill editor html loaded, than calls checks for Program Type field toggling.
*/
function waitForEditorLoading() {
var checkContent = setInterval(function() {
var $modal = $('.xblock-editor');
var content = $modal.html();
if (content) {
clearInterval(checkContent);
checkProblemTypeShouldBeVisible($modal);
}
}, 10);
}
// Initial call
waitForEditorLoading();
var $librarySelect = $("select[name='Library']");
$(document).on('change', $librarySelect, waitForEditorLoading)
var $libraryContentEditors = $('.xblock-header.xblock-header-library_content');
var $editBtns = $libraryContentEditors.find('.action-item.action-edit');
$(document).on('click', $editBtns, waitForEditorLoading)

38
xmodule/library_content_module.py
View File

@@ -8,7 +8,6 @@ import logging
import random
from copy import copy
from gettext import ngettext
from rest_framework import status
import bleach
from django.conf import settings
@@ -16,8 +15,10 @@ from django.utils.functional import classproperty
from lazy import lazy
from lxml import etree
from lxml.etree import XMLSyntaxError
from opaque_keys.edx.locator import LibraryLocator
from opaque_keys import InvalidKeyError
from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
from pkg_resources import resource_string
from rest_framework import status
from web_fragments.fragment import Fragment
from webob import Response
from xblock.completable import XBlockCompletionMode
@@ -29,16 +30,15 @@ from xmodule.mako_module import MakoTemplateBlockBase
from xmodule.studio_editable import StudioEditableBlock
from xmodule.util.xmodule_django import add_webpack_to_fragment
from xmodule.validation import StudioValidation, StudioValidationMessage
from xmodule.xml_module import XmlMixin
from xmodule.x_module import (
STUDENT_VIEW,
HTMLSnippet,
ResourceTemplates,
shim_xmodule_js,
STUDENT_VIEW,
XModuleMixin,
XModuleToXBlockMixin,
shim_xmodule_js,
)
from xmodule.xml_module import XmlMixin
# 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
@@ -189,9 +189,14 @@ class LibraryContentBlock(
@property
def source_library_key(self):
"""
Convenience method to get the library ID as a LibraryLocator and not just a string
Convenience method to get the library ID as a LibraryLocator and not just a string.
Supports either library v1 or library v2 locators.
"""
return LibraryLocator.from_string(self.source_library_id)
try:
return LibraryLocator.from_string(self.source_library_id)
except InvalidKeyError:
return LibraryLocatorV2.from_string(self.source_library_id)
@classmethod
def make_selection(cls, selected, children, max_count, mode):
@@ -456,6 +461,7 @@ class LibraryContentBlock(
fragment = Fragment(
self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context())
)
fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_edit_helpers.js'))
add_webpack_to_fragment(fragment, 'LibraryContentBlockStudio')
shim_xmodule_js(fragment, self.studio_js_module_name)
return fragment
@@ -517,6 +523,22 @@ class LibraryContentBlock(
self.tools.update_children(self, user_perms)
return Response()
@XBlock.json_handler
def is_v2_library(self, data, suffix=''): # lint-amnesty, pylint: disable=unused-argument
"""
Check the library version by library_id.
This is a temporary handler needed for hiding the Problem Type xblock editor field for V2 libraries.
"""
lib_key = data.get('library_key')
try:
LibraryLocatorV2.from_string(lib_key)
except InvalidKeyError:
is_v2 = False
else:
is_v2 = True
return {'is_v2': is_v2}
# Copy over any overridden settings the course author may have applied to the blocks.
def _copy_overrides(self, store, user_id, source, dest):
"""

160
xmodule/library_tools.py
View File

@@ -5,20 +5,28 @@ import hashlib
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.core.exceptions import PermissionDenied
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator, LibraryUsageLocatorV2, BlockUsageLocator
from opaque_keys.edx.locator import (
BlockUsageLocator,
LibraryLocator,
LibraryLocatorV2,
LibraryUsageLocator,
LibraryUsageLocatorV2,
)
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 openedx.core.lib import blockstore_api
from common.djangoapps.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
from xmodule.modulestore.exceptions import ItemNotFoundError
from common.djangoapps.student.auth import has_studio_write_access
from openedx.core.djangoapps.content_libraries import api as library_api
from openedx.core.djangoapps.content_libraries.models import ContentLibrary
from openedx.core.djangoapps.xblock.api import load_block
from openedx.core.lib import blockstore_api
def normalize_key_for_search(library_key):
""" Normalizes library key for use with search indexing """
@@ -27,39 +35,63 @@ def normalize_key_for_search(library_key):
class LibraryToolsService:
"""
Service that allows LibraryContentBlock to interact with libraries in the
modulestore.
Service for LibraryContentBlock and LibrarySourcedBlock.
Allows to interact with libraries in the modulestore and blockstore.
"""
def __init__(self, modulestore, user_id):
self.store = modulestore
self.user_id = user_id
def _get_library(self, library_key):
def _get_library(self, library_key, is_v2_lib):
"""
Given a library key like "library-v1:ProblemX+PR0B", return the
'library' XBlock with meta-information about the library.
Helper method to get either V1 or V2 library.
A specific version may be specified.
Given a library key like "library-v1:ProblemX+PR0B" (V1) or "lib:RG:rg-1" (v2), return the 'library'.
is_v2_lib (bool) indicates which library storage should be requested:
True - blockstore (V2 library);
False - modulestore (V1 library).
Returns None on error.
"""
if not isinstance(library_key, LibraryLocator):
library_key = LibraryLocator.from_string(library_key)
try:
return self.store.get_library(
library_key, remove_version=False, remove_branch=False, head_validation=False
)
except ItemNotFoundError:
return None
if is_v2_lib:
try:
return library_api.get_library(library_key)
except ContentLibrary.DoesNotExist:
return None
else:
try:
return self.store.get_library(
library_key, remove_version=False, remove_branch=False, head_validation=False
)
except ItemNotFoundError:
return None
def get_library_version(self, lib_key):
"""
Get the version (an ObjectID) of the given library.
Returns None if the library does not exist.
Get the version of the given library.
The return value (library version) could be:
ObjectID - for V1 library;
int - for V2 library.
None - if the library does not exist.
"""
library = self._get_library(lib_key)
if not isinstance(lib_key, (LibraryLocator, LibraryLocatorV2)):
try:
lib_key = LibraryLocator.from_string(lib_key)
is_v2_lib = False
except InvalidKeyError:
lib_key = LibraryLocatorV2.from_string(lib_key)
is_v2_lib = True
else:
is_v2_lib = isinstance(lib_key, LibraryLocatorV2)
library = self._get_library(lib_key, is_v2_lib)
if library:
if is_v2_lib:
return library.version
# We need to know the library's version so ensure it's set in library.location.library_key.version_guid
assert library.location.library_key.version_guid is not None
return library.location.library_key.version_guid
@@ -137,11 +169,13 @@ class LibraryToolsService:
def update_children(self, dest_block, user_perms=None, version=None):
"""
This method is to be used when the library that a LibraryContentBlock
references has been updated. It will re-fetch all matching blocks from
the libraries, and copy them as children of dest_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.
Update xBlock's children.
Re-fetch all matching blocks from the libraries, and copy them as children of dest_block.
The children will be given new block_ids.
NOTE: V1 libraies blocks definition ID should be the
exact same definition ID used in the copy block.
This method will update dest_block's 'source_library_version' field to
store the version number of the libraries used, so we easily determine
@@ -155,49 +189,69 @@ class LibraryToolsService:
return
source_blocks = []
library_key = dest_block.source_library_key
if version:
is_v2_lib = isinstance(library_key, LibraryLocatorV2)
if version and not is_v2_lib:
library_key = library_key.replace(branch=ModuleStoreEnum.BranchName.library, version_guid=version)
library = self._get_library(library_key)
library = self._get_library(library_key, is_v2_lib)
if library is None:
raise ValueError(f"Requested library {library_key} not found.")
if user_perms and not user_perms.can_read(library_key):
raise PermissionDenied()
filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE)
if filter_children:
# Apply simple filtering based on CAPA problem types:
source_blocks.extend(self._problem_type_filter(library, dest_block.capa_type))
else:
source_blocks.extend(library.children)
with self.store.bulk_operations(dest_block.location.course_key):
dest_block.source_library_version = str(library.location.library_key.version_guid)
filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE)
if not is_v2_lib:
if user_perms and not user_perms.can_read(library_key):
raise PermissionDenied()
if filter_children:
# Apply simple filtering based on CAPA problem types:
source_blocks.extend(self._problem_type_filter(library, dest_block.capa_type))
else:
source_blocks.extend(library.children)
with self.store.bulk_operations(dest_block.location.course_key):
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, 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
else:
# TODO: add filtering by capa_type when V2 library will support different problem types
source_blocks = library_api.get_library_blocks(library_key, block_types=None)
source_block_ids = [str(block.usage_key) for block in source_blocks]
dest_block.source_library_version = str(library.version)
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, 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
self.import_from_blockstore(dest_block, source_block_ids)
def list_available_libraries(self):
"""
List all known libraries.
Returns tuples of (LibraryLocator, display_name)
Collects V1 libraries along with V2.
Returns tuples of (library key, display_name).
"""
return [
user = User.objects.get(id=self.user_id)
v1_libs = [
(lib.location.library_key.replace(version_guid=None, branch=None), lib.display_name)
for lib in self.store.get_library_summaries()
]
v2_query = library_api.get_libraries_for_user(user)
v2_libs_with_meta = library_api.get_metadata_from_index(v2_query)
v2_libs = [(lib.key, lib.title) for lib in v2_libs_with_meta]
return v1_libs + v2_libs
def import_from_blockstore(self, dest_block, blockstore_block_ids):
"""
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):
@@ -254,7 +308,7 @@ class LibraryToolsService:
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:
if new_block.parent.block_id != dest_parent_key.block_id:
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,

7
xmodule/modulestore/split_mongo/caching_descriptor_system.py
View File

@@ -3,12 +3,12 @@
import logging
import sys
from crum import get_current_user
from fs.osfs import OSFS
from lazy import lazy
from opaque_keys.edx.locator import BlockUsageLocator, DefinitionLocator, LocalId
from xblock.fields import ScopeIds
from xblock.runtime import KeyValueStore, KvsFieldData
from xmodule.error_module import ErrorBlock
from xmodule.errortracker import exc_info_to_str
from xmodule.library_tools import LibraryToolsService
@@ -74,7 +74,10 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li
self.module_data = module_data
self.default_class = default_class
self.local_modules = {}
self._services['library_tools'] = LibraryToolsService(modulestore, user_id=None)
user = get_current_user()
user_id = user.id if user else None
self._services['library_tools'] = LibraryToolsService(modulestore, user_id=user_id)
@lazy
def _parent_map(self): # lint-amnesty, pylint: disable=missing-function-docstring

33
xmodule/tests/test_library_content.py
View File

@@ -4,16 +4,18 @@ Basic unit tests for LibraryContentBlock
Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`.
"""
from unittest.mock import MagicMock, Mock, patch
import ddt
import ddt
from bson.objectid import ObjectId
from fs.memoryfs import MemoryFS
from lxml import etree
from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
from rest_framework import status
from search.search_engine_base import SearchEngine
from web_fragments.fragment import Fragment
from xblock.runtime import Runtime as VanillaRuntime
from rest_framework import status
from xmodule.capa_module import ProblemBlock
from xmodule.library_content_module import ANY_CAPA_TYPE_VALUE, LibraryContentBlock
from xmodule.library_tools import LibraryToolsService
from xmodule.modulestore import ModuleStoreEnum
@@ -22,7 +24,6 @@ from xmodule.modulestore.tests.utils import MixedSplitTestCase
from xmodule.tests import get_test_system
from xmodule.validation import StudioValidationMessage
from xmodule.x_module import AUTHOR_VIEW
from xmodule.capa_module import ProblemBlock
from .test_course_module import DummySystem as TestImportSystem
@@ -74,6 +75,32 @@ class LibraryContentTest(MixedSplitTestCase):
module.xmodule_runtime = module_system
@ddt.ddt
class LibraryContentGeneralTest(LibraryContentTest):
"""
Test the base functionality of the LibraryContentBlock.
"""
@ddt.data(
('library-v1:ProblemX+PR0B', LibraryLocator),
('lib:ORG:test-1', LibraryLocatorV2)
)
@ddt.unpack
def test_source_library_key(self, library_key, expected_locator_type):
"""
Test the source_library_key property of the xblock.
The method should correctly work either with V1 or V2 libraries.
"""
library = self.make_block(
"library_content",
self.vertical,
max_count=1,
source_library_id=library_key
)
assert isinstance(library.source_library_key, expected_locator_type)
class TestLibraryContentExportImport(LibraryContentTest):
"""
Export and import tests for LibraryContentBlock

143
xmodule/tests/test_library_tools.py
View File

@@ -1,35 +1,49 @@
"""
Tests for library tools service.
"""
from unittest.mock import patch
import ddt
from bson.objectid import ObjectId
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 common.djangoapps.student.roles import CourseInstructorRole
from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
from xmodule.library_tools import LibraryToolsService
from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory
from xmodule.modulestore.tests.utils import MixedSplitTestCase
from common.djangoapps.student.roles import CourseInstructorRole
from common.djangoapps.student.tests.factories import UserFactory
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
class LibraryToolsServiceTest(MixedSplitTestCase):
"""
Tests for library service.
"""
@ddt.ddt
class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest):
"""
Tests for LibraryToolsService.
Tests interaction with blockstore-based (V2) and mongo-based (V1) content libraries.
"""
def setUp(self):
super().setUp()
UserFactory(is_staff=True, id=self.user_id)
self.tools = LibraryToolsService(self.store, self.user_id)
def test_list_available_libraries(self):
"""
Test listing of libraries.
Should include either V1 or V2 libraries.
"""
# create V1 library
_ = LibraryFactory.create(modulestore=self.store)
# create V2 library
self._create_library(slug="testlib1_preview", title="Test Library 1", description="Testing XBlocks")
all_libraries = self.tools.list_available_libraries()
assert all_libraries
assert len(all_libraries) == 1
assert len(all_libraries) == 2
@patch('xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.get_library_summaries')
def test_list_available_libraries_fetch(self, mock_get_library_summaries):
@@ -39,15 +53,6 @@ class LibraryToolsServiceTest(MixedSplitTestCase):
_ = self.tools.list_available_libraries()
assert 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")
@@ -94,3 +99,105 @@ class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest):
imported_html_block = self.store.get_item(imported_unit_block.children[0])
assert 'Hello world' not in imported_html_block.data
assert 'Foo bar' in imported_html_block.data
def test_get_v2_library_version(self):
"""
Test get_library_version for V2 libraries.
Covers getting results for either library key as a string or LibraryLocatorV2.
NOTE:
We don't publish library updates so the library version will always be 0.
"""
lib = self._create_library(slug="testlib1-slug", title="Test Library 1", description="Testing Library 1")
# use library key as a string for getting the library version
result = self.tools.get_library_version(lib['id'])
assert isinstance(result, int)
assert result == 0
# now do the same but use library key as a LibraryLocatorV2
result2 = self.tools.get_library_version(LibraryLocatorV2.from_string(lib['id']))
assert isinstance(result, int)
assert result2 == 0
def test_get_v1_library_version(self):
"""
Test get_library_version for V1 libraries.
Covers getting results for either string library key or LibraryLocator.
"""
lib_key = LibraryFactory.create(modulestore=self.store).location.library_key
# Re-load the library from the modulestore, explicitly including version information:
lib = self.store.get_library(lib_key, remove_version=False, remove_branch=False)
# check the result using the LibraryLocator
assert isinstance(lib_key, LibraryLocator)
result = self.tools.get_library_version(lib_key)
assert result
assert isinstance(result, ObjectId)
assert result == lib.location.library_key.version_guid
# the same check for string representation of the LibraryLocator
str_key = str(lib_key)
result = self.tools.get_library_version(str_key)
assert result
assert isinstance(result, ObjectId)
assert result == lib.location.library_key.version_guid
@ddt.data(
'library-v1:Fake+Key', # V1 library key
'lib:Fake:V-2', # V2 library key
LibraryLocator.from_string('library-v1:Fake+Key'),
LibraryLocatorV2.from_string('lib:Fake:V-2'),
)
def test_get_library_version_no_library(self, lib_key):
"""
Test get_library_version result when the library does not exist.
Provided lib_key's are valid V1 or V2 keys.
"""
assert self.tools.get_library_version(lib_key) is None
def test_update_children_for_v2_lib(self):
"""
Test update_children with V2 library as a source.
As for now, covers usage of update_children for the library content module only.
"""
library = self._create_library(
slug="cool-v2-lib", title="The best Library", description="Spectacular description"
)
self._add_block_to_library(library["id"], "unit", "unit1_id")["id"] # pylint: disable=expression-not-assigned
course = CourseFactory.create(modulestore=self.store, user_id=self.user.id)
CourseInstructorRole(course.id).add_users(self.user)
content_block = self.make_block(
"library_content",
course,
max_count=1,
source_library_id=library['id']
)
assert len(content_block.children) == 0
self.tools.update_children(content_block)
content_block = self.store.get_item(content_block.location)
assert len(content_block.children) == 1
def test_update_children_for_v1_lib(self):
"""
Test update_children with V1 library as a source.
As for now, covers usage of update_children for the library content module only.
"""
library = LibraryFactory.create(modulestore=self.store)
self.make_block("html", library, data="Hello world from the block")
course = CourseFactory.create(modulestore=self.store)
content_block = self.make_block(
"library_content",
course,
max_count=1,
source_library_id=str(library.location.library_key)
)
assert len(content_block.children) == 0
self.tools.update_children(content_block)
content_block = self.store.get_item(content_block.location)
assert len(content_block.children) == 1