Skip to content

Revert "[BD-14] feat: implement V2 libraries usage for library content block" #30872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rayzhou-bit merged 1 commit into from
Aug 17, 2022
Merged

Revert "[BD-14] feat: implement V2 libraries usage for library content block" #30872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 0 additions & 54 deletions xmodule/assets/library_content/public/js/library_content_edit_helpers.js
View file
Open in desktop

This file was deleted.

38 changes: 8 additions & 30 deletions xmodule/library_content_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,16 @@
import random
from copy import copy
from gettext import ngettext
from rest_framework import status

import bleach
from django.conf import settings
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
Expand All @@ -30,15 +29,16 @@
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
Expand Down Expand Up @@ -189,14 +189,9 @@ def completion_mode(cls): # pylint: disable=no-self-argument
@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):
Expand Down Expand Up @@ -461,7 +456,6 @@ def studio_view(self, _context):
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
Expand Down Expand Up @@ -523,22 +517,6 @@ def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint:
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):
"""
Expand Down
158 changes: 52 additions & 106 deletions xmodule/library_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,19 @@

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 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
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


def normalize_key_for_search(library_key):
Expand All @@ -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
Expand Down Expand Up @@ -169,13 +137,11 @@ def can_use_library_content(self, block):

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
Expand All @@ -189,69 +155,49 @@ def update_children(self, dest_block, user_perms=None, version=None):
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):
Expand Down Expand Up @@ -308,7 +254,7 @@ def generate_block_key(source_key, dest_parent_key):
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,
Expand Down
Loading