Revert "feat: provisionally support V2 libraries in LibraryContentBlo…

…ck (randomized only) (#33263)"

This reverts commit e800ae7.

cms/djangoapps/contentstore/views/component.py

@@ -195,9 +195,6 @@ def container_handler(request, usage_key_string):
 
             # Get the status of the user's clipboard so they can paste components if they have something to paste
             user_clipboard = content_staging_api.get_user_clipboard_json(request.user.id, request)
-            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,
                 'context_course': course,  # Needed only for display of menus at top of page.
@@ -206,7 +203,6 @@ def container_handler(request, usage_key_string):
                 'xblock_locator': xblock.location,
                 'unit': unit,
                 'is_unit_page': is_unit_page,
-                'is_collapsible': is_library_xblock,
                 'subsection': subsection,
                 'section': section,
                 'position': index,
@@ -222,7 +218,6 @@ def container_handler(request, usage_key_string):
                 'templates': CONTAINER_TEMPLATES,
                 # Status of the user's clipboard, exactly as would be returned from the "GET clipboard" REST API.
                 'user_clipboard': user_clipboard,
-                'is_fullwidth_content': is_library_xblock,
             })
     else:
         return HttpResponseBadRequest("Only supports HTML requests")

cms/djangoapps/contentstore/views/preview.py

@@ -45,7 +45,7 @@
     wrap_xblock_aside
 )
 
-from ..utils import get_visibility_partition_info, StudioPermissionsService
+from ..utils import get_visibility_partition_info
 from .access import get_user_role
 from .session_kv_store import SessionKeyValueStore
 
@@ -198,7 +198,6 @@ def _prepare_runtime_for_preview(request, block):
         deprecated_anonymous_user_id = anonymous_id_for_user(request.user, None)
 
     services = {
-        "studio_user_permissions": StudioPermissionsService(request.user),
         "i18n": XBlockI18nService,
         'mako': mako_service,
         "settings": SettingsService(),
@@ -311,9 +310,6 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
             'is_reorderable': is_reorderable,
             'can_edit': can_edit,
             'can_edit_visibility': context.get('can_edit_visibility', is_course),
-            'is_loading': context.get('is_loading', False),
-            'is_selected': context.get('is_selected', False),
-            'selectable': context.get('selectable', False),
             'selected_groups_label': selected_groups_label,
             'can_add': context.get('can_add', True),
             'can_move': context.get('can_move', is_course),

cms/djangoapps/contentstore/views/tests/test_block.py

@@ -211,7 +211,7 @@ def test_get_empty_container_fragment(self):
         self.assertNotRegex(html, r"wrapper-xblock[^-]+")
 
         # Verify that the header and article tags are still added
-        self.assertIn('<header class="xblock-header xblock-header-vertical ">', html)
+        self.assertIn('<header class="xblock-header xblock-header-vertical">', html)
         self.assertIn('<article class="xblock-render">', html)
 
     def test_get_container_fragment(self):
@@ -233,7 +233,7 @@ def test_get_container_fragment(self):
 
         # Verify that the Studio nesting wrapper has been added
         self.assertIn("level-nesting", html)
-        self.assertIn('<header class="xblock-header xblock-header-vertical ">', html)
+        self.assertIn('<header class="xblock-header xblock-header-vertical">', html)
         self.assertIn('<article class="xblock-render">', html)
 
         # Verify that the Studio element wrapper has been added
@@ -950,135 +950,6 @@ def test_shallow_duplicate(self):
         self.assertEqual(len(destination_chapter.get_children()), 0)
         self.assertEqual(destination_chapter.display_name, "Source Chapter")
 
-    def test_duplicate_library_content_block(self):  # pylint: disable=too-many-statements
-        """
-        Test the LibraryContentBlock's special duplication process.
-        """
-        store = modulestore()
-
-        # Create a library with two blocks (HTML 1 and HTML 2).
-        # These are the "Original" version of the blocks.
-        lib = LibraryFactory()
-        BlockFactory(
-            parent=lib,
-            category="html",
-            display_name="HTML 1 Title (Original)",
-            data="HTML 1 Content (Original)",
-            publish_item=False,
-        )
-        BlockFactory(
-            parent=lib,
-            category="html",
-            display_name="HTML 2 Title (Original)",
-            data="HTML 2 Content (Original)",
-            publish_item=False,
-        )
-        original_lib_version = store.get_library(
-            lib.location.library_key, remove_version=False, remove_branch=False,
-        ).location.library_key.version_guid
-        assert original_lib_version is not None
-
-        # Create a library content block (lc), point it out our library, and sync it.
-        unit = BlockFactory(
-            parent_location=self.seq_usage_key,
-            category="vertical",
-            display_name="Parent Unit of LC and its Dupe",
-            publish_item=False,
-        )
-        lc = BlockFactory(
-            parent=unit,
-            category="library_content",
-            source_library_id=str(lib.location.library_key),
-            display_name="LC Block",
-            max_count=1,
-            publish_item=False,
-        )
-        lc.sync_from_library()
-        lc = store.get_item(lc.location)  # we must reload because sync_from_library happens out-of-thread
-        assert lc.source_library_version == str(original_lib_version)
-        lc_html_1 = store.get_item(lc.children[0])
-        lc_html_2 = store.get_item(lc.children[1])
-        assert lc_html_1.display_name == "HTML 1 Title (Original)"
-        assert lc_html_2.display_name == "HTML 2 Title (Original)"
-        assert lc_html_1.data == "HTML 1 Content (Original)"
-        assert lc_html_2.data == "HTML 2 Content (Original)"
-
-        # Override the title and data of HTML 1 under lc ("Course Override").
-        # Note that title is settings-scoped and data is content-scoped.
-        lc_html_1.display_name = "HTML 1 Title (Course Override)"
-        lc_html_1.data = "HTML 1 Content (Course Override)"
-        store.update_item(lc_html_1, self.user.id)
-
-        # Now, update the titles and contents of both HTML 1 and HTML 2 ("Lib Update").
-        # This will yield a new version of the library (updated_lib_version).
-        lib_html_1 = store.get_item(lib.children[0])
-        lib_html_2 = store.get_item(lib.children[1])
-        assert lib_html_1.display_name == "HTML 1 Title (Original)"
-        assert lib_html_2.display_name == "HTML 2 Title (Original)"
-        lib_html_1.display_name = "HTML 1 Title (Lib Update)"
-        lib_html_2.display_name = "HTML 2 Title (Lib Update)"
-        lib_html_1.data = "HTML 1 Content (Lib Update)"
-        lib_html_2.data = "HTML 2 Content (Lib Update)"
-        store.update_item(lib_html_1, self.user.id)
-        store.update_item(lib_html_2, self.user.id)
-        updated_lib_version = store.get_library(
-            lib.location.library_key, remove_version=False, remove_branch=False,
-        ).location.library_key.version_guid
-        assert updated_lib_version is not None
-        assert updated_lib_version != original_lib_version
-
-        # DUPLICATE lc.
-        # Unit should now contain both lc and dupe.
-        # All settings should match between lc and dupe.
-        dupe = store.get_item(
-            self._duplicate_item(
-                parent_usage_key=unit.location,
-                source_usage_key=lc.location,
-                display_name="Dupe LC Block",
-            )
-        )
-        lc = store.get_item(lc.location)
-        unit = store.get_item(unit.location)
-        assert unit.children == [lc.location, dupe.location]
-        assert len(lc.children) == len(dupe.children) == 2
-        assert lc.max_count == dupe.max_count == 1
-        assert lc.source_library_id == dupe.source_library_id == str(lib.location.library_key)
-        assert lc.source_library_version == dupe.source_library_version == str(original_lib_version)
-
-        # The lc block's children should remain unchanged.
-        # That is: HTML 1 has overrides, HTML 2 has originals.
-        lc_html_1 = store.get_item(lc.children[0])
-        assert lc_html_1.display_name == "HTML 1 Title (Course Override)"
-        assert lc_html_1.data == "HTML 1 Content (Course Override)"
-        lc_html_2 = store.get_item(lc.children[1])
-        assert lc_html_2.display_name == "HTML 2 Title (Original)"
-        assert lc_html_2.data == "HTML 2 Content (Original)"
-
-        # Now, the dupe's children should copy *settings* overrides over from the lc block,
-        # but we don't actually expect it to copy *content* overrides over from the lc block.
-        # (Yes, this is weird. One would expect it to copy all fields from the lc block, whether settings or content.
-        #  But that's the existing behavior, so we're going to test for it, for now at least.
-        #  We may change this in the future: https://github.com/openedx/edx-platform/issues/33739)
-        dupe_html_1 = store.get_item(dupe.children[0])
-        assert dupe_html_1.display_name == "HTML 1 Title (Course Override)"  # <- as you'd expect
-        assert dupe_html_1.data == "HTML 1 Content (Original)"  # <- weird!
-        dupe_html_2 = store.get_item(dupe.children[1])
-        assert dupe_html_2.display_name == "HTML 2 Title (Original)"  # <- as you'd expect
-        assert dupe_html_2.data == "HTML 2 Content (Original)"  # <- as you'd expect
-
-        # Finally, upgrade the dupe's library version, and make sure it pulls in updated library block *content*,
-        # whilst preserving *settings overrides* (specifically, HTML 1's title override).
-        dupe.sync_from_library(upgrade_to_latest=True)
-        dupe = store.get_item(dupe.location)
-        assert dupe.source_library_version == str(updated_lib_version)
-        assert len(dupe.children) == 2
-        dupe_html_1 = store.get_item(dupe.children[0])
-        dupe_html_2 = store.get_item(dupe.children[1])
-        assert dupe_html_1.display_name == "HTML 1 Title (Course Override)"
-        assert dupe_html_1.data == "HTML 1 Content (Lib Update)"
-        assert dupe_html_2.display_name == "HTML 2 Title (Lib Update)"
-        assert dupe_html_2.data == "HTML 2 Content (Lib Update)"
-
 
 @ddt.ddt
 class TestMoveItem(ItemTest):

cms/djangoapps/contentstore/views/tests/test_container_page.py

@@ -167,7 +167,7 @@ def test_library_content_preview_html(self):
         self.assertEqual(len(lc_block.children), 0)
 
         # Refresh children to be reflected in lc_block
-        lc_block = self._upgrade_and_sync(lc_block)
+        lc_block = self._refresh_children(lc_block)
         self.assertEqual(len(lc_block.children), 1)
 
         self.validate_preview_html(

cms/djangoapps/contentstore/views/tests/test_import_export.py

@@ -1112,7 +1112,7 @@ def _setup_source_course_with_library_content(self, publish=False, version=None)
         lc_block = self._add_library_content_block(
             vertical, self.lib_key, publish_item=publish, other_settings=dict(source_library_version=version)
         )
-        lc_block.sync_from_library(upgrade_to_latest=True)
+        self._refresh_children(lc_block)
 
     def get_lib_content_block_children(self, block_location):
         """

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

@@ -8,14 +8,15 @@
 contentstore/views/block.py to this file, because the logic is reused in another view now.
 Along with it, we moved the business logic of the other views in that file, since that is related.
 """
+
 import logging
 from datetime import datetime
 from uuid import uuid4
 
 from attrs import asdict
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
-from django.contrib.auth.models import User  # pylint: disable=imported-auth-user
+from django.contrib.auth.models import (User)  # lint-amnesty, pylint: disable=imported-auth-user
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponse, HttpResponseBadRequest
 from django.utils.timezone import timezone
@@ -56,15 +57,37 @@
 from openedx.core.lib.gating import api as gating_api
 from openedx.core.lib.cache_utils import request_cached
 from openedx.core.toggles import ENTRANCE_EXAMS
-from xmodule.course_block import DEFAULT_START_DATE
-from xmodule.library_tools import LibraryToolsService
-from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum
-from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES
-from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError
-from xmodule.modulestore.inheritance import own_metadata
-from xmodule.services import ConfigurationService, SettingsService, TeamsConfigurationService
-from xmodule.tabs import CourseTabList
+from xmodule.course_block import (
+    DEFAULT_START_DATE,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.library_tools import (
+    LibraryToolsService,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore import (
+    EdxJSONEncoder,
+    ModuleStoreEnum,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.django import (
+    modulestore,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.draft_and_published import (
+    DIRECT_ONLY_CATEGORIES,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.exceptions import (
+    InvalidLocationError,
+    ItemNotFoundError,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.inheritance import (
+    own_metadata,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.services import (
+    ConfigurationService,
+    SettingsService,
+    TeamsConfigurationService,
+)  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.tabs import (
+    CourseTabList,
+)  # lint-amnesty, pylint: disable=wrong-import-order
 
 from ..utils import (
     ancestor_has_staff_lock,
@@ -157,7 +180,6 @@ def handle_xblock(request, usage_key_string=None):
     the public CMS API.
     """
     if usage_key_string:
-
         usage_key = usage_key_with_run(usage_key_string)
 
         access_check = (
@@ -198,15 +220,15 @@ def handle_xblock(request, usage_key_string=None):
             _delete_item(usage_key, request.user)
             return JsonResponse()
         else:  # Since we have a usage_key, we are updating an existing xblock.
-            modified_xblock = modify_xblock(usage_key, request)
-            return modified_xblock
+            return modify_xblock(usage_key, request)
 
     elif request.method in ("PUT", "POST"):
         if "duplicate_source_locator" in request.json:
             parent_usage_key = usage_key_with_run(request.json["parent_locator"])
             duplicate_source_usage_key = usage_key_with_run(
                 request.json["duplicate_source_locator"]
             )
+
             source_course = duplicate_source_usage_key.course_key
             dest_course = parent_usage_key.course_key
             if not has_studio_write_access(
@@ -233,7 +255,6 @@ def handle_xblock(request, usage_key_string=None):
                 request.user,
                 request.json.get("display_name"),
             )
-
             return JsonResponse(
                 {
                     "locator": str(dest_usage_key),
@@ -277,6 +298,7 @@ def handle_xblock(request, usage_key_string=None):
 
 def modify_xblock(usage_key, request):
     request_data = request.json
+    print(f'In modify_xblock with data = {request_data.get("data")}, fields = {request_data.get("fields")}')
     return _save_xblock(
         request.user,
         get_xblock(usage_key, request.user),
@@ -338,27 +360,21 @@ def load_services_for_studio(runtime, user):
 def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
     """
     Updates the xblock in the modulestore.
-    But before doing so, it calls the xblock's editor_saved callback function,
-    and after doing so, it calls the xblock's post_editor_saved callback function.
-
-    TODO: Remove getattrs from this function.
-          See https://github.com/openedx/edx-platform/issues/33715
+    But before doing so, it calls the xblock's editor_saved callback function.
     """
-    if old_metadata is None:
-        old_metadata = own_metadata(xblock)
-    if old_content is None:
-        old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content)
-    if hasattr(xblock, "editor_saved"):
+    if callable(getattr(xblock, "editor_saved", None)):
+        if old_metadata is None:
+            old_metadata = own_metadata(xblock)
+        if old_content is None:
+            old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content)
         load_services_for_studio(xblock.runtime, user)
         xblock.editor_saved(user, old_metadata, old_content)
-    xblock_updated = modulestore().update_item(xblock, user.id)
-    if hasattr(xblock_updated, "post_editor_saved"):
-        load_services_for_studio(xblock_updated.runtime, user)
-        xblock_updated.post_editor_saved(user, old_metadata, old_content)
-    return xblock_updated
+
+    # Update after the callback so any changes made in the callback will get persisted.
+    return modulestore().update_item(xblock, user.id)
 
 
-def _save_xblock(
+def _save_xblock(  # lint-amnesty, pylint: disable=too-many-statements
     user,
     xblock,
     data=None,
@@ -373,11 +389,12 @@ def _save_xblock(
     publish=None,
     fields=None,
     summary_configuration_enabled=None,
-):  # lint-amnesty, pylint: disable=too-many-statements
+):
     """
     Saves xblock w/ its fields. Has special processing for grader_type, publish, and nullout and Nones in metadata.
     nullout means to truly set the field to None whereas nones in metadata mean to unset them (so they revert
     to default).
+
     """
     store = modulestore()
     # Perform all xblock changes within a (single-versioned) transaction
@@ -864,8 +881,6 @@ def _duplicate_block(
             # Allow an XBlock to do anything fancy it may need to when duplicated from another block.
             # These blocks may handle their own children or parenting if needed. Let them return booleans to
             # let us know if we need to handle these or not.
-            # TODO: Make this a proper method in the base class so we don't need getattr.
-            #       See https://github.com/openedx/edx-platform/issues/33715
             load_services_for_studio(dest_block.runtime, user)
             children_handled = dest_block.studio_post_duplicate(store, source_item)