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
This commit is contained in:
Kyle McCormick
2024-10-15 11:32:01 -04:00
committed by GitHub
parent 70df3deea6
commit 2bbd8ecd18
31 changed files with 180 additions and 604 deletions

View File

@@ -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 = (
'<!-- Comment -->\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

View File

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

View File

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