feat!: remove all references to content library types (#35726)

At one point, we envisioned having different kinds of libraries, e.g. a "Video" library would be distinct from a "Problem" library. Later on, we decided on a more generalized form of Libraries, where any given library can hold any combination of content–which would then be organized using collections and tagging. Due to this shift in perspective, these values haven't actually been used for a long time. This is just getting rid of them altogether.
2024-10-25 13:16:56 -05:00
parent ab6d8cb6c2
commit e8cdb06410
13 changed files with 35 additions and 131 deletions
--- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
+++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
@@ -408,7 +408,6 @@ class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase):
        self.store = modulestore()

        self.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",
--- a/openedx/core/djangoapps/content/search/tests/test_api.py
+++ b/openedx/core/djangoapps/content/search/tests/test_api.py
@@ -115,7 +115,6 @@ class TestSearchApi(ModuleStoreTestCase):

        # Create a content library:
        self.library = library_api.create_library(
-            library_type=library_api.COMPLEX,
            org=OrganizationFactory.create(short_name="org1"),
            slug="lib",
            title="Library",
--- a/openedx/core/djangoapps/content_libraries/api.py
+++ b/openedx/core/djangoapps/content_libraries/api.py
@@ -114,7 +114,7 @@ from openedx.core.lib.xblock_serializer.api import serialize_modulestore_block_f
 from xmodule.modulestore.django import modulestore

 from . import permissions, tasks
-from .constants import ALL_RIGHTS_RESERVED, COMPLEX
+from .constants import ALL_RIGHTS_RESERVED
 from .models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask

 log = logging.getLogger(__name__)
@@ -176,7 +176,6 @@ class ContentLibraryMetadata:
    description = attr.ib("")
    num_blocks = attr.ib(0)
    version = attr.ib(0)
-    type = attr.ib(default=COMPLEX)
    last_published = attr.ib(default=None, type=datetime)
    last_draft_created = attr.ib(default=None, type=datetime)
    last_draft_created_by = attr.ib(default=None, type=datetime)
@@ -306,15 +305,13 @@ class LibraryXBlockType:
 # ============


-def get_libraries_for_user(user, org=None, library_type=None, text_search=None, order=None):
+def get_libraries_for_user(user, org=None, text_search=None, order=None):
    """
    Return content libraries that the user has permission to view.
    """
    filter_kwargs = {}
    if org:
        filter_kwargs['org__short_name'] = org
-    if library_type:
-        filter_kwargs['type'] = library_type
    qs = ContentLibrary.objects.filter(**filter_kwargs) \
                               .select_related('learning_package', 'org') \
                               .order_by('org__short_name', 'slug')
@@ -361,7 +358,6 @@ def get_metadata(queryset, text_search=None):
        ContentLibraryMetadata(
            key=lib.library_key,
            title=lib.learning_package.title if lib.learning_package else "",
-            type=lib.type,
            description="",
            version=0,
            allow_public_learning=lib.allow_public_learning,
@@ -446,7 +442,6 @@ def get_library(library_key):
    return ContentLibraryMetadata(
        key=library_key,
        title=learning_package.title,
-        type=ref.type,
        description=ref.learning_package.description,
        num_blocks=num_blocks,
        version=version,
@@ -474,7 +469,6 @@ def create_library(
        allow_public_learning=False,
        allow_public_read=False,
        library_license=ALL_RIGHTS_RESERVED,
-        library_type=COMPLEX,
 ):
    """
    Create a new content library.
@@ -491,8 +485,6 @@ def create_library(

    allow_public_read: Allow anyone to view blocks (including source) in Studio?

-    library_type: Deprecated parameter, not really used. Set to COMPLEX.
-
    Returns a ContentLibraryMetadata instance.
    """
    assert isinstance(org, Organization)
@@ -502,7 +494,6 @@ def create_library(
            ref = ContentLibrary.objects.create(
                org=org,
                slug=slug,
-                type=library_type,
                allow_public_learning=allow_public_learning,
                allow_public_read=allow_public_read,
                license=library_license,
@@ -526,7 +517,6 @@ def create_library(
    return ContentLibraryMetadata(
        key=ref.library_key,
        title=title,
-        type=library_type,
        description=description,
        num_blocks=0,
        version=0,
@@ -611,7 +601,6 @@ def update_library(
        description=None,
        allow_public_learning=None,
        allow_public_read=None,
-        library_type=None,
        library_license=None,
 ):
    """
@@ -621,7 +610,7 @@ def update_library(
    A value of None means "don't change".
    """
    lib_obj_fields = [
-        allow_public_learning, allow_public_read, library_type, library_license
+        allow_public_learning, allow_public_read, library_license
    ]
    lib_obj_changed = any(field is not None for field in lib_obj_fields)
    learning_pkg_changed = any(field is not None for field in [title, description])
@@ -640,10 +629,6 @@ def update_library(
                content_lib.allow_public_learning = allow_public_learning
            if allow_public_read is not None:
                content_lib.allow_public_read = allow_public_read
-            if library_type is not None:
-                # TODO: Get rid of this field entirely, and remove library_type
-                # from any functions that take it as an argument.
-                content_lib.library_type = library_type
            if library_license is not None:
                content_lib.library_license = library_license
            content_lib.save()
@@ -856,13 +841,6 @@ def validate_can_add_block_to_library(
    """
    assert isinstance(library_key, LibraryLocatorV2)
    content_library = ContentLibrary.objects.get_by_key(library_key)  # type: ignore[attr-defined]
-    if content_library.type != COMPLEX:
-        if block_type != content_library.type:
-            raise IncompatibleTypesError(
-                _('Block type "{block_type}" is not compatible with library type "{library_type}".').format(
-                    block_type=block_type, library_type=content_library.type,
-                )
-            )

    # If adding a component would take us over our max, return an error.
    component_count = authoring_api.get_all_drafts(content_library.learning_package.id).count()
@@ -1288,10 +1266,7 @@ def get_allowed_block_types(library_key):  # pylint: disable=unused-argument
    # TODO: return support status and template options
    # See cms/djangoapps/contentstore/views/component.py
    block_types = sorted(name for name, class_ in XBlock.load_classes())
-    lib = get_library(library_key)
-    if lib.type != COMPLEX:
-        # Problem and Video libraries only permit XBlocks of the same name.
-        block_types = (name for name in block_types if name == lib.type)
+
    info = []
    for block_type in block_types:
        # TODO: unify the contentstore helper with the xblock.api version of
--- a/openedx/core/djangoapps/content_libraries/constants.py
+++ b/openedx/core/djangoapps/content_libraries/constants.py
@@ -2,16 +2,6 @@
 from django.utils.translation import gettext_lazy as _


-VIDEO = 'video'
-COMPLEX = 'complex'
-PROBLEM = 'problem'
-
-LIBRARY_TYPES = (
-    (VIDEO, _('Video')),
-    (COMPLEX, _('Complex')),
-    (PROBLEM, _('Problem')),
-)
-
 # These are all the licenses we support so far.
 ALL_RIGHTS_RESERVED = ''
 CC_4_BY = 'CC:4.0:BY'
--- a/openedx/core/djangoapps/content_libraries/migrations/0011_remove_contentlibrary_bundle_uuid_and_more.py
+++ b/openedx/core/djangoapps/content_libraries/migrations/0011_remove_contentlibrary_bundle_uuid_and_more.py
@@ -0,0 +1,21 @@
+# Generated by Django 4.2.16 on 2024-10-24 20:21
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('content_libraries', '0010_contentlibrary_learning_package_and_more'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='contentlibrary',
+            name='bundle_uuid',
+        ),
+        migrations.RemoveField(
+            model_name='contentlibrary',
+            name='type',
+        ),
+    ]
--- a/openedx/core/djangoapps/content_libraries/models.py
+++ b/openedx/core/djangoapps/content_libraries/models.py
@@ -54,8 +54,7 @@ from pylti1p3.grade import Grade

 from opaque_keys.edx.django.models import UsageKeyField
 from openedx.core.djangoapps.content_libraries.constants import (
-    LIBRARY_TYPES, COMPLEX, LICENSE_OPTIONS,
-    ALL_RIGHTS_RESERVED,
+    LICENSE_OPTIONS, ALL_RIGHTS_RESERVED,
 )
 from openedx_learning.api.authoring_models import LearningPackage
 from organizations.models import Organization  # lint-amnesty, pylint: disable=wrong-import-order
@@ -101,18 +100,6 @@ class ContentLibrary(models.Model):
    org = models.ForeignKey(Organization, on_delete=models.PROTECT, null=False)
    slug = models.SlugField(allow_unicode=True)

-    # We no longer use the ``bundle_uuid`` and ``type`` fields, but we'll leave
-    # them in the model until after the Redwood release, just in case someone
-    # out there was using v2 libraries. We don't expect this, since it wasn't in
-    # a usable state, but there's always a chance someone managed to do it and
-    # is still using it. By keeping the schema backwards compatible, the thought
-    # is that they would update to the latest version, notice their libraries
-    # aren't working correctly, and still have the ability to recover their data
-    # if the code was rolled back.
-    # TODO: Remove these fields after the Redwood release is cut.
-    bundle_uuid = models.UUIDField(unique=True, null=True, default=None)
-    type = models.CharField(max_length=25, default=COMPLEX, choices=LIBRARY_TYPES)
-
    license = models.CharField(max_length=25, default=ALL_RIGHTS_RESERVED, choices=LICENSE_OPTIONS)
    learning_package = models.OneToOneField(
        LearningPackage,
--- a/openedx/core/djangoapps/content_libraries/serializers.py
+++ b/openedx/core/djangoapps/content_libraries/serializers.py
@@ -11,8 +11,6 @@ from opaque_keys import InvalidKeyError

 from openedx_learning.api.authoring_models import Collection
 from openedx.core.djangoapps.content_libraries.constants import (
-    LIBRARY_TYPES,
-    COMPLEX,
    ALL_RIGHTS_RESERVED,
    LICENSE_OPTIONS,
 )
@@ -37,10 +35,8 @@ class ContentLibraryMetadataSerializer(serializers.Serializer):
    # begins with 'lib:'. (The numeric ID of the ContentLibrary object in MySQL
    # is not exposed via this API.)
    id = serializers.CharField(source="key", read_only=True)
-    type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=COMPLEX)
    org = serializers.SlugField(source="key.org")
    slug = serializers.CharField(source="key.slug", validators=(validate_unicode_slug, ))
-    bundle_uuid = serializers.UUIDField(format='hex_verbose', read_only=True)
    title = serializers.CharField()
    description = serializers.CharField(allow_blank=True)
    num_blocks = serializers.IntegerField(read_only=True)
@@ -86,7 +82,6 @@ class ContentLibraryUpdateSerializer(serializers.Serializer):
    description = serializers.CharField()
    allow_public_learning = serializers.BooleanField()
    allow_public_read = serializers.BooleanField()
-    type = serializers.ChoiceField(choices=LIBRARY_TYPES)
    license = serializers.ChoiceField(choices=LICENSE_OPTIONS)


@@ -118,7 +113,7 @@ class ContentLibraryPermissionSerializer(ContentLibraryPermissionLevelSerializer
    group_name = serializers.CharField(source="group.name", allow_null=True, allow_blank=False, default=None)


-class BaseFilterSerializer(serializers.Serializer):
+class ContentLibraryFilterSerializer(serializers.Serializer):
    """
    Base serializer for filtering listings on the content library APIs.
    """
@@ -127,13 +122,6 @@ class BaseFilterSerializer(serializers.Serializer):
    order = serializers.CharField(default=None, required=False)


-class ContentLibraryFilterSerializer(BaseFilterSerializer):
-    """
-    Serializer for filtering library listings.
-    """
-    type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=None, required=False)
-
-
 class CollectionMetadataSerializer(serializers.Serializer):
    """
    Serializer for CollectionMetadata
--- a/openedx/core/djangoapps/content_libraries/tests/base.py
+++ b/openedx/core/djangoapps/content_libraries/tests/base.py
@@ -9,7 +9,7 @@ from organizations.models import Organization
 from rest_framework.test import APITransactionTestCase, APIClient

 from common.djangoapps.student.tests.factories import UserFactory
-from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED
+from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED
 from openedx.core.djangolib.testing.utils import skip_unless_cms

 # Define the URLs here - don't use reverse() because we want to detect
@@ -124,7 +124,7 @@ class ContentLibrariesRestApiTest(APITransactionTestCase):
        self.client = old_client  # pylint: disable=attribute-defined-outside-init

    def _create_library(
-        self, slug, title, description="", org=None, library_type=COMPLEX,
+        self, slug, title, description="", org=None,
        license_type=ALL_RIGHTS_RESERVED, expect_response=200,
    ):
        """ Create a library """
@@ -135,7 +135,6 @@ class ContentLibrariesRestApiTest(APITransactionTestCase):
            "slug": slug,
            "title": title,
            "description": description,
-            "type": library_type,
            "license": license_type,
        }, expect_response)

--- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
@@ -25,7 +25,7 @@ from organizations.models import Organization
 from rest_framework.test import APITestCase

 from common.djangoapps.student.tests.factories import UserFactory
-from openedx.core.djangoapps.content_libraries.constants import CC_4_BY, COMPLEX, PROBLEM, VIDEO
+from openedx.core.djangoapps.content_libraries.constants import CC_4_BY
 from openedx.core.djangoapps.content_libraries.tests.base import (
    URL_BLOCK_GET_HANDLER_URL,
    URL_BLOCK_METADATA_URL,
@@ -100,7 +100,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix
            "title": "A Tést Lꜟطrary",
            "description": "Just Téstꜟng",
            "version": 0,
-            "type": COMPLEX,
            "license": CC_4_BY,
            "has_unpublished_changes": False,
            "has_unpublished_deletes": False,
@@ -199,13 +198,13 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix
        Test the filters in the list libraries API
        """
        self._create_library(
-            slug="test-lib-filter-1", title="Fob", description="Bar", library_type=VIDEO,
+            slug="test-lib-filter-1", title="Fob", description="Bar",
        )
        self._create_library(
            slug="test-lib-filter-2", title="Library-Title-2", description="Bar-2",
        )
        self._create_library(
-            slug="l3", title="Library-Title-3", description="Description", library_type=VIDEO,
+            slug="l3", title="Library-Title-3", description="Description",
        )

        Organization.objects.get_or_create(
@@ -215,7 +214,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix
        self._create_library(
            slug="l4", title="Library-Title-4",
            description="Library-Description", org='org-test',
-            library_type=VIDEO,
        )
        self._create_library(
            slug="l5", title="Library-Title-5", description="Library-Description",
@@ -225,14 +223,11 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix
        assert len(self._list_libraries()) == 5
        assert len(self._list_libraries({'org': 'org-test'})) == 2
        assert len(self._list_libraries({'text_search': 'test-lib-filter'})) == 2
-        assert len(self._list_libraries({'text_search': 'test-lib-filter', 'type': VIDEO})) == 1
        assert len(self._list_libraries({'text_search': 'library-title'})) == 4
-        assert len(self._list_libraries({'text_search': 'library-title', 'type': VIDEO})) == 2
        assert len(self._list_libraries({'text_search': 'bar'})) == 2
        assert len(self._list_libraries({'text_search': 'org-test'})) == 2
        assert len(self._list_libraries({'org': 'org-test',
                                         'text_search': 'library-title-4'})) == 1
-        assert len(self._list_libraries({'type': VIDEO})) == 3

        self.assertOrderEqual(
            self._list_libraries({'order': 'title'}),
@@ -496,27 +491,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix
        assert len(self._get_library_blocks(lib['id'], {'block_type': 'problem'})['results']) == 3
        assert len(self._get_library_blocks(lib['id'], {'block_type': 'squirrel'})['results']) == 0

-    @ddt.data(
-        ('video-problem', VIDEO, 'problem', 400),
-        ('video-video', VIDEO, 'video', 200),
-        ('problem-problem', PROBLEM, 'problem', 200),
-        ('problem-video', PROBLEM, 'video', 400),
-        ('complex-video', COMPLEX, 'video', 200),
-        ('complex-problem', COMPLEX, 'problem', 200),
-    )
-    @ddt.unpack
-    def test_library_blocks_type_constrained(self, slug, library_type, block_type, expect_response):
-        """
-        Test that type-constrained libraries enforce their constraint when adding an XBlock.
-        """
-        lib = self._create_library(
-            slug=slug, title="A Test Library", description="Testing XBlocks", library_type=library_type,
-        )
-        lib_id = lib["id"]
-
-        # Add a 'problem' XBlock to the library:
-        self._add_block_to_library(lib_id, block_type, 'test-block', expect_response=expect_response)
-
    def test_library_not_found(self):
        """Test that requests fail with 404 when the library does not exist"""
        valid_not_found_key = 'lb:valid:key:video:1'
@@ -755,24 +729,6 @@ class ContentLibrariesTestCase(ContentLibrariesRestApiTest, OpenEdxEventsTestMix
            # Second block should throw error
            self._add_block_to_library(lib_id, "problem", "problem1", expect_response=400)

-    @ddt.data(
-        ('complex-types', COMPLEX, False),
-        ('video-types', VIDEO, True),
-        ('problem-types', PROBLEM, True),
-    )
-    @ddt.unpack
-    def test_block_types(self, slug, library_type, constrained):
-        """
-        Test that the permitted block types listing for a library change based on type.
-        """
-        lib = self._create_library(slug=slug, title='Test Block Types', library_type=library_type)
-        types = self._get_library_block_types(lib['id'])
-        if constrained:
-            assert len(types) == 1
-            assert types[0]['block_type'] == library_type
-        else:
-            assert len(types) > 1
-
    def test_content_library_create_event(self):
        """
        Check that CONTENT_LIBRARY_CREATED event is sent when a content library is created.
--- a/openedx/core/djangoapps/content_libraries/tests/test_models.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_models.py
@@ -15,7 +15,6 @@ from organizations.models import Organization
 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2

 from ..models import ALL_RIGHTS_RESERVED
-from ..models import COMPLEX
 from ..models import ContentLibrary
 from ..models import LtiGradedResource
 from ..models import LtiProfile
@@ -35,7 +34,6 @@ class ContentLibraryTest(TestCase):
        return ContentLibrary.objects.create(
            org=org,
            slug='foobar',
-            type=COMPLEX,
            allow_public_learning=False,
            allow_public_read=False,
            license=ALL_RIGHTS_RESERVED,
--- a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py
@@ -21,7 +21,7 @@ from openedx.core.djangoapps.content_libraries.tests.base import (
    URL_BLOCK_FIELDS_URL,
 )
 from openedx.core.djangoapps.content_libraries.tests.user_state_block import UserStateTestBlock
-from openedx.core.djangoapps.content_libraries.constants import COMPLEX, ALL_RIGHTS_RESERVED
+from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED
 from openedx.core.djangoapps.dark_lang.models import DarkLangConfig
 from openedx.core.djangoapps.xblock import api as xblock_api
 from openedx.core.djangolib.testing.utils import skip_unless_lms, skip_unless_cms
@@ -50,7 +50,6 @@ class ContentLibraryContentTestMixin:
        _, slug = self.id().rsplit('.', 1)
        with transaction.atomic():
            self.library = library_api.create_library(
-                library_type=COMPLEX,
                org=self.organization,
                slug=slugify(slug),
                title=(f"{slug} Test Lib"),
--- a/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py
+++ b/openedx/core/djangoapps/content_libraries/tests/test_views_lti.py
@@ -5,8 +5,6 @@ Tests for LTI views.
 from django.conf import settings
 from django.test import TestCase, override_settings

-from openedx.core.djangoapps.content_libraries.constants import PROBLEM
-
 from .base import (
    ContentLibrariesRestApiTest,
    URL_LIB_LTI_JWKS,
@@ -60,10 +58,10 @@ class LibraryBlockLtiUrlViewTestMixin:
        """

        library = self._create_library(
-            slug="libgg", title="A Test Library", description="Testing library", library_type=PROBLEM,
+            slug="libgg", title="A Test Library", description="Testing library",
        )

-        block = self._add_block_to_library(library['id'], PROBLEM, PROBLEM)
+        block = self._add_block_to_library(library['id'], 'problem', 'problem')
        usage_key = str(block['id'])

        url = f'/api/libraries/v2/blocks/{usage_key}/lti/'
--- a/openedx/core/djangoapps/content_libraries/views.py
+++ b/openedx/core/djangoapps/content_libraries/views.py
@@ -227,14 +227,12 @@ class LibraryRootView(GenericAPIView):
        serializer = ContentLibraryFilterSerializer(data=request.query_params)
        serializer.is_valid(raise_exception=True)
        org = serializer.validated_data['org']
-        library_type = serializer.validated_data['type']
        text_search = serializer.validated_data['text_search']
        order = serializer.validated_data['order']

        queryset = api.get_libraries_for_user(
            request.user,
            org=org,
-            library_type=library_type,
            text_search=text_search,
            order=order,
        )
@@ -259,7 +257,6 @@ class LibraryRootView(GenericAPIView):
        data = dict(serializer.validated_data)
        # Converting this over because using the reserved names 'type' and 'license' would shadow the built-in
        # definitions elsewhere.
-        data['library_type'] = data.pop('type')
        data['library_license'] = data.pop('license')
        key_data = data.pop("key")
        # Move "slug" out of the "key.slug" pseudo-field that the serializer added:
@@ -313,8 +310,6 @@ class LibraryDetailsView(APIView):
        serializer.is_valid(raise_exception=True)
        data = dict(serializer.validated_data)
        # Prevent ourselves from shadowing global names.
-        if 'type' in data:
-            data['library_type'] = data.pop('type')
        if 'license' in data:
            data['library_license'] = data.pop('license')
        try: