From 2bbd8ecd180fd299de2c4ee4bc1542ccb97b50bb Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Tue, 15 Oct 2024 11:32:01 -0400 Subject: [PATCH] feat!: Remove outdated Libraries Relaunch cruft (#35644) The V2 libraries project had a few past iterations which were never launched. This commit cleans up pieces from those which we don't need for the real Libraries Relaunch MVP in Sumac: * Remove ENABLE_LIBRARY_AUTHORING_MICROFRONTEND, LIBRARY_AUTHORING_FRONTEND_URL, and REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND, all of which are obsolete now that library authoring has been merged into https://github.com/openedx/frontend-app-authoring. More details on the new Content Libraries configuration settings are here: https://github.com/openedx/frontend-app-authoring/issues/1334 * Remove dangling support for syncing V2 (learning core-backed) library content using the LibraryContentBlock. This code was all based on an older understanding of V2 Content Libraries, where the libraries were smaller and versioned as a whole rather then versioned by-item. Reference to V2 libraries will be done on a per-block basis using the upstream/downstream system, described here: https://github.com/openedx/edx-platform/blob/master/docs/decisions/0020-upstream-downstream.rst It's important that we remove this support now so that OLX course authors don't stuble upon it and use it, which would be buggy and complicate future migrations. * Remove the "mode" parameter from LibraryContentBlock. The only supported mode was and is "random". We will not be adding any further modes. Going forward for V2, we will have an ItemBank block for randomizing items (regardless of source), which can be synthesized with upstream referenced as described above. Existing LibraryContentBlocks will be migrated. * Finally, some renamings: * LibraryContentBlock -> LegacyLibraryContentBlock * LibraryToolsService -> LegacyLibraryToolsService * LibrarySummary -> LegacyLibrarySummary Module names and the old OLX tag (library_content) are unchanged. Closes: https://github.com/openedx/frontend-app-authoring/issues/1115 --- cms/djangoapps/contentstore/config/waffle.py | 16 +- .../rest_api/v1/serializers/home.py | 2 - .../contentstore/rest_api/v1/views/home.py | 2 - .../rest_api/v1/views/tests/test_home.py | 2 - cms/djangoapps/contentstore/utils.py | 13 +- cms/djangoapps/contentstore/views/library.py | 16 -- .../contentstore/views/tests/test_block.py | 2 +- .../views/tests/test_clipboard_paste.py | 43 ++-- cms/envs/common.py | 15 +- cms/envs/devstack.py | 3 - cms/templates/index.html | 4 - docs/docs_settings.py | 1 - .../transformers/library_content.py | 8 +- lms/djangoapps/courseware/block_render.py | 4 +- lms/djangoapps/grades/rest_api/v1/views.py | 4 +- .../core/djangoapps/content_libraries/api.py | 79 +------ .../djangoapps/content_libraries/tasks.py | 195 +++--------------- openedx/core/lib/xblock_utils/__init__.py | 2 +- .../completion_integration/test_services.py | 4 +- setup.py | 2 +- webpack.builtinblocks.config.js | 9 +- .../public/js/library_content_edit_helpers.js | 54 ----- xmodule/library_content_block.py | 77 ++----- xmodule/library_tools.py | 75 +++---- xmodule/modulestore/mixed.py | 2 +- .../split_mongo/caching_descriptor_system.py | 4 +- xmodule/modulestore/split_mongo/split.py | 6 +- xmodule/tests/test_library_content.py | 63 +++--- xmodule/tests/test_library_tools.py | 73 ++----- xmodule/tests/test_randomize_block.py | 2 +- xmodule/vertical_block.py | 2 +- 31 files changed, 180 insertions(+), 604 deletions(-) delete mode 100644 xmodule/assets/library_content/public/js/library_content_edit_helpers.js diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index f84290ba83..ae0e6ea467 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -4,7 +4,7 @@ waffle switches for the contentstore app. """ -from edx_toggles.toggles import WaffleFlag, WaffleSwitch +from edx_toggles.toggles import WaffleSwitch from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag @@ -26,20 +26,6 @@ SHOW_REVIEW_RULES_FLAG = CourseWaffleFlag( # lint-amnesty, pylint: disable=togg f'{WAFFLE_NAMESPACE}.show_review_rules', __name__, LOG_PREFIX ) -# Waffle flag to redirect to the library authoring MFE. -# .. toggle_name: contentstore.library_authoring_mfe -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Toggles the new micro-frontend-based implementation of the library authoring experience. -# .. toggle_use_cases: temporary, open_edx -# .. toggle_creation_date: 2020-08-03 -# .. toggle_target_removal_date: 2020-12-31 -# .. toggle_warning: Also set settings.LIBRARY_AUTHORING_MICROFRONTEND_URL and ENABLE_LIBRARY_AUTHORING_MICROFRONTEND. -# .. toggle_tickets: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4106944527/Libraries+Relaunch+Proposal+For+Product+Review -REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND = WaffleFlag( - f'{WAFFLE_NAMESPACE}.library_authoring_mfe', __name__, LOG_PREFIX -) - # .. toggle_name: studio.custom_relative_dates # .. toggle_implementation: CourseWaffleFlag diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index 0aa06d8b8d..4c3e2a4321 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -59,10 +59,8 @@ class CourseHomeSerializer(serializers.Serializer): libraries = LibraryViewSerializer(many=True, required=False, allow_null=True) libraries_enabled = serializers.BooleanField() taxonomies_enabled = serializers.BooleanField() - library_authoring_mfe_url = serializers.CharField() taxonomy_list_mfe_url = serializers.CharField() optimization_enabled = serializers.BooleanField() - redirect_to_library_authoring_mfe = serializers.BooleanField() request_course_creator_url = serializers.CharField() rerun_creator_status = serializers.BooleanField() show_new_library_button = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index d72042cff6..ff476090ee 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -59,9 +59,7 @@ class HomePageView(APIView): "in_process_course_actions": [], "libraries": [], "libraries_enabled": true, - "library_authoring_mfe_url": "//localhost:3001/course/course-v1:edX+P315+2T2023", "optimization_enabled": true, - "redirect_to_library_authoring_mfe": false, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": true, "show_new_library_button": true, diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index a8b4cf5e39..c3c9652e5d 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -52,10 +52,8 @@ class HomePageViewTest(CourseTestCase): "libraries": [], "libraries_enabled": True, "taxonomies_enabled": True, - "library_authoring_mfe_url": settings.LIBRARY_AUTHORING_MICROFRONTEND_URL, "taxonomy_list_mfe_url": 'http://course-authoring-mfe/taxonomies', "optimization_enabled": False, - "redirect_to_library_authoring_mfe": False, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": True, "show_new_library_button": True, diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 214193918e..035e44d5ce 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -98,7 +98,7 @@ from cms.djangoapps.contentstore.toggles import ( ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.djangoapps.models.settings.course_metadata import CourseMetadata -from xmodule.library_tools import LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order @@ -1265,7 +1265,7 @@ def load_services_for_studio(runtime, user): "settings": SettingsService(), "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) + "library_tools": LegacyLibraryToolsService(modulestore(), user.id) } runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access @@ -1671,9 +1671,7 @@ def get_home_context(request, no_course=False): ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) from cms.djangoapps.contentstore.views.library import ( - LIBRARY_AUTHORING_MICROFRONTEND_URL, LIBRARIES_ENABLED, - should_redirect_to_library_authoring_mfe, user_can_view_create_library_button, ) @@ -1699,12 +1697,9 @@ def get_home_context(request, no_course=False): 'in_process_course_actions': in_process_course_actions, 'libraries_enabled': LIBRARIES_ENABLED, 'taxonomies_enabled': not is_tagging_feature_disabled(), - 'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), - 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, - 'show_new_library_button': user_can_view_create_library_button(user) - and not should_redirect_to_library_authoring_mfe(), + 'show_new_library_button': user_can_view_create_library_button(user), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(user), @@ -2202,7 +2197,7 @@ class StudioPermissionsService: Deprecated. To be replaced by a more general authorization service. - Only used by LibraryContentBlock (and library_tools.py). + Only used by LegacyLibraryContentBlock (and library_tools.py). """ def __init__(self, user): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 8c314caa66..340cadb4e2 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -41,7 +41,6 @@ from common.djangoapps.student.roles import ( ) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json -from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND from ..utils import add_instructor, reverse_library_url from .component import CONTAINER_TEMPLATES, get_component_templates from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import create_xblock_info @@ -52,21 +51,6 @@ __all__ = ['library_handler', 'manage_library_users'] log = logging.getLogger(__name__) LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) -ENABLE_LIBRARY_AUTHORING_MICROFRONTEND = settings.FEATURES.get('ENABLE_LIBRARY_AUTHORING_MICROFRONTEND', False) -LIBRARY_AUTHORING_MICROFRONTEND_URL = settings.LIBRARY_AUTHORING_MICROFRONTEND_URL - - -def should_redirect_to_library_authoring_mfe(): - """ - Boolean helper method, returns whether or not to redirect to the Library - Authoring MFE based on settings and flags. - """ - - return ( - ENABLE_LIBRARY_AUTHORING_MICROFRONTEND and - LIBRARY_AUTHORING_MICROFRONTEND_URL and - REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND.is_enabled() - ) def _user_can_create_library_for_org(user, org=None): diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 80a2535598..f3e20b45b2 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -982,7 +982,7 @@ class TestDuplicateItem(ItemTest, DuplicateHelper, OpenEdxEventsTestMixin): def test_duplicate_library_content_block(self): # pylint: disable=too-many-statements """ - Test the LibraryContentBlock's special duplication process. + Test the LegacyLibraryContentBlock's special duplication process. """ store = modulestore() diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 76dba57f1d..7979a422a3 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -7,13 +7,11 @@ import ddt from opaque_keys.edx.keys import UsageKey from rest_framework.test import APIClient from openedx_tagging.core.tagging.models import Tag -from organizations.models import Organization from xmodule.modulestore.django import contentstore, modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course -from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory, LibraryFactory from cms.djangoapps.contentstore.utils import reverse_usage_url -from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_tagging import api as tagging_api CLIPBOARD_ENDPOINT = "/api/content-staging/v1/clipboard/" @@ -165,12 +163,12 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): publish_item=True, ).location - library = ClipboardLibraryContentPasteTestCase.setup_library() + library = ClipboardPasteFromV1LibraryTestCase.setup_library() with self.store.bulk_operations(course_key): library_content_block_key = BlockFactory.create( parent=self.store.get_item(unit_key), category="library_content", - source_library_id=str(library.key), + source_library_id=str(library.context_key), display_name="LC Block", publish_item=True, ).location @@ -393,27 +391,27 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. -class ClipboardLibraryContentPasteTestCase(ModuleStoreTestCase): +class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase): """ - Test Clipboard Paste functionality with library content + Test Clipboard Paste functionality with legacy (v1) library content """ def setUp(self): """ - Set up a v2 Content Library and a library content block + Set up a v1 Content Library and a library content block """ super().setUp() self.client = APIClient() self.client.login(username=self.user.username, password=self.user_password) self.store = modulestore() - library = self.setup_library() + self.library = self.setup_library() # Create a library content block (lc), point it out our library, and sync it. self.course = CourseFactory.create(display_name='Course') self.orig_lc_block = BlockFactory.create( parent=self.course, category="library_content", - source_library_id=str(library.key), + source_library_id=str(self.library.context_key), display_name="LC Block", publish_item=False, ) @@ -426,18 +424,15 @@ class ClipboardLibraryContentPasteTestCase(ModuleStoreTestCase): @classmethod def setup_library(cls): """ - Creates and returns a content library. + Creates and returns a legacy content library with 1 problem """ - library = library_api.create_library( - library_type=library_api.COMPLEX, - org=Organization.objects.create(name="Test Org", short_name="CL-TEST"), - slug="lib", - title="Library", - ) - # Populate it with a problem: - problem_key = library_api.create_library_block(library.key, "problem", "p1").usage_key - library_api.set_library_block_olx(problem_key, """ - + library = LibraryFactory.create(display_name='Library') + lib_block = BlockFactory.create( + parent_location=library.usage_key, + category="problem", + display_name="MCQ", + max_attempts=1, + data=""" @@ -445,9 +440,9 @@ class ClipboardLibraryContentPasteTestCase(ModuleStoreTestCase): Right - - """) - library_api.publish_changes(library.key) + """, + publish_item=False, + ) return library def test_paste_library_content_block(self): diff --git a/cms/envs/common.py b/cms/envs/common.py index 7521cc21fa..63221ee0b0 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -434,18 +434,6 @@ FEATURES = { # .. toggle_tickets: https://openedx.atlassian.net/browse/DEPR-58 'DEPRECATE_OLD_COURSE_KEYS_IN_STUDIO': True, - # .. toggle_name: FEATURES['ENABLE_LIBRARY_AUTHORING_MICROFRONTEND'] - # .. toggle_implementation: DjangoSetting - # .. toggle_default: False - # .. toggle_description: Set to True to enable the Library Authoring MFE - # .. toggle_use_cases: temporary - # .. toggle_creation_date: 2020-06-20 - # .. toggle_target_removal_date: 2020-12-31 - # .. toggle_tickets: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4106944527/Libraries+Relaunch+Proposal+For+Product+Review - # .. toggle_warning: Also set settings.LIBRARY_AUTHORING_MICROFRONTEND_URL and see - # REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND for rollout. - 'ENABLE_LIBRARY_AUTHORING_MICROFRONTEND': False, - # .. toggle_name: FEATURES['DISABLE_COURSE_CREATION'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -601,7 +589,6 @@ IDA_LOGOUT_URI_LIST = [] COURSE_AUTHORING_MICROFRONTEND_URL = None DISCUSSIONS_MICROFRONTEND_URL = None DISCUSSIONS_MFE_FEEDBACK_URL = None -LIBRARY_AUTHORING_MICROFRONTEND_URL = None # .. toggle_name: ENABLE_AUTHN_RESET_PASSWORD_HIBP_POLICY # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -2779,6 +2766,7 @@ WIKI_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-runni CUSTOM_PAGES_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/pages.html#adding-custom-pages" COURSE_LIVE_HELP_URL = "https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/course_assets/course_live.html" ORA_SETTINGS_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/pages.html#configuring-course-level-open-response-assessment-settings" +# pylint: enable=line-too-long # keys for big blue button live provider COURSE_LIVE_GLOBAL_CREDENTIALS = {} @@ -2810,6 +2798,7 @@ DISCUSSIONS_INCONTEXT_FEEDBACK_URL = '' # Learn More link in upgraded discussion notification alert # pylint: disable=line-too-long DISCUSSIONS_INCONTEXT_LEARNMORE_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/manage_discussions/discussions.html" +# pylint: enable=line-too-long #### django-simple-history## # disable indexing on date field its coming django-simple-history. diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 1d3a510cdc..1200a61b06 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -174,9 +174,6 @@ FEATURES['ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES'] = True ################### FRONTEND APPLICATION PUBLISHER URL ################### FEATURES['FRONTEND_APP_PUBLISHER_URL'] = 'http://localhost:18400' -################### FRONTEND APPLICATION LIBRARY AUTHORING ################### -LIBRARY_AUTHORING_MICROFRONTEND_URL = 'http://localhost:3001' - ################### FRONTEND APPLICATION COURSE AUTHORING ################### COURSE_AUTHORING_MICROFRONTEND_URL = 'http://localhost:2001' diff --git a/cms/templates/index.html b/cms/templates/index.html index 766d68da78..e558597307 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -348,9 +348,6 @@ from openedx.core.djangolib.js_utils import ( % endif % if libraries_enabled: - % if redirect_to_library_authoring_mfe: -
  • ${_("Libraries")}
    • - %else:
  • % if split_studio_home: ${_("Libraries")} @@ -358,7 +355,6 @@ from openedx.core.djangolib.js_utils import ( ${_("Libraries")} % endif
    • - % endif % endif % if taxonomies_enabled:
  • ${_("Taxonomies")}
    • diff --git a/docs/docs_settings.py b/docs/docs_settings.py index 5bc9b15946..f12848876e 100644 --- a/docs/docs_settings.py +++ b/docs/docs_settings.py @@ -14,7 +14,6 @@ from cms.envs.common import ( # lint-amnesty, pylint: disable=unused-import ADVANCED_PROBLEM_TYPES, COURSE_IMPORT_EXPORT_STORAGE, GIT_EXPORT_DEFAULT_IDENT, - LIBRARY_AUTHORING_MICROFRONTEND_URL, SCRAPE_YOUTUBE_THUMBNAILS_JOB_QUEUE, VIDEO_TRANSCRIPT_MIGRATIONS_JOB_QUEUE, UPDATE_SEARCH_INDEX_JOB_QUEUE, diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index 616cf68f4b..10ef8c2138 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -14,7 +14,7 @@ from openedx.core.djangoapps.content.block_structure.transformer import ( BlockStructureTransformer, FilteringTransformerMixin ) -from xmodule.library_content_block import LibraryContentBlock # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.library_content_block import LegacyLibraryContentBlock # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from ..utils import get_student_module_as_dict @@ -47,7 +47,6 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo Collects any information that's necessary to execute this transformer's transform method. """ - block_structure.request_xblock_fields('mode') block_structure.request_xblock_fields('max_count') block_structure.request_xblock_fields('category') store = modulestore() @@ -83,7 +82,6 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo if library_children: all_library_children.update(library_children) selected = [] - mode = block_structure.get_xblock_field(block_key, 'mode') max_count = block_structure.get_xblock_field(block_key, 'max_count') if max_count < 0: max_count = len(library_children) @@ -100,7 +98,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo # Update selected previous_count = len(selected) - block_keys = LibraryContentBlock.make_selection(selected, library_children, max_count, mode) + block_keys = LegacyLibraryContentBlock.make_selection(selected, library_children, max_count) selected = block_keys['selected'] # Save back any changes @@ -176,7 +174,7 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo with tracker.get_tracker().context(full_event_name, context): tracker.emit(full_event_name, event_data) - LibraryContentBlock.publish_selected_children_events( + LegacyLibraryContentBlock.publish_selected_children_events( block_keys, format_block_keys, publish_event, diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 1bae903224..de92692ce4 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -45,7 +45,7 @@ from lms.djangoapps.teams.services import TeamsService from openedx.core.lib.xblock_services.call_to_action import CallToActionService from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError, ProcessingError -from xmodule.library_tools import LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.modulestore.django import XBlockI18nService, modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.partitions.partitions_service import PartitionService @@ -626,7 +626,7 @@ def prepare_runtime_for_user( ), 'completion': CompletionService(user=user, context_key=course_id) if user and user.is_authenticated else None, 'i18n': XBlockI18nService, - 'library_tools': LibraryToolsService(store, user_id=user.id if user else None), + 'library_tools': LegacyLibraryToolsService(store, user_id=user.id if user else None), 'partitions': PartitionService(course_id=course_id, cache=DEFAULT_REQUEST_CACHE.data), 'settings': SettingsService(), 'user_tags': UserTagsService(user=user, course_id=course_id), diff --git a/lms/djangoapps/grades/rest_api/v1/views.py b/lms/djangoapps/grades/rest_api/v1/views.py index 5f49f2299a..b6835fd61e 100644 --- a/lms/djangoapps/grades/rest_api/v1/views.py +++ b/lms/djangoapps/grades/rest_api/v1/views.py @@ -378,7 +378,7 @@ class SubmissionHistoryView(GradeViewMixin, PaginatedAPIView): def get(self, request, course_id=None): """ Get submission history details. This submission history is related to only - ProblemBlock and it doesn't support LibraryContentBlock or ContentLibraries + ProblemBlock and it doesn't support LegacyLibraryContentBlock or ContentLibraries as of now. **Usecases**: @@ -463,7 +463,7 @@ class SubmissionHistoryView(GradeViewMixin, PaginatedAPIView): @staticmethod def get_problem_blocks(course): """ Get a list of problem xblock for the course. - This doesn't support LibraryContentBlock or ContentLibraries + This doesn't support LegacyLibraryContentBlock or ContentLibraries as of now """ blocks = [] diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 6f45a10daf..f30d5f0477 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -76,7 +76,6 @@ from opaque_keys.edx.locator import ( LibraryLocator as LibraryLocatorV1, LibraryCollectionLocator, ) -from opaque_keys import InvalidKeyError from openedx_events.content_authoring.data import ( ContentLibraryData, LibraryBlockData, @@ -99,10 +98,7 @@ from xblock.exceptions import XBlockNotFoundError from openedx.core.djangoapps.xblock.api import get_component_from_usage_key, xblock_type_display_name from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_for_learning_core -from xmodule.library_root_xblock import LibraryRoot as LibraryRootV1 -from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import ItemNotFoundError from . import permissions, tasks from .constants import ALL_RIGHTS_RESERVED, COMPLEX @@ -421,8 +417,8 @@ def get_library(library_key): # updated version of content that a course could pull in. But more recently, # we've decided to do those version references at the level of the # individual blocks being used, since a Learning Core backed library is - # intended to be used for many LibraryContentBlocks and not 1:1 like v1 - # libraries. The top level version stays for now because LibraryContentBlock + # intended to be referenced in multiple course locations and not 1:1 like v1 + # libraries. The top level version stays for now because LegacyLibraryContentBlock # uses it, but that should hopefully change before the Redwood release. version = 0 if last_publish_log is None else last_publish_log.pk published_by = None @@ -1340,77 +1336,6 @@ def get_library_collection_from_usage_key( raise ContentLibraryCollectionNotFound from exc -# V1/V2 Compatibility Helpers -# (Should be removed as part of -# https://github.com/openedx/edx-platform/issues/32457) -# ====================================================== - -def get_v1_or_v2_library( - library_id: str | LibraryLocatorV1 | LibraryLocatorV2, - version: str | int | None, -) -> LibraryRootV1 | ContentLibraryMetadata | None: - """ - Fetch either a V1 or V2 content library from a V1/V2 key (or key string) and version. - - V1 library versions are Mongo ObjectID strings. - V2 library versions can be positive ints, or strings of positive ints. - Passing version=None will return the latest version the library. - - Returns None if not found. - If key is invalid, raises InvalidKeyError. - For V1, if key has a version, it is ignored in favor of `version`. - For V2, if version is provided but it isn't an int or parseable to one, we raise a ValueError. - - Examples: - * get_v1_or_v2_library("library-v1:ProblemX+PR0B", None) -> - * get_v1_or_v2_library("library-v1:ProblemX+PR0B", "65ff...") -> - * get_v1_or_v2_library("lib:RG:rg-1", None) -> - * get_v1_or_v2_library("lib:RG:rg-1", "36") -> - * get_v1_or_v2_library("lib:RG:rg-1", "xyz") -> - * get_v1_or_v2_library("notakey", "xyz") -> - - If you just want to get a V2 library, use `get_library` instead. - """ - library_key: LibraryLocatorV1 | LibraryLocatorV2 - if isinstance(library_id, str): - try: - library_key = LibraryLocatorV1.from_string(library_id) - except InvalidKeyError: - library_key = LibraryLocatorV2.from_string(library_id) - else: - library_key = library_id - if isinstance(library_key, LibraryLocatorV2): - v2_version: int | None - if version: - v2_version = int(version) - else: - v2_version = None - try: - library = get_library(library_key) - if v2_version is not None and library.version != v2_version: - raise NotImplementedError( - f"Tried to load version {v2_version} of learning_core-based library {library_key}. " - f"Currently, only the latest version ({library.version}) may be loaded. " - "This is a known issue. " - "It will be fixed before the production release of learning_core-based (V2) content libraries. " - ) - return library - except ContentLibrary.DoesNotExist: - return None - elif isinstance(library_key, LibraryLocatorV1): - v1_version: str | None - if version: - v1_version = str(version) - else: - v1_version = None - store = modulestore() - library_key = library_key.for_branch(ModuleStoreEnum.BranchName.library).for_version(v1_version) - try: - return store.get_library(library_key, remove_version=False, remove_branch=False, head_validation=False) - except ItemNotFoundError: - return None - - # Import from Courseware # ====================== diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 9f4f7aaaf7..f56b4adfe3 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -21,33 +21,20 @@ import logging from celery import shared_task from celery_utils.logged_task import LoggedTask from celery.utils.log import get_task_logger -from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user -from django.core.exceptions import PermissionDenied from edx_django_utils.monitoring import set_code_owner_attribute, set_code_owner_attribute_from_module -from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import ( - BlockUsageLocator, - LibraryLocatorV2, - LibraryUsageLocatorV2, - LibraryLocator as LibraryLocatorV1 -) from user_tasks.tasks import UserTask, UserTaskStatus from xblock.fields import Scope -from common.djangoapps.student.auth import has_studio_write_access from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.content_libraries import api as library_api -from openedx.core.djangoapps.xblock.api import load_block +from opaque_keys.edx.locator import BlockUsageLocator from openedx.core.lib import ensure_cms from xmodule.capa_block import ProblemBlock -from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LibraryContentBlock -from xmodule.library_root_xblock import LibraryRoot as LibraryRootV1 +from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.mixed import MixedModuleStore -from xmodule.modulestore.split_mongo import BlockKey -from xmodule.util.keys import derive_key from . import api from .models import ContentLibraryBlockImportTask @@ -84,77 +71,6 @@ def import_blocks_from_course(import_task_id, course_key_str, use_course_key_as_ ) -def _import_block(store, user_id, source_block, dest_parent_key): - """ - Recursively import a learning core block and its children.` - """ - def generate_block_key(source_key, dest_parent_key): - """ - Deterministically generate an ID for the new block and return the key. - Keys are generated such that they appear identical to a v1 library with - the same input block_id, library name, library organization, and parent block using derive_key - """ - if not isinstance(source_key.lib_key, LibraryLocatorV2): - raise TypeError(f"Expected source library key of type LibraryLocatorV2, got {source_key.lib_key} instead.") - source_key_as_v1_course_key = LibraryLocatorV1( - org=source_key.lib_key.org, - library=source_key.lib_key.slug, - branch='library' - ) - derived_block_key = derive_key( - source=source_key_as_v1_course_key.make_usage_key(source_key.block_type, source_key.block_id), - dest_parent=BlockKey(dest_parent_key.block_type, dest_parent_key.block_id), - ) - return dest_parent_key.context_key.make_usage_key(*derived_block_key) - - source_key = source_block.scope_ids.usage_id - new_block_key = generate_block_key(source_key, dest_parent_key) - try: - new_block = store.get_item(new_block_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, - ) - ) - except ItemNotFoundError: - new_block = store.create_child( - user_id, - dest_parent_key, - source_key.block_type, - block_id=new_block_key.block_id, - ) - - # Prepare a list of this block's static assets; any assets that are referenced as /static/{path} (the - # recommended way for referencing them) will stop working, and so we rewrite the url when importing. - # Copying assets not advised because modulestore doesn't namespace assets to each block like learning core, which - # might cause conflicts when the same filename is used across imported blocks. - if isinstance(source_key, LibraryUsageLocatorV2): - all_assets = library_api.get_library_block_static_asset_files(source_key) - else: - all_assets = [] - - for field_name, field in source_block.fields.items(): - if field.scope not in (Scope.settings, Scope.content): - continue # Only copy authored field data - if field.is_set_on(source_block) or field.is_set_on(new_block): - field_value = getattr(source_block, field_name) - setattr(new_block, field_name, field_value) - new_block.save() - store.update_item(new_block, user_id) - - if new_block.has_children: - # Delete existing children in the new block, which can be reimported again if they still exist in the - # source library - for existing_child_key in new_block.children: - store.delete_item(existing_child_key, user_id) - # Now import the children - for child in source_block.get_children(): - _import_block(store, user_id, child, new_block_key) - - return new_block_key - - def _filter_child(store, usage_key, capa_type): """ Return whether this block is both a problem and has a `capa_type` which is included in the filter. @@ -172,49 +88,6 @@ def _problem_type_filter(store, library, capa_type): return [key for key in library.children if _filter_child(store, key, capa_type)] -def _import_from_learning_core(user_id, store, dest_block, source_block_ids): - """ - Imports a block from a learning-core-based learning context (usually a - content library) into modulestore, as a new child of dest_block. - Any existing children of dest_block are replaced. - """ - dest_key = dest_block.scope_ids.usage_id - if not isinstance(dest_key, BlockUsageLocator): - raise TypeError(f"Destination {dest_key} should be a modulestore course.") - if user_id is None: - raise ValueError("Cannot check user permissions - LibraryTools user_id is None") - - if len(set(source_block_ids)) != len(source_block_ids): - # We don't support importing the exact same block twice because it would break the way we generate new IDs - # for each block and then overwrite existing copies of blocks when re-importing the same blocks. - raise ValueError("One or more library component IDs is a duplicate.") - - dest_course_key = dest_key.context_key - user = User.objects.get(id=user_id) - if not has_studio_write_access(user, dest_course_key): - raise PermissionDenied() - - # Read the source block; this will also confirm that user has permission to read it. - # (This could be slow and use lots of memory, except for the fact that LibraryContentBlock which calls this - # should be limiting the number of blocks to a reasonable limit. We load them all now instead of one at a - # time in order to raise any errors before we start actually copying blocks over.) - orig_blocks = [load_block(UsageKey.from_string(key), user) for key in source_block_ids] - - with store.bulk_operations(dest_course_key): - child_ids_updated = set() - - for block in orig_blocks: - new_block_id = _import_block(store, user_id, block, dest_key) - child_ids_updated.add(new_block_id) - - # Remove any existing children that are no longer used - for old_child_id in set(dest_block.children) - child_ids_updated: - store.delete_item(old_child_id, user_id) - # If this was called from a handler, it will save dest_block at the end, so we must update - # dest_block.children to avoid it saving the old value of children and deleting the new ones. - dest_block.children = store.get_item(dest_key).children - - class LibrarySyncChildrenTask(UserTask): # pylint: disable=abstract-method """ Base class for tasks which operate upon library_content children. @@ -244,7 +117,7 @@ def sync_from_library( self: LibrarySyncChildrenTask, user_id: int, dest_block_id: str, - library_version: str | int | None, + library_version: str | None, ) -> None: """ Celery task to update the children of the library_content block at `dest_block_id`. @@ -300,8 +173,8 @@ def _sync_children( task: LibrarySyncChildrenTask, store: MixedModuleStore, user_id: int, - dest_block: LibraryContentBlock, - library_version: int | str | None, + dest_block: LegacyLibraryContentBlock, + library_version: str | None, ) -> None: """ Implementation helper for `sync_from_library` and `duplicate_children` Celery tasks. @@ -309,41 +182,29 @@ def _sync_children( Can update children with a specific library `library_version`, or latest (`library_version=None`). """ source_blocks = [] - library_key = dest_block.source_library_key - filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE) - library = library_api.get_v1_or_v2_library(library_key, version=library_version) - if not library: + library_key = dest_block.source_library_key.for_branch( + ModuleStoreEnum.BranchName.library + ).for_version(library_version) + try: + library = store.get_library(library_key, remove_version=False, remove_branch=False, head_validation=False) + except ItemNotFoundError: task.status.fail(f"Requested library {library_key} not found.") - elif isinstance(library, LibraryRootV1): - if filter_children: - # Apply simple filtering based on CAPA problem types: - source_blocks.extend(_problem_type_filter(store, library, dest_block.capa_type)) - else: - source_blocks.extend(library.children) - with store.bulk_operations(dest_block.scope_ids.usage_id.context_key): - try: - dest_block.source_library_version = str(library.location.library_key.version_guid) - store.update_item(dest_block, user_id) - dest_block.children = store.copy_from_template( - source_blocks, dest_block.location, user_id, head_validation=True - ) - # ^-- copy_from_template updates the children in the DB - # but we must also set .children here to avoid overwriting the DB again - except Exception as exception: # pylint: disable=broad-except - TASK_LOGGER.exception('Error importing children for %s', dest_block.scope_ids.usage_id, exc_info=True) - if task.status.state != UserTaskStatus.FAILED: - task.status.fail({'raw_error_msg': str(exception)}) - raise - elif isinstance(library, library_api.ContentLibraryMetadata): - # TODO: add filtering by capa_type when V2 library will support different problem types + return + filter_children = (dest_block.capa_type != ANY_CAPA_TYPE_VALUE) + if filter_children: + # Apply simple filtering based on CAPA problem types: + source_blocks.extend(_problem_type_filter(store, library, dest_block.capa_type)) + else: + source_blocks.extend(library.children) + with store.bulk_operations(dest_block.scope_ids.usage_id.context_key): try: - source_block_ids = [ - str(library_api.LibraryXBlockMetadata.from_component(library_key, component).usage_key) - for component in library_api.get_library_components(library_key) - ] - _import_from_learning_core(user_id, store, dest_block, source_block_ids) - dest_block.source_library_version = str(library.version) + dest_block.source_library_version = str(library.location.library_key.version_guid) store.update_item(dest_block, user_id) + dest_block.children = store.copy_from_template( + source_blocks, dest_block.location, user_id, head_validation=True + ) + # ^-- copy_from_template updates the children in the DB + # but we must also set .children here to avoid overwriting the DB again except Exception as exception: # pylint: disable=broad-except TASK_LOGGER.exception('Error importing children for %s', dest_block.scope_ids.usage_id, exc_info=True) if task.status.state != UserTaskStatus.FAILED: @@ -354,8 +215,8 @@ def _sync_children( def _copy_overrides( store: MixedModuleStore, user_id: int, - source_block: LibraryContentBlock, - dest_block: LibraryContentBlock + source_block: LegacyLibraryContentBlock, + dest_block: LegacyLibraryContentBlock ) -> None: """ Copy any overrides the user has made on children of `source` over to the children of `dest_block`, recursively. diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index 26127dbfb3..a8b76541b6 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -452,7 +452,7 @@ def xblock_resource_pkg(block): ProblemBlock, and most other built-in blocks currently. Handling for these assets does not interact with this function. 2. The (preferred) standard XBlock runtime resource loading system, used by - LibraryContentBlock. Handling for these assets *does* interact with this + LegacyLibraryContentBlock. Handling for these assets *does* interact with this function. We hope to migrate to (2) eventually, tracked by: diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index f4088678d9..7a6fad8ece 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -10,7 +10,7 @@ from completion.test_utils import CompletionWaffleTestMixin from django.conf import settings from django.test import override_settings from opaque_keys.edx.keys import CourseKey -from xmodule.library_tools import LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory, LibraryFactory from xmodule.tests import prepare_block_runtime @@ -122,7 +122,7 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest Bind a block (part of self.course) so we can access student-specific data. """ prepare_block_runtime(block.runtime, course_id=block.location.course_key) - block.runtime._services.update({'library_tools': LibraryToolsService(self.store, self.user.id)}) # lint-amnesty, pylint: disable=protected-access + block.runtime._services.update({'library_tools': LegacyLibraryToolsService(self.store, self.user.id)}) # lint-amnesty, pylint: disable=protected-access def get_block(descriptor): """Mocks module_system get_block_for_descriptor function""" diff --git a/setup.py b/setup.py index 28a25cc914..21c8e537c9 100644 --- a/setup.py +++ b/setup.py @@ -21,7 +21,7 @@ XBLOCKS = [ "html = xmodule.html_block:HtmlBlock", "image = xmodule.template_block:TranslateCustomTagBlock", "library = xmodule.library_root_xblock:LibraryRoot", - "library_content = xmodule.library_content_block:LibraryContentBlock", + "library_content = xmodule.library_content_block:LegacyLibraryContentBlock", "lti = xmodule.lti_block:LTIBlock", "poll_question = xmodule.poll_block:PollBlock", "problem = xmodule.capa_block:ProblemBlock", diff --git a/webpack.builtinblocks.config.js b/webpack.builtinblocks.config.js index d86f891dc6..1c5a9b1e0e 100644 --- a/webpack.builtinblocks.config.js +++ b/webpack.builtinblocks.config.js @@ -38,6 +38,10 @@ module.exports = { './xmodule/js/src/xmodule.js', './xmodule/js/src/html/edit.js' ], + LibraryContentBlockEditor: [ + './xmodule/js/src/xmodule.js', + './xmodule/js/src/vertical/edit.js' + ], LTIBlockDisplay: [ './xmodule/js/src/xmodule.js', './xmodule/js/src/lti/lti.js' @@ -46,11 +50,6 @@ module.exports = { './xmodule/js/src/xmodule.js', './xmodule/js/src/raw/edit/metadata-only.js' ], - LibraryContentBlockDisplay: './xmodule/js/src/xmodule.js', - LibraryContentBlockEditor: [ - './xmodule/js/src/xmodule.js', - './xmodule/js/src/vertical/edit.js' - ], PollBlockDisplay: [ './xmodule/js/src/xmodule.js', './xmodule/js/src/javascript_loader.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 deleted file mode 100644 index 564cc5fb0f..0000000000 --- a/xmodule/assets/library_content/public/js/library_content_edit_helpers.js +++ /dev/null @@ -1,54 +0,0 @@ -/* JavaScript for special editing operations that can be done on LibraryContentXBlock */ -// This is a temporary UI improvements that will be removed when V2 content libraries became -// fully functional - -/** - * Toggle the "Problem Type" settings section depending on selected library type. - * As for now, the V2 libraries don't support different problem types, so they can't be - * filtered by it. We're hiding the Problem Type field for them. - */ -function checkProblemTypeShouldBeVisible(editor) { - var libraries = editor.find('.wrapper-comp-settings.metadata_edit.is-active') - .data().metadata.source_library_id.options; - var selectedIndex = $("select[name='Library']", editor)[0].selectedIndex; - var libraryKey = libraries[selectedIndex].value; - var url = URI('/xblock') - .segment(editor.find('.xblock.xblock-studio_view.xblock-studio_view-library_content.xblock-initialized') - .data('usage-id')) - .segment('handler') - .segment('is_v2_library'); - - $.ajax({ - type: 'POST', - url: url, - data: JSON.stringify({'library_key': libraryKey}), - success: function(data) { - var problemTypeSelect = editor.find("select[name='Problem Type']") - .parents("li.field.comp-setting-entry.metadata_entry"); - data.is_v2 ? problemTypeSelect.hide() : problemTypeSelect.show(); - } - }); -} - -/** - * Waits untill editor html loaded, than calls checks for Program Type field toggling. - */ -function waitForEditorLoading() { - var checkContent = setInterval(function() { - var $modal = $('.xblock-editor'); - var content = $modal.html(); - if (content) { - clearInterval(checkContent); - checkProblemTypeShouldBeVisible($modal); - } - }, 10); -} -// Initial call -waitForEditorLoading(); - -var $librarySelect = $("select[name='Library']"); -$(document).on('change', $librarySelect, waitForEditorLoading) - -var $libraryContentEditors = $('.xblock-header.xblock-header-library_content'); -var $editBtns = $libraryContentEditors.find('.action-item.action-edit'); -$(document).on('click', $editBtns, waitForEditorLoading) diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 09a5d1dee1..adb07101d0 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -1,5 +1,12 @@ """ -LibraryContent: The XBlock used to include blocks from a library in a course. +LegacyLibraryContent: The XBlock used to randomly select a subset of blocks from a "v1" (modulestore-backed) library. + +In Studio, it's called the "Randomized Content Module". + +In the long-term, this block is deprecated in favor of "v2" (learning core-backed) library references: +https://github.com/openedx/edx-platform/issues/32457 + +We need to retain backwards-compatibility, but please do not build any new features into this. """ from __future__ import annotations @@ -15,8 +22,7 @@ from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.utils.functional import classproperty from lxml import etree from lxml.etree import XMLSyntaxError -from opaque_keys import InvalidKeyError -from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryLocator from rest_framework import status from web_fragments.fragment import Fragment from webob import Response @@ -78,7 +84,7 @@ class LibraryToolsUnavailable(ValueError): @XBlock.wants('studio_user_permissions') # Only available in CMS. @XBlock.wants('user') @XBlock.needs('mako') -class LibraryContentBlock( +class LegacyLibraryContentBlock( MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, @@ -87,7 +93,7 @@ class LibraryContentBlock( StudioEditableBlock, ): """ - An XBlock whose children are chosen dynamically from a content library. + An XBlock whose children are chosen dynamically from a legacy (v1) content library. Can be used to create randomized assessments among other things. Note: technically, all matching blocks from the content library are added @@ -135,17 +141,6 @@ class LibraryContentBlock( display_name=_("Library Version"), scope=Scope.settings, ) - mode = String( - display_name=_("Mode"), - help=_("Determines how content is drawn from the library"), - default="random", - values=[ - {"display_name": _("Choose n at random"), "value": "random"} - # Future addition: Choose a new random set of n every time the student refreshes the block, for self tests - # Future addition: manually selected blocks - ], - scope=Scope.settings, - ) max_count = Integer( display_name=_("Count"), help=_("Enter the number of components to display to each student. Set it to -1 to display all components."), @@ -179,15 +174,12 @@ class LibraryContentBlock( """ Convenience method to get the library ID as a LibraryLocator and not just a string. - Supports either library v1 or library v2 locators. + Supports only v1 libraries. """ - try: - return LibraryLocator.from_string(self.source_library_id) - except InvalidKeyError: - return LibraryLocatorV2.from_string(self.source_library_id) + return LibraryLocator.from_string(self.source_library_id) @classmethod - def make_selection(cls, selected, children, max_count, mode): + def make_selection(cls, selected, children, max_count): """ Dynamically selects block_ids indicating which of the possible children are displayed to the current user. @@ -195,7 +187,6 @@ class LibraryContentBlock( selected - list of (block_type, block_id) tuples assigned to this student children - children of this block max_count - number of components to display to each student - mode - how content is drawn from the library Returns: A dict containing the following keys: @@ -231,12 +222,9 @@ class LibraryContentBlock( if num_to_add > 0: # We need to select [more] blocks to display to this user: pool = valid_block_keys - selected_keys - if mode == "random": - num_to_add = min(len(pool), num_to_add) - added_block_keys = set(rand.sample(list(pool), num_to_add)) - # We now have the correct n random children to show for this user. - else: - raise NotImplementedError("Unsupported mode.") + num_to_add = min(len(pool), num_to_add) + added_block_keys = set(rand.sample(list(pool), num_to_add)) + # We now have the correct n random children to show for this user. selected_keys |= added_block_keys if any((invalid_block_keys, overlimit_block_keys, added_block_keys)): @@ -334,7 +322,7 @@ class LibraryContentBlock( if max_count < 0: max_count = len(self.children) - block_keys = self.make_selection(self.selected, self.children, max_count, "random") # pylint: disable=no-member + block_keys = self.make_selection(self.selected, self.children, max_count) # pylint: disable=no-member # Publish events for analytics purposes: lib_tools = self.get_tools() @@ -467,7 +455,6 @@ class LibraryContentBlock( fragment = Fragment( self.runtime.service(self, 'mako').render_cms_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_js_to_fragment(fragment, 'LibraryContentBlockEditor') shim_xmodule_js(fragment, self.studio_js_module_name) return fragment @@ -481,16 +468,12 @@ class LibraryContentBlock( @property def non_editable_metadata_fields(self): non_editable_fields = super().non_editable_metadata_fields - # The only supported mode is currently 'random'. - # Add the mode field to non_editable_metadata_fields so that it doesn't - # render in the edit form. non_editable_fields.extend([ - LibraryContentBlock.mode, - LibraryContentBlock.source_library_version, + LegacyLibraryContentBlock.source_library_version, ]) return non_editable_fields - def get_tools(self, to_read_library_content: bool = False) -> 'LibraryToolsService': + def get_tools(self, to_read_library_content: bool = False) -> 'LegacyLibraryToolsService': """ Grab the library tools service and confirm that it'll work for us. Else, raise LibraryToolsUnavailable. """ @@ -564,22 +547,6 @@ class LibraryContentBlock( library_version=(None if upgrade_to_latest else self.source_library_version), ) - @XBlock.json_handler - def is_v2_library(self, data, suffix=''): # 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} - @XBlock.handler def children_are_syncing(self, request, suffix=''): # pylint: disable=unused-argument """ @@ -809,14 +776,14 @@ class LibraryContentBlock( return xml_object -class LibrarySummary: +class LegacyLibrarySummary: """ A library summary object which contains the fields required for library listing on studio. """ def __init__(self, library_locator, display_name): """ - Initialize LibrarySummary + Initialize LegacyLibrarySummary Arguments: library_locator (LibraryLocator): LibraryLocator object of the library. diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index 2c077a8884..7f9e83a937 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -1,18 +1,17 @@ """ -XBlock runtime services for LibraryContentBlock +XBlock runtime services for LegacyLibraryContentBlock """ from __future__ import annotations from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user -from django.conf import settings from django.core.exceptions import ObjectDoesNotExist +from opaque_keys.edx.locator import LibraryLocator from user_tasks.models import UserTaskStatus from openedx.core.lib import ensure_cms -from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_libraries import tasks as library_tasks -from xmodule.library_content_block import LibraryContentBlock -from xmodule.library_root_xblock import LibraryRoot as LibraryRootV1 +from xmodule.library_content_block import LegacyLibraryContentBlock +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.exceptions import ItemNotFoundError @@ -21,9 +20,9 @@ def normalize_key_for_search(library_key): return library_key.replace(version_guid=None, branch=None) -class LibraryToolsService: +class LegacyLibraryToolsService: """ - Service for LibraryContentBlock. + Service for LegacyLibraryContentBlock. Allows to interact with libraries in the modulestore and learning core. @@ -33,24 +32,31 @@ class LibraryToolsService: self.store = modulestore self.user_id = user_id - def get_latest_library_version(self, lib_key) -> str | None: + def get_latest_library_version(self, library_id: str | LibraryLocator) -> str | None: """ Get the version of the given library as string. The return value (library version) could be: str() - for V1 library; - str() - for V2 library. None - if the library does not exist. """ - library = library_api.get_v1_or_v2_library(lib_key, version=None) + library_key: LibraryLocator + if isinstance(library_id, str): + library_key = LibraryLocator.from_string(library_id) + else: + library_key = library_id + library_key = library_key.for_branch(ModuleStoreEnum.BranchName.library).for_version(None) + try: + library = self.store.get_library( + library_key, remove_version=False, remove_branch=False, head_validation=False + ) + except ItemNotFoundError: + return None if not library: return None - elif isinstance(library, LibraryRootV1): - # 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 str(library.location.library_key.version_guid) - elif isinstance(library, library_api.ContentLibraryMetadata): - return str(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 str(library.location.library_key.version_guid) def create_block_analytics_summary(self, course_key, block_keys): """ @@ -96,7 +102,7 @@ class LibraryToolsService: """ return self.store.check_supports(block.location.course_key, 'copy_from_template') - def trigger_library_sync(self, dest_block: LibraryContentBlock, library_version: str | int | None) -> None: + def trigger_library_sync(self, dest_block: LegacyLibraryContentBlock, library_version: str | None) -> None: """ Queue task to synchronize the children of `dest_block` with it source library (at `library_version` or latest). @@ -118,16 +124,20 @@ class LibraryToolsService: `dest_block.children`. """ ensure_cms("library_content block children may only be synced in a CMS context") - if not isinstance(dest_block, LibraryContentBlock): + if not isinstance(dest_block, LegacyLibraryContentBlock): raise ValueError(f"Can only sync children for library_content blocks, not {dest_block.tag} blocks.") if not dest_block.source_library_id: dest_block.source_library_version = "" return - library_key = dest_block.source_library_key - if not library_api.get_v1_or_v2_library(library_key, version=library_version): + library_key = dest_block.source_library_key.for_branch( + ModuleStoreEnum.BranchName.library + ).for_version(library_version) + try: + self.store.get_library(library_key, remove_version=False, remove_branch=False, head_validation=False) + except ItemNotFoundError as exc: if library_version: - raise ObjectDoesNotExist(f"Version {library_version} of library {library_key} not found.") - raise ObjectDoesNotExist(f"Library {library_key} not found.") + raise ObjectDoesNotExist(f"Version {library_version} of library {library_key} not found.") from exc + raise ObjectDoesNotExist(f"Library {library_key} not found.") from exc # TODO: This task is synchronous until we can figure out race conditions with import. # These race conditions lead to failed imports of library content from course import. @@ -140,12 +150,14 @@ class LibraryToolsService: ), ) - def trigger_duplication(self, source_block: LibraryContentBlock, dest_block: LibraryContentBlock) -> None: + def trigger_duplication( + self, source_block: LegacyLibraryContentBlock, dest_block: LegacyLibraryContentBlock + ) -> None: """ Queue a task to duplicate the children of `source_block` to `dest_block`. """ ensure_cms("library_content block children may only be duplicated in a CMS context") - if not isinstance(dest_block, LibraryContentBlock): + if not isinstance(dest_block, LegacyLibraryContentBlock): raise ValueError(f"Can only duplicate children for library_content blocks, not {dest_block.tag} blocks.") if source_block.scope_ids.usage_id.context_key != source_block.scope_ids.usage_id.context_key: raise ValueError( @@ -163,7 +175,7 @@ class LibraryToolsService: dest_block_id=str(dest_block.scope_ids.usage_id), ) - def are_children_syncing(self, library_content_block: LibraryContentBlock) -> bool: + def are_children_syncing(self, library_content_block: LegacyLibraryContentBlock) -> bool: """ Is a task currently running to sync the children of `library_content_block`? @@ -179,21 +191,12 @@ class LibraryToolsService: def list_available_libraries(self): """ - List all known libraries. + List all known legacy libraries. - Collects Only V2 Libaries if the FEATURES[ENABLE_LIBRARY_AUTHORING_MICROFRONTEND] setting is True. - Otherwise, return all v1 and v2 libraries. Returns tuples of (library key, display_name). """ user = User.objects.get(id=self.user_id) - v1_libs = [ + return [ (lib.location.library_key.replace(version_guid=None, branch=None), lib.display_name) for lib in self.store.get_library_summaries() ] - v2_query = library_api.get_libraries_for_user(user) - v2_libs_with_meta = library_api.get_metadata(v2_query) - v2_libs = [(lib.key, lib.title) for lib in v2_libs_with_meta] - - if settings.FEATURES.get('ENABLE_LIBRARY_AUTHORING_MICROFRONTEND'): - return v2_libs - return v1_libs + v2_libs diff --git a/xmodule/modulestore/mixed.py b/xmodule/modulestore/mixed.py index e1ea6640ac..fb5be17041 100644 --- a/xmodule/modulestore/mixed.py +++ b/xmodule/modulestore/mixed.py @@ -343,7 +343,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): @strip_key def get_library_summaries(self, **kwargs): """ - Returns a list of LibrarySummary objects. + Returns a list of LegacyLibrarySummary objects. Information contains `location`, `display_name`, `locator` of the libraries in this modulestore. """ library_summaries = {} diff --git a/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/xmodule/modulestore/split_mongo/caching_descriptor_system.py index b0965d63fe..a83fec32ba 100644 --- a/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -13,7 +13,7 @@ from xblock.runtime import KeyValueStore, KvsFieldData from xmodule.error_block import ErrorBlock from xmodule.errortracker import exc_info_to_str -from xmodule.library_tools import LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.mako_block import MakoDescriptorSystem from xmodule.modulestore import BlockData from xmodule.modulestore.edit_info import EditInfoRuntimeMixin @@ -78,7 +78,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li user = get_current_user() user_id = user.id if user else None - self._services['library_tools'] = LibraryToolsService(modulestore, user_id=user_id) + self._services['library_tools'] = LegacyLibraryToolsService(modulestore, user_id=user_id) # Cache of block field datas, keyed by the XBlock instance (since the ScopeId changes!) self.block_field_datas = weakref.WeakKeyDictionary() diff --git a/xmodule/modulestore/split_mongo/split.py b/xmodule/modulestore/split_mongo/split.py index 64e19420a1..e69a2ca0e5 100644 --- a/xmodule/modulestore/split_mongo/split.py +++ b/xmodule/modulestore/split_mongo/split.py @@ -81,7 +81,7 @@ from xmodule.assetstore import AssetMetadata from xmodule.course_block import CourseSummary from xmodule.error_block import ErrorBlock from xmodule.errortracker import null_error_tracker -from xmodule.library_content_block import LibrarySummary +from xmodule.library_content_block import LegacyLibrarySummary from xmodule.modulestore import ( BlockData, BulkOperationsMixin, @@ -1029,7 +1029,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): @autoretry_read() def get_library_summaries(self, **kwargs): """ - Returns a list of `LibrarySummary` objects. + Returns a list of `LegacyLibrarySummary` objects. kwargs can be valid db fields to match against active_versions collection e.g org='example_org'. """ @@ -1057,7 +1057,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): display_name = library_block_fields['display_name'] libraries_summaries.append( - LibrarySummary(library_locator, display_name) + LegacyLibrarySummary(library_locator, display_name) ) return libraries_summaries diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index e30e19922b..9914ef2d90 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -1,7 +1,5 @@ """ -Basic unit tests for LibraryContentBlock - -Higher-level tests are in `cms/djangoapps/contentstore/tests/test_libraries.py`. +Basic unit tests for LegacyLibraryContentBlock """ from unittest.mock import MagicMock, Mock, patch @@ -9,15 +7,15 @@ 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 opaque_keys.edx.locator import LibraryLocator 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 openedx.core.djangolib.testing.utils import skip_unless_cms -from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LibraryContentBlock -from xmodule.library_tools import LibraryToolsService +from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase @@ -33,15 +31,15 @@ dummy_render = lambda block, _: Fragment(block.data) # pylint: disable=invalid- @skip_unless_cms -class LibraryContentTest(MixedSplitTestCase): +class LegacyLibraryContentTest(MixedSplitTestCase): """ - Base class for tests of LibraryContentBlock (library_content_block.py) + Base class for tests of LegacyLibraryContentBlock (library_content_block.py) """ def setUp(self): super().setUp() self.user_id = UserFactory().id - self.tools = LibraryToolsService(self.store, self.user_id) + self.tools = LegacyLibraryToolsService(self.store, self.user_id) self.library = LibraryFactory.create(modulestore=self.store) self.lib_blocks = [ self.make_block("html", self.library, data=f"Hello world from block {i}") @@ -88,29 +86,22 @@ class LibraryContentTest(MixedSplitTestCase): @ddt.ddt -class LibraryContentGeneralTest(LibraryContentTest): +class LegacyLibraryContentGeneralTest(LegacyLibraryContentTest): """ - Test the base functionality of the LibraryContentBlock. + Test the base functionality of the LegacyLibraryContentBlock. """ - @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): + def test_source_library_key(self): """ 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 + source_library_id='library-v1:ProblemX+PR0B', ) - assert isinstance(library.source_library_key, expected_locator_type) + assert isinstance(library.source_library_key, LibraryLocator) def test_initial_sync_from_library(self): """ @@ -133,9 +124,9 @@ class LibraryContentGeneralTest(LibraryContentTest): assert len(self.lc_block.children) == len(self.lib_blocks) -class TestLibraryContentExportImport(LibraryContentTest): +class TestLibraryContentExportImport(LegacyLibraryContentTest): """ - Export and import tests for LibraryContentBlock + Export and import tests for LegacyLibraryContentBlock """ def setUp(self): super().setUp() @@ -173,7 +164,6 @@ class TestLibraryContentExportImport(LibraryContentTest): assert imported_lc_block.display_name == self.lc_block.display_name assert imported_lc_block.source_library_id == self.lc_block.source_library_id assert imported_lc_block.source_library_version == self.lc_block.source_library_version - assert imported_lc_block.mode == self.lc_block.mode assert imported_lc_block.max_count == self.lc_block.max_count assert imported_lc_block.capa_type == self.lc_block.capa_type assert len(imported_lc_block.children) == len(self.lc_block.children) @@ -195,13 +185,13 @@ class TestLibraryContentExportImport(LibraryContentTest): # Now import it. olx_element = etree.fromstring(exported_olx) - imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None) + imported_lc_block = LegacyLibraryContentBlock.parse_xml(olx_element, self.runtime, None) self._verify_xblock_properties(imported_lc_block) def test_xml_import_with_comments(self): """ - Test that XML comments within LibraryContentBlock are ignored during the import. + Test that XML comments within LegacyLibraryContentBlock are ignored during the import. """ olx_with_comments = ( '\n' @@ -219,15 +209,15 @@ class TestLibraryContentExportImport(LibraryContentTest): # Import the olx. olx_element = etree.fromstring(olx_with_comments) - imported_lc_block = LibraryContentBlock.parse_xml(olx_element, self.runtime, None) + imported_lc_block = LegacyLibraryContentBlock.parse_xml(olx_element, self.runtime, None) self._verify_xblock_properties(imported_lc_block) @ddt.ddt -class LibraryContentBlockTestMixin: +class LegacyLibraryContentBlockTestMixin: """ - Basic unit tests for LibraryContentBlock + Basic unit tests for LegacyLibraryContentBlock """ problem_types = [ ["multiplechoiceresponse"], ["optionresponse"], ["optionresponse", "coderesponse"], @@ -424,8 +414,7 @@ class LibraryContentBlockTestMixin: Test the settings that are marked as "non-editable". """ non_editable_metadata_fields = self.lc_block.non_editable_metadata_fields - assert LibraryContentBlock.mode in non_editable_metadata_fields - assert LibraryContentBlock.display_name not in non_editable_metadata_fields + assert LegacyLibraryContentBlock.display_name not in non_editable_metadata_fields def test_overlimit_blocks_chosen_randomly(self): """ @@ -503,7 +492,7 @@ search_index_mock = Mock(spec=SearchEngine) # pylint: disable=invalid-name @patch.object(SearchEngine, 'get_search_engine', Mock(return_value=None, autospec=True)) -class TestLibraryContentBlockWithSearchIndex(LibraryContentBlockTestMixin, LibraryContentTest): +class TestLegacyLibraryContentBlockWithSearchIndex(LegacyLibraryContentBlockTestMixin, LegacyLibraryContentTest): """ Tests for library container with mocked search engine response. """ @@ -532,9 +521,9 @@ class TestLibraryContentBlockWithSearchIndex(LibraryContentBlockTestMixin, Libra ) @patch('xmodule.html_block.HtmlBlock.author_view', dummy_render, create=True) @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) -class TestLibraryContentRender(LibraryContentTest): +class TestLibraryContentRender(LegacyLibraryContentTest): """ - Rendering unit tests for LibraryContentBlock + Rendering unit tests for LegacyLibraryContentBlock """ def setUp(self): @@ -559,9 +548,9 @@ class TestLibraryContentRender(LibraryContentTest): # but some js initialization should happen -class TestLibraryContentAnalytics(LibraryContentTest): +class TestLibraryContentAnalytics(LegacyLibraryContentTest): """ - Test analytics features of LibraryContentBlock + Test analytics features of LegacyLibraryContentBlock """ def setUp(self): @@ -573,7 +562,7 @@ class TestLibraryContentAnalytics(LibraryContentTest): def _assert_event_was_published(self, event_type): """ - Check that a LibraryContentBlock analytics event was published by self.lc_block. + Check that a LegacyLibraryContentBlock analytics event was published by self.lc_block. """ assert self.publisher.called assert len(self.publisher.call_args[0]) == 3 # pylint:disable=unsubscriptable-object diff --git a/xmodule/tests/test_library_tools.py b/xmodule/tests/test_library_tools.py index f93066cd5c..30b007c3d9 100644 --- a/xmodule/tests/test_library_tools.py +++ b/xmodule/tests/test_library_tools.py @@ -1,23 +1,20 @@ """ -Tests for library tools service (only used by CMS) +Tests for legacy library tools service (only used by CMS) -Currently, the only known user of the LibraryToolsService is the -LibraryContentBlock, so these tests are all written with only that +The only known user of the LegacyLibraryToolsService is the +LegacyLibraryContentBlock, so these tests are all written with only that block type in mind. """ from unittest import mock import ddt -from django.conf import settings -from django.test import override_settings -from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryLocator -from common.djangoapps.student.roles import CourseInstructorRole from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import skip_unless_cms from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest -from xmodule.library_tools import LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.modulestore.tests.factories import CourseFactory, LibraryFactory from xmodule.modulestore.tests.utils import MixedSplitTestCase @@ -26,34 +23,23 @@ from xmodule.modulestore.tests.utils import MixedSplitTestCase @ddt.ddt class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest): """ - Tests for LibraryToolsService. - - Tests interaction with learning-core-based (V2) and mongo-based (V1) content libraries. + Tests for LegacyLibraryToolsService. """ def setUp(self): super().setUp() UserFactory(is_staff=True, id=self.user_id) - self.tools = LibraryToolsService(self.store, self.user_id) + self.tools = LegacyLibraryToolsService(self.store, self.user_id) def test_list_available_libraries(self): """ - Test listing of libraries. - - Collects Only V2 Libaries if the FEATURES[ENABLE_LIBRARY_AUTHORING_MICROFRONTEND] setting is True. - Otherwise, return all v1 and v2 libraries. + Test listing of v1 libraries. """ # create V1 library _ = LibraryFactory.create(modulestore=self.store) - # create V2 library + # create V2 library (should not be included in this list) self._create_library(slug="testlib1_preview", title="Test Library 1", description="Testing XBlocks") all_libraries = self.tools.list_available_libraries() - assert all_libraries - assert len(all_libraries) == 2 - - with override_settings(FEATURES={**settings.FEATURES, "ENABLE_LIBRARY_AUTHORING_MICROFRONTEND": True}): - all_libraries = self.tools.list_available_libraries() - assert all_libraries - assert len(all_libraries) == 1 + assert len(all_libraries) == 1 @mock.patch('xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.get_library_summaries') def test_list_available_libraries_fetch(self, mock_get_library_summaries): @@ -63,7 +49,7 @@ class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest): _ = self.tools.list_available_libraries() assert mock_get_library_summaries.called - def test_get_latest_v1_library_version(self): + def test_get_latest_library_version(self): """ Test get_v1_library_version for V1 libraries. @@ -84,49 +70,16 @@ class ContentLibraryToolsTest(MixedSplitTestCase, ContentLibrariesRestApiTest): assert result == str(lib.location.library_key.version_guid) @ddt.data( - 'library-v1:Fake+Key', # V1 library key - 'lib:Fake:V-2', # V2 library key + 'library-v1:Fake+Key', LibraryLocator.from_string('library-v1:Fake+Key'), - LibraryLocatorV2.from_string('lib:Fake:V-2'), ) def test_get_latest_library_version_no_library(self, lib_key): """ Test get_latest_library_version result when the library does not exist. - - Provided lib_key's are valid V1 or V2 keys. """ assert self.tools.get_latest_library_version(lib_key) is None - def test_update_children_for_v2_lib(self): - """ - Test update_children with V2 library as a source. - """ - 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") - - 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 - - # Populate children from library - self.tools.trigger_library_sync(content_block, library_version=None) - - # The updates happen in a Celery task, so this particular content_block instance is no updated. - # We must re-instantiate it from modulstore in order to see the updated children list. - content_block = self.store.get_item(content_block.location) - - assert len(content_block.children) == 1 - - def test_update_children_for_v1_lib(self): + def test_update_children(self): """ Test update_children with V1 library as a source. diff --git a/xmodule/tests/test_randomize_block.py b/xmodule/tests/test_randomize_block.py index deebdfe4f1..52413faad3 100644 --- a/xmodule/tests/test_randomize_block.py +++ b/xmodule/tests/test_randomize_block.py @@ -16,7 +16,7 @@ from .test_course_block import DummySystem as TestImportSystem class RandomizeBlockTest(MixedSplitTestCase): """ - Base class for tests of LibraryContentBlock (library_content_block.py) + Base class for tests of RandomizeBlock (randomize_block.py) """ maxDiff = None diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index 2a10ae4449..81621a7a7b 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -188,7 +188,7 @@ class VerticalBlock( if has_access_error: return True - # Check child nodes if they exist (e.g. randomized library question aka LibraryContentBlock) + # Check child nodes if they exist (e.g. randomized library question aka LegacyLibraryContentBlock) for child in block.get_children(): has_access_error = getattr(child, 'has_access_error', False) if has_access_error: