From 8f39b143bec6f2de3ef480615457923f73ba090a Mon Sep 17 00:00:00 2001 From: Eugene Dyudyunov Date: Thu, 16 Jun 2022 10:44:03 +0300 Subject: [PATCH 01/71] feat: implement V2 libraries usage for library content block 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; - unit tests added/updated. --- xmodule/library_content_module.py | 26 +-- xmodule/library_tools.py | 158 ++++++++++++------ .../split_mongo/caching_descriptor_system.py | 7 +- xmodule/tests/test_library_content.py | 33 +++- xmodule/tests/test_library_tools.py | 141 ++++++++++++++-- 5 files changed, 280 insertions(+), 85 deletions(-) diff --git a/xmodule/library_content_module.py b/xmodule/library_content_module.py index 0d9d4e080f10..f26aa0e259ed 100644 --- a/xmodule/library_content_module.py +++ b/xmodule/library_content_module.py @@ -8,37 +8,36 @@ import random from copy import copy from gettext import ngettext -from rest_framework import status import bleach +from capa.responsetypes import registry 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.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 from xblock.core import XBlock -from xblock.fields import Integer, List, Scope, String, Boolean - -from xmodule.capa.responsetypes import registry +from xblock.fields import Boolean, Integer, List, Scope, String 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 +188,14 @@ 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 + 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): diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index fd99f41dbd49..e4730813d29a 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -5,20 +5,28 @@ 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 @@ def can_use_library_content(self, block): 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 @@ def update_children(self, dest_block, user_perms=None, version=None): 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) + 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 @@ 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 != 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, diff --git a/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 6849cc2c6811..d5c7c357ddf3 100644 --- a/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -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 @@ def __init__(self, modulestore, course_entry, default_class, module_data, lazy, 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 diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index 3e1785746b46..6d7ced75930a 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -4,16 +4,18 @@ 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.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 @@ def get_module(descriptor): 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 diff --git a/xmodule/tests/test_library_tools.py b/xmodule/tests/test_library_tools.py index 950307b6963d..0e5f59aac45a 100644 --- a/xmodule/tests/test_library_tools.py +++ b/xmodule/tests/test_library_tools.py @@ -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 @@ def test_list_available_libraries_fetch(self, mock_get_library_summaries): _ = 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 @@ def test_import_from_blockstore(self): 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 From 6337ba884bbfd1e4797294c271f1646ca6c40426 Mon Sep 17 00:00:00 2001 From: Eugene Dyudyunov Date: Thu, 14 Jul 2022 10:10:00 +0300 Subject: [PATCH 02/71] feat: hide problem type field for v2 libs --- .../public/js/library_content_edit_helpers.js | 60 +++++++++++++++++++ xmodule/library_content_module.py | 17 ++++++ 2 files changed, 77 insertions(+) create mode 100644 xmodule/assets/library_content/public/js/library_content_edit_helpers.js diff --git a/xmodule/assets/library_content/public/js/library_content_edit_helpers.js b/xmodule/assets/library_content/public/js/library_content_edit_helpers.js new file mode 100644 index 000000000000..bed4fafd37de --- /dev/null +++ b/xmodule/assets/library_content/public/js/library_content_edit_helpers.js @@ -0,0 +1,60 @@ +/* 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(); + } + }); +} + +var $librarySelect = $("select[name='Library']"); + +$(document).on('change', $librarySelect, (e) => { + waitForEditorLoading(); +}) + +$libraryContentEditors = $('.xblock-header.xblock-header-library_content'); +$editBtns = $libraryContentEditors.find('.action-item.action-edit'); +$(document).on('click', $editBtns, (e) => { + console.log('edit clicked') + waitForEditorLoading(); +}) + +/** + * Waits untill editor html loaded, than calls checks for Program Type field toggling. + */ +function waitForEditorLoading() { + var checkContent = setInterval(function() { + $modal = $('.xblock-editor'); + content = $modal.html(); + if (content) { + clearInterval(checkContent); + checkProblemTypeShouldBeVisible($modal); + } + }, 10); +} +// Initial call +waitForEditorLoading(); diff --git a/xmodule/library_content_module.py b/xmodule/library_content_module.py index f26aa0e259ed..4b3e6bcb03d8 100644 --- a/xmodule/library_content_module.py +++ b/xmodule/library_content_module.py @@ -460,6 +460,7 @@ 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 @@ -521,6 +522,22 @@ 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): """ From 5fc70928b1a4c318d138265e7d7bb45490274a4f Mon Sep 17 00:00:00 2001 From: Eugene Dyudyunov Date: Thu, 14 Jul 2022 12:37:10 +0300 Subject: [PATCH 03/71] style: formating the code --- .../public/js/library_content_edit_helpers.js | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/xmodule/assets/library_content/public/js/library_content_edit_helpers.js b/xmodule/assets/library_content/public/js/library_content_edit_helpers.js index bed4fafd37de..564cc5fb0fbe 100644 --- a/xmodule/assets/library_content/public/js/library_content_edit_helpers.js +++ b/xmodule/assets/library_content/public/js/library_content_edit_helpers.js @@ -30,26 +30,13 @@ function checkProblemTypeShouldBeVisible(editor) { }); } -var $librarySelect = $("select[name='Library']"); - -$(document).on('change', $librarySelect, (e) => { - waitForEditorLoading(); -}) - -$libraryContentEditors = $('.xblock-header.xblock-header-library_content'); -$editBtns = $libraryContentEditors.find('.action-item.action-edit'); -$(document).on('click', $editBtns, (e) => { - console.log('edit clicked') - waitForEditorLoading(); -}) - /** * Waits untill editor html loaded, than calls checks for Program Type field toggling. */ function waitForEditorLoading() { var checkContent = setInterval(function() { - $modal = $('.xblock-editor'); - content = $modal.html(); + var $modal = $('.xblock-editor'); + var content = $modal.html(); if (content) { clearInterval(checkContent); checkProblemTypeShouldBeVisible($modal); @@ -58,3 +45,10 @@ function waitForEditorLoading() { } // 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) From 8d01fbb688c424fdedf48c3ed3dea38cb5635479 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Mon, 20 Jun 2022 13:56:56 +0300 Subject: [PATCH 04/71] feat: Edit modal for Library Sourced Content --- .../public/js/library_source_edit.js | 36 +++ xmodule/library_sourced_block.py | 231 ++++++++++++++---- xmodule/library_tools.py | 5 +- xmodule/static_content.py | 2 + 4 files changed, 229 insertions(+), 45 deletions(-) create mode 100644 xmodule/assets/library_source_block/public/js/library_source_edit.js diff --git a/xmodule/assets/library_source_block/public/js/library_source_edit.js b/xmodule/assets/library_source_block/public/js/library_source_edit.js new file mode 100644 index 000000000000..6f1624467097 --- /dev/null +++ b/xmodule/assets/library_source_block/public/js/library_source_edit.js @@ -0,0 +1,36 @@ +/* JavaScript for special editing operations that can be done on LibraryContentXBlock */ +window.LibrarySourceAuthorView = function(runtime, element) { + 'use strict'; + var $element = $(element); + var usage_id = $element.data('usage-id'); + // The "Update Now" button is not a child of 'element', as it is in the validation message area + // But it is still inside this xblock's wrapper element, which we can easily find: + var $wrapper = $element.parents('*[data-locator="' + usage_id + '"]'); + + // We can't bind to the button itself because in the bok choy test environment, + // it may not yet exist at this point in time... not sure why. + $wrapper.on('click', '.library-update-btn', function(e) { + e.preventDefault(); + // Update the XBlock with the latest matching content from the library: + runtime.notify('save', { + state: 'start', + element: element, + message: gettext('Updating with latest library content') + }); + $.post(runtime.handlerUrl(element, 'refresh_children')).done(function() { + runtime.notify('save', { + state: 'end', + element: element + }); + if ($element.closest('.wrapper-xblock').is(':not(.level-page)')) { + // We are on a course unit page. The notify('save') should refresh this block, + // but that is only working on the container page view of this block. + // Why? On the unit page, this XBlock's runtime has no reference to the + // XBlockContainerPage - only the top-level XBlock (a vertical) runtime does. + // But unfortunately there is no way to get a reference to our parent block's + // JS 'runtime' object. So instead we must refresh the whole page: + location.reload(); + } + }); + }); +}; diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index d96741d6e447..ab34f633e5c9 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -1,20 +1,27 @@ """ Library Sourced Content XBlock """ +import json import logging - from copy import copy -from mako.template import Template as MakoTemplate + +import bleach +from lazy import lazy +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from pkg_resources import resource_string +from web_fragments.fragment import Fragment +from webob import Response from xblock.core import XBlock -from xblock.fields import Scope, String, List +from xblock.fields import List, Scope, String from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader -from xblockutils.studio_editable import StudioEditableXBlockMixin -from webob import Response -from web_fragments.fragment import Fragment +from xmodule.mako_module import MakoTemplateBlockBase from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin +from xmodule.util.xmodule_django import add_webpack_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage +from xmodule.x_module import HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js log = logging.getLogger(__name__) loader = ResourceLoader(__name__) @@ -25,7 +32,15 @@ @XBlock.wants('library_tools') # Only needed in studio -class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlock): +@XBlock.wants('studio_user_permissions') # Only available in studio +class LibrarySourcedBlock( + MakoTemplateBlockBase, + XModuleToXBlockMixin, + HTMLSnippet, + ResourceTemplates, + XModuleMixin, + EditableChildrenMixin, +): """ Library Sourced Content XBlock @@ -42,6 +57,17 @@ class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlo display_name=_("Display Name"), scope=Scope.content, ) + source_library_id = String( + display_name=_("Library"), + help=_("Select the library from which you want to draw content."), + scope=Scope.settings, + values_provider=lambda instance: instance.source_library_values(), + ) + source_library_version = String( + # This is a hidden field that stores the version of source_library when we last pulled content from it + display_name=_("Library Version"), + scope=Scope.settings, + ) source_block_ids = List( display_name=_("Library Blocks List"), help=_("Enter the IDs of the library XBlocks that you wish to use."), @@ -50,38 +76,42 @@ class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlo editable_fields = ("display_name", "source_block_ids") has_children = True has_author_view = True + resources_dir = 'assets/library_source_block' + + preview_view_js = { + 'js': [], + 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + } + preview_view_css = { + 'scss': [], + } + + mako_template = 'widgets/metadata-edit.html' + studio_js_module_name = "VerticalDescriptor" + studio_view_js = { + 'js': [ + resource_string(__name__, 'js/src/vertical/edit.js'), + ], + 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + } + studio_view_css = { + 'scss': [], + } MAX_BLOCKS_ALLOWED = 10 def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if not self.source_block_ids: - self.has_children = False - - def studio_view(self, context): + def studio_view(self, _context): """ - Render a form for editing this XBlock + Return the studio view. """ - fragment = Fragment() - static_content = ResourceLoader('common.djangoapps.pipeline_mako').load_unicode('templates/static_content.html') - render_react = MakoTemplate(static_content, default_filters=[]).get_def('renderReact') - react_content = render_react.render( - component="LibrarySourcedBlockPicker", - id="library-sourced-block-picker", - props={ - 'selectedXblocks': self.source_block_ids, - } + fragment = Fragment( + self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context()) ) - fragment.content = loader.render_django_template('templates/library-sourced-block-studio-view.html', { - 'react_content': react_content, - 'save_url': self.runtime.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('LibrarySourceBlockStudioView') + add_webpack_to_fragment(fragment, 'LibrarySourcedBlockStudio') + shim_xmodule_js(fragment, self.studio_js_module_name) return fragment @@ -94,6 +124,8 @@ def author_view(self, context): # 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) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_source_edit.js')) + fragment.initialize_js('LibrarySourceAuthorView') return fragment def student_view(self, context): @@ -124,6 +156,42 @@ def validate_field_data(self, validation, data): ) ) + def _validate_library_version(self, validation, lib_tools, version, library_key): + """ + Validates library version + """ + latest_version = lib_tools.get_library_version(library_key) + if latest_version is not None: + if version is None or version != str(latest_version): + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + _('This component is out of date. The library has new content.'), + # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + # See https://openedx.atlassian.net/browse/TNL-993 + action_class='library-update-btn', + # Translators: {refresh_icon} placeholder is substituted to "↻" (without double quotes) + action_label=_("{refresh_icon} Update now.").format(refresh_icon="↻") + ) + ) + return False + else: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.ERROR, + _('Library is invalid, corrupt, or has been deleted.'), + action_class='edit-button', + action_label=_("Edit Library List.") + ) + ) + return False + return True + + def _set_validation_error_if_empty(self, validation, summary): + """ Helper method to only set validation summary if it's empty """ + if validation.empty: + validation.set_summary(summary) + def validate(self): """ Validates the state of this library_sourced_xblock Instance. This is the override of the general XBlock method, @@ -132,28 +200,103 @@ def validate(self): validation = super().validate() validation = StudioValidation.copy(validation) - if not self.source_block_ids: + if not self.source_library_id: validation.set_summary( StudioValidationMessage( StudioValidationMessage.NOT_CONFIGURED, - _("No XBlock has been configured for this component. Use the editor to select the target blocks."), + _("A library has not yet been selected."), action_class='edit-button', - action_label=_("Open Editor") + action_label=_("Select a Library.") ) ) + self._validate_library_version(validation, self.tools, self.source_library_version, self.source_library_key) return validation - @XBlock.handler - def submit_studio_edits(self, data, suffix=''): + def source_library_values(self): + """ + Return a list of possible values for self.source_library_id + """ + user_perms = self.runtime.service(self, 'studio_user_permissions') + all_libraries = [ + (key, bleach.clean(name)) for key, name in self.tools.list_available_libraries() + if user_perms.can_read(key) or self.source_library_id == str(key) + ] + all_libraries.sort(key=lambda entry: entry[1]) # Sort by name + if self.source_library_id and self.source_library_key not in [entry[0] for entry in all_libraries]: + all_libraries.append((self.source_library_id, _("Invalid Library"))) + all_libraries = [("", _("No Library Selected"))] + all_libraries + values = [{"display_name": name, "value": str(key)} for key, name in all_libraries] + return values + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super().non_editable_metadata_fields + non_editable_fields.extend([ + LibrarySourcedBlock.source_block_ids, + LibrarySourcedBlock.source_library_version, + ]) + return non_editable_fields + + @property + def source_library_key(self): """ - Save changes to this block, applying edits made in Studio. + Convenience method to get the library ID as a LibraryLocator and not just a string """ - 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_ids) - except Exception as err: # pylint: disable=broad-except - log.exception(err) - return Response(_("Importing Library Block failed - are the IDs valid and readable?"), status=400) - return response + return LibraryLocator.from_string(self.source_library_id) + except InvalidKeyError: + return LibraryLocatorV2.from_string(self.source_library_id) + + @lazy + def tools(self): + """ + Grab the library tools service or raise an error. + """ + return self.runtime.service(self, 'library_tools') + + @XBlock.handler + def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: disable=unused-argument + """ + Refresh children: + This method is to be used when any of the libraries that this block + references have been updated. It will re-fetch all matching blocks from + the libraries, and copy them as children of this block. The children + will be given new block_ids, but the definition ID used should be the + exact same definition ID used in the library. + + This method will update this block's 'source_library_id' field to store + the version number of the libraries used, so we easily determine if + this block is up to date or not. + """ + user_perms = self.runtime.service(self, 'studio_user_permissions') + if not self.tools: + return Response("Library Tools unavailable in current runtime.", status=400) + self.tools.update_children(self, user_perms) + return Response() + + def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument + """ + If source_library_id has been edited, refresh_children automatically. + """ + old_source_library_id = old_metadata.get('source_library_id', []) + if old_source_library_id != self.source_library_id: + try: + self.refresh_children() + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + + @XBlock.json_handler + def set_block_ids(self, data, suffix=''): + """ + Save source_block_ids. + """ + if data.get('source_library_id'): + self.source_library_id = data.get('source_library_id') + return {'source_library_id': self.source_library_id} + + @XBlock.handler + def get_block_ids(self, data, suffix=''): + """ + Return source_block_ids. + """ + return Response(json.dumps({'source_library_id': self.source_library_id})) diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index e4730813d29a..985f9e0f4266 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -200,7 +200,10 @@ def update_children(self, dest_block, user_perms=None, version=None): if library is None: raise ValueError(f"Requested library {library_key} not found.") - filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE) + if hasattr(dest_block, 'capa_type'): + filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE) + else: + filter_children = None if not is_v2_lib: if user_perms and not user_perms.can_read(library_key): diff --git a/xmodule/static_content.py b/xmodule/static_content.py index 3c891cd750c2..ea6931513ad8 100755 --- a/xmodule/static_content.py +++ b/xmodule/static_content.py @@ -24,6 +24,7 @@ from xmodule.conditional_module import ConditionalBlock from xmodule.html_module import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock from xmodule.library_content_module import LibraryContentBlock +from xmodule.library_sourced_block import LibrarySourcedBlock from xmodule.lti_module import LTIBlock from xmodule.poll_module import PollBlock from xmodule.seq_module import SequenceBlock @@ -78,6 +79,7 @@ class VideoBlock(HTMLSnippet): # lint-amnesty, pylint: disable=abstract-method CustomTagBlock, HtmlBlock, LibraryContentBlock, + LibrarySourcedBlock, LTIBlock, PollBlock, ProblemBlock, From 7d4c2351e52f6a86466a243079691faa5b1f050e Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Mon, 20 Jun 2022 13:57:27 +0300 Subject: [PATCH 05/71] feat: remove react studio view for Library Sourced Content --- webpack.common.config.js | 13 - .../LibrarySourcedBlockPicker.jsx | 223 ------------------ .../public/js/library_source_block.js | 58 ----- xmodule/assets/library_source_block/style.css | 58 ----- .../library-sourced-block-studio-view.html | 19 -- 5 files changed, 371 deletions(-) delete mode 100644 xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx delete mode 100644 xmodule/assets/library_source_block/public/js/library_source_block.js delete mode 100644 xmodule/assets/library_source_block/style.css delete mode 100644 xmodule/templates/library-sourced-block-studio-view.html diff --git a/webpack.common.config.js b/webpack.common.config.js index 7aa2d02d1042..53c242c095f9 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -72,7 +72,6 @@ module.exports = Merge.smart({ // Studio Import: './cms/static/js/features/import/factories/import.js', CourseOrLibraryListing: './cms/static/js/features_jsx/studio/CourseOrLibraryListing.jsx', - LibrarySourcedBlockPicker: './xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx', // eslint-disable-line max-len 'js/factories/textbooks': './cms/static/js/factories/textbooks.js', 'js/factories/container': './cms/static/js/factories/container.js', 'js/factories/context_course': './cms/static/js/factories/context_course.js', @@ -342,18 +341,6 @@ module.exports = Merge.smart({ { test: /logger/, loader: 'imports-loader?this=>window' - }, - { - test: /\.css$/, - use: [ - 'style-loader', - { - loader: 'css-loader', - options: { - modules: true - } - } - ] } ] }, diff --git a/xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx b/xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx deleted file mode 100644 index 74b1d8b485a2..000000000000 --- a/xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx +++ /dev/null @@ -1,223 +0,0 @@ -/* globals gettext */ - -import 'whatwg-fetch'; -import PropTypes from 'prop-types'; -import React from 'react'; -import _ from 'underscore'; -import styles from './style.css'; - -class LibrarySourcedBlockPicker extends React.Component { - constructor(props) { - super(props); - this.state = { - libraries: [], - xblocks: [], - searchedLibrary: '', - libraryLoading: false, - xblocksLoading: false, - selectedLibrary: undefined, - selectedXblocks: new Set(this.props.selectedXblocks), - }; - this.onLibrarySearchInput = this.onLibrarySearchInput.bind(this); - this.onXBlockSearchInput = this.onXBlockSearchInput.bind(this); - this.onLibrarySelected = this.onLibrarySelected.bind(this); - this.onXblockSelected = this.onXblockSelected.bind(this); - this.onDeleteClick = this.onDeleteClick.bind(this); - } - - componentDidMount() { - this.fetchLibraries(); - } - - fetchLibraries(textSearch='', page=1, append=false) { - this.setState({ - libraries: append ? this.state.libraries : [], - libraryLoading: true, - }, async function() { - try { - let res = await fetch(`/api/libraries/v2/?pagination=true&page=${page}&text_search=${textSearch}`); - res = await res.json(); - this.setState({ - libraries: this.state.libraries.concat(res.results), - libraryLoading: false, - }, () => { - if (res.next) { - this.fetchLibraries(textSearch, page+1, true); - } - }); - } catch (error) { - $('#library-sourced-block-picker').trigger('error', { - title: 'Could not fetch library', - message: error, - }); - this.setState({ - libraries: [], - libraryLoading: false, - }); - } - }); - } - - fetchXblocks(library, textSearch='', page=1, append=false) { - this.setState({ - xblocks: append ? this.state.xblocks : [], - xblocksLoading: true, - }, async function() { - try { - let res = await fetch(`/api/libraries/v2/${library}/blocks/?pagination=true&page=${page}&text_search=${textSearch}`); - res = await res.json(); - this.setState({ - xblocks: this.state.xblocks.concat(res.results), - xblocksLoading: false, - }, () => { - if (res.next) { - this.fetchXblocks(library, textSearch, page+1, true); - } - }); - } catch (error) { - $('#library-sourced-block-picker').trigger('error', { - title: 'Could not fetch xblocks', - message: error, - }); - this.setState({ - xblocks: [], - xblocksLoading: false, - }); - } - }); - } - - onLibrarySearchInput(event) { - event.persist() - this.setState({ - searchedLibrary: event.target.value, - }); - if (!this.debouncedFetchLibraries) { - this.debouncedFetchLibraries = _.debounce(value => { - this.fetchLibraries(value); - }, 300); - } - this.debouncedFetchLibraries(event.target.value); - } - - onXBlockSearchInput(event) { - event.persist() - if (!this.debouncedFetchXblocks) { - this.debouncedFetchXblocks = _.debounce(value => { - this.fetchXblocks(this.state.selectedLibrary, value); - }, 300); - } - this.debouncedFetchXblocks(event.target.value); - } - - onLibrarySelected(event) { - this.setState({ - selectedLibrary: event.target.value, - }); - this.fetchXblocks(event.target.value); - } - - onXblockSelected(event) { - let state = new Set(this.state.selectedXblocks); - if (event.target.checked) { - state.add(event.target.value); - } else { - state.delete(event.target.value); - } - this.setState({ - selectedXblocks: state, - }, this.updateList); - } - - onDeleteClick(event) { - let value; - if (event.target.tagName == 'SPAN') { - value = event.target.parentElement.dataset.value; - } else { - value = event.target.dataset.value; - } - let state = new Set(this.state.selectedXblocks); - state.delete(value); - this.setState({ - selectedXblocks: state, - }, this.updateList); - } - - updateList(list) { - $('#library-sourced-block-picker').trigger('selected-xblocks', { - sourceBlockIds: Array.from(this.state.selectedXblocks), - }); - } - - render() { - return ( -
-
-
-

- - Hitting 'Save and Import' will import the latest versions of the selected blocks, overwriting any changes done to this block post-import. -

-
-
-
-
- -
- { - this.state.libraries.map(lib => ( -
- - -
- )) - } - { this.state.libraryLoading && {gettext('Loading...')} } -
-
-
- -
- { - this.state.xblocks.map(block => ( -
- - -
- )) - } - { this.state.xblocksLoading && {gettext('Loading...')} } -
-
-
-

{gettext('Selected blocks')}

-
    - { - Array.from(this.state.selectedXblocks).map(block => ( -
  • - - -
    • - )) - } -
-
-
-
- ); - } -} - -LibrarySourcedBlockPicker.propTypes = { - selectedXblocks: PropTypes.array, -}; - -LibrarySourcedBlockPicker.defaultProps = { - selectedXblocks: [], -}; - -export { LibrarySourcedBlockPicker }; // eslint-disable-line import/prefer-default-export diff --git a/xmodule/assets/library_source_block/public/js/library_source_block.js b/xmodule/assets/library_source_block/public/js/library_source_block.js deleted file mode 100644 index 9674080d59dc..000000000000 --- a/xmodule/assets/library_source_block/public/js/library_source_block.js +++ /dev/null @@ -1,58 +0,0 @@ -/* JavaScript for allowing editing options on LibrarySourceBlock's studio view */ -window.LibrarySourceBlockStudioView = function(runtime, element) { - 'use strict'; - var self = this; - - $('#library-sourced-block-picker', element).on('selected-xblocks', function(e, params) { - self.sourceBlockIds = params.sourceBlockIds; - }); - - $('#library-sourced-block-picker', element).on('error', function(e, params) { - runtime.notify('error', {title: gettext(params.title), message: params.message}); - }); - - $('.save-button', element).on('click', function(e) { - e.preventDefault(); - var url = $(e.target).data('submit-url'); - var data = { - values: { - source_block_ids: self.sourceBlockIds - }, - defaults: ['display_name'] - }; - - 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}); - }); - }); - - $('.cancel-button', element).on('click', function(e) { - e.preventDefault(); - runtime.notify('cancel', {}); - }); -}; diff --git a/xmodule/assets/library_source_block/style.css b/xmodule/assets/library_source_block/style.css deleted file mode 100644 index 4892f20405c0..000000000000 --- a/xmodule/assets/library_source_block/style.css +++ /dev/null @@ -1,58 +0,0 @@ -.column { - display: flex; - flex-direction: column; - margin: 10px; - max-width: 300px; - flex-grow: 1; -} -.elementList { - margin-top: 10px; -} -input.search { - width: 100% !important; - height: auto !important; -} -.element > input[type='checkbox'], -.element > input[type='radio'] { - position: absolute; - width: 0 !important; - height: 0 !important; - top: -9999px; -} -.element > .elementItem { - display: flex; - flex-grow: 1; - padding: 0.625rem 1.25rem; - border: 1px solid rgba(0, 0, 0, 0.25); -} -.element + .element > label { - border-top: 0; -} -.element > input[type='checkbox']:focus + label, -.element > input[type='radio']:focus + label, -.element > input:hover + label { - background: #f6f6f7; - cursor: pointer; -} -.element > input:checked + label { - background: #23419f; - color: #fff; -} -.element > input[type='checkbox']:checked:focus + label, -.element > input[type='radio']:checked:focus + label, -.element > input:checked:hover + label { - background: #193787; - cursor: pointer; -} -.selectedBlocks { - padding: 12px 8px 20px; -} -button.remove { - background: #e00; - color: #fff; - border: solid rgba(0,0,0,0.25) 1px; -} -button.remove:focus, -button.remove:hover { - background: #d00; -} diff --git a/xmodule/templates/library-sourced-block-studio-view.html b/xmodule/templates/library-sourced-block-studio-view.html deleted file mode 100644 index 08a2882b51c5..000000000000 --- a/xmodule/templates/library-sourced-block-studio-view.html +++ /dev/null @@ -1,19 +0,0 @@ -{% load i18n %} -
-
-
- {{ react_content|safe }} -
-
-
- -
-
From 726824be4381e23530b552c34c733114af1ebf7e Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Mon, 20 Jun 2022 18:51:13 +0300 Subject: [PATCH 06/71] feat: add "selectable" context for author_view --- xmodule/library_sourced_block.py | 50 +++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index ab34f633e5c9..4ab0cf6b6e49 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -16,6 +16,7 @@ from xblock.fields import List, Scope, String from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader +from xblockutils.studio_editable import StudioEditableXBlockMixin from xmodule.mako_module import MakoTemplateBlockBase from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin @@ -34,6 +35,7 @@ @XBlock.wants('library_tools') # Only needed in studio @XBlock.wants('studio_user_permissions') # Only available in studio class LibrarySourcedBlock( + StudioEditableXBlockMixin, MakoTemplateBlockBase, XModuleToXBlockMixin, HTMLSnippet, @@ -53,7 +55,7 @@ class LibrarySourcedBlock( """ display_name = String( help=_("The display name for this component."), - default="Library Sourced Content", + default="Library Reference Block", display_name=_("Display Name"), scope=Scope.content, ) @@ -73,7 +75,7 @@ class LibrarySourcedBlock( help=_("Enter the IDs of the library XBlocks that you wish to use."), scope=Scope.content, ) - editable_fields = ("display_name", "source_block_ids") + editable_fields = ("source_block_ids",) has_children = True has_author_view = True @@ -103,6 +105,33 @@ class LibrarySourcedBlock( def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" + def render_children(self, context, fragment, can_reorder=False, can_add=False): + """ + Renders the children of the module with HTML appropriate for Studio. If can_reorder is True, + then the children will be rendered to support drag and drop. + """ + contents = [] + + for child in self.get_children(): # pylint: disable=no-member + if can_reorder: + context['reorderable_items'].add(child.location) + context['can_add'] = can_add + context['selected'] = str(child.location) in self.source_block_ids + rendered_child = child.render(StudioEditableModule.get_preview_view_name(child), context) + fragment.add_fragment_resources(rendered_child) + + contents.append({ + 'id': str(child.location), + 'content': rendered_child.content + }) + + fragment.add_content(self.runtime.service(self, 'mako').render_template("studio_render_children_view.html", { # pylint: disable=no-member + 'items': contents, + 'xblock_context': context, + 'can_add': can_add, + 'can_reorder': can_reorder, + })) + def studio_view(self, _context): """ Return the studio view. @@ -209,6 +238,8 @@ def validate(self): action_label=_("Select a Library.") ) ) + return validation + self._validate_library_version(validation, self.tools, self.source_library_version, self.source_library_key) return validation @@ -285,18 +316,11 @@ def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint except ValueError: pass # The validation area will display an error message, no need to do anything now. - @XBlock.json_handler - def set_block_ids(self, data, suffix=''): - """ - Save source_block_ids. - """ - if data.get('source_library_id'): - self.source_library_id = data.get('source_library_id') - return {'source_library_id': self.source_library_id} - @XBlock.handler - def get_block_ids(self, data, suffix=''): + def get_block_ids(self, request, suffix=''): # lint-amnesty, pylint: disable=unused-argument """ Return source_block_ids. """ - return Response(json.dumps({'source_library_id': self.source_library_id})) + return Response(json.dumps({'source_block_ids': self.source_block_ids})) + +StudioEditableModule = EditableChildrenMixin From d8685837f99e42af0e96d4bb6eb335c25b8085dc Mon Sep 17 00:00:00 2001 From: ihor-romaniuk Date: Fri, 24 Jun 2022 14:32:50 +0300 Subject: [PATCH 07/71] feat: add selection functionality to library components --- cms/djangoapps/contentstore/views/preview.py | 1 + cms/static/js/views/pages/container.js | 66 +++++++++++++++++++- cms/static/sass/elements/_xblocks.scss | 13 ++++ cms/templates/container.html | 7 +++ cms/templates/studio_xblock_wrapper.html | 9 +++ 5 files changed, 93 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index fd0bdf0dc57a..d9103d634663 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -309,6 +309,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, + 'is_selected': context.get('selected', False), 'can_edit': context.get('can_edit', True), 'can_edit_visibility': context.get('can_edit_visibility', xblock.scope_ids.usage_id.context_key.is_course), 'selected_groups_label': selected_groups_label, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 347f7ea77b7f..7b5ebe19bb2a 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -6,10 +6,10 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page 'common/js/components/utils/view_utils', 'js/views/container', 'js/views/xblock', 'js/views/components/add_xblock', 'js/views/modals/edit_xblock', 'js/views/modals/move_xblock_modal', 'js/models/xblock_info', 'js/views/xblock_string_field_editor', 'js/views/xblock_access_editor', - 'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils'], + 'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils', 'js/utils/module'], function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, MoveXBlockModal, XBlockInfo, XBlockStringFieldEditor, XBlockAccessEditor, - ContainerSubviews, UnitOutlineView, XBlockUtils) { + ContainerSubviews, UnitOutlineView, XBlockUtils, ModuleUtils) { 'use strict'; var XBlockContainerPage = BasePage.extend({ // takes XBlockInfo as a model @@ -20,7 +20,9 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page 'click .duplicate-button': 'duplicateXBlock', 'click .move-button': 'showMoveXBlockModal', 'click .delete-button': 'deleteXBlock', - 'click .new-component-button': 'scrollToNewComponentButtons' + 'click .save-button': 'saveSelectedLibraryComponents', + 'click .new-component-button': 'scrollToNewComponentButtons', + 'change .header-library-checkbox': 'toggleLibraryComponent' }, options: { @@ -95,6 +97,10 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page model: this.model }); this.unitOutlineView.render(); + + this.selectedLibraryComponents = []; + this.storeSelectedLibraryComponents = []; + this.getSelectedLibraryComponents(); } this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved); @@ -300,6 +306,60 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page }); }, + getSelectedLibraryComponents: function() { + var self = this; + var locator = this.$el.find('.studio-xblock-wrapper').data('locator'); + $.getJSON( + ModuleUtils.getUpdateUrl(locator) + '/handler/get_block_ids', + function(data) { + self.selectedLibraryComponents = Array.from(data.source_block_ids); + self.storeSelectedLibraryComponents = Array.from(data.source_block_ids); + } + ); + }, + + saveSelectedLibraryComponents: function(e) { + var self = this; + var locator = this.$el.find('.studio-xblock-wrapper').data('locator'); + e.preventDefault(); + $.postJSON( + ModuleUtils.getUpdateUrl(locator) + '/handler/submit_studio_edits', + {values: {source_block_ids: self.storeSelectedLibraryComponents}}, + function() { + self.selectedLibraryComponents = Array.from(self.storeSelectedLibraryComponents); + self.toggleSaveButton(); + } + ); + }, + + toggleLibraryComponent: function(event) { + var componentId = $(event.target).closest('.studio-xblock-wrapper').data('locator'); + var storeIndex = this.storeSelectedLibraryComponents.indexOf(componentId); + if (storeIndex > -1) { + this.storeSelectedLibraryComponents.splice(storeIndex, 1); + this.toggleSaveButton(); + } else { + this.storeSelectedLibraryComponents.push(componentId); + this.toggleSaveButton(); + } + }, + + toggleSaveButton: function() { + var $saveButton = $('.nav-actions .save-button'); + if (JSON.stringify(this.selectedLibraryComponents.sort()) === JSON.stringify(this.storeSelectedLibraryComponents.sort())) { + $saveButton.addClass('is-hidden'); + window.removeEventListener('beforeunload', this.onBeforePageUnloadCallback); + } else { + $saveButton.removeClass('is-hidden'); + window.addEventListener('beforeunload', this.onBeforePageUnloadCallback); + } + }, + + onBeforePageUnloadCallback: function (event) { + event.preventDefault(); + event.returnValue = ''; + }, + onDelete: function(xblockElement) { // get the parent so we can remove this component from its parent. var xblockView = this.xblockView, diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index d6e8aff5d66a..7574a732cb48 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -43,6 +43,19 @@ display: flex; align-items: center; + .header-library-checkbox { + margin-right: 10px; + width: 17px; + height: 17px; + cursor: pointer; + vertical-align: middle; + } + + .header-library-checkbox-label { + vertical-align: middle; + cursor: pointer; + } + .header-details { @extend %cont-truncated; diff --git a/cms/templates/container.html b/cms/templates/container.html index 17de3d20ea76..909360832610 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -144,6 +144,13 @@

${_("Page Actions")}

% else: + % if True: +
  • + +
    • + % endif
  • diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 1bd2c3070e30..e5bcfe19c730 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -73,7 +73,16 @@ % endif
    + % if True: + + + % else: ${label} + % endif % if selected_groups_label:

    ${selected_groups_label}

    % endif From ffbd97790aebf3251e63343d3f8460c7b644b292 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Thu, 23 Jun 2022 15:07:52 +0300 Subject: [PATCH 08/71] feat: show selected blocks for student_view --- xmodule/library_sourced_block.py | 84 +++++++++++++++++--------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index 4ab0cf6b6e49..aa6a9a022d74 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -8,13 +8,13 @@ import bleach from lazy import lazy from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 from pkg_resources import resource_string from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock from xblock.fields import List, Scope, String -from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin @@ -22,7 +22,7 @@ from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin from xmodule.util.xmodule_django import add_webpack_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage -from xmodule.x_module import HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js +from xmodule.x_module import STUDENT_VIEW, HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js log = logging.getLogger(__name__) loader = ResourceLoader(__name__) @@ -100,7 +100,6 @@ class LibrarySourcedBlock( studio_view_css = { 'scss': [], } - MAX_BLOCKS_ALLOWED = 10 def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" @@ -117,7 +116,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): context['reorderable_items'].add(child.location) context['can_add'] = can_add context['selected'] = str(child.location) in self.source_block_ids - rendered_child = child.render(StudioEditableModule.get_preview_view_name(child), context) + rendered_child = child.render(EditableChildrenMixin.get_preview_view_name(child), context) fragment.add_fragment_resources(rendered_child) contents.append({ @@ -157,33 +156,40 @@ def author_view(self, context): fragment.initialize_js('LibrarySourceAuthorView') return fragment - def student_view(self, context): + def _get_selected_child_blocks(self): """ - Renders the view that learners see. + Generator returning XBlock instances of the children selected for the + current user. """ - 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_field_data(self, validation, data): + for block_id in self.source_block_ids: + usage_key = UsageKey.from_string(block_id) + yield self.runtime.get_block(usage_key) + + def student_view(self, context): """ - Validate this block's field data. Instead of checking fields like self.name, check the - fields set on data, e.g. data.name. This allows the same validation method to be re-used - for the studio editor. + Renders the view that learners see. """ - if len(data.source_block_ids) > self.MAX_BLOCKS_ALLOWED: - # Because importing library blocks is an expensive operation - validation.add( - ValidationMessage( - ValidationMessage.ERROR, - _("A maximum of {0} components may be added.").format(self.MAX_BLOCKS_ALLOWED) - ) - ) + fragment = Fragment() + child_context = {} if not context else copy(context) + + fragment.add_content('
    ') + for child in self._get_selected_child_blocks(): + if child is None: + # TODO: Fix the underlying issue in TNL-7424 + # This shouldn't be happening, but does for an as-of-now + # unknown reason. Until we address the underlying issue, + # let's at least log the error explicitly, ignore the + # exception, and prevent the page from resulting in a + # 500-response. + log.error('Skipping display for child block that is None') + continue + for displayable in child.displayable_items(): + rendered_child = displayable.render(STUDENT_VIEW, child_context) + fragment.add_content(rendered_child.content) + fragment.add_fragment_resources(rendered_child) + + fragment.add_content('
    ') + return fragment def _validate_library_version(self, validation, lib_tools, version, library_key): """ @@ -285,6 +291,17 @@ def tools(self): """ return self.runtime.service(self, 'library_tools') + def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument + """ + If source_library_id has been edited, refresh_children automatically. + """ + old_source_library_id = old_metadata.get('source_library_id', []) + if old_source_library_id != self.source_library_id: + try: + self.refresh_children() + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + @XBlock.handler def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: disable=unused-argument """ @@ -305,22 +322,9 @@ def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: self.tools.update_children(self, user_perms) return Response() - def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument - """ - If source_library_id has been edited, refresh_children automatically. - """ - old_source_library_id = old_metadata.get('source_library_id', []) - if old_source_library_id != self.source_library_id: - try: - self.refresh_children() - except ValueError: - pass # The validation area will display an error message, no need to do anything now. - @XBlock.handler def get_block_ids(self, request, suffix=''): # lint-amnesty, pylint: disable=unused-argument """ Return source_block_ids. """ return Response(json.dumps({'source_block_ids': self.source_block_ids})) - -StudioEditableModule = EditableChildrenMixin From 13280bda9526b8d8128178eb0216f828882d72d0 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 24 Jun 2022 15:06:49 +0300 Subject: [PATCH 09/71] test: update tests for LibrarySourcedBlock --- xmodule/tests/test_library_sourced_block.py | 66 +++++++++------------ 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/xmodule/tests/test_library_sourced_block.py b/xmodule/tests/test_library_sourced_block.py index 246a6fa05a94..5b4de29582d6 100644 --- a/xmodule/tests/test_library_sourced_block.py +++ b/xmodule/tests/test_library_sourced_block.py @@ -2,63 +2,53 @@ Tests for Source from Library XBlock """ -from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest -from common.djangoapps.student.roles import CourseInstructorRole -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.library_tools import LibraryToolsService +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory +from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.tests import get_test_system from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=unused-import +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest + -class LibrarySourcedBlockTestCase(ContentLibrariesRestApiTest): +class LibrarySourcedBlockTestCase(MixedSplitTestCase, ContentLibrariesRestApiTest): """ Tests for LibraryToolsService which interact with blockstore-based content libraries """ def setUp(self): super().setUp() - self.store = modulestore() + self.tools = LibraryToolsService(self.store, self.user_id) + self.library = LibraryFactory.create(modulestore=self.store) + # Add content to the library + self.lib_blocks = [ + self.make_block("html", self.library, data=f"Hello world from block {i}") + for i in range(1, 5) + ] # 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 - self.source_block = ItemFactory.create( - category="library_sourced", - parent=course, - parent_location=course.location, - user_id=self.user.id, - modulestore=self.store + self.course = CourseFactory.create(modulestore=self.store) + self.chapter = self.make_block("chapter", self.course) + self.sequential = self.make_block("sequential", self.chapter) + self.vertical = self.make_block("vertical", self.sequential) + # Add a LibrarySourcedBlock to the course + self.source_block = self.make_block( + "library_sourced", + self.vertical, + source_library_id=str(self.library.location.library_key) ) self.submit_url = f'/xblock/{self.source_block.scope_ids.usage_id}/handler/submit_studio_edits' 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_1 = self._add_block_to_library(library["id"], "html", "html_student_preview_1")["id"] - self._set_library_block_olx(html_block_1, 'Student Preview Test 1') - html_block_2 = self._add_block_to_library(library["id"], "html", "html_student_preview_2")["id"] - self._set_library_block_olx(html_block_2, 'Student Preview Test 2') - # Import the html blocks from the library to the course - post_data = {"values": {"source_block_ids": [html_block_1, html_block_2]}, "defaults": ["display_name"]} + self.source_block.refresh_children() + # Save children block_ids as source_block_ids + source_block_ids = [str(child) for child in self.source_block.children] + post_data = {"values": {"source_block_ids": source_block_ids}} res = self.client.post(self.submit_url, data=post_data, format='json') # Check if student_view renders the children correctly res = self.get_block_view(self.source_block, STUDENT_VIEW) - assert 'Student Preview Test 1' in res - assert 'Student Preview Test 2' in res - - def test_block_limits(self): - # Create a blockstore content library - library = self._create_library(slug="testlib2_preview", title="Test Library 2", description="Testing XBlocks") - # Add content to the library - blocks = [self._add_block_to_library(library["id"], "html", f"block_{i}")["id"] for i in range(11)] - - # Import the html blocks from the library to the course - post_data = {"values": {"source_block_ids": blocks}, "defaults": ["display_name"]} - res = self.client.post(self.submit_url, data=post_data, format='json') - assert res.status_code == 400 - assert res.json()['error']['messages'][0]['text'] == 'A maximum of 10 components may be added.' + for i in range(1, 5): + assert f"Hello world from block {i}" in res def get_block_view(self, block, view, context=None): """ From c4d75191e3066d9ae9c74c41c773b174140d6fe1 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 24 Jun 2022 18:48:39 +0300 Subject: [PATCH 10/71] feat: add context for library_sourced container page --- cms/djangoapps/contentstore/views/component.py | 3 ++- cms/djangoapps/contentstore/views/preview.py | 3 ++- cms/static/js/views/pages/container.js | 4 +++- cms/templates/container.html | 2 +- cms/templates/studio_xblock_wrapper.html | 2 +- xmodule/library_sourced_block.py | 16 +++++++++++++--- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index c8173c72283f..34914ef98f4b 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -202,7 +202,8 @@ def container_handler(request, usage_key_string): 'xblock_info': xblock_info, 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, - 'templates': CONTAINER_TEMPLATES + 'templates': CONTAINER_TEMPLATES, + 'is_sourced_block': xblock.location.block_type == 'library_sourced', }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index d9103d634663..ba418ff8be23 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -309,7 +309,8 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, - 'is_selected': context.get('selected', False), + 'is_selected': context.get('is_selected', False), + 'selectable': context.get('selectable', False), 'can_edit': context.get('can_edit', True), 'can_edit_visibility': context.get('can_edit_visibility', xblock.scope_ids.usage_id.context_key.is_course), 'selected_groups_label': selected_groups_label, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 7b5ebe19bb2a..d1e36e32ac84 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -44,6 +44,7 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page BasePage.prototype.initialize.call(this, options); this.viewClass = options.viewClass || this.defaultViewClass; this.isLibraryPage = (this.model.attributes.category === 'library'); + this.isLibrarySourced = (this.model.attributes.category === 'library_sourced'); this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -97,7 +98,8 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page model: this.model }); this.unitOutlineView.render(); - + } + if (this.isLibrarySourced) { this.selectedLibraryComponents = []; this.storeSelectedLibraryComponents = []; this.getSelectedLibraryComponents(); diff --git a/cms/templates/container.html b/cms/templates/container.html index 909360832610..6310e39d3e2d 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -144,7 +144,7 @@

    ${_("Page Actions")}

    • % else: - % if True: + % if is_sourced_block:
  • % endif
    - % if True: + % if selectable:
    • % else: + % if True: +
  • + +
    • + % endif
  • diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 1bd2c3070e30..e5bcfe19c730 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -73,7 +73,16 @@ % endif
    + % if True: + + + % else: ${label} + % endif % if selected_groups_label:

    ${selected_groups_label}

    % endif From 9e0d22d6f92fa1fa16c10214bbe78aa4f5ba6760 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Thu, 23 Jun 2022 15:07:52 +0300 Subject: [PATCH 26/71] feat: show selected blocks for student_view --- xmodule/library_sourced_block.py | 84 +++++++++++++++++--------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index 4ab0cf6b6e49..aa6a9a022d74 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -8,13 +8,13 @@ import bleach from lazy import lazy from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 from pkg_resources import resource_string from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock from xblock.fields import List, Scope, String -from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin @@ -22,7 +22,7 @@ from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin from xmodule.util.xmodule_django import add_webpack_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage -from xmodule.x_module import HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js +from xmodule.x_module import STUDENT_VIEW, HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js log = logging.getLogger(__name__) loader = ResourceLoader(__name__) @@ -100,7 +100,6 @@ class LibrarySourcedBlock( studio_view_css = { 'scss': [], } - MAX_BLOCKS_ALLOWED = 10 def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" @@ -117,7 +116,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): context['reorderable_items'].add(child.location) context['can_add'] = can_add context['selected'] = str(child.location) in self.source_block_ids - rendered_child = child.render(StudioEditableModule.get_preview_view_name(child), context) + rendered_child = child.render(EditableChildrenMixin.get_preview_view_name(child), context) fragment.add_fragment_resources(rendered_child) contents.append({ @@ -157,33 +156,40 @@ def author_view(self, context): fragment.initialize_js('LibrarySourceAuthorView') return fragment - def student_view(self, context): + def _get_selected_child_blocks(self): """ - Renders the view that learners see. + Generator returning XBlock instances of the children selected for the + current user. """ - 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_field_data(self, validation, data): + for block_id in self.source_block_ids: + usage_key = UsageKey.from_string(block_id) + yield self.runtime.get_block(usage_key) + + def student_view(self, context): """ - Validate this block's field data. Instead of checking fields like self.name, check the - fields set on data, e.g. data.name. This allows the same validation method to be re-used - for the studio editor. + Renders the view that learners see. """ - if len(data.source_block_ids) > self.MAX_BLOCKS_ALLOWED: - # Because importing library blocks is an expensive operation - validation.add( - ValidationMessage( - ValidationMessage.ERROR, - _("A maximum of {0} components may be added.").format(self.MAX_BLOCKS_ALLOWED) - ) - ) + fragment = Fragment() + child_context = {} if not context else copy(context) + + fragment.add_content('
    ') + for child in self._get_selected_child_blocks(): + if child is None: + # TODO: Fix the underlying issue in TNL-7424 + # This shouldn't be happening, but does for an as-of-now + # unknown reason. Until we address the underlying issue, + # let's at least log the error explicitly, ignore the + # exception, and prevent the page from resulting in a + # 500-response. + log.error('Skipping display for child block that is None') + continue + for displayable in child.displayable_items(): + rendered_child = displayable.render(STUDENT_VIEW, child_context) + fragment.add_content(rendered_child.content) + fragment.add_fragment_resources(rendered_child) + + fragment.add_content('
    ') + return fragment def _validate_library_version(self, validation, lib_tools, version, library_key): """ @@ -285,6 +291,17 @@ def tools(self): """ return self.runtime.service(self, 'library_tools') + def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument + """ + If source_library_id has been edited, refresh_children automatically. + """ + old_source_library_id = old_metadata.get('source_library_id', []) + if old_source_library_id != self.source_library_id: + try: + self.refresh_children() + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + @XBlock.handler def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: disable=unused-argument """ @@ -305,22 +322,9 @@ def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: self.tools.update_children(self, user_perms) return Response() - def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument - """ - If source_library_id has been edited, refresh_children automatically. - """ - old_source_library_id = old_metadata.get('source_library_id', []) - if old_source_library_id != self.source_library_id: - try: - self.refresh_children() - except ValueError: - pass # The validation area will display an error message, no need to do anything now. - @XBlock.handler def get_block_ids(self, request, suffix=''): # lint-amnesty, pylint: disable=unused-argument """ Return source_block_ids. """ return Response(json.dumps({'source_block_ids': self.source_block_ids})) - -StudioEditableModule = EditableChildrenMixin From aab474227bc867f8f97da8a90b1d5f70941b70a4 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 24 Jun 2022 15:06:49 +0300 Subject: [PATCH 27/71] test: update tests for LibrarySourcedBlock --- xmodule/tests/test_library_sourced_block.py | 66 +++++++++------------ 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/xmodule/tests/test_library_sourced_block.py b/xmodule/tests/test_library_sourced_block.py index 246a6fa05a94..5b4de29582d6 100644 --- a/xmodule/tests/test_library_sourced_block.py +++ b/xmodule/tests/test_library_sourced_block.py @@ -2,63 +2,53 @@ Tests for Source from Library XBlock """ -from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest -from common.djangoapps.student.roles import CourseInstructorRole -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.library_tools import LibraryToolsService +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory +from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.tests import get_test_system from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=unused-import +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest + -class LibrarySourcedBlockTestCase(ContentLibrariesRestApiTest): +class LibrarySourcedBlockTestCase(MixedSplitTestCase, ContentLibrariesRestApiTest): """ Tests for LibraryToolsService which interact with blockstore-based content libraries """ def setUp(self): super().setUp() - self.store = modulestore() + self.tools = LibraryToolsService(self.store, self.user_id) + self.library = LibraryFactory.create(modulestore=self.store) + # Add content to the library + self.lib_blocks = [ + self.make_block("html", self.library, data=f"Hello world from block {i}") + for i in range(1, 5) + ] # 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 - self.source_block = ItemFactory.create( - category="library_sourced", - parent=course, - parent_location=course.location, - user_id=self.user.id, - modulestore=self.store + self.course = CourseFactory.create(modulestore=self.store) + self.chapter = self.make_block("chapter", self.course) + self.sequential = self.make_block("sequential", self.chapter) + self.vertical = self.make_block("vertical", self.sequential) + # Add a LibrarySourcedBlock to the course + self.source_block = self.make_block( + "library_sourced", + self.vertical, + source_library_id=str(self.library.location.library_key) ) self.submit_url = f'/xblock/{self.source_block.scope_ids.usage_id}/handler/submit_studio_edits' 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_1 = self._add_block_to_library(library["id"], "html", "html_student_preview_1")["id"] - self._set_library_block_olx(html_block_1, 'Student Preview Test 1') - html_block_2 = self._add_block_to_library(library["id"], "html", "html_student_preview_2")["id"] - self._set_library_block_olx(html_block_2, 'Student Preview Test 2') - # Import the html blocks from the library to the course - post_data = {"values": {"source_block_ids": [html_block_1, html_block_2]}, "defaults": ["display_name"]} + self.source_block.refresh_children() + # Save children block_ids as source_block_ids + source_block_ids = [str(child) for child in self.source_block.children] + post_data = {"values": {"source_block_ids": source_block_ids}} res = self.client.post(self.submit_url, data=post_data, format='json') # Check if student_view renders the children correctly res = self.get_block_view(self.source_block, STUDENT_VIEW) - assert 'Student Preview Test 1' in res - assert 'Student Preview Test 2' in res - - def test_block_limits(self): - # Create a blockstore content library - library = self._create_library(slug="testlib2_preview", title="Test Library 2", description="Testing XBlocks") - # Add content to the library - blocks = [self._add_block_to_library(library["id"], "html", f"block_{i}")["id"] for i in range(11)] - - # Import the html blocks from the library to the course - post_data = {"values": {"source_block_ids": blocks}, "defaults": ["display_name"]} - res = self.client.post(self.submit_url, data=post_data, format='json') - assert res.status_code == 400 - assert res.json()['error']['messages'][0]['text'] == 'A maximum of 10 components may be added.' + for i in range(1, 5): + assert f"Hello world from block {i}" in res def get_block_view(self, block, view, context=None): """ From 1cfeca91b993c270f51c2cf5bb18500afee0db78 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 24 Jun 2022 18:48:39 +0300 Subject: [PATCH 28/71] feat: add context for library_sourced container page --- cms/djangoapps/contentstore/views/component.py | 3 ++- cms/djangoapps/contentstore/views/preview.py | 3 ++- cms/static/js/views/pages/container.js | 4 +++- cms/templates/container.html | 2 +- cms/templates/studio_xblock_wrapper.html | 2 +- xmodule/library_sourced_block.py | 16 +++++++++++++--- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index c8173c72283f..34914ef98f4b 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -202,7 +202,8 @@ def container_handler(request, usage_key_string): 'xblock_info': xblock_info, 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, - 'templates': CONTAINER_TEMPLATES + 'templates': CONTAINER_TEMPLATES, + 'is_sourced_block': xblock.location.block_type == 'library_sourced', }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index d9103d634663..ba418ff8be23 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -309,7 +309,8 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, - 'is_selected': context.get('selected', False), + 'is_selected': context.get('is_selected', False), + 'selectable': context.get('selectable', False), 'can_edit': context.get('can_edit', True), 'can_edit_visibility': context.get('can_edit_visibility', xblock.scope_ids.usage_id.context_key.is_course), 'selected_groups_label': selected_groups_label, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 7b5ebe19bb2a..d1e36e32ac84 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -44,6 +44,7 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page BasePage.prototype.initialize.call(this, options); this.viewClass = options.viewClass || this.defaultViewClass; this.isLibraryPage = (this.model.attributes.category === 'library'); + this.isLibrarySourced = (this.model.attributes.category === 'library_sourced'); this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -97,7 +98,8 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page model: this.model }); this.unitOutlineView.render(); - + } + if (this.isLibrarySourced) { this.selectedLibraryComponents = []; this.storeSelectedLibraryComponents = []; this.getSelectedLibraryComponents(); diff --git a/cms/templates/container.html b/cms/templates/container.html index 909360832610..6310e39d3e2d 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -144,7 +144,7 @@

    ${_("Page Actions")}

    • % else: - % if True: + % if is_sourced_block:
  • % endif
    - % if True: + % if selectable:
    • + % if is_collapsible: +
  • + + + ${_("Collapse All")} + +
    • + % endif % endif @@ -169,8 +177,7 @@

    ${_("Page Actions")}

    - -
    +
    diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 4a6740ea5d36..a7d89adc7bbe 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -16,7 +16,6 @@ xblock_url = xblock_studio_url(xblock) show_inline = xblock.has_children and not xblock_url section_class = "level-nesting" if show_inline else "level-element" -collapsible_class = "is-collapsible" if xblock.has_children else "" label = determine_label(xblock.display_name_with_default, xblock.scope_ids.block_type) messages = xblock.validate().to_json() block_is_unit = is_unit(xblock) @@ -47,7 +46,7 @@
    % endif -
    % endif + % if can_collapse: +
  • + +
    • + % endif % endif % if can_add: From ccd0fb01adf3047509e48610b3cfecc2ee01ee40 Mon Sep 17 00:00:00 2001 From: ihor-romaniuk Date: Tue, 14 Jun 2022 09:01:03 +0300 Subject: [PATCH 36/71] fix: prevent click on collapse button --- cms/static/js/views/pages/container.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 83475e134d6b..1bc00b19c638 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -219,14 +219,14 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page }, toggleChildrenPreviews: function toggleChildrenPreviews(event) { - var $button = $(event.currentTarget), - $el = $(this.xblockView.el); + event.preventDefault(); + var $button = $(event.currentTarget); if ($button.hasClass('collapsed')) { this.updateCollapseAllButton(true); - $el.find('.wrapper-xblock.is-collapsible').removeClass('is-collapsed'); + this.$el.find('.wrapper-xblock.is-collapsible').removeClass('is-collapsed'); } else { this.updateCollapseAllButton(false); - $el.find('.wrapper-xblock.is-collapsible').addClass('is-collapsed'); + this.$el.find('.wrapper-xblock.is-collapsible').addClass('is-collapsed'); } }, From f3ec7c8a66c8cba50f62df524cebc5bd4fe1e9db Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Wed, 15 Jun 2022 14:07:04 +0300 Subject: [PATCH 37/71] feat: add context for library xblocks --- cms/djangoapps/contentstore/views/component.py | 9 +++++---- cms/djangoapps/contentstore/views/preview.py | 3 +-- xmodule/library_content_module.py | 1 + xmodule/library_sourced_block.py | 1 + 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 920097d9ff20..03344b6fc7c3 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -184,6 +184,9 @@ def container_handler(request, usage_key_string): break index += 1 + library_problem_types = [problem_type['component'] for problem_type in LIBRARY_PROBLEM_TYPES] + is_library_xblock = xblock.location.block_type in library_problem_types + return render_to_response('container.html', { 'language_code': request.LANGUAGE_CODE, 'context_course': course, # Needed only for display of menus at top of page. @@ -192,8 +195,7 @@ def container_handler(request, usage_key_string): 'xblock_locator': xblock.location, 'unit': unit, 'is_unit_page': is_unit_page, - # ToDo: Add required context for library container only - 'is_collapsible': True, + 'is_collapsible': is_library_xblock, 'subsection': subsection, 'section': section, 'position': index, @@ -208,8 +210,7 @@ def container_handler(request, usage_key_string): 'published_preview_link': lms_link, 'templates': CONTAINER_TEMPLATES, 'is_sourced_block': xblock.location.block_type == 'library_sourced', - # ToDo: Add required context for library container only - 'is_fullwidth_content': True + 'is_fullwidth_content': is_library_xblock, }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 94c1c3a19897..73cfdc956ff6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -316,8 +316,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_loading': context.get('is_loading', False), 'is_selected': context.get('is_selected', False), 'selectable': context.get('selectable', False), - # ToDo: Add required context for library container only - 'can_collapse': context.get('can_collapse', True), + 'can_collapse': context.get('can_collapse', False), 'can_edit': context.get('can_edit', True), 'can_edit_visibility': context.get('can_edit_visibility', xblock.scope_ids.usage_id.context_key.is_course), 'selected_groups_label': selected_groups_label, diff --git a/xmodule/library_content_module.py b/xmodule/library_content_module.py index 6daab59217e3..e68ab0781ac7 100644 --- a/xmodule/library_content_module.py +++ b/xmodule/library_content_module.py @@ -445,6 +445,7 @@ def author_view(self, context): })) context['can_edit_visibility'] = False context['can_move'] = False + context['can_collapse'] = True self.render_children(context, fragment, can_reorder=False, can_add=False) # else: When shown on a unit page, don't show any sort of preview - # just the status of this block in the validation area. diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index c9ab381f4ef1..abaea724b20f 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -166,6 +166,7 @@ def author_view(self, context): # to make edits to the children context['can_move'] = False context['selectable'] = True + context['can_collapse'] = True self.render_children(context, fragment, can_reorder=False, can_add=False) return fragment context['is_loading'] = is_loading From bdd0dd8b5422d3a9b2bdb37fbcceb3aa092ad120 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Tue, 19 Jul 2022 14:50:19 +0300 Subject: [PATCH 38/71] feat: rename View button to Configure --- cms/djangoapps/contentstore/views/preview.py | 1 + cms/templates/studio_xblock_wrapper.html | 2 +- xmodule/library_sourced_block.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 73cfdc956ff6..9495c345d5c9 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -315,6 +315,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_reorderable': is_reorderable, 'is_loading': context.get('is_loading', False), 'is_selected': context.get('is_selected', False), + 'is_sourced_block': context.get('is_sourced_block', False), 'selectable': context.get('selectable', False), 'can_collapse': context.get('can_collapse', False), 'can_edit': context.get('can_edit', True), diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index a7d89adc7bbe..90b5d1289102 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -171,7 +171,7 @@
  • ## Translators: this is a verb describing the action of viewing more details - ${_('View')} + ${_('Configure') if is_sourced_block else _('View')}
    • diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index abaea724b20f..ab26ca4d8c1f 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -169,6 +169,7 @@ def author_view(self, context): context['can_collapse'] = True self.render_children(context, fragment, can_reorder=False, can_add=False) return fragment + context['is_sourced_block'] = True context['is_loading'] = is_loading fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_source_edit.js')) fragment.initialize_js('LibrarySourceAuthorView') From f6723d09898744cbdf959a32dcd9ff215ac88fa3 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Thu, 21 Jul 2022 13:34:49 +0300 Subject: [PATCH 39/71] fix: fix errors --- cms/djangoapps/contentstore/views/component.py | 4 ++-- docs/docs_settings.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 03344b6fc7c3..05a0da67b61d 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -184,8 +184,8 @@ def container_handler(request, usage_key_string): break index += 1 - library_problem_types = [problem_type['component'] for problem_type in LIBRARY_PROBLEM_TYPES] - is_library_xblock = xblock.location.block_type in library_problem_types + library_block_types = [problem_type['component'] for problem_type in LIBRARY_BLOCK_TYPES] + is_library_xblock = xblock.location.block_type in library_block_types return render_to_response('container.html', { 'language_code': request.LANGUAGE_CODE, diff --git a/docs/docs_settings.py b/docs/docs_settings.py index 8c1b813df17d..5bc9b1594697 100644 --- a/docs/docs_settings.py +++ b/docs/docs_settings.py @@ -37,7 +37,6 @@ 'cms.djangoapps.course_creators', 'cms.djangoapps.xblock_config.apps.XBlockConfig', 'lms.djangoapps.lti_provider', - 'user_tasks', ]) From 2954fdf54f152a1e2344980419a7b1a4d8fa2ac7 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Tue, 26 Jul 2022 14:31:51 +0300 Subject: [PATCH 40/71] fix: 500 error appears if instructor doesn't choose the Library --- xmodule/library_content_module.py | 5 +++-- xmodule/library_sourced_block.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/xmodule/library_content_module.py b/xmodule/library_content_module.py index e68ab0781ac7..876faa88d652 100644 --- a/xmodule/library_content_module.py +++ b/xmodule/library_content_module.py @@ -722,10 +722,11 @@ def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint def post_editor_saved(self): """ - If source_library_id has been edited, refresh_children automatically. + If xblock has been edited, refresh_children automatically. """ try: - self.refresh_children() + if self.source_library_id: + self.refresh_children() except ValueError: pass # The validation area will display an error message, no need to do anything now. diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index ab26ca4d8c1f..f568f70e8242 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -327,7 +327,8 @@ def post_editor_saved(self): # lint-amnesty, pylint: disable=unused-argument If source_library_id has been edited, refresh_children automatically. """ try: - self.refresh_children() + if self.source_library_id: + self.refresh_children() except ValueError: pass # The validation area will display an error message, no need to do anything now. From b69d173ca1c38f0b0f0eb841953eedcaa2445c23 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Tue, 26 Jul 2022 17:51:05 +0300 Subject: [PATCH 41/71] feat: Problem with resave xblock --- cms/djangoapps/contentstore/tests/test_libraries.py | 2 +- xmodule/library_content_module.py | 4 ++-- xmodule/library_sourced_block.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index b1897fdb5787..5bebe5b1f760 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -358,7 +358,7 @@ def test_change_after_first_sync(self): ) self.assertEqual(resp.status_code, 200) lc_block = modulestore().get_item(lc_block.location) - self.assertEqual(len(lc_block.children), 0) # Children deleted due to a changing source_library_id. + self.assertEqual(len(lc_block.children), 1) # Children should not be deleted due to a bad setting. def test_refreshes_children_if_libraries_change(self): """ Tests that children are automatically refreshed if libraries list changes """ diff --git a/xmodule/library_content_module.py b/xmodule/library_content_module.py index 876faa88d652..441ccf32e2cb 100644 --- a/xmodule/library_content_module.py +++ b/xmodule/library_content_module.py @@ -10,7 +10,6 @@ from gettext import ngettext import bleach -from capa.responsetypes import registry from django.conf import settings from django.utils.functional import classproperty from lazy import lazy @@ -26,6 +25,7 @@ from xblock.core import XBlock from xblock.fields import Boolean, Integer, List, Scope, String +from xmodule.capa.responsetypes import registry from xmodule.mako_module import MakoTemplateBlockBase from xmodule.studio_editable import StudioEditableBlock from xmodule.util.xmodule_django import add_webpack_to_fragment @@ -714,8 +714,8 @@ def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint """ If source_library_id is empty, clear source_library_version and children. """ - self.children = [] # lint-amnesty, pylint: disable=attribute-defined-outside-init if not self.source_library_id: + self.children = [] # lint-amnesty, pylint: disable=attribute-defined-outside-init self.source_library_version = "" else: self.source_library_version = str(self.tools.get_library_version(self.source_library_id)) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index f568f70e8242..f0d2b2525bff 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -316,8 +316,8 @@ def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint """ If source_library_id is empty, clear source_library_version and children. """ - self.children = [] # lint-amnesty, pylint: disable=attribute-defined-outside-init if not self.source_library_id: + self.children = [] # lint-amnesty, pylint: disable=attribute-defined-outside-init self.source_library_version = "" else: self.source_library_version = str(self.tools.get_library_version(self.source_library_id)) From 5d04c53872222d2b8ac44c0b114a5f50ed355248 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Tue, 26 Jul 2022 14:45:28 +0300 Subject: [PATCH 42/71] fix: "Configure" replace to "View" --- cms/djangoapps/contentstore/views/preview.py | 1 - cms/templates/studio_xblock_wrapper.html | 1 + xmodule/library_sourced_block.py | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 9495c345d5c9..73cfdc956ff6 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -315,7 +315,6 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_reorderable': is_reorderable, 'is_loading': context.get('is_loading', False), 'is_selected': context.get('is_selected', False), - 'is_sourced_block': context.get('is_sourced_block', False), 'selectable': context.get('selectable', False), 'can_collapse': context.get('can_collapse', False), 'can_edit': context.get('can_edit', True), diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 90b5d1289102..9bddde5e072a 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -19,6 +19,7 @@ label = determine_label(xblock.display_name_with_default, xblock.scope_ids.block_type) messages = xblock.validate().to_json() block_is_unit = is_unit(xblock) +is_sourced_block = xblock.location.block_type == 'library_sourced' %> <%namespace name='static' file='static_content.html'/> diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index f0d2b2525bff..d1dfb6135a8e 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -169,7 +169,6 @@ def author_view(self, context): context['can_collapse'] = True self.render_children(context, fragment, can_reorder=False, can_add=False) return fragment - context['is_sourced_block'] = True context['is_loading'] = is_loading fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_source_edit.js')) fragment.initialize_js('LibrarySourceAuthorView') From 3a4805f78fd4d1101f496ebf9bc98c81b05d779f Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 22 Jul 2022 15:25:30 +0300 Subject: [PATCH 43/71] feat: remove library component from library category --- cms/djangoapps/contentstore/views/component.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 05a0da67b61d..a527d67c12ee 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -316,7 +316,7 @@ def create_support_legend_dict(): templates_for_category = [] component_class = _load_mixed_class(category) - if support_level_without_template: + if support_level_without_template and category != 'library': # add the default template with localized display name # TODO: Once mixins are defined per-application, rather than per-runtime, # this should use a cms mixed-in class. (cpennington) From bd003311cbf3ff99f7e551b464c6368e64c80552 Mon Sep 17 00:00:00 2001 From: Eugene Dyudyunov Date: Thu, 16 Jun 2022 10:44:03 +0300 Subject: [PATCH 44/71] feat: implement V2 libraries usage for library content block 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. --- .../public/js/library_content_edit_helpers.js | 54 ++++++ xmodule/library_content_module.py | 38 ++++- xmodule/library_tools.py | 158 ++++++++++++------ .../split_mongo/caching_descriptor_system.py | 7 +- xmodule/tests/test_library_content.py | 33 +++- xmodule/tests/test_library_tools.py | 141 ++++++++++++++-- 6 files changed, 349 insertions(+), 82 deletions(-) create mode 100644 xmodule/assets/library_content/public/js/library_content_edit_helpers.js diff --git a/xmodule/assets/library_content/public/js/library_content_edit_helpers.js b/xmodule/assets/library_content/public/js/library_content_edit_helpers.js new file mode 100644 index 000000000000..564cc5fb0fbe --- /dev/null +++ b/xmodule/assets/library_content/public/js/library_content_edit_helpers.js @@ -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) diff --git a/xmodule/library_content_module.py b/xmodule/library_content_module.py index 0d9d4e080f10..66858d4d1f36 100644 --- a/xmodule/library_content_module.py +++ b/xmodule/library_content_module.py @@ -8,7 +8,6 @@ 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 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.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 @@ 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 + 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 @@ 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 @@ -517,6 +523,22 @@ 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): """ diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index fd99f41dbd49..e4730813d29a 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -5,20 +5,28 @@ 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 @@ def can_use_library_content(self, block): 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 @@ def update_children(self, dest_block, user_perms=None, version=None): 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) + 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 @@ 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 != 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, diff --git a/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 6849cc2c6811..d5c7c357ddf3 100644 --- a/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -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 @@ def __init__(self, modulestore, course_entry, default_class, module_data, lazy, 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 diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index 3e1785746b46..6d7ced75930a 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -4,16 +4,18 @@ 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.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 @@ def get_module(descriptor): 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 diff --git a/xmodule/tests/test_library_tools.py b/xmodule/tests/test_library_tools.py index 950307b6963d..0e5f59aac45a 100644 --- a/xmodule/tests/test_library_tools.py +++ b/xmodule/tests/test_library_tools.py @@ -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 @@ def test_list_available_libraries_fetch(self, mock_get_library_summaries): _ = 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 @@ def test_import_from_blockstore(self): 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 From f4295152fe99c5a6f0a49f64e1925f3431acaeb6 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Tue, 9 Aug 2022 16:01:28 +0300 Subject: [PATCH 45/71] fix: fix updating preview for library xblocks after uploading childrens --- .../library_content/public/js/library_content_edit.js | 6 +++++- .../library_source_block/public/js/library_source_edit.js | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/xmodule/assets/library_content/public/js/library_content_edit.js b/xmodule/assets/library_content/public/js/library_content_edit.js index 004e1ee2b0a3..08e877137d32 100644 --- a/xmodule/assets/library_content/public/js/library_content_edit.js +++ b/xmodule/assets/library_content/public/js/library_content_edit.js @@ -37,12 +37,16 @@ window.LibraryContentAuthorView = function(runtime, element) { var $loader = $wrapper.find('.ui-loading'); var $xblockHeader = $wrapper.find('.xblock-header'); if (!$loader.hasClass('is-hidden')) { - var timer = setInterval(function() { + var timer = setInterval(function() { $.get(runtime.handlerUrl(element, 'get_import_task_status'), function( data ) { if (data.status == 'Succeeded') { $loader.addClass('is-hidden'); $xblockHeader.removeClass('is-hidden'); clearInterval(timer); + runtime.notify('save', { + state: 'end', + element: element + }); } }) }, 1000); diff --git a/xmodule/assets/library_source_block/public/js/library_source_edit.js b/xmodule/assets/library_source_block/public/js/library_source_edit.js index 58f479e0a77d..23a56576827a 100644 --- a/xmodule/assets/library_source_block/public/js/library_source_edit.js +++ b/xmodule/assets/library_source_block/public/js/library_source_edit.js @@ -43,6 +43,10 @@ window.LibrarySourceAuthorView = function(runtime, element) { $loader.addClass('is-hidden'); $xblockHeader.removeClass('is-hidden'); clearInterval(timer); + runtime.notify('save', { + state: 'end', + element: element + }); } }) }, 1000); From d7e808068b38c3604e6b8c0929396e1ce0e0bb76 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Wed, 10 Aug 2022 13:18:40 +0300 Subject: [PATCH 46/71] feat: add validation for empty library --- xmodule/library_sourced_block.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index d1dfb6135a8e..c95b082bd18a 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -266,6 +266,17 @@ def validate(self): ) return validation + if not self.children: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.NOT_CONFIGURED, + _("There are no problem types in the specified libraries."), + action_class='edit-button', + action_label=_("Select another Library.") + ) + ) + return validation + self._validate_library_version(validation, self.tools, self.source_library_version, self.source_library_key) return validation From fba1ae705d038ee57f5a22f27148d71f2c1c2a76 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Mon, 20 Jun 2022 13:56:56 +0300 Subject: [PATCH 47/71] feat: Edit modal for Library Sourced Content --- .../public/js/library_source_edit.js | 36 +++ xmodule/library_sourced_block.py | 231 ++++++++++++++---- xmodule/library_tools.py | 5 +- xmodule/static_content.py | 2 + 4 files changed, 229 insertions(+), 45 deletions(-) create mode 100644 xmodule/assets/library_source_block/public/js/library_source_edit.js diff --git a/xmodule/assets/library_source_block/public/js/library_source_edit.js b/xmodule/assets/library_source_block/public/js/library_source_edit.js new file mode 100644 index 000000000000..6f1624467097 --- /dev/null +++ b/xmodule/assets/library_source_block/public/js/library_source_edit.js @@ -0,0 +1,36 @@ +/* JavaScript for special editing operations that can be done on LibraryContentXBlock */ +window.LibrarySourceAuthorView = function(runtime, element) { + 'use strict'; + var $element = $(element); + var usage_id = $element.data('usage-id'); + // The "Update Now" button is not a child of 'element', as it is in the validation message area + // But it is still inside this xblock's wrapper element, which we can easily find: + var $wrapper = $element.parents('*[data-locator="' + usage_id + '"]'); + + // We can't bind to the button itself because in the bok choy test environment, + // it may not yet exist at this point in time... not sure why. + $wrapper.on('click', '.library-update-btn', function(e) { + e.preventDefault(); + // Update the XBlock with the latest matching content from the library: + runtime.notify('save', { + state: 'start', + element: element, + message: gettext('Updating with latest library content') + }); + $.post(runtime.handlerUrl(element, 'refresh_children')).done(function() { + runtime.notify('save', { + state: 'end', + element: element + }); + if ($element.closest('.wrapper-xblock').is(':not(.level-page)')) { + // We are on a course unit page. The notify('save') should refresh this block, + // but that is only working on the container page view of this block. + // Why? On the unit page, this XBlock's runtime has no reference to the + // XBlockContainerPage - only the top-level XBlock (a vertical) runtime does. + // But unfortunately there is no way to get a reference to our parent block's + // JS 'runtime' object. So instead we must refresh the whole page: + location.reload(); + } + }); + }); +}; diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index d96741d6e447..ab34f633e5c9 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -1,20 +1,27 @@ """ Library Sourced Content XBlock """ +import json import logging - from copy import copy -from mako.template import Template as MakoTemplate + +import bleach +from lazy import lazy +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from pkg_resources import resource_string +from web_fragments.fragment import Fragment +from webob import Response from xblock.core import XBlock -from xblock.fields import Scope, String, List +from xblock.fields import List, Scope, String from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader -from xblockutils.studio_editable import StudioEditableXBlockMixin -from webob import Response -from web_fragments.fragment import Fragment +from xmodule.mako_module import MakoTemplateBlockBase from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin +from xmodule.util.xmodule_django import add_webpack_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage +from xmodule.x_module import HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js log = logging.getLogger(__name__) loader = ResourceLoader(__name__) @@ -25,7 +32,15 @@ @XBlock.wants('library_tools') # Only needed in studio -class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlock): +@XBlock.wants('studio_user_permissions') # Only available in studio +class LibrarySourcedBlock( + MakoTemplateBlockBase, + XModuleToXBlockMixin, + HTMLSnippet, + ResourceTemplates, + XModuleMixin, + EditableChildrenMixin, +): """ Library Sourced Content XBlock @@ -42,6 +57,17 @@ class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlo display_name=_("Display Name"), scope=Scope.content, ) + source_library_id = String( + display_name=_("Library"), + help=_("Select the library from which you want to draw content."), + scope=Scope.settings, + values_provider=lambda instance: instance.source_library_values(), + ) + source_library_version = String( + # This is a hidden field that stores the version of source_library when we last pulled content from it + display_name=_("Library Version"), + scope=Scope.settings, + ) source_block_ids = List( display_name=_("Library Blocks List"), help=_("Enter the IDs of the library XBlocks that you wish to use."), @@ -50,38 +76,42 @@ class LibrarySourcedBlock(StudioEditableXBlockMixin, EditableChildrenMixin, XBlo editable_fields = ("display_name", "source_block_ids") has_children = True has_author_view = True + resources_dir = 'assets/library_source_block' + + preview_view_js = { + 'js': [], + 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + } + preview_view_css = { + 'scss': [], + } + + mako_template = 'widgets/metadata-edit.html' + studio_js_module_name = "VerticalDescriptor" + studio_view_js = { + 'js': [ + resource_string(__name__, 'js/src/vertical/edit.js'), + ], + 'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'), + } + studio_view_css = { + 'scss': [], + } MAX_BLOCKS_ALLOWED = 10 def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if not self.source_block_ids: - self.has_children = False - - def studio_view(self, context): + def studio_view(self, _context): """ - Render a form for editing this XBlock + Return the studio view. """ - fragment = Fragment() - static_content = ResourceLoader('common.djangoapps.pipeline_mako').load_unicode('templates/static_content.html') - render_react = MakoTemplate(static_content, default_filters=[]).get_def('renderReact') - react_content = render_react.render( - component="LibrarySourcedBlockPicker", - id="library-sourced-block-picker", - props={ - 'selectedXblocks': self.source_block_ids, - } + fragment = Fragment( + self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context()) ) - fragment.content = loader.render_django_template('templates/library-sourced-block-studio-view.html', { - 'react_content': react_content, - 'save_url': self.runtime.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('LibrarySourceBlockStudioView') + add_webpack_to_fragment(fragment, 'LibrarySourcedBlockStudio') + shim_xmodule_js(fragment, self.studio_js_module_name) return fragment @@ -94,6 +124,8 @@ def author_view(self, context): # 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) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_source_edit.js')) + fragment.initialize_js('LibrarySourceAuthorView') return fragment def student_view(self, context): @@ -124,6 +156,42 @@ def validate_field_data(self, validation, data): ) ) + def _validate_library_version(self, validation, lib_tools, version, library_key): + """ + Validates library version + """ + latest_version = lib_tools.get_library_version(library_key) + if latest_version is not None: + if version is None or version != str(latest_version): + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + _('This component is out of date. The library has new content.'), + # TODO: change this to action_runtime_event='...' once the unit page supports that feature. + # See https://openedx.atlassian.net/browse/TNL-993 + action_class='library-update-btn', + # Translators: {refresh_icon} placeholder is substituted to "↻" (without double quotes) + action_label=_("{refresh_icon} Update now.").format(refresh_icon="↻") + ) + ) + return False + else: + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.ERROR, + _('Library is invalid, corrupt, or has been deleted.'), + action_class='edit-button', + action_label=_("Edit Library List.") + ) + ) + return False + return True + + def _set_validation_error_if_empty(self, validation, summary): + """ Helper method to only set validation summary if it's empty """ + if validation.empty: + validation.set_summary(summary) + def validate(self): """ Validates the state of this library_sourced_xblock Instance. This is the override of the general XBlock method, @@ -132,28 +200,103 @@ def validate(self): validation = super().validate() validation = StudioValidation.copy(validation) - if not self.source_block_ids: + if not self.source_library_id: validation.set_summary( StudioValidationMessage( StudioValidationMessage.NOT_CONFIGURED, - _("No XBlock has been configured for this component. Use the editor to select the target blocks."), + _("A library has not yet been selected."), action_class='edit-button', - action_label=_("Open Editor") + action_label=_("Select a Library.") ) ) + self._validate_library_version(validation, self.tools, self.source_library_version, self.source_library_key) return validation - @XBlock.handler - def submit_studio_edits(self, data, suffix=''): + def source_library_values(self): + """ + Return a list of possible values for self.source_library_id + """ + user_perms = self.runtime.service(self, 'studio_user_permissions') + all_libraries = [ + (key, bleach.clean(name)) for key, name in self.tools.list_available_libraries() + if user_perms.can_read(key) or self.source_library_id == str(key) + ] + all_libraries.sort(key=lambda entry: entry[1]) # Sort by name + if self.source_library_id and self.source_library_key not in [entry[0] for entry in all_libraries]: + all_libraries.append((self.source_library_id, _("Invalid Library"))) + all_libraries = [("", _("No Library Selected"))] + all_libraries + values = [{"display_name": name, "value": str(key)} for key, name in all_libraries] + return values + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super().non_editable_metadata_fields + non_editable_fields.extend([ + LibrarySourcedBlock.source_block_ids, + LibrarySourcedBlock.source_library_version, + ]) + return non_editable_fields + + @property + def source_library_key(self): """ - Save changes to this block, applying edits made in Studio. + Convenience method to get the library ID as a LibraryLocator and not just a string """ - 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_ids) - except Exception as err: # pylint: disable=broad-except - log.exception(err) - return Response(_("Importing Library Block failed - are the IDs valid and readable?"), status=400) - return response + return LibraryLocator.from_string(self.source_library_id) + except InvalidKeyError: + return LibraryLocatorV2.from_string(self.source_library_id) + + @lazy + def tools(self): + """ + Grab the library tools service or raise an error. + """ + return self.runtime.service(self, 'library_tools') + + @XBlock.handler + def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: disable=unused-argument + """ + Refresh children: + This method is to be used when any of the libraries that this block + references have been updated. It will re-fetch all matching blocks from + the libraries, and copy them as children of this block. The children + will be given new block_ids, but the definition ID used should be the + exact same definition ID used in the library. + + This method will update this block's 'source_library_id' field to store + the version number of the libraries used, so we easily determine if + this block is up to date or not. + """ + user_perms = self.runtime.service(self, 'studio_user_permissions') + if not self.tools: + return Response("Library Tools unavailable in current runtime.", status=400) + self.tools.update_children(self, user_perms) + return Response() + + def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument + """ + If source_library_id has been edited, refresh_children automatically. + """ + old_source_library_id = old_metadata.get('source_library_id', []) + if old_source_library_id != self.source_library_id: + try: + self.refresh_children() + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + + @XBlock.json_handler + def set_block_ids(self, data, suffix=''): + """ + Save source_block_ids. + """ + if data.get('source_library_id'): + self.source_library_id = data.get('source_library_id') + return {'source_library_id': self.source_library_id} + + @XBlock.handler + def get_block_ids(self, data, suffix=''): + """ + Return source_block_ids. + """ + return Response(json.dumps({'source_library_id': self.source_library_id})) diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index e4730813d29a..985f9e0f4266 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -200,7 +200,10 @@ def update_children(self, dest_block, user_perms=None, version=None): if library is None: raise ValueError(f"Requested library {library_key} not found.") - filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE) + if hasattr(dest_block, 'capa_type'): + filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE) + else: + filter_children = None if not is_v2_lib: if user_perms and not user_perms.can_read(library_key): diff --git a/xmodule/static_content.py b/xmodule/static_content.py index 3c891cd750c2..ea6931513ad8 100755 --- a/xmodule/static_content.py +++ b/xmodule/static_content.py @@ -24,6 +24,7 @@ from xmodule.conditional_module import ConditionalBlock from xmodule.html_module import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock from xmodule.library_content_module import LibraryContentBlock +from xmodule.library_sourced_block import LibrarySourcedBlock from xmodule.lti_module import LTIBlock from xmodule.poll_module import PollBlock from xmodule.seq_module import SequenceBlock @@ -78,6 +79,7 @@ class VideoBlock(HTMLSnippet): # lint-amnesty, pylint: disable=abstract-method CustomTagBlock, HtmlBlock, LibraryContentBlock, + LibrarySourcedBlock, LTIBlock, PollBlock, ProblemBlock, From b87a23726b777ffdf433dd0294fe8b06fe60f314 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Mon, 20 Jun 2022 13:57:27 +0300 Subject: [PATCH 48/71] feat: remove react studio view for Library Sourced Content --- webpack.common.config.js | 13 - .../LibrarySourcedBlockPicker.jsx | 223 ------------------ .../public/js/library_source_block.js | 58 ----- xmodule/assets/library_source_block/style.css | 58 ----- .../library-sourced-block-studio-view.html | 19 -- 5 files changed, 371 deletions(-) delete mode 100644 xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx delete mode 100644 xmodule/assets/library_source_block/public/js/library_source_block.js delete mode 100644 xmodule/assets/library_source_block/style.css delete mode 100644 xmodule/templates/library-sourced-block-studio-view.html diff --git a/webpack.common.config.js b/webpack.common.config.js index 7aa2d02d1042..53c242c095f9 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -72,7 +72,6 @@ module.exports = Merge.smart({ // Studio Import: './cms/static/js/features/import/factories/import.js', CourseOrLibraryListing: './cms/static/js/features_jsx/studio/CourseOrLibraryListing.jsx', - LibrarySourcedBlockPicker: './xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx', // eslint-disable-line max-len 'js/factories/textbooks': './cms/static/js/factories/textbooks.js', 'js/factories/container': './cms/static/js/factories/container.js', 'js/factories/context_course': './cms/static/js/factories/context_course.js', @@ -342,18 +341,6 @@ module.exports = Merge.smart({ { test: /logger/, loader: 'imports-loader?this=>window' - }, - { - test: /\.css$/, - use: [ - 'style-loader', - { - loader: 'css-loader', - options: { - modules: true - } - } - ] } ] }, diff --git a/xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx b/xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx deleted file mode 100644 index 74b1d8b485a2..000000000000 --- a/xmodule/assets/library_source_block/LibrarySourcedBlockPicker.jsx +++ /dev/null @@ -1,223 +0,0 @@ -/* globals gettext */ - -import 'whatwg-fetch'; -import PropTypes from 'prop-types'; -import React from 'react'; -import _ from 'underscore'; -import styles from './style.css'; - -class LibrarySourcedBlockPicker extends React.Component { - constructor(props) { - super(props); - this.state = { - libraries: [], - xblocks: [], - searchedLibrary: '', - libraryLoading: false, - xblocksLoading: false, - selectedLibrary: undefined, - selectedXblocks: new Set(this.props.selectedXblocks), - }; - this.onLibrarySearchInput = this.onLibrarySearchInput.bind(this); - this.onXBlockSearchInput = this.onXBlockSearchInput.bind(this); - this.onLibrarySelected = this.onLibrarySelected.bind(this); - this.onXblockSelected = this.onXblockSelected.bind(this); - this.onDeleteClick = this.onDeleteClick.bind(this); - } - - componentDidMount() { - this.fetchLibraries(); - } - - fetchLibraries(textSearch='', page=1, append=false) { - this.setState({ - libraries: append ? this.state.libraries : [], - libraryLoading: true, - }, async function() { - try { - let res = await fetch(`/api/libraries/v2/?pagination=true&page=${page}&text_search=${textSearch}`); - res = await res.json(); - this.setState({ - libraries: this.state.libraries.concat(res.results), - libraryLoading: false, - }, () => { - if (res.next) { - this.fetchLibraries(textSearch, page+1, true); - } - }); - } catch (error) { - $('#library-sourced-block-picker').trigger('error', { - title: 'Could not fetch library', - message: error, - }); - this.setState({ - libraries: [], - libraryLoading: false, - }); - } - }); - } - - fetchXblocks(library, textSearch='', page=1, append=false) { - this.setState({ - xblocks: append ? this.state.xblocks : [], - xblocksLoading: true, - }, async function() { - try { - let res = await fetch(`/api/libraries/v2/${library}/blocks/?pagination=true&page=${page}&text_search=${textSearch}`); - res = await res.json(); - this.setState({ - xblocks: this.state.xblocks.concat(res.results), - xblocksLoading: false, - }, () => { - if (res.next) { - this.fetchXblocks(library, textSearch, page+1, true); - } - }); - } catch (error) { - $('#library-sourced-block-picker').trigger('error', { - title: 'Could not fetch xblocks', - message: error, - }); - this.setState({ - xblocks: [], - xblocksLoading: false, - }); - } - }); - } - - onLibrarySearchInput(event) { - event.persist() - this.setState({ - searchedLibrary: event.target.value, - }); - if (!this.debouncedFetchLibraries) { - this.debouncedFetchLibraries = _.debounce(value => { - this.fetchLibraries(value); - }, 300); - } - this.debouncedFetchLibraries(event.target.value); - } - - onXBlockSearchInput(event) { - event.persist() - if (!this.debouncedFetchXblocks) { - this.debouncedFetchXblocks = _.debounce(value => { - this.fetchXblocks(this.state.selectedLibrary, value); - }, 300); - } - this.debouncedFetchXblocks(event.target.value); - } - - onLibrarySelected(event) { - this.setState({ - selectedLibrary: event.target.value, - }); - this.fetchXblocks(event.target.value); - } - - onXblockSelected(event) { - let state = new Set(this.state.selectedXblocks); - if (event.target.checked) { - state.add(event.target.value); - } else { - state.delete(event.target.value); - } - this.setState({ - selectedXblocks: state, - }, this.updateList); - } - - onDeleteClick(event) { - let value; - if (event.target.tagName == 'SPAN') { - value = event.target.parentElement.dataset.value; - } else { - value = event.target.dataset.value; - } - let state = new Set(this.state.selectedXblocks); - state.delete(value); - this.setState({ - selectedXblocks: state, - }, this.updateList); - } - - updateList(list) { - $('#library-sourced-block-picker').trigger('selected-xblocks', { - sourceBlockIds: Array.from(this.state.selectedXblocks), - }); - } - - render() { - return ( -
    -
    -
    -

    - - Hitting 'Save and Import' will import the latest versions of the selected blocks, overwriting any changes done to this block post-import. -

    -
    -
    -
    -
    - -
    - { - this.state.libraries.map(lib => ( -
    - - -
    - )) - } - { this.state.libraryLoading && {gettext('Loading...')} } -
    -
    -
    - -
    - { - this.state.xblocks.map(block => ( -
    - - -
    - )) - } - { this.state.xblocksLoading && {gettext('Loading...')} } -
    -
    -
    -

    {gettext('Selected blocks')}

    -
      - { - Array.from(this.state.selectedXblocks).map(block => ( -
    • - - -
      • - )) - } -
    -
    -
    -
    - ); - } -} - -LibrarySourcedBlockPicker.propTypes = { - selectedXblocks: PropTypes.array, -}; - -LibrarySourcedBlockPicker.defaultProps = { - selectedXblocks: [], -}; - -export { LibrarySourcedBlockPicker }; // eslint-disable-line import/prefer-default-export diff --git a/xmodule/assets/library_source_block/public/js/library_source_block.js b/xmodule/assets/library_source_block/public/js/library_source_block.js deleted file mode 100644 index 9674080d59dc..000000000000 --- a/xmodule/assets/library_source_block/public/js/library_source_block.js +++ /dev/null @@ -1,58 +0,0 @@ -/* JavaScript for allowing editing options on LibrarySourceBlock's studio view */ -window.LibrarySourceBlockStudioView = function(runtime, element) { - 'use strict'; - var self = this; - - $('#library-sourced-block-picker', element).on('selected-xblocks', function(e, params) { - self.sourceBlockIds = params.sourceBlockIds; - }); - - $('#library-sourced-block-picker', element).on('error', function(e, params) { - runtime.notify('error', {title: gettext(params.title), message: params.message}); - }); - - $('.save-button', element).on('click', function(e) { - e.preventDefault(); - var url = $(e.target).data('submit-url'); - var data = { - values: { - source_block_ids: self.sourceBlockIds - }, - defaults: ['display_name'] - }; - - 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}); - }); - }); - - $('.cancel-button', element).on('click', function(e) { - e.preventDefault(); - runtime.notify('cancel', {}); - }); -}; diff --git a/xmodule/assets/library_source_block/style.css b/xmodule/assets/library_source_block/style.css deleted file mode 100644 index 4892f20405c0..000000000000 --- a/xmodule/assets/library_source_block/style.css +++ /dev/null @@ -1,58 +0,0 @@ -.column { - display: flex; - flex-direction: column; - margin: 10px; - max-width: 300px; - flex-grow: 1; -} -.elementList { - margin-top: 10px; -} -input.search { - width: 100% !important; - height: auto !important; -} -.element > input[type='checkbox'], -.element > input[type='radio'] { - position: absolute; - width: 0 !important; - height: 0 !important; - top: -9999px; -} -.element > .elementItem { - display: flex; - flex-grow: 1; - padding: 0.625rem 1.25rem; - border: 1px solid rgba(0, 0, 0, 0.25); -} -.element + .element > label { - border-top: 0; -} -.element > input[type='checkbox']:focus + label, -.element > input[type='radio']:focus + label, -.element > input:hover + label { - background: #f6f6f7; - cursor: pointer; -} -.element > input:checked + label { - background: #23419f; - color: #fff; -} -.element > input[type='checkbox']:checked:focus + label, -.element > input[type='radio']:checked:focus + label, -.element > input:checked:hover + label { - background: #193787; - cursor: pointer; -} -.selectedBlocks { - padding: 12px 8px 20px; -} -button.remove { - background: #e00; - color: #fff; - border: solid rgba(0,0,0,0.25) 1px; -} -button.remove:focus, -button.remove:hover { - background: #d00; -} diff --git a/xmodule/templates/library-sourced-block-studio-view.html b/xmodule/templates/library-sourced-block-studio-view.html deleted file mode 100644 index 08a2882b51c5..000000000000 --- a/xmodule/templates/library-sourced-block-studio-view.html +++ /dev/null @@ -1,19 +0,0 @@ -{% load i18n %} -
    -
    -
    - {{ react_content|safe }} -
    -
    -
    - -
    -
    From fa58d16fd305607197d130ec21e2dba680b4030b Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Mon, 20 Jun 2022 18:51:13 +0300 Subject: [PATCH 49/71] feat: add "selectable" context for author_view --- xmodule/library_sourced_block.py | 50 +++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index ab34f633e5c9..4ab0cf6b6e49 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -16,6 +16,7 @@ from xblock.fields import List, Scope, String from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader +from xblockutils.studio_editable import StudioEditableXBlockMixin from xmodule.mako_module import MakoTemplateBlockBase from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin @@ -34,6 +35,7 @@ @XBlock.wants('library_tools') # Only needed in studio @XBlock.wants('studio_user_permissions') # Only available in studio class LibrarySourcedBlock( + StudioEditableXBlockMixin, MakoTemplateBlockBase, XModuleToXBlockMixin, HTMLSnippet, @@ -53,7 +55,7 @@ class LibrarySourcedBlock( """ display_name = String( help=_("The display name for this component."), - default="Library Sourced Content", + default="Library Reference Block", display_name=_("Display Name"), scope=Scope.content, ) @@ -73,7 +75,7 @@ class LibrarySourcedBlock( help=_("Enter the IDs of the library XBlocks that you wish to use."), scope=Scope.content, ) - editable_fields = ("display_name", "source_block_ids") + editable_fields = ("source_block_ids",) has_children = True has_author_view = True @@ -103,6 +105,33 @@ class LibrarySourcedBlock( def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" + def render_children(self, context, fragment, can_reorder=False, can_add=False): + """ + Renders the children of the module with HTML appropriate for Studio. If can_reorder is True, + then the children will be rendered to support drag and drop. + """ + contents = [] + + for child in self.get_children(): # pylint: disable=no-member + if can_reorder: + context['reorderable_items'].add(child.location) + context['can_add'] = can_add + context['selected'] = str(child.location) in self.source_block_ids + rendered_child = child.render(StudioEditableModule.get_preview_view_name(child), context) + fragment.add_fragment_resources(rendered_child) + + contents.append({ + 'id': str(child.location), + 'content': rendered_child.content + }) + + fragment.add_content(self.runtime.service(self, 'mako').render_template("studio_render_children_view.html", { # pylint: disable=no-member + 'items': contents, + 'xblock_context': context, + 'can_add': can_add, + 'can_reorder': can_reorder, + })) + def studio_view(self, _context): """ Return the studio view. @@ -209,6 +238,8 @@ def validate(self): action_label=_("Select a Library.") ) ) + return validation + self._validate_library_version(validation, self.tools, self.source_library_version, self.source_library_key) return validation @@ -285,18 +316,11 @@ def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint except ValueError: pass # The validation area will display an error message, no need to do anything now. - @XBlock.json_handler - def set_block_ids(self, data, suffix=''): - """ - Save source_block_ids. - """ - if data.get('source_library_id'): - self.source_library_id = data.get('source_library_id') - return {'source_library_id': self.source_library_id} - @XBlock.handler - def get_block_ids(self, data, suffix=''): + def get_block_ids(self, request, suffix=''): # lint-amnesty, pylint: disable=unused-argument """ Return source_block_ids. """ - return Response(json.dumps({'source_library_id': self.source_library_id})) + return Response(json.dumps({'source_block_ids': self.source_block_ids})) + +StudioEditableModule = EditableChildrenMixin From f5e8241d3fa522c57e5cc1a290312526effc37b3 Mon Sep 17 00:00:00 2001 From: ihor-romaniuk Date: Fri, 24 Jun 2022 14:32:50 +0300 Subject: [PATCH 50/71] feat: add selection functionality to library components --- cms/djangoapps/contentstore/views/preview.py | 1 + cms/static/js/views/pages/container.js | 66 +++++++++++++++++++- cms/static/sass/elements/_xblocks.scss | 13 ++++ cms/templates/container.html | 7 +++ cms/templates/studio_xblock_wrapper.html | 9 +++ 5 files changed, 93 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index fd0bdf0dc57a..d9103d634663 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -309,6 +309,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, + 'is_selected': context.get('selected', False), 'can_edit': context.get('can_edit', True), 'can_edit_visibility': context.get('can_edit_visibility', xblock.scope_ids.usage_id.context_key.is_course), 'selected_groups_label': selected_groups_label, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 347f7ea77b7f..7b5ebe19bb2a 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -6,10 +6,10 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page 'common/js/components/utils/view_utils', 'js/views/container', 'js/views/xblock', 'js/views/components/add_xblock', 'js/views/modals/edit_xblock', 'js/views/modals/move_xblock_modal', 'js/models/xblock_info', 'js/views/xblock_string_field_editor', 'js/views/xblock_access_editor', - 'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils'], + 'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils', 'js/utils/module'], function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, MoveXBlockModal, XBlockInfo, XBlockStringFieldEditor, XBlockAccessEditor, - ContainerSubviews, UnitOutlineView, XBlockUtils) { + ContainerSubviews, UnitOutlineView, XBlockUtils, ModuleUtils) { 'use strict'; var XBlockContainerPage = BasePage.extend({ // takes XBlockInfo as a model @@ -20,7 +20,9 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page 'click .duplicate-button': 'duplicateXBlock', 'click .move-button': 'showMoveXBlockModal', 'click .delete-button': 'deleteXBlock', - 'click .new-component-button': 'scrollToNewComponentButtons' + 'click .save-button': 'saveSelectedLibraryComponents', + 'click .new-component-button': 'scrollToNewComponentButtons', + 'change .header-library-checkbox': 'toggleLibraryComponent' }, options: { @@ -95,6 +97,10 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page model: this.model }); this.unitOutlineView.render(); + + this.selectedLibraryComponents = []; + this.storeSelectedLibraryComponents = []; + this.getSelectedLibraryComponents(); } this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved); @@ -300,6 +306,60 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page }); }, + getSelectedLibraryComponents: function() { + var self = this; + var locator = this.$el.find('.studio-xblock-wrapper').data('locator'); + $.getJSON( + ModuleUtils.getUpdateUrl(locator) + '/handler/get_block_ids', + function(data) { + self.selectedLibraryComponents = Array.from(data.source_block_ids); + self.storeSelectedLibraryComponents = Array.from(data.source_block_ids); + } + ); + }, + + saveSelectedLibraryComponents: function(e) { + var self = this; + var locator = this.$el.find('.studio-xblock-wrapper').data('locator'); + e.preventDefault(); + $.postJSON( + ModuleUtils.getUpdateUrl(locator) + '/handler/submit_studio_edits', + {values: {source_block_ids: self.storeSelectedLibraryComponents}}, + function() { + self.selectedLibraryComponents = Array.from(self.storeSelectedLibraryComponents); + self.toggleSaveButton(); + } + ); + }, + + toggleLibraryComponent: function(event) { + var componentId = $(event.target).closest('.studio-xblock-wrapper').data('locator'); + var storeIndex = this.storeSelectedLibraryComponents.indexOf(componentId); + if (storeIndex > -1) { + this.storeSelectedLibraryComponents.splice(storeIndex, 1); + this.toggleSaveButton(); + } else { + this.storeSelectedLibraryComponents.push(componentId); + this.toggleSaveButton(); + } + }, + + toggleSaveButton: function() { + var $saveButton = $('.nav-actions .save-button'); + if (JSON.stringify(this.selectedLibraryComponents.sort()) === JSON.stringify(this.storeSelectedLibraryComponents.sort())) { + $saveButton.addClass('is-hidden'); + window.removeEventListener('beforeunload', this.onBeforePageUnloadCallback); + } else { + $saveButton.removeClass('is-hidden'); + window.addEventListener('beforeunload', this.onBeforePageUnloadCallback); + } + }, + + onBeforePageUnloadCallback: function (event) { + event.preventDefault(); + event.returnValue = ''; + }, + onDelete: function(xblockElement) { // get the parent so we can remove this component from its parent. var xblockView = this.xblockView, diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index d6e8aff5d66a..7574a732cb48 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -43,6 +43,19 @@ display: flex; align-items: center; + .header-library-checkbox { + margin-right: 10px; + width: 17px; + height: 17px; + cursor: pointer; + vertical-align: middle; + } + + .header-library-checkbox-label { + vertical-align: middle; + cursor: pointer; + } + .header-details { @extend %cont-truncated; diff --git a/cms/templates/container.html b/cms/templates/container.html index 17de3d20ea76..909360832610 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -144,6 +144,13 @@

    ${_("Page Actions")}

    % else: + % if True: +
  • + +
    • + % endif
  • diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 1bd2c3070e30..e5bcfe19c730 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -73,7 +73,16 @@ % endif
    + % if True: + + + % else: ${label} + % endif % if selected_groups_label:

    ${selected_groups_label}

    % endif From eae44a6749796bdb186667d7fd4a284c8aedd8a6 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Thu, 23 Jun 2022 15:07:52 +0300 Subject: [PATCH 51/71] feat: show selected blocks for student_view --- xmodule/library_sourced_block.py | 84 +++++++++++++++++--------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/xmodule/library_sourced_block.py b/xmodule/library_sourced_block.py index 4ab0cf6b6e49..aa6a9a022d74 100644 --- a/xmodule/library_sourced_block.py +++ b/xmodule/library_sourced_block.py @@ -8,13 +8,13 @@ import bleach from lazy import lazy from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 from pkg_resources import resource_string from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock from xblock.fields import List, Scope, String -from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin @@ -22,7 +22,7 @@ from xmodule.studio_editable import StudioEditableBlock as EditableChildrenMixin from xmodule.util.xmodule_django import add_webpack_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage -from xmodule.x_module import HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js +from xmodule.x_module import STUDENT_VIEW, HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js log = logging.getLogger(__name__) loader = ResourceLoader(__name__) @@ -100,7 +100,6 @@ class LibrarySourcedBlock( studio_view_css = { 'scss': [], } - MAX_BLOCKS_ALLOWED = 10 def __str__(self): return f"LibrarySourcedBlock: {self.display_name}" @@ -117,7 +116,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): context['reorderable_items'].add(child.location) context['can_add'] = can_add context['selected'] = str(child.location) in self.source_block_ids - rendered_child = child.render(StudioEditableModule.get_preview_view_name(child), context) + rendered_child = child.render(EditableChildrenMixin.get_preview_view_name(child), context) fragment.add_fragment_resources(rendered_child) contents.append({ @@ -157,33 +156,40 @@ def author_view(self, context): fragment.initialize_js('LibrarySourceAuthorView') return fragment - def student_view(self, context): + def _get_selected_child_blocks(self): """ - Renders the view that learners see. + Generator returning XBlock instances of the children selected for the + current user. """ - 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_field_data(self, validation, data): + for block_id in self.source_block_ids: + usage_key = UsageKey.from_string(block_id) + yield self.runtime.get_block(usage_key) + + def student_view(self, context): """ - Validate this block's field data. Instead of checking fields like self.name, check the - fields set on data, e.g. data.name. This allows the same validation method to be re-used - for the studio editor. + Renders the view that learners see. """ - if len(data.source_block_ids) > self.MAX_BLOCKS_ALLOWED: - # Because importing library blocks is an expensive operation - validation.add( - ValidationMessage( - ValidationMessage.ERROR, - _("A maximum of {0} components may be added.").format(self.MAX_BLOCKS_ALLOWED) - ) - ) + fragment = Fragment() + child_context = {} if not context else copy(context) + + fragment.add_content('
    ') + for child in self._get_selected_child_blocks(): + if child is None: + # TODO: Fix the underlying issue in TNL-7424 + # This shouldn't be happening, but does for an as-of-now + # unknown reason. Until we address the underlying issue, + # let's at least log the error explicitly, ignore the + # exception, and prevent the page from resulting in a + # 500-response. + log.error('Skipping display for child block that is None') + continue + for displayable in child.displayable_items(): + rendered_child = displayable.render(STUDENT_VIEW, child_context) + fragment.add_content(rendered_child.content) + fragment.add_fragment_resources(rendered_child) + + fragment.add_content('
    ') + return fragment def _validate_library_version(self, validation, lib_tools, version, library_key): """ @@ -285,6 +291,17 @@ def tools(self): """ return self.runtime.service(self, 'library_tools') + def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument + """ + If source_library_id has been edited, refresh_children automatically. + """ + old_source_library_id = old_metadata.get('source_library_id', []) + if old_source_library_id != self.source_library_id: + try: + self.refresh_children() + except ValueError: + pass # The validation area will display an error message, no need to do anything now. + @XBlock.handler def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: disable=unused-argument """ @@ -305,22 +322,9 @@ def refresh_children(self, request=None, suffix=None): # lint-amnesty, pylint: self.tools.update_children(self, user_perms) return Response() - def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint: disable=unused-argument - """ - If source_library_id has been edited, refresh_children automatically. - """ - old_source_library_id = old_metadata.get('source_library_id', []) - if old_source_library_id != self.source_library_id: - try: - self.refresh_children() - except ValueError: - pass # The validation area will display an error message, no need to do anything now. - @XBlock.handler def get_block_ids(self, request, suffix=''): # lint-amnesty, pylint: disable=unused-argument """ Return source_block_ids. """ return Response(json.dumps({'source_block_ids': self.source_block_ids})) - -StudioEditableModule = EditableChildrenMixin From d7c340caeba3c53a9a4ca9b932a00e61c875e8b0 Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 24 Jun 2022 15:06:49 +0300 Subject: [PATCH 52/71] test: update tests for LibrarySourcedBlock --- xmodule/tests/test_library_sourced_block.py | 66 +++++++++------------ 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/xmodule/tests/test_library_sourced_block.py b/xmodule/tests/test_library_sourced_block.py index 246a6fa05a94..5b4de29582d6 100644 --- a/xmodule/tests/test_library_sourced_block.py +++ b/xmodule/tests/test_library_sourced_block.py @@ -2,63 +2,53 @@ Tests for Source from Library XBlock """ -from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest -from common.djangoapps.student.roles import CourseInstructorRole -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.library_tools import LibraryToolsService +from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory +from xmodule.modulestore.tests.utils import MixedSplitTestCase from xmodule.tests import get_test_system from xmodule.x_module import STUDENT_VIEW # lint-amnesty, pylint: disable=unused-import +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest + -class LibrarySourcedBlockTestCase(ContentLibrariesRestApiTest): +class LibrarySourcedBlockTestCase(MixedSplitTestCase, ContentLibrariesRestApiTest): """ Tests for LibraryToolsService which interact with blockstore-based content libraries """ def setUp(self): super().setUp() - self.store = modulestore() + self.tools = LibraryToolsService(self.store, self.user_id) + self.library = LibraryFactory.create(modulestore=self.store) + # Add content to the library + self.lib_blocks = [ + self.make_block("html", self.library, data=f"Hello world from block {i}") + for i in range(1, 5) + ] # 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 - self.source_block = ItemFactory.create( - category="library_sourced", - parent=course, - parent_location=course.location, - user_id=self.user.id, - modulestore=self.store + self.course = CourseFactory.create(modulestore=self.store) + self.chapter = self.make_block("chapter", self.course) + self.sequential = self.make_block("sequential", self.chapter) + self.vertical = self.make_block("vertical", self.sequential) + # Add a LibrarySourcedBlock to the course + self.source_block = self.make_block( + "library_sourced", + self.vertical, + source_library_id=str(self.library.location.library_key) ) self.submit_url = f'/xblock/{self.source_block.scope_ids.usage_id}/handler/submit_studio_edits' 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_1 = self._add_block_to_library(library["id"], "html", "html_student_preview_1")["id"] - self._set_library_block_olx(html_block_1, 'Student Preview Test 1') - html_block_2 = self._add_block_to_library(library["id"], "html", "html_student_preview_2")["id"] - self._set_library_block_olx(html_block_2, 'Student Preview Test 2') - # Import the html blocks from the library to the course - post_data = {"values": {"source_block_ids": [html_block_1, html_block_2]}, "defaults": ["display_name"]} + self.source_block.refresh_children() + # Save children block_ids as source_block_ids + source_block_ids = [str(child) for child in self.source_block.children] + post_data = {"values": {"source_block_ids": source_block_ids}} res = self.client.post(self.submit_url, data=post_data, format='json') # Check if student_view renders the children correctly res = self.get_block_view(self.source_block, STUDENT_VIEW) - assert 'Student Preview Test 1' in res - assert 'Student Preview Test 2' in res - - def test_block_limits(self): - # Create a blockstore content library - library = self._create_library(slug="testlib2_preview", title="Test Library 2", description="Testing XBlocks") - # Add content to the library - blocks = [self._add_block_to_library(library["id"], "html", f"block_{i}")["id"] for i in range(11)] - - # Import the html blocks from the library to the course - post_data = {"values": {"source_block_ids": blocks}, "defaults": ["display_name"]} - res = self.client.post(self.submit_url, data=post_data, format='json') - assert res.status_code == 400 - assert res.json()['error']['messages'][0]['text'] == 'A maximum of 10 components may be added.' + for i in range(1, 5): + assert f"Hello world from block {i}" in res def get_block_view(self, block, view, context=None): """ From 40792513df58043658cab4310a2e2bcbe318ec1b Mon Sep 17 00:00:00 2001 From: Sagirov Eugeniy Date: Fri, 24 Jun 2022 18:48:39 +0300 Subject: [PATCH 53/71] feat: add context for library_sourced container page --- cms/djangoapps/contentstore/views/component.py | 3 ++- cms/djangoapps/contentstore/views/preview.py | 3 ++- cms/static/js/views/pages/container.js | 4 +++- cms/templates/container.html | 2 +- cms/templates/studio_xblock_wrapper.html | 2 +- xmodule/library_sourced_block.py | 16 +++++++++++++--- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index da15dc5c84dc..72f7ff924400 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -204,7 +204,8 @@ def container_handler(request, usage_key_string): 'xblock_info': xblock_info, 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, - 'templates': CONTAINER_TEMPLATES + 'templates': CONTAINER_TEMPLATES, + 'is_sourced_block': xblock.location.block_type == 'library_sourced', }) else: return HttpResponseBadRequest("Only supports HTML requests") diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index d9103d634663..ba418ff8be23 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -309,7 +309,8 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, - 'is_selected': context.get('selected', False), + 'is_selected': context.get('is_selected', False), + 'selectable': context.get('selectable', False), 'can_edit': context.get('can_edit', True), 'can_edit_visibility': context.get('can_edit_visibility', xblock.scope_ids.usage_id.context_key.is_course), 'selected_groups_label': selected_groups_label, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 7b5ebe19bb2a..d1e36e32ac84 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -44,6 +44,7 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page BasePage.prototype.initialize.call(this, options); this.viewClass = options.viewClass || this.defaultViewClass; this.isLibraryPage = (this.model.attributes.category === 'library'); + this.isLibrarySourced = (this.model.attributes.category === 'library_sourced'); this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -97,7 +98,8 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page model: this.model }); this.unitOutlineView.render(); - + } + if (this.isLibrarySourced) { this.selectedLibraryComponents = []; this.storeSelectedLibraryComponents = []; this.getSelectedLibraryComponents(); diff --git a/cms/templates/container.html b/cms/templates/container.html index 909360832610..6310e39d3e2d 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -144,7 +144,7 @@

    ${_("Page Actions")}

    • % else: - % if True: + % if is_sourced_block:
  • % endif
    - % if True: + % if selectable: