feat: follow migrated legacy library content block (#37405)

* feat: show item bank ui for migrated legacy library content

* feat: migrate legacy content block to item bank block on view in studio

* fix: duplicate and copy issues

* refactor: migration location and add tests

* fix: lint issues

* fix: item bank and library content children view add button functionality

Newly added blocks from library in children view page of item bank block
and migrated library content block were not displayed automatically.

* fix: lint issues

* fix: lint issues

* feat: only migrate if same version of library is migrated

* refactor: migrate block on request

* fix: component reload on migration

* fix: tests

* refactor: comments and message wordings

* refactor: update alert text

* docs: add context

* fix: component links not being created on migrating legacy blocks

* fix: api docs and types

* refactor: use inheritance and specific parent method call

* fix: imports

* fix: api typing

* fix: upstream_version check

* refactor: rename variables

* refactor: parsing entity keys to usage_keys

xmodule/tests/test_library_content.py

@@ -7,13 +7,17 @@ import ddt
 from bson.objectid import ObjectId
 from fs.memoryfs import MemoryFS
 from lxml import etree
-from opaque_keys.edx.locator import LibraryLocator
+from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2
+from organizations.tests.factories import OrganizationFactory
 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 common.djangoapps.student.tests.factories import UserFactory
+from openedx.core.djangoapps.content_libraries import api as lib_api
 from openedx.core.djangolib.testing.utils import skip_unless_cms
+from xmodule.capa_block import ProblemBlock
 from xmodule.library_content_block import ANY_CAPA_TYPE_VALUE, LegacyLibraryContentBlock
 from xmodule.library_tools import LegacyLibraryToolsService
 from xmodule.modulestore import ModuleStoreEnum
@@ -22,8 +26,6 @@ from xmodule.modulestore.tests.utils import MixedSplitTestCase
 from xmodule.tests import prepare_block_runtime
 from xmodule.validation import StudioValidationMessage
 from xmodule.x_module import AUTHOR_VIEW
-from xmodule.capa_block import ProblemBlock
-from common.djangoapps.student.tests.factories import UserFactory
 
 from .test_course_block import DummySystem as TestImportSystem
 
@@ -42,7 +44,12 @@ class LegacyLibraryContentTest(MixedSplitTestCase):
         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}")
+            self.make_block(
+                "html",
+                self.library,
+                data=f"Hello world from block {i}",
+                display_name=f"html {i}"
+            )
             for i in range(1, 5)
         ]
         self.course = CourseFactory.create(modulestore=self.store)
@@ -84,6 +91,18 @@ class LegacyLibraryContentTest(MixedSplitTestCase):
 
         block.runtime.get_block_for_descriptor = get_block
 
+    def _verify_xblock_properties(self, imported_lc_block):
+        """
+        Check the new XBlock has the same properties as the old one.
+        """
+        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.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)
+        assert imported_lc_block.children == self.lc_block.children
+
 
 @ddt.ddt
 class LegacyLibraryContentGeneralTest(LegacyLibraryContentTest):
@@ -133,15 +152,14 @@ class TestLibraryContentExportImport(LegacyLibraryContentTest):
         self._sync_lc_block_from_library()
 
         self.expected_olx = (
-            '<library_content display_name="{block.display_name}" max_count="{block.max_count}"'
-            ' source_library_id="{block.source_library_id}" source_library_version="{block.source_library_version}">\n'
-            '  <html url_name="{block.children[0].block_id}"/>\n'
-            '  <html url_name="{block.children[1].block_id}"/>\n'
-            '  <html url_name="{block.children[2].block_id}"/>\n'
-            '  <html url_name="{block.children[3].block_id}"/>\n'
+            f'<library_content display_name="{self.lc_block.display_name}" max_count="{self.lc_block.max_count}"'
+            f' source_library_id="{self.lc_block.source_library_id}" '
+            f'source_library_version="{self.lc_block.source_library_version}">\n'
+            f'  <html url_name="{self.lc_block.children[0].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[1].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[2].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[3].block_id}"/>\n'
             '</library_content>\n'
-        ).format(
-            block=self.lc_block,
         )
 
         # Set the virtual FS to export the olx to.
@@ -157,27 +175,13 @@ class TestLibraryContentExportImport(LegacyLibraryContentTest):
         node = etree.Element("unknown_root")
         self.lc_block.add_xml_to_node(node)
 
-    def _verify_xblock_properties(self, imported_lc_block):
-        """
-        Check the new XBlock has the same properties as the old one.
-        """
-        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.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)
-        assert imported_lc_block.children == self.lc_block.children
-
     def test_xml_export_import_cycle(self):
         """
         Test the export-import cycle.
         """
         # Read back the olx.
-        with self.export_fs.open('{dir}/{file_name}.xml'.format(
-            dir=self.lc_block.scope_ids.usage_id.block_type,
-            file_name=self.lc_block.scope_ids.usage_id.block_id
-        )) as f:
+        file_path = f'{self.lc_block.scope_ids.usage_id.block_type}/{self.lc_block.scope_ids.usage_id.block_id}.xml'
+        with self.export_fs.open(file_path) as f:
             exported_olx = f.read()
 
         # And compare.
@@ -195,16 +199,15 @@ class TestLibraryContentExportImport(LegacyLibraryContentTest):
         """
         olx_with_comments = (
             '<!-- Comment -->\n'
-            '<library_content display_name="{block.display_name}" max_count="{block.max_count}"'
-            ' source_library_id="{block.source_library_id}" source_library_version="{block.source_library_version}">\n'
+            f'<library_content display_name="{self.lc_block.display_name}" max_count="{self.lc_block.max_count}"'
+            f' source_library_id="{self.lc_block.source_library_id}" '
+            f'source_library_version="{self.lc_block.source_library_version}">\n'
             '<!-- Comment -->\n'
-            '  <html url_name="{block.children[0].block_id}"/>\n'
-            '  <html url_name="{block.children[1].block_id}"/>\n'
-            '  <html url_name="{block.children[2].block_id}"/>\n'
-            '  <html url_name="{block.children[3].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[0].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[1].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[2].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[3].block_id}"/>\n'
             '</library_content>\n'
-        ).format(
-            block=self.lc_block,
         )
 
         # Import the olx.
@@ -234,7 +237,7 @@ class LegacyLibraryContentBlockTestMixin:
         """ Helper function to create empty CAPA problem definition """
         problem = "<problem>"
         for problem_type in args:
-            problem += "<{problem_type}></{problem_type}>".format(problem_type=problem_type)
+            problem += f"<{problem_type}></{problem_type}>"
         problem += "</problem>"
         return problem
 
@@ -500,7 +503,7 @@ class TestLegacyLibraryContentBlockWithSearchIndex(LegacyLibraryContentBlockTest
 
     def _get_search_response(self, field_dictionary=None):
         """ Mocks search response as returned by search engine """
-        target_type = field_dictionary.get('problem_types')
+        target_type = (field_dictionary or {}).get('problem_types')
         matched_block_locations = [
             key for key, problem_types in
             self.problem_type_lookup.items() if target_type in problem_types
@@ -726,3 +729,110 @@ class TestLibraryContentAnalytics(LegacyLibraryContentTest):
                  'original_usage_key': str(keep_block_lib_usage_key),
                  'original_usage_version': str(keep_block_lib_version), 'descendants': []}]
         assert event_data['reason'] == 'invalid'
+
+
+@patch(
+    'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render
+)
+@patch('xmodule.html_block.HtmlBlock.author_view', dummy_render, create=True)
+@patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: [])
+class TestMigratedLibraryContentRender(LegacyLibraryContentTest):
+    """
+    Rendering unit tests for LegacyLibraryContentBlock
+    """
+
+    def setUp(self):
+        from cms.djangoapps.modulestore_migrator import api
+        from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy
+        super().setUp()
+        user = UserFactory()
+        self._sync_lc_block_from_library()
+        self.organization = OrganizationFactory()
+        self.lib_key_v2 = LibraryLocatorV2.from_string(
+            f"lib:{self.organization.short_name}:test-key"
+        )
+        lib_api.create_library(
+            org=self.organization,
+            slug=self.lib_key_v2.slug,
+            title="Test Library",
+        )
+        self.library_v2 = lib_api.ContentLibrary.objects.get(slug=self.lib_key_v2.slug)
+        api.start_migration_to_library(
+            user=user,
+            source_key=self.library.location.library_key,
+            target_library_key=self.library_v2.library_key,
+            target_collection_slug=None,
+            composition_level=CompositionLevel.Component.value,
+            repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
+            preserve_url_slugs=True,
+            forward_source_to_target=True,
+        )
+        # Migrate block
+        self.lc_block.upgrade_to_v2_library(None, None)
+
+    def test_preview_view(self):
+        """ Test preview view rendering """
+        assert len(self.lc_block.children) == len(self.lib_blocks)
+        self._bind_course_block(self.lc_block)
+        rendered = self.lc_block.render(AUTHOR_VIEW, {'root_xblock': self.lc_block})
+        assert 'Hello world from block 1' in rendered.content
+        assert 'Hello world from block 2' in rendered.content
+        assert 'Hello world from block 3' in rendered.content
+        assert 'Hello world from block 4' in rendered.content
+
+    def test_author_view(self):
+        """ Test author view rendering """
+        assert len(self.lc_block.children) == len(self.lib_blocks)
+        self._bind_course_block(self.lc_block)
+        rendered = self.lc_block.render(AUTHOR_VIEW, {})
+        # content should be similar to ItemBankBlock
+        assert 'Learners will see 1 of the 4 selected components' in rendered.content
+        assert '<li>html 1</li>' in rendered.content
+        assert '<li>html 2</li>' in rendered.content
+        assert '<li>html 3</li>' in rendered.content
+        assert '<li>html 4</li>' in rendered.content
+
+    def test_xml_export_import_cycle(self):
+        """
+        Test the export-import cycle.
+        """
+        # Render block to migrate it first
+        self.lc_block.render(AUTHOR_VIEW, {})
+        # Set the virtual FS to export the olx to.
+        export_fs = MemoryFS()
+        self.lc_block.runtime.export_fs = export_fs  # pylint: disable=protected-access
+
+        # Export the olx.
+        node = etree.Element("unknown_root")
+        self.lc_block.add_xml_to_node(node)
+
+        # Read back the olx.
+        file_path = f'{self.lc_block.scope_ids.usage_id.block_type}/{self.lc_block.scope_ids.usage_id.block_id}.xml'
+        with export_fs.open(file_path) as f:
+            exported_olx = f.read()
+
+        expected_olx_export = (
+            f'<library_content display_name="{self.lc_block.display_name}" is_migrated_to_v2="true"'
+            f' max_count="{self.lc_block.max_count}" source_library_id="{self.lc_block.source_library_id}" '
+            f'source_library_version="{self.lc_block.source_library_version}">\n'
+            f'  <html url_name="{self.lc_block.children[0].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[1].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[2].block_id}"/>\n'
+            f'  <html url_name="{self.lc_block.children[3].block_id}"/>\n'
+            '</library_content>\n'
+        )
+        # And compare.
+        assert exported_olx == expected_olx_export
+
+        # Now import it.
+        runtime = TestImportSystem(load_error_blocks=True, course_id=self.lc_block.location.course_key)
+        runtime.resources_fs = export_fs
+        olx_element = etree.fromstring(exported_olx)
+        imported_lc_block = LegacyLibraryContentBlock.parse_xml(olx_element, runtime, None)
+
+        self._verify_xblock_properties(imported_lc_block)
+        # Verify migration info in the child
+        assert imported_lc_block.is_migrated_to_v2
+        for child in imported_lc_block.get_children():
+            assert child.xml_attributes.get('upstream') is not None
+            assert str(child.xml_attributes.get('upstream_version')) == '0'