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

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

Browse Source 
This reverts commit dcf1cc14b6.
This commit is contained in:
connorhaugh
2022-08-17 14:43:31 -04:00
committed by GitHub
parent 3946503960
commit dd1367823d
6 changed files with 82 additions and 349 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
View File

@@ -1,54 +0,0 @@
/* 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,6 +8,7 @@ import logging
import random
from copy import copy
from gettext import ngettext
from rest_framework import status
import bleach
from django.conf import settings
@@ -15,10 +16,8 @@ from django.utils.functional import classproperty
from lazy import lazy
from lxml import etree
from lxml.etree import XMLSyntaxError
from opaque_keys import InvalidKeyError
from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
from opaque_keys.edx.locator import LibraryLocator
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
@@ -30,15 +29,16 @@ 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,14 +189,9 @@ class LibraryContentBlock(
@property
def source_library_key(self):
"""
Convenience method to get the library ID as a LibraryLocator and not just a string.
Supports either library v1 or library v2 locators.
Convenience method to get the library ID as a LibraryLocator and not just a string
"""
try:
return LibraryLocator.from_string(self.source_library_id)
except InvalidKeyError:
return LibraryLocatorV2.from_string(self.source_library_id)
return LibraryLocator.from_string(self.source_library_id)
@classmethod
def make_selection(cls, selected, children, max_count, mode):
@@ -461,7 +456,6 @@ 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
@@ -523,22 +517,6 @@ 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):
"""

158
xmodule/library_tools.py
View File

@@ -5,28 +5,20 @@ 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 (
BlockUsageLocator,
LibraryLocator,
LibraryLocatorV2,
LibraryUsageLocator,
LibraryUsageLocatorV2,
)
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 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 """
@@ -35,63 +27,39 @@ def normalize_key_for_search(library_key):
class LibraryToolsService:
"""
Service for LibraryContentBlock and LibrarySourcedBlock.
Allows to interact with libraries in the modulestore and blockstore.
Service that allows LibraryContentBlock to interact with libraries in the
modulestore.
"""
def __init__(self, modulestore, user_id):
self.store = modulestore
self.user_id = user_id
def _get_library(self, library_key, is_v2_lib):
def _get_library(self, library_key):
"""
Helper method to get either V1 or V2 library.
Given a library key like "library-v1:ProblemX+PR0B", return the
'library' XBlock with meta-information about the library.
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).
A specific version may be specified.
Returns None on error.
"""
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
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
def get_library_version(self, lib_key):
"""
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.
Get the version (an ObjectID) of the given library.
Returns None if the library does not exist.
"""
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)
library = self._get_library(lib_key)
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
@@ -169,13 +137,11 @@ class LibraryToolsService:
def update_children(self, dest_block, user_perms=None, version=None):
"""
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 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.
This method will update dest_block's 'source_library_version' field to
store the version number of the libraries used, so we easily determine
@@ -189,69 +155,49 @@ class LibraryToolsService:
return
source_blocks = []
library_key = dest_block.source_library_key
is_v2_lib = isinstance(library_key, LibraryLocatorV2)
if version and not is_v2_lib:
if version:
library_key = library_key.replace(branch=ModuleStoreEnum.BranchName.library, version_guid=version)
library = self._get_library(library_key, is_v2_lib)
library = self._get_library(library_key)
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 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
if filter_children:
# Apply simple filtering based on CAPA problem types:
source_blocks.extend(self._problem_type_filter(library, dest_block.capa_type))
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)
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)
self.import_from_blockstore(dest_block, source_block_ids)
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
def list_available_libraries(self):
"""
List all known libraries.
Collects V1 libraries along with V2.
Returns tuples of (library key, display_name).
Returns tuples of (LibraryLocator, display_name)
"""
user = User.objects.get(id=self.user_id)
v1_libs = [
return [
(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):
@@ -308,7 +254,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.block_id != dest_parent_key.block_id:
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,

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,10 +74,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li
self.module_data = module_data
self.default_class = default_class
self.local_modules = {}
user = get_current_user()
user_id = user.id if user else None
self._services['library_tools'] = LibraryToolsService(modulestore, user_id=user_id)
self._services['library_tools'] = LibraryToolsService(modulestore, user_id=None)
@lazy
def _parent_map(self): # lint-amnesty, pylint: disable=missing-function-docstring

33
xmodule/tests/test_library_content.py
View File

@@ -4,18 +4,16 @@ 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
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
@@ -24,6 +22,7 @@ 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
@@ -75,32 +74,6 @@ 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

141
xmodule/tests/test_library_tools.py
View File

@@ -1,49 +1,35 @@
"""
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 opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
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 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
@ddt.ddt
class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest):
class LibraryToolsServiceTest(MixedSplitTestCase):
"""
Tests for LibraryToolsService.
Tests interaction with blockstore-based (V2) and mongo-based (V1) content libraries.
Tests for library service.
"""
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) == 2
assert len(all_libraries) == 1
@patch('xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.get_library_summaries')
def test_list_available_libraries_fetch(self, mock_get_library_summaries):
@@ -53,6 +39,15 @@ class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest):
_ = 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")
@@ -99,105 +94,3 @@ 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